From ece0c7e5d31a6ae97274a7db80dbabb7564fdc72 Mon Sep 17 00:00:00 2001 From: Boaz Shuster Date: Thu, 29 Jul 2021 08:18:58 +0300 Subject: Support template unit files in podman generate systemd Signed-off-by: Boaz Shuster --- cmd/podman/generate/systemd.go | 11 +++++++- docs/source/markdown/podman-generate-systemd.1.md | 6 ++++ pkg/api/handlers/libpod/generate.go | 34 ++++++++++++----------- pkg/bindings/generate/types.go | 2 ++ pkg/bindings/generate/types_systemd_options.go | 15 ++++++++++ pkg/domain/entities/generate.go | 2 ++ pkg/domain/infra/tunnel/generate.go | 2 +- pkg/systemd/generate/common.go | 4 +-- pkg/systemd/generate/containers.go | 31 +++++++++++++++++++-- pkg/systemd/generate/pods.go | 5 ++++ test/system/250-systemd.bats | 29 +++++++++++++++++++ 11 files changed, 119 insertions(+), 22 deletions(-) diff --git a/cmd/podman/generate/systemd.go b/cmd/podman/generate/systemd.go index 2ab33c26b..cdc103865 100644 --- a/cmd/podman/generate/systemd.go +++ b/cmd/podman/generate/systemd.go @@ -21,6 +21,7 @@ import ( const ( restartPolicyFlagName = "restart-policy" timeFlagName = "time" + newFlagName = "new" ) var ( @@ -53,10 +54,11 @@ func init() { flags := systemdCmd.Flags() flags.BoolVarP(&systemdOptions.Name, "name", "n", false, "Use container/pod names instead of IDs") 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") _ = systemdCmd.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone) - flags.BoolVarP(&systemdOptions.New, "new", "", false, "Create a new container or pod instead of starting an existing one") + 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") containerPrefixFlagName := "container-prefix" @@ -93,6 +95,13 @@ func systemd(cmd *cobra.Command, args []string) error { logrus.Warnln("The generated units should be placed on your remote system") } + if cmd.Flags().Changed(newFlagName) && !systemdOptions.New && systemdOptions.TemplateUnitFile { + return errors.New("--template cannot be set with --new=false") + } + if !systemdOptions.New && systemdOptions.TemplateUnitFile { + systemdOptions.New = true + } + reports, err := registry.ContainerEngine().GenerateSystemd(registry.GetContext(), args[0], systemdOptions) if err != nil { return err diff --git a/docs/source/markdown/podman-generate-systemd.1.md b/docs/source/markdown/podman-generate-systemd.1.md index 8393aec11..fe5d1e46c 100644 --- a/docs/source/markdown/podman-generate-systemd.1.md +++ b/docs/source/markdown/podman-generate-systemd.1.md @@ -59,6 +59,12 @@ Set the systemd unit name prefix for pods. The default is *pod*. Set the systemd unit name separator between the name/id of a container/pod and the prefix. The default is *-*. +#### **--template** + +Add template specifiers to run multiple services from the systemd unit file. + +Note that if `--new` was not set to true, it is set to true by default. However, if `--new` is set to `false` explicitly the command will fail. + ## EXAMPLES ### Generate and print a systemd unit file for a container diff --git a/pkg/api/handlers/libpod/generate.go b/pkg/api/handlers/libpod/generate.go index 117c5e2aa..5205d875d 100644 --- a/pkg/api/handlers/libpod/generate.go +++ b/pkg/api/handlers/libpod/generate.go @@ -17,14 +17,15 @@ func GenerateSystemd(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) query := struct { - Name bool `schema:"useName"` - New bool `schema:"new"` - NoHeader bool `schema:"noHeader"` - RestartPolicy *string `schema:"restartPolicy"` - StopTimeout uint `schema:"stopTimeout"` - ContainerPrefix string `schema:"containerPrefix"` - PodPrefix string `schema:"podPrefix"` - Separator string `schema:"separator"` + Name bool `schema:"useName"` + New bool `schema:"new"` + NoHeader bool `schema:"noHeader"` + TemplateUnitFile bool `schema:"templateUnitFile"` + RestartPolicy *string `schema:"restartPolicy"` + StopTimeout uint `schema:"stopTimeout"` + ContainerPrefix string `schema:"containerPrefix"` + PodPrefix string `schema:"podPrefix"` + Separator string `schema:"separator"` }{ StopTimeout: util.DefaultContainerConfig().Engine.StopTimeout, ContainerPrefix: "container", @@ -40,14 +41,15 @@ func GenerateSystemd(w http.ResponseWriter, r *http.Request) { containerEngine := abi.ContainerEngine{Libpod: runtime} options := entities.GenerateSystemdOptions{ - Name: query.Name, - New: query.New, - NoHeader: query.NoHeader, - RestartPolicy: query.RestartPolicy, - StopTimeout: &query.StopTimeout, - ContainerPrefix: query.ContainerPrefix, - PodPrefix: query.PodPrefix, - Separator: query.Separator, + Name: query.Name, + New: query.New, + NoHeader: query.NoHeader, + TemplateUnitFile: query.TemplateUnitFile, + RestartPolicy: query.RestartPolicy, + StopTimeout: &query.StopTimeout, + ContainerPrefix: query.ContainerPrefix, + PodPrefix: query.PodPrefix, + Separator: query.Separator, } report, err := containerEngine.GenerateSystemd(r.Context(), utils.GetName(r), options) diff --git a/pkg/bindings/generate/types.go b/pkg/bindings/generate/types.go index 3c9ea87d4..6f2594604 100644 --- a/pkg/bindings/generate/types.go +++ b/pkg/bindings/generate/types.go @@ -16,6 +16,8 @@ type SystemdOptions struct { New *bool // NoHeader - Removes autogenerated by Podman and timestamp if set to true NoHeader *bool + // TemplateUnitFile - Create a template unit file that uses the identity specifiers + TemplateUnitFile *bool // RestartPolicy - systemd restart policy. RestartPolicy *string // StopTimeout - time when stopping the container. diff --git a/pkg/bindings/generate/types_systemd_options.go b/pkg/bindings/generate/types_systemd_options.go index 7a778a52b..b26aa7fc2 100644 --- a/pkg/bindings/generate/types_systemd_options.go +++ b/pkg/bindings/generate/types_systemd_options.go @@ -62,6 +62,21 @@ func (o *SystemdOptions) GetNoHeader() bool { return *o.NoHeader } +// WithTemplateUnitFile set field TemplateUnitFile to given value +func (o *SystemdOptions) WithTemplateUnitFile(value bool) *SystemdOptions { + o.TemplateUnitFile = &value + return o +} + +// GetTemplateUnitFile returns value of field TemplateUnitFile +func (o *SystemdOptions) GetTemplateUnitFile() bool { + if o.TemplateUnitFile == nil { + var z bool + return z + } + return *o.TemplateUnitFile +} + // WithRestartPolicy set field RestartPolicy to given value func (o *SystemdOptions) WithRestartPolicy(value string) *SystemdOptions { o.RestartPolicy = &value diff --git a/pkg/domain/entities/generate.go b/pkg/domain/entities/generate.go index 7809c5241..dfb5bfc6c 100644 --- a/pkg/domain/entities/generate.go +++ b/pkg/domain/entities/generate.go @@ -20,6 +20,8 @@ type GenerateSystemdOptions struct { Separator string // NoHeader - skip header generation NoHeader bool + // TemplateUnitFile - make use of %i and %I to differentiate between the different instances of the unit + TemplateUnitFile bool } // GenerateSystemdReport diff --git a/pkg/domain/infra/tunnel/generate.go b/pkg/domain/infra/tunnel/generate.go index 9f69abb1a..3a35dd59c 100644 --- a/pkg/domain/infra/tunnel/generate.go +++ b/pkg/domain/infra/tunnel/generate.go @@ -8,7 +8,7 @@ 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) + options := new(generate.SystemdOptions).WithUseName(opts.Name).WithContainerPrefix(opts.ContainerPrefix).WithNew(opts.New).WithNoHeader(opts.NoHeader).WithTemplateUnitFile(opts.TemplateUnitFile) options.WithPodPrefix(opts.PodPrefix).WithSeparator(opts.Separator) if opts.RestartPolicy != nil { options.WithRestartPolicy(*opts.RestartPolicy) diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go index 3515bb3b7..24c85a27e 100644 --- a/pkg/systemd/generate/common.go +++ b/pkg/systemd/generate/common.go @@ -23,7 +23,7 @@ func validateRestartPolicy(restart string) error { return errors.Errorf("%s is not a valid restart policy", restart) } -const headerTemplate = `# {{{{.ServiceName}}}}.service +const headerTemplate = `# {{{{.ServiceName}}}}{{{{- if (eq .IdentifySpecifier true) }}}}@{{{{- end}}}}.service {{{{- if (eq .GenerateNoHeader false) }}}} # autogenerated by Podman {{{{.PodmanVersion}}}} {{{{- if .TimeStamp}}}} @@ -32,7 +32,7 @@ const headerTemplate = `# {{{{.ServiceName}}}}.service {{{{- end}}}} [Unit] -Description=Podman {{{{.ServiceName}}}}.service +Description=Podman {{{{.ServiceName}}}}.service{{{{- if (eq .IdentifySpecifier true) }}}} for %I{{{{- end}}}} Documentation=man:podman-generate-systemd(1) Wants=network-online.target After=network-online.target diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index 037652a6d..c0a49c614 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -90,6 +90,8 @@ type containerInfo struct { // Location of the RunRoot for the container. Required for ensuring the tmpfs // or volume exists and is mounted when coming online at boot. RunRoot string + // Add %i and %I to description and execute parts + IdentifySpecifier bool } const containerTemplate = headerTemplate + ` @@ -99,7 +101,7 @@ After={{{{- range $index, $value := .BoundToServices -}}}}{{{{if $index}}}} {{{{ {{{{- end}}}} [Service] -Environment={{{{.EnvVariable}}}}=%n +Environment={{{{.EnvVariable}}}}=%n{{{{- if (eq .IdentifySpecifier true) }}}}-%i{{{{- end}}}} {{{{- if .ExtraEnvs}}}} Environment={{{{- range $index, $value := .ExtraEnvs -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}{{{{end}}}} {{{{- end}}}} @@ -273,7 +275,6 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst "--rm", ) remainingCmd := info.CreateCommand[index:] - // Presence check for certain flags/options. fs := pflag.NewFlagSet("args", pflag.ContinueOnError) fs.ParseErrorsWhitelist.UnknownFlags = true @@ -389,6 +390,32 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst startCommand = append(startCommand, remainingCmd...) startCommand = escapeSystemdArguments(startCommand) + if options.TemplateUnitFile { + info.IdentifySpecifier = true + runIx := -1 + nameIx := -1 + for argIx, arg := range startCommand { + if arg == "run" { + runIx = argIx + continue + } + if arg == "--name" { + nameIx = argIx + 1 + break + } + if strings.HasPrefix(arg, "--name=") { + nameIx = argIx + break + } + } + if nameIx == -1 { + startCommand = append(startCommand[:runIx+1], startCommand[runIx:]...) + startCommand[runIx+1] = fmt.Sprintf("--name=%s-%%i", info.ServiceName) + fmt.Println(startCommand) + } else { + startCommand[nameIx] = fmt.Sprintf("%s-%%i", startCommand[nameIx]) + } + } info.ExecStart = strings.Join(startCommand, " ") } diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go index e755b8eea..38f7e8e3e 100644 --- a/pkg/systemd/generate/pods.go +++ b/pkg/systemd/generate/pods.go @@ -79,6 +79,8 @@ type podInfo struct { // Location of the RunRoot for the pod. Required for ensuring the tmpfs // or volume exists and is mounted when coming online at boot. RunRoot string + // Add %i and %I to description and execute parts - this should not be used + IdentifySpecifier bool } const podTemplate = headerTemplate + `Requires={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}} @@ -108,6 +110,9 @@ WantedBy=multi-user.target default.target // Based on the options, the return value might be the content of all units or // the files they been written to. func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (map[string]string, error) { + if options.TemplateUnitFile { + return nil, errors.New("--template is not supported for pods") + } // Error out if the pod has no infra container, which we require to be the // main service. if !pod.HasInfraContainer() { diff --git a/test/system/250-systemd.bats b/test/system/250-systemd.bats index 98241c309..d4fb7ab08 100644 --- a/test/system/250-systemd.bats +++ b/test/system/250-systemd.bats @@ -201,4 +201,33 @@ LISTEN_FDNAMES=listen_fdnames" "LISTEN Environment passed: $context" check_listen_env "$stdenv" "podman start" } +@test "podman generate - systemd template" { + cname=$(random_string) + run_podman run -dt --name $cname $IMAGE top + + run_podman generate systemd --template -n $cname + is "$output" ".*%I.*" "%I indentifiers in template" + is "$output" ".*%i.*" "%i indentifiers in template" +} + +@test "podman generate - systemd template no support for pod" { + cname=$(random_string) + podname=$(random_string) + run_podman pod create --name $podname + run_podman run --pod $podname -dt --name $cname $IMAGE top + + run_podman 125 generate systemd --new --template -n $podname + is "$output" ".*--template is not supported for pods.*" "Error message contains 'not supported'" + + run_podman rm -f $cname + run_podman pod rm -f $podname +} + +@test "podman generate - systemd template only used on --new" { + cname=$(random_string) + run_podman create --name $cname $IMAGE top + run_podman 125 generate systemd --new=false --template -n $cname + is "$output" ".*--template cannot be set" "Error message should be '--template requires --new'" +} + # vim: filetype=sh -- cgit v1.2.3-54-g00ecf From 7494876000daa3f2a58d07f89561fd3ea447819a Mon Sep 17 00:00:00 2001 From: Boaz Shuster Date: Thu, 28 Oct 2021 13:31:54 +0300 Subject: Use systemctl in local system test Signed-off-by: Boaz Shuster --- test/system/250-systemd.bats | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/test/system/250-systemd.bats b/test/system/250-systemd.bats index d4fb7ab08..28e28b30c 100644 --- a/test/system/250-systemd.bats +++ b/test/system/250-systemd.bats @@ -9,6 +9,7 @@ load helpers.systemd SERVICE_NAME="podman_test_$(random_string)" UNIT_FILE="$UNIT_DIR/$SERVICE_NAME.service" +TEMPLATE_FILE_PREFIX="$UNIT_DIR/$SERVICE_NAME" function setup() { skip_if_remote "systemd tests are meaningless over remote" @@ -206,8 +207,37 @@ LISTEN_FDNAMES=listen_fdnames" "LISTEN Environment passed: $context" run_podman run -dt --name $cname $IMAGE top run_podman generate systemd --template -n $cname - is "$output" ".*%I.*" "%I indentifiers in template" - is "$output" ".*%i.*" "%i indentifiers in template" + echo "$output" > "$TEMPLATE_FILE_PREFIX@.service" + run_podman rm -f $cname + + systemctl daemon-reload + + INSTANCE="$SERVICE_NAME@1.service" + run systemctl start "$INSTANCE" + if [ $status -ne 0 ]; then + die "Error starting systemd unit $INSTANCE, output: $output" + fi + + run systemctl status "$INSTANCE" + if [ $status -ne 0 ]; then + die "Non-zero status of systemd unit $INSTANCE, output: $output" + fi + + run systemctl stop "$INSTANCE" + if [ $status -ne 0 ]; then + die "Error stopping systemd unit $INSTANCE, output: $output" + fi + + if [[ -z "$status" ]]; then + run systemctl is-active "$INSTANCE" + if [ $status -ne 0 ]; then + die "Error checking stauts of systemd unit $INSTANCE, output: $output" + fi + is "$output" "$status" "$INSTANCE not in expected state" + fi + + rm -f "$TEMPLATE_FILE_PREFIX@.service" + systemctl daemon-reload } @test "podman generate - systemd template no support for pod" { -- cgit v1.2.3-54-g00ecf From 15eb016017e39c7370d169790e15744746d54cd7 Mon Sep 17 00:00:00 2001 From: Boaz Shuster Date: Mon, 1 Nov 2021 19:53:20 +0200 Subject: add unit test to containers_test Signed-off-by: Boaz Shuster --- pkg/systemd/generate/containers_test.go | 73 ++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go index f46513459..85aeb92b8 100644 --- a/pkg/systemd/generate/containers_test.go +++ b/pkg/systemd/generate/containers_test.go @@ -519,6 +519,32 @@ ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all +[Install] +WantedBy=multi-user.target default.target +` + + templateGood := `# container-foo@.service +# autogenerated by Podman CI + +[Unit] +Description=Podman container-foo.service for %I +Documentation=man:podman-generate-systemd(1) +Wants=network-online.target +After=network-online.target +RequiresMountsFor=/var/run/containers/storage + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n-%i +Restart=on-failure +StartLimitBurst=42 +TimeoutStopSec=70 +ExecStartPre=/bin/rm -f %t/%n.ctr-id +ExecStart=/usr/bin/podman run --name=container-foo-%i --cidfile=%t/%n.ctr-id --cgroups=no-conmon --rm --sdnotify=conmon -d awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id +ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id +Type=notify +NotifyAccess=all + [Install] WantedBy=multi-user.target default.target ` @@ -529,6 +555,7 @@ WantedBy=multi-user.target default.target new bool noHeader bool wantErr bool + template bool }{ {"good with id", @@ -547,6 +574,7 @@ WantedBy=multi-user.target default.target false, false, false, + false, }, {"good with noHeader", containerInfo{ @@ -564,6 +592,7 @@ WantedBy=multi-user.target default.target false, true, false, + false, }, {"good with name", containerInfo{ @@ -581,6 +610,7 @@ WantedBy=multi-user.target default.target false, false, false, + false, }, {"good with name and bound to", containerInfo{ @@ -599,6 +629,7 @@ WantedBy=multi-user.target default.target false, false, false, + false, }, {"good with name and generic", containerInfo{ @@ -617,6 +648,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with name and sdnotify", containerInfo{ @@ -635,6 +667,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with explicit short detach param", containerInfo{ @@ -653,6 +686,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with explicit short detach param and podInfo", containerInfo{ @@ -674,6 +708,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with explicit full detach param", containerInfo{ @@ -692,6 +727,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with id and no param", containerInfo{ @@ -710,6 +746,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with explicit detach=true param", containerInfo{ @@ -728,6 +765,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with explicit detach=false param", containerInfo{ @@ -746,6 +784,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with explicit detach=false param", containerInfo{ @@ -764,6 +803,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with multiple detach=false params", containerInfo{ @@ -782,6 +822,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with multiple shorthand params detach first", containerInfo{ @@ -800,6 +841,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with multiple shorthand params detach last", containerInfo{ @@ -818,6 +860,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with container create", containerInfo{ @@ -836,6 +879,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with journald log tag (see #9034)", containerInfo{ @@ -854,6 +898,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with special chars", containerInfo{ @@ -872,6 +917,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with ID files", containerInfo{ @@ -890,6 +936,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with pod ID files", containerInfo{ @@ -911,6 +958,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with environment variables", containerInfo{ @@ -930,6 +978,7 @@ WantedBy=multi-user.target default.target true, false, false, + false, }, {"good with restart policy", containerInfo{ @@ -948,14 +997,34 @@ WantedBy=multi-user.target default.target true, false, false, + false, + }, + {"good template", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-foo", + ContainerNameOrID: "foo", + PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", + CreateCommand: []string{"I'll get stripped", "create", "--restart", "on-failure:42", "awesome-image:latest"}, + }, + templateGood, + true, + false, + false, + true, }, } for _, tt := range tests { test := tt t.Run(tt.name, func(t *testing.T) { opts := entities.GenerateSystemdOptions{ - New: test.new, - NoHeader: test.noHeader, + New: test.new, + NoHeader: test.noHeader, + TemplateUnitFile: test.template, } test.info.RestartPolicy = define.DefaultRestartPolicy got, err := executeContainerTemplate(&test.info, opts) -- cgit v1.2.3-54-g00ecf From 6ee3b33d3825177033763e51f756209102e0a309 Mon Sep 17 00:00:00 2001 From: Boaz Shuster Date: Mon, 8 Nov 2021 16:56:06 +0200 Subject: change from run to create in 250-systemd.bats Signed-off-by: Boaz Shuster --- pkg/systemd/generate/containers.go | 4 ++++ test/system/250-systemd.bats | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index c0a49c614..a4b18ec88 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -394,6 +394,10 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst info.IdentifySpecifier = true runIx := -1 nameIx := -1 + // Add systemd identify specifier next to the name value + // to set a name to the container according to the parameters passed to systemd. + // In case no --name set for that container, use ServiceName specified + // in the containerInfo struct. for argIx, arg := range startCommand { if arg == "run" { runIx = argIx diff --git a/test/system/250-systemd.bats b/test/system/250-systemd.bats index 28e28b30c..1c778a5e3 100644 --- a/test/system/250-systemd.bats +++ b/test/system/250-systemd.bats @@ -204,7 +204,7 @@ LISTEN_FDNAMES=listen_fdnames" "LISTEN Environment passed: $context" @test "podman generate - systemd template" { cname=$(random_string) - run_podman run -dt --name $cname $IMAGE top + run_podman create --name $cname $IMAGE top run_podman generate systemd --template -n $cname echo "$output" > "$TEMPLATE_FILE_PREFIX@.service" -- cgit v1.2.3-54-g00ecf From 73e1cdfe9e41a8748b75b9461c087e4cee5d2183 Mon Sep 17 00:00:00 2001 From: Boaz Shuster Date: Thu, 11 Nov 2021 11:45:36 +0200 Subject: export adding id-specifier code to setContainerNameForTemplate Signed-off-by: Boaz Shuster --- pkg/systemd/generate/containers.go | 69 ++++++++++++++++++++------------- pkg/systemd/generate/containers_test.go | 46 ++++++++++++++++++++++ 2 files changed, 89 insertions(+), 26 deletions(-) diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index a4b18ec88..95ff13371 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -206,6 +206,46 @@ func containerServiceName(ctr *libpod.Container, options entities.GenerateSystem return nameOrID, serviceName } +// setContainerNameForTemplate updates startCommand to contain the name argument with +// a value that includes the identify specifier. +// In case startCommand doesn't contain that argument it's added after "run" and its +// value will be set to info.ServiceName concated with the identify specifier %i. +func setContainerNameForTemplate(startCommand []string, info *containerInfo) ([]string, error) { + // find the index of "--name" in the command slice + nameIx := -1 + for argIx, arg := range startCommand { + if arg == "--name" { + nameIx = argIx + 1 + break + } + if strings.HasPrefix(arg, "--name=") { + nameIx = argIx + break + } + } + switch { + case nameIx == -1: + // if not found, add --name argument in the command slice before the "run" argument. + // it's assumed that the command slice contains this argument. + runIx := -1 + for argIx, arg := range startCommand { + if arg == "run" { + runIx = argIx + break + } + } + if runIx == -1 { + return startCommand, fmt.Errorf("\"run\" is missing in the command arguments") + } + startCommand = append(startCommand[:runIx+1], startCommand[runIx:]...) + startCommand[runIx+1] = fmt.Sprintf("--name=%s-%%i", info.ServiceName) + default: + // append the identity specifier (%i) to the end of the --name value + startCommand[nameIx] = fmt.Sprintf("%s-%%i", startCommand[nameIx]) + } + return startCommand, nil +} + // executeContainerTemplate executes the container template on the specified // containerInfo. Note that the containerInfo is also post processed and // completed, which allows for an easier unit testing. @@ -392,32 +432,9 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst startCommand = escapeSystemdArguments(startCommand) if options.TemplateUnitFile { info.IdentifySpecifier = true - runIx := -1 - nameIx := -1 - // Add systemd identify specifier next to the name value - // to set a name to the container according to the parameters passed to systemd. - // In case no --name set for that container, use ServiceName specified - // in the containerInfo struct. - for argIx, arg := range startCommand { - if arg == "run" { - runIx = argIx - continue - } - if arg == "--name" { - nameIx = argIx + 1 - break - } - if strings.HasPrefix(arg, "--name=") { - nameIx = argIx - break - } - } - if nameIx == -1 { - startCommand = append(startCommand[:runIx+1], startCommand[runIx:]...) - startCommand[runIx+1] = fmt.Sprintf("--name=%s-%%i", info.ServiceName) - fmt.Println(startCommand) - } else { - startCommand[nameIx] = fmt.Sprintf("%s-%%i", startCommand[nameIx]) + startCommand, err = setContainerNameForTemplate(startCommand, info) + if err != nil { + return "", err } } info.ExecStart = strings.Join(startCommand, " ") diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go index 85aeb92b8..eab2c2e67 100644 --- a/pkg/systemd/generate/containers_test.go +++ b/pkg/systemd/generate/containers_test.go @@ -1,6 +1,7 @@ package generate import ( + "fmt" "testing" "github.com/containers/podman/v3/pkg/domain/entities" @@ -1036,3 +1037,48 @@ WantedBy=multi-user.target default.target }) } } + +func TestSetContainerNameForTemplate(t *testing.T) { + tt := []struct { + name string + startCommand []string + info *containerInfo + expected []string + err error + }{ + { + name: "no name argument is set", + startCommand: []string{"/usr/bin/podman", "run", "busybox", "top"}, + info: &containerInfo{ServiceName: "container-122"}, + expected: []string{"/usr/bin/podman", "run", "--name=container-122-%i", "busybox", "top"}, + err: nil, + }, + { + name: "--name=value is used in arguments", + startCommand: []string{"/usr/bin/podman", "run", "--name=lovely_james", "busybox", "top"}, + info: &containerInfo{}, + expected: []string{"/usr/bin/podman", "run", "--name=lovely_james-%i", "busybox", "top"}, + err: nil, + }, + { + name: "--name value is used in arguments", + startCommand: []string{"/usr/bin/podman", "run", "--name", "lovely_james", "busybox", "top"}, + info: &containerInfo{}, + expected: []string{"/usr/bin/podman", "run", "--name", "lovely_james-%i", "busybox", "top"}, + err: nil, + }, + { + name: "--name value is used in arguments", + startCommand: []string{"/usr/bin/podman", "create", "busybox", "top"}, + info: &containerInfo{}, + expected: []string{"/usr/bin/podman", "create", "busybox", "top"}, + err: fmt.Errorf("\"run\" is missing in the command arguments"), + }, + } + + for _, te := range tt { + res, err := setContainerNameForTemplate(te.startCommand, te.info) + assert.Equal(t, te.err, err) + assert.Equal(t, te.expected, res) + } +} -- cgit v1.2.3-54-g00ecf