From 714e5a13d9586ac6a3a6a1ee1b2ec15a43058350 Mon Sep 17 00:00:00 2001 From: Nirmal Patel Date: Sat, 19 Feb 2022 17:55:23 -0500 Subject: Separator is no longer prepended when prefix is empty on podman generate systemd When podman generate systemd is invoked, it previously did not check if container-prefix or pod-prefix are empty. When these are empty, the file name starts with the separator, which is hyphen by default. This results in files like '-containername.service'. The code now checks if these prefixes are empty. If they are, the filename no longer adds a separator. Instead, it uses name or ID of the container or pod. Closes #13272 Signed-off-by: Nirmal Patel --- pkg/api/handlers/libpod/generate.go | 34 ++++++++++++++------- pkg/systemd/generate/common.go | 14 +++++++++ pkg/systemd/generate/containers.go | 4 ++- pkg/systemd/generate/containers_test.go | 42 ++++++++++++++++++++++++++ pkg/systemd/generate/pods.go | 3 +- pkg/systemd/generate/pods_test.go | 44 ++++++++++++++++++++++++++++ test/e2e/generate_systemd_test.go | 52 +++++++++++++++++++++++++++++++++ 7 files changed, 180 insertions(+), 13 deletions(-) diff --git a/pkg/api/handlers/libpod/generate.go b/pkg/api/handlers/libpod/generate.go index 7e08dd4a8..28785b00d 100644 --- a/pkg/api/handlers/libpod/generate.go +++ b/pkg/api/handlers/libpod/generate.go @@ -25,18 +25,15 @@ func GenerateSystemd(w http.ResponseWriter, r *http.Request) { RestartSec uint `schema:"restartSec"` StopTimeout uint `schema:"stopTimeout"` StartTimeout uint `schema:"startTimeout"` - ContainerPrefix string `schema:"containerPrefix"` - PodPrefix string `schema:"podPrefix"` - Separator string `schema:"separator"` + ContainerPrefix *string `schema:"containerPrefix"` + PodPrefix *string `schema:"podPrefix"` + Separator *string `schema:"separator"` Wants []string `schema:"wants"` After []string `schema:"after"` Requires []string `schema:"requires"` }{ - StartTimeout: 0, - StopTimeout: util.DefaultContainerConfig().Engine.StopTimeout, - ContainerPrefix: "container", - PodPrefix: "pod", - Separator: "-", + StartTimeout: 0, + StopTimeout: util.DefaultContainerConfig().Engine.StopTimeout, } if err := decoder.Decode(&query, r.URL.Query()); err != nil { @@ -44,6 +41,21 @@ func GenerateSystemd(w http.ResponseWriter, r *http.Request) { return } + var ContainerPrefix = "container" + if query.ContainerPrefix != nil { + ContainerPrefix = *query.ContainerPrefix + } + + var PodPrefix = "pod" + if query.PodPrefix != nil { + PodPrefix = *query.PodPrefix + } + + var Separator = "-" + if query.Separator != nil { + Separator = *query.Separator + } + containerEngine := abi.ContainerEngine{Libpod: runtime} options := entities.GenerateSystemdOptions{ Name: query.Name, @@ -53,9 +65,9 @@ func GenerateSystemd(w http.ResponseWriter, r *http.Request) { RestartPolicy: query.RestartPolicy, StartTimeout: &query.StartTimeout, StopTimeout: &query.StopTimeout, - ContainerPrefix: query.ContainerPrefix, - PodPrefix: query.PodPrefix, - Separator: query.Separator, + ContainerPrefix: ContainerPrefix, + PodPrefix: PodPrefix, + Separator: Separator, RestartSec: &query.RestartSec, Wants: query.Wants, After: query.After, diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go index a6f8f7cd4..e53d37897 100644 --- a/pkg/systemd/generate/common.go +++ b/pkg/systemd/generate/common.go @@ -137,3 +137,17 @@ func removeArg(arg string, args []string) []string { } return newArgs } + +// This function is used to get name of systemd service from prefix, separator, and +// container/pod name. If prefix is empty, the service name does not include the +// separator. This is to avoid a situation where service name starts with the separator +// which is usually hyphen. +func getServiceName(prefix string, separator string, name string) string { + serviceName := name + + if len(prefix) > 0 { + serviceName = prefix + separator + name + } + + return serviceName +} diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index ea829c810..c01bb1baf 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -236,7 +236,9 @@ func containerServiceName(ctr *libpod.Container, options entities.GenerateSystem if options.Name { nameOrID = ctr.Name() } - serviceName := fmt.Sprintf("%s%s%s", options.ContainerPrefix, options.Separator, nameOrID) + + serviceName := getServiceName(options.ContainerPrefix, options.Separator, nameOrID) + return nameOrID, serviceName } diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go index 2f653a4b9..b9bf7c317 100644 --- a/pkg/systemd/generate/containers_test.go +++ b/pkg/systemd/generate/containers_test.go @@ -87,6 +87,30 @@ ExecStopPost=/usr/bin/podman stop -t 10 foobar PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking +[Install] +WantedBy=default.target +` + + goodNameEmptyContainerPrefix := `# foobar.service +# autogenerated by Podman CI + +[Unit] +Description=Podman foobar.service +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 +Restart=on-failure +TimeoutStopSec=70 +ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar +ExecStopPost=/usr/bin/podman stop -t 10 foobar +PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +Type=forking + [Install] WantedBy=default.target ` @@ -1206,6 +1230,24 @@ WantedBy=default.target false, true, }, + {"good with name and empty container-prefix", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "foobar", + ContainerNameOrID: "foobar", + PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", + }, + goodNameEmptyContainerPrefix, + false, + false, + false, + false, + }, } for _, tt := range tests { test := tt diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go index 003c23e77..78ae6391b 100644 --- a/pkg/systemd/generate/pods.go +++ b/pkg/systemd/generate/pods.go @@ -242,7 +242,8 @@ func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) ( nameOrID = pod.Name() ctrNameOrID = infraCtr.Name() } - serviceName := fmt.Sprintf("%s%s%s", options.PodPrefix, options.Separator, nameOrID) + + serviceName := getServiceName(options.PodPrefix, options.Separator, nameOrID) info := podInfo{ ServiceName: serviceName, diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go index b37e0825b..dcb18780c 100644 --- a/pkg/systemd/generate/pods_test.go +++ b/pkg/systemd/generate/pods_test.go @@ -67,6 +67,32 @@ WantedBy=default.target podGood := serviceInfo + headerInfo + podContent podGoodNoHeaderInfo := serviceInfo + podContent + podGoodWithEmptyPrefix := `# 123abc.service +# autogenerated by Podman CI + +[Unit] +Description=Podman 123abc.service +Documentation=man:podman-generate-systemd(1) +Wants=network-online.target +After=network-online.target +RequiresMountsFor=/var/run/containers/storage +Requires=container-1.service container-2.service +Before=container-1.service container-2.service + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=on-failure +TimeoutStopSec=102 +ExecStart=/usr/bin/podman start jadda-jadda-infra +ExecStop=/usr/bin/podman stop -t 42 jadda-jadda-infra +ExecStopPost=/usr/bin/podman stop -t 42 jadda-jadda-infra +PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +Type=forking + +[Install] +WantedBy=default.target +` + podGoodCustomWants := `# pod-123abc.service # autogenerated by Podman CI @@ -580,6 +606,24 @@ WantedBy=default.target false, false, }, + {"pod with empty pod-prefix", + podInfo{ + Executable: "/usr/bin/podman", + ServiceName: "123abc", + InfraNameOrID: "jadda-jadda-infra", + PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", + RequiredServices: []string{"container-1", "container-2"}, + CreateCommand: []string{"podman", "pod", "create", "--name", "foo", "bar=arg with space"}, + }, + podGoodWithEmptyPrefix, + false, + false, + false, + }, } for _, tt := range tests { diff --git a/test/e2e/generate_systemd_test.go b/test/e2e/generate_systemd_test.go index 55b9a8037..e4b854332 100644 --- a/test/e2e/generate_systemd_test.go +++ b/test/e2e/generate_systemd_test.go @@ -423,6 +423,20 @@ var _ = Describe("Podman generate systemd", func() { }) + It("podman generate systemd --container-prefix ''", func() { + n := podmanTest.Podman([]string{"create", "--name", "foo", "alpine", "top"}) + n.WaitWithDefaultTimeout() + Expect(n).Should(Exit(0)) + + session := podmanTest.Podman([]string{"generate", "systemd", "--name", "--container-prefix", "", "foo"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + // Grepping the output (in addition to unit tests) + Expect(session.OutputToString()).To(ContainSubstring("# foo.service")) + + }) + It("podman generate systemd --separator _", func() { n := podmanTest.Podman([]string{"create", "--name", "foo", "alpine", "top"}) n.WaitWithDefaultTimeout() @@ -485,6 +499,44 @@ var _ = Describe("Podman generate systemd", func() { Expect(session.OutputToString()).To(ContainSubstring("BindsTo=p_foo.service")) }) + It("podman generate systemd pod --pod-prefix '' --container-prefix '' --separator _ change all prefixes/separator", func() { + n := podmanTest.Podman([]string{"pod", "create", "--name", "foo"}) + n.WaitWithDefaultTimeout() + Expect(n).Should(Exit(0)) + + n = podmanTest.Podman([]string{"create", "--pod", "foo", "--name", "foo-1", "alpine", "top"}) + n.WaitWithDefaultTimeout() + Expect(n).Should(Exit(0)) + + n = podmanTest.Podman([]string{"create", "--pod", "foo", "--name", "foo-2", "alpine", "top"}) + n.WaitWithDefaultTimeout() + Expect(n).Should(Exit(0)) + + // test systemd generate with empty pod prefix + session1 := podmanTest.Podman([]string{"generate", "systemd", "--pod-prefix", "", "--name", "foo"}) + session1.WaitWithDefaultTimeout() + Expect(session1).Should(Exit(0)) + + // Grepping the output (in addition to unit tests) + Expect(session1.OutputToString()).To(ContainSubstring("# foo.service")) + Expect(session1.OutputToString()).To(ContainSubstring("Requires=container-foo-1.service container-foo-2.service")) + Expect(session1.OutputToString()).To(ContainSubstring("# container-foo-1.service")) + Expect(session1.OutputToString()).To(ContainSubstring("BindsTo=foo.service")) + + // test systemd generate with empty container and pod prefix + session2 := podmanTest.Podman([]string{"generate", "systemd", "--container-prefix", "", "--pod-prefix", "", "--separator", "_", "--name", "foo"}) + session2.WaitWithDefaultTimeout() + Expect(session2).Should(Exit(0)) + + // Grepping the output (in addition to unit tests) + Expect(session2.OutputToString()).To(ContainSubstring("# foo.service")) + Expect(session2.OutputToString()).To(ContainSubstring("Requires=foo-1.service foo-2.service")) + Expect(session2.OutputToString()).To(ContainSubstring("# foo-1.service")) + Expect(session2.OutputToString()).To(ContainSubstring("# foo-2.service")) + Expect(session2.OutputToString()).To(ContainSubstring("BindsTo=foo.service")) + + }) + It("podman generate systemd pod with containers --new", func() { tmpDir, err := ioutil.TempDir("", "") Expect(err).To(BeNil()) -- cgit v1.2.3-54-g00ecf