generate systemd: add --start-timeout flag

Add a new flag to set the start timeout for a generated systemd unit.
To make naming consistent, add a new --stop-timeout flag as well and let
the previous --time map to it.

Fixes: #11618
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>

cmd/podman/generate/systemd.go

@@ -19,16 +19,19 @@ import (
 )
 
 const (
-	restartPolicyFlagName = "restart-policy"
+	startTimeoutFlagName      = "start-timeout"
-	timeFlagName          = "time"
+	stopTimeoutFlagName       = "stop-timeout"
-	newFlagName           = "new"
+	stopTimeoutCompatFlagName = "time"
+	restartPolicyFlagName     = "restart-policy"
+	newFlagName               = "new"
 )
 
 var (
 	files              bool
 	format             string
-	systemdTimeout     uint
 	systemdRestart     string
+	startTimeout       uint
+	stopTimeout        uint
 	systemdOptions     = entities.GenerateSystemdOptions{}
 	systemdDescription = `Generate systemd units for a pod or container.
   The generated units can later be controlled via systemctl(1).`
@@ -56,8 +59,17 @@ func init() {
 	flags.BoolVarP(&files, "files", "f", false, "Generate .service files instead of printing to stdout")
 	flags.BoolVar(&systemdOptions.TemplateUnitFile, "template", false, "Make it a template file and use %i and %I specifiers. Working only for containers")
 
-	flags.UintVarP(&systemdTimeout, timeFlagName, "t", containerConfig.Engine.StopTimeout, "Stop timeout override")
+	flags.UintVarP(&startTimeout, startTimeoutFlagName, "", 0, "Start timeout override")
-	_ = systemdCmd.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone)
+	_ = systemdCmd.RegisterFlagCompletionFunc(startTimeoutFlagName, completion.AutocompleteNone)
+
+	// NOTE: initially, there was only a --time/-t flag which mapped to
+	// stop-timeout. To remain backwards compatible create a hidden flag
+	// that maps to StopTimeout.
+	flags.UintVarP(&stopTimeout, stopTimeoutFlagName, "", containerConfig.Engine.StopTimeout, "Stop timeout override")
+	_ = systemdCmd.RegisterFlagCompletionFunc(stopTimeoutFlagName, completion.AutocompleteNone)
+	flags.UintVarP(&stopTimeout, stopTimeoutCompatFlagName, "t", containerConfig.Engine.StopTimeout, "Backwards alias for --stop-timeout")
+	_ = flags.MarkHidden("time")
+
 	flags.BoolVar(&systemdOptions.New, newFlagName, false, "Create a new container or pod instead of starting an existing one")
 	flags.BoolVarP(&systemdOptions.NoHeader, "no-header", "", false, "Skip header generation")
 
@@ -84,9 +96,6 @@ func init() {
 }
 
 func systemd(cmd *cobra.Command, args []string) error {
-	if cmd.Flags().Changed(timeFlagName) {
-		systemdOptions.StopTimeout = &systemdTimeout
-	}
 	if cmd.Flags().Changed(restartPolicyFlagName) {
 		systemdOptions.RestartPolicy = &systemdRestart
 	}
@@ -102,6 +111,23 @@ func systemd(cmd *cobra.Command, args []string) error {
 		systemdOptions.New = true
 	}
 
+	if cmd.Flags().Changed(startTimeoutFlagName) {
+		systemdOptions.StartTimeout = &startTimeout
+	}
+	setStopTimeout := 0
+	if cmd.Flags().Changed(stopTimeoutFlagName) {
+		setStopTimeout++
+	}
+	if cmd.Flags().Changed(stopTimeoutCompatFlagName) {
+		setStopTimeout++
+	}
+	switch setStopTimeout {
+	case 1:
+		systemdOptions.StopTimeout = &stopTimeout
+	case 2:
+		return fmt.Errorf("%s and %s are redundant and cannot be used together", stopTimeoutFlagName, stopTimeoutCompatFlagName)
+	}
+
 	reports, err := registry.ContainerEngine().GenerateSystemd(registry.GetContext(), args[0], systemdOptions)
 	if err != nil {
 		return err

docs/source/markdown/podman-generate-systemd.1.md

@@ -38,9 +38,13 @@ Note that `--new` only works on containers and pods created directly via Podman
 
 Do not generate the header including meta data such as the Podman version and the timestamp.
 
-#### **--time**, **-t**=*value*
+#### **--start-timeout** =*value*
 
-Override the default stop timeout for the container with the given value.
+Override the default start timeout for the container with the given value in seconds.
+
+#### **--stop-timeout** =*value*
+
+Override the default stop timeout for the container with the given value in seconds.
 
 #### **--restart-policy**=*policy*
 

pkg/api/handlers/libpod/generate.go

@@ -23,10 +23,12 @@ func GenerateSystemd(w http.ResponseWriter, r *http.Request) {
 		TemplateUnitFile bool    `schema:"templateUnitFile"`
 		RestartPolicy    *string `schema:"restartPolicy"`
 		StopTimeout      uint    `schema:"stopTimeout"`
+		StartTimeout     uint    `schema:"startTimeout"`
 		ContainerPrefix  string  `schema:"containerPrefix"`
 		PodPrefix        string  `schema:"podPrefix"`
 		Separator        string  `schema:"separator"`
 	}{
+		StartTimeout:    0,
 		StopTimeout:     util.DefaultContainerConfig().Engine.StopTimeout,
 		ContainerPrefix: "container",
 		PodPrefix:       "pod",
@@ -46,6 +48,7 @@ func GenerateSystemd(w http.ResponseWriter, r *http.Request) {
 		NoHeader:         query.NoHeader,
 		TemplateUnitFile: query.TemplateUnitFile,
 		RestartPolicy:    query.RestartPolicy,
+		StartTimeout:     &query.StartTimeout,
 		StopTimeout:      &query.StopTimeout,
 		ContainerPrefix:  query.ContainerPrefix,
 		PodPrefix:        query.PodPrefix,

pkg/api/server/register_generate.go

@@ -37,10 +37,15 @@ func (s *APIServer) registerGenerateHandlers(r *mux.Router) error {
 	//    default: false
 	//    description: Do not generate the header including the Podman version and the timestamp.
 	//  - in: query
-	//    name: time
+	//    name: startTimeout
+	//    type: integer
+	//    default: 0
+	//    description: Start timeout in seconds.
+	//  - in: query
+	//    name: stopTimeout
 	//    type: integer
 	//    default: 10
-	//    description: Stop timeout override.
+	//    description: Stop timeout in seconds.
 	//  - in: query
 	//    name: restartPolicy
 	//    default: on-failure

pkg/bindings/generate/types.go

@@ -20,6 +20,8 @@ type SystemdOptions struct {
 	TemplateUnitFile *bool
 	// RestartPolicy - systemd restart policy.
 	RestartPolicy *string
+	// StartTimeout - time when starting the container.
+	StartTimeout *uint
 	// StopTimeout - time when stopping the container.
 	StopTimeout *uint
 	// ContainerPrefix - systemd unit name prefix for containers

pkg/bindings/generate/types_systemd_options.go

@@ -92,6 +92,21 @@ func (o *SystemdOptions) GetRestartPolicy() string {
 	return *o.RestartPolicy
 }
 
+// WithStartTimeout set field StartTimeout to given value
+func (o *SystemdOptions) WithStartTimeout(value uint) *SystemdOptions {
+	o.StartTimeout = &value
+	return o
+}
+
+// GetStartTimeout returns value of field StartTimeout
+func (o *SystemdOptions) GetStartTimeout() uint {
+	if o.StartTimeout == nil {
+		var z uint
+		return z
+	}
+	return *o.StartTimeout
+}
+
 // WithStopTimeout set field StopTimeout to given value
 func (o *SystemdOptions) WithStopTimeout(value uint) *SystemdOptions {
 	o.StopTimeout = &value

pkg/domain/entities/generate.go

@@ -10,6 +10,8 @@ type GenerateSystemdOptions struct {
 	New bool
 	// RestartPolicy - systemd restart policy.
 	RestartPolicy *string
+	// StartTimeout - time when starting the container.
+	StartTimeout *uint
 	// StopTimeout - time when stopping the container.
 	StopTimeout *uint
 	// ContainerPrefix - systemd unit name prefix for containers

pkg/domain/infra/tunnel/generate.go

@@ -8,14 +8,18 @@ import (
 )
 
 func (ic *ContainerEngine) GenerateSystemd(ctx context.Context, nameOrID string, opts entities.GenerateSystemdOptions) (*entities.GenerateSystemdReport, error) {
-	options := new(generate.SystemdOptions).WithUseName(opts.Name).WithContainerPrefix(opts.ContainerPrefix).WithNew(opts.New).WithNoHeader(opts.NoHeader).WithTemplateUnitFile(opts.TemplateUnitFile)
+	options := new(generate.SystemdOptions).WithUseName(opts.Name).WithContainerPrefix(opts.ContainerPrefix).WithNew(opts.New).WithNoHeader(opts.NoHeader).WithTemplateUnitFile(opts.TemplateUnitFile).WithPodPrefix(opts.PodPrefix).WithSeparator(opts.Separator)
-	options.WithPodPrefix(opts.PodPrefix).WithSeparator(opts.Separator)
+
+	if opts.StartTimeout != nil {
+		options.WithStartTimeout(*opts.StartTimeout)
+	}
+	if opts.StopTimeout != nil {
+		options.WithStopTimeout(*opts.StopTimeout)
+	}
 	if opts.RestartPolicy != nil {
 		options.WithRestartPolicy(*opts.RestartPolicy)
 	}
-	if to := opts.StopTimeout; to != nil {
+
-		options.WithStopTimeout(*opts.StopTimeout)
-	}
 	return generate.Systemd(ic.ClientCtx, nameOrID, options)
 }
 

pkg/systemd/generate/containers.go

@@ -73,6 +73,8 @@ type containerInfo struct {
 	ExecStartPre string
 	// ExecStart of the unit.
 	ExecStart string
+	// TimeoutStartSec of the unit.
+	TimeoutStartSec uint
 	// TimeoutStopSec of the unit.
 	TimeoutStopSec uint
 	// ExecStop of the unit.
@@ -109,6 +111,9 @@ Restart={{{{.RestartPolicy}}}}
 {{{{- if .StartLimitBurst}}}}
 StartLimitBurst={{{{.StartLimitBurst}}}}
 {{{{- end}}}}
+{{{{- if ne .TimeoutStartSec 0}}}}
+TimeoutStartSec={{{{.TimeoutStartSec}}}}
+{{{{- end}}}}
 TimeoutStopSec={{{{.TimeoutStopSec}}}}
 {{{{- if .ExecStartPre}}}}
 ExecStartPre={{{{.ExecStartPre}}}}
@@ -148,9 +153,14 @@ func ContainerUnit(ctr *libpod.Container, options entities.GenerateSystemdOption
 }
 
 func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSystemdOptions) (*containerInfo, error) {
-	timeout := ctr.StopTimeout()
+	stopTimeout := ctr.StopTimeout()
 	if options.StopTimeout != nil {
-		timeout = *options.StopTimeout
+		stopTimeout = *options.StopTimeout
+	}
+
+	startTimeout := uint(0)
+	if options.StartTimeout != nil {
+		startTimeout = *options.StartTimeout
 	}
 
 	config := ctr.Config()
@@ -185,7 +195,8 @@ func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSyste
 		ContainerNameOrID: nameOrID,
 		RestartPolicy:     define.DefaultRestartPolicy,
 		PIDFile:           conmonPidFile,
-		StopTimeout:       timeout,
+		TimeoutStartSec:   startTimeout,
+		StopTimeout:       stopTimeout,
 		GenerateTimestamp: true,
 		CreateCommand:     createCommand,
 		RunRoot:           runRoot,

pkg/systemd/generate/pods.go

@@ -195,9 +195,9 @@ func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) (
 		return nil, errors.Wrap(err, "could not find infra container")
 	}
 
-	timeout := infraCtr.StopTimeout()
+	stopTimeout := infraCtr.StopTimeout()
 	if options.StopTimeout != nil {
-		timeout = *options.StopTimeout
+		stopTimeout = *options.StopTimeout
 	}
 
 	config := infraCtr.Config()
@@ -223,7 +223,7 @@ func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) (
 		ServiceName:       serviceName,
 		InfraNameOrID:     ctrNameOrID,
 		PIDFile:           conmonPidFile,
-		StopTimeout:       timeout,
+		StopTimeout:       stopTimeout,
 		GenerateTimestamp: true,
 		CreateCommand:     createCommand,
 	}

test/e2e/generate_systemd_test.go

@@ -147,6 +147,15 @@ var _ = Describe("Podman generate systemd", func() {
 		session := podmanTest.Podman([]string{"generate", "systemd", "--time", "5", "nginx"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
+		Expect(session.OutputToString()).To(ContainSubstring("TimeoutStopSec=65"))
+		Expect(session.OutputToString()).ToNot(ContainSubstring("TimeoutStartSec="))
+		Expect(session.OutputToString()).To(ContainSubstring("podman stop -t 5"))
+
+		session = podmanTest.Podman([]string{"generate", "systemd", "--stop-timeout", "5", "--start-timeout", "123", "nginx"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		Expect(session.OutputToString()).To(ContainSubstring("TimeoutStartSec=123"))
+		Expect(session.OutputToString()).To(ContainSubstring("TimeoutStopSec=65"))
 		Expect(session.OutputToString()).To(ContainSubstring("podman stop -t 5"))
 	})
 

test/system/250-systemd.bats

@@ -138,7 +138,7 @@ function service_cleanup() {
 }
 
 # Regression test for #11438
-@test "podman generate systemd - restart policy" {
+@test "podman generate systemd - restart policy & timeouts" {
     cname=$(random_string)
     run_podman create --restart=always --name $cname $IMAGE
     run_podman generate systemd --new $cname