From 5747fd71c98238d42cf9cd307fdf4d59747a6e1d Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 26 Jan 2021 15:38:05 +0100 Subject: remote exec: write conmon error on hijacked connection Make sure to write error from conmon on the hijacked http connection. This fixes issues where errors were not reported on the client side, for instance, when specified command was not found on the container. To future generations: I am sorry. The code is complex, and there are many interdependencies among the concurrent goroutines. I added more complexity on top but I don't have a good idea of how to reduce complexity in the available time. Fixes: #8281 Signed-off-by: Valentin Rothberg --- test/system/075-exec.bats | 2 -- 1 file changed, 2 deletions(-) (limited to 'test') diff --git a/test/system/075-exec.bats b/test/system/075-exec.bats index c028e16c9..badf44c49 100644 --- a/test/system/075-exec.bats +++ b/test/system/075-exec.bats @@ -6,8 +6,6 @@ load helpers @test "podman exec - basic test" { - skip_if_remote "FIXME: pending #7241" - rand_filename=$(random_string 20) rand_content=$(random_string 50) -- cgit v1.2.3-54-g00ecf From bb88db783c1756291e8047a1a7ffd214477f7f9b Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 19 Jan 2021 13:41:50 -0500 Subject: Ensure the Volumes field in Compat Create is honored Docker has, for unclear reasons, three separate fields in their Create Container struct in which volumes can be placed. Right now we support two of those - Binds and Mounts, which (roughly) correspond to `-v` and `--mount` respectively. Unfortunately, we did not support the third, `Volumes`, which is used for anonymous named volumes created by `-v` (e.g. `-v /test`). It seems that volumes listed here are *not* included in the remaining two from my investigation, so it should be safe to just append them into our handling of the `Binds` (`-v`) field. Fixes #8649 Signed-off-by: Matthew Heon --- cmd/podman/common/create_opts.go | 26 ++++++++++++++++++++++++-- test/apiv2/20-containers.at | 9 +++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go index f252618ce..f3918d233 100644 --- a/cmd/podman/common/create_opts.go +++ b/cmd/podman/common/create_opts.go @@ -3,6 +3,7 @@ package common import ( "fmt" "net" + "path/filepath" "strconv" "strings" @@ -383,8 +384,29 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, cgroup } // volumes - if volumes := cc.HostConfig.Binds; len(volumes) > 0 { - cliOpts.Volume = volumes + volDestinations := make(map[string]bool) + for _, vol := range cc.HostConfig.Binds { + cliOpts.Volume = append(cliOpts.Volume, vol) + // Extract the destination so we don't add duplicate mounts in + // the volumes phase. + splitVol := strings.SplitN(vol, ":", 3) + switch len(splitVol) { + case 1: + volDestinations[vol] = true + default: + volDestinations[splitVol[1]] = true + } + } + // Anonymous volumes are added differently from other volumes, in their + // own special field, for reasons known only to Docker. Still use the + // format of `-v` so we can just append them in there. + // Unfortunately, these may be duplicates of existing mounts in Binds. + // So... We need to catch that. + for vol := range cc.Volumes { + if _, ok := volDestinations[filepath.Clean(vol)]; ok { + continue + } + cliOpts.Volume = append(cliOpts.Volume, vol) } if len(cc.HostConfig.BlkioWeightDevice) > 0 { devices := make([]string, 0, len(cc.HostConfig.BlkioWeightDevice)) diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index decdc4754..0da196e46 100644 --- a/test/apiv2/20-containers.at +++ b/test/apiv2/20-containers.at @@ -237,3 +237,12 @@ t GET containers/$cid/json 200 \ t DELETE containers/$cid 204 t DELETE images/${MultiTagName}?force=true 200 # vim: filetype=sh + +# Test Volumes field adds an anonymous volume +t POST containers/create '"Image":"'$IMAGE'","Volumes":{"/test":{}}' 201 \ + .Id~[0-9a-f]\\{64\\} +cid=$(jq -r '.Id' <<<"$output") +t GET containers/$cid/json 200 \ + .Mounts[0].Destination="/test" + +t DELETE containers/$cid?v=true 204 -- cgit v1.2.3-54-g00ecf From b4aa54c7561f7093f795093a72c75248018f3517 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 27 Jan 2021 10:42:22 +0100 Subject: Fix podman history --no-trunc for the CREATED BY field Fixes #9120 Signed-off-by: Paul Holzinger --- cmd/podman/images/history.go | 2 +- test/e2e/history_test.go | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/cmd/podman/images/history.go b/cmd/podman/images/history.go index 964c7a975..af40dd73a 100644 --- a/cmd/podman/images/history.go +++ b/cmd/podman/images/history.go @@ -162,7 +162,7 @@ func (h historyReporter) Size() string { } func (h historyReporter) CreatedBy() string { - if len(h.ImageHistoryLayer.CreatedBy) > 45 { + if !opts.noTrunc && len(h.ImageHistoryLayer.CreatedBy) > 45 { return h.ImageHistoryLayer.CreatedBy[:45-3] + "..." } return h.ImageHistoryLayer.CreatedBy diff --git a/test/e2e/history_test.go b/test/e2e/history_test.go index fea3f4d43..1c57c60de 100644 --- a/test/e2e/history_test.go +++ b/test/e2e/history_test.go @@ -65,6 +65,23 @@ var _ = Describe("Podman history", func() { session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) Expect(len(session.OutputToStringArray())).To(BeNumerically(">", 0)) + + session = podmanTest.Podman([]string{"history", "--no-trunc", "--format", "{{.ID}}", ALPINE}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + lines := session.OutputToStringArray() + Expect(len(lines)).To(BeNumerically(">", 0)) + // the image id must be 64 chars long + Expect(len(lines[0])).To(BeNumerically("==", 64)) + + session = podmanTest.Podman([]string{"history", "--no-trunc", "--format", "{{.CreatedBy}}", ALPINE}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + lines = session.OutputToStringArray() + Expect(len(lines)).To(BeNumerically(">", 0)) + Expect(session.OutputToString()).ToNot(ContainSubstring("...")) + // the second line in the alpine history contains a command longer than 45 chars + Expect(len(lines[1])).To(BeNumerically(">", 45)) }) It("podman history with json flag", func() { -- cgit v1.2.3-54-g00ecf From 2393dd304de3453fabfb7eac99ef6b2fb308bd29 Mon Sep 17 00:00:00 2001 From: baude Date: Mon, 25 Jan 2021 13:58:04 -0600 Subject: Add default net info in container inspect when inspecting a container that is only connected to the default network, we should populate the default network in the container inspect information. Fixes: #6618 Signed-off-by: baude MH: Small fixes, added another test Signed-off-by: Matthew Heon --- libpod/networking_linux.go | 8 +++++--- test/e2e/inspect_test.go | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) (limited to 'test') diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index addf1814c..94b8563fe 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -955,7 +955,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e if c.state.NetNS == nil { // We still want to make dummy configurations for each CNI net // the container joined. - if len(networks) > 0 && !isDefault { + if len(networks) > 0 { settings.Networks = make(map[string]*define.InspectAdditionalNetwork, len(networks)) for _, net := range networks { cniNet := new(define.InspectAdditionalNetwork) @@ -976,7 +976,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e } // If we have CNI networks - handle that here - if len(networks) > 0 && !isDefault { + if len(networks) > 0 { if len(networks) != len(c.state.NetworkStatus) { return nil, errors.Wrapf(define.ErrInternal, "network inspection mismatch: asked to join %d CNI network(s) %v, but have information on %d network(s)", len(networks), networks, len(c.state.NetworkStatus)) } @@ -1006,7 +1006,9 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e settings.Networks[name] = addedNet } - return settings, nil + if !isDefault { + return settings, nil + } } // If not joining networks, we should have at most 1 result diff --git a/test/e2e/inspect_test.go b/test/e2e/inspect_test.go index 97f77414e..8fc9721f9 100644 --- a/test/e2e/inspect_test.go +++ b/test/e2e/inspect_test.go @@ -443,4 +443,27 @@ var _ = Describe("Podman inspect", func() { Expect(inspect.OutputToString()).To(Equal(`"{"80/tcp":[{"HostIp":"","HostPort":"8080"}]}"`)) }) + It("Verify container inspect has default network", func() { + SkipIfRootless("Requires root CNI networking") + ctrName := "testctr" + session := podmanTest.Podman([]string{"run", "-d", "--name", ctrName, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(BeZero()) + + inspect := podmanTest.InspectContainer(ctrName) + Expect(len(inspect)).To(Equal(1)) + Expect(len(inspect[0].NetworkSettings.Networks)).To(Equal(1)) + }) + + It("Verify stopped container still has default network in inspect", func() { + SkipIfRootless("Requires root CNI networking") + ctrName := "testctr" + session := podmanTest.Podman([]string{"create", "--name", ctrName, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(BeZero()) + + inspect := podmanTest.InspectContainer(ctrName) + Expect(len(inspect)).To(Equal(1)) + Expect(len(inspect[0].NetworkSettings.Networks)).To(Equal(1)) + }) }) -- cgit v1.2.3-54-g00ecf From c48753bfa0d56b855f71dc6ad62e8eff62cf98e1 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 21 Jan 2021 15:41:14 +0100 Subject: workdir presence checks A container's workdir can be specified via the CLI via `--workdir` and via an image config with the CLI having precedence. Since images have a tendency to specify workdirs without necessarily shipping the paths with the root FS, make sure that Podman creates the workdir. When specified via the CLI, do not create the path, but check for its existence and return a human-friendly error. NOTE: `crun` is performing a similar check that would yield exit code 127. With this change, however, Podman performs the check and yields exit code 126. Since this is specific to `crun`, I do not consider it to be a breaking change of Podman. Fixes: #9040 Signed-off-by: Valentin Rothberg --- libpod/container_internal_linux.go | 63 +++++++++++++++++++++++++------- libpod/container_path_resolution.go | 47 ++++++++++++++++++++++-- pkg/specgen/generate/container.go | 14 ------- pkg/specgen/generate/container_create.go | 12 +++++- test/e2e/run_working_dir_test.go | 6 +-- test/system/030-run.bats | 21 +++++++++++ test/system/070-build.bats | 4 +- 7 files changed, 127 insertions(+), 40 deletions(-) (limited to 'test') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 0553cc59c..9c5989d23 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -174,25 +174,60 @@ func (c *Container) prepare() error { return err } - // Ensure container entrypoint is created (if required) - if c.config.CreateWorkingDir { - workdir, err := securejoin.SecureJoin(c.state.Mountpoint, c.WorkingDir()) - if err != nil { - return errors.Wrapf(err, "error creating path to container %s working dir", c.ID()) - } - rootUID := c.RootUID() - rootGID := c.RootGID() + // Make sure the workdir exists + if err := c.resolveWorkDir(); err != nil { + return err + } - if err := os.MkdirAll(workdir, 0755); err != nil { - if os.IsExist(err) { - return nil + return nil +} + +// resolveWorkDir resolves the container's workdir and, depending on the +// configuration, will create it, or error out if it does not exist. +// Note that the container must be mounted before. +func (c *Container) resolveWorkDir() error { + workdir := c.WorkingDir() + + // If the specified workdir is a subdir of a volume or mount, + // we don't need to do anything. The runtime is taking care of + // that. + if isPathOnVolume(c, workdir) || isPathOnBindMount(c, workdir) { + logrus.Debugf("Workdir %q resolved to a volume or mount", workdir) + return nil + } + + _, resolvedWorkdir, err := c.resolvePath(c.state.Mountpoint, workdir) + if err != nil { + return err + } + logrus.Debugf("Workdir %q resolved to host path %q", workdir, resolvedWorkdir) + + // No need to create it (e.g., `--workdir=/foo`), so let's make sure + // the path exists on the container. + if !c.config.CreateWorkingDir { + if _, err := os.Stat(resolvedWorkdir); err != nil { + if os.IsNotExist(err) { + return errors.Errorf("workdir %q does not exist on container %s", workdir, c.ID()) } - return errors.Wrapf(err, "error creating container %s working dir", c.ID()) + // This might be a serious error (e.g., permission), so + // we need to return the full error. + return errors.Wrapf(err, "error detecting workdir %q on container %s", workdir, c.ID()) } + } + + // Ensure container entrypoint is created (if required). + rootUID := c.RootUID() + rootGID := c.RootGID() - if err := os.Chown(workdir, rootUID, rootGID); err != nil { - return errors.Wrapf(err, "error chowning container %s working directory to container root", c.ID()) + if err := os.MkdirAll(resolvedWorkdir, 0755); err != nil { + if os.IsExist(err) { + return nil } + return errors.Wrapf(err, "error creating container %s workdir", c.ID()) + } + + if err := os.Chown(resolvedWorkdir, rootUID, rootGID); err != nil { + return errors.Wrapf(err, "error chowning container %s workdir to container root", c.ID()) } return nil diff --git a/libpod/container_path_resolution.go b/libpod/container_path_resolution.go index 68f174278..805b3b947 100644 --- a/libpod/container_path_resolution.go +++ b/libpod/container_path_resolution.go @@ -13,6 +13,11 @@ import ( // resolveContainerPaths resolves the container's mount point and the container // path as specified by the user. Both may resolve to paths outside of the // container's mount point when the container path hits a volume or bind mount. +// +// It returns a bool, indicating whether containerPath resolves outside of +// mountPoint (e.g., via a mount or volume), the resolved root (e.g., container +// mount, bind mount or volume) and the resolved path on the root (absolute to +// the host). func (container *Container) resolvePath(mountPoint string, containerPath string) (string, string, error) { // Let's first make sure we have a path relative to the mount point. pathRelativeToContainerMountPoint := containerPath @@ -97,8 +102,8 @@ func (container *Container) resolvePath(mountPoint string, containerPath string) return mountPoint, resolvedPathOnTheContainerMountPoint, nil } -// findVolume checks if the specified container path matches a volume inside -// the container. It returns a matching volume or nil. +// findVolume checks if the specified containerPath matches the destination +// path of a Volume. Returns a matching Volume or nil. func findVolume(c *Container, containerPath string) (*Volume, error) { runtime := c.Runtime() cleanedContainerPath := filepath.Clean(containerPath) @@ -110,8 +115,25 @@ func findVolume(c *Container, containerPath string) (*Volume, error) { return nil, nil } -// findBindMount checks if the specified container path matches a bind mount -// inside the container. It returns a matching mount or nil. +// isPathOnVolume returns true if the specified containerPath is a subdir of any +// Volume's destination. +func isPathOnVolume(c *Container, containerPath string) bool { + cleanedContainerPath := filepath.Clean(containerPath) + for _, vol := range c.Config().NamedVolumes { + if cleanedContainerPath == filepath.Clean(vol.Dest) { + return true + } + for dest := vol.Dest; dest != "/"; dest = filepath.Dir(dest) { + if cleanedContainerPath == dest { + return true + } + } + } + return false +} + +// findBindMounts checks if the specified containerPath matches the destination +// path of a Mount. Returns a matching Mount or nil. func findBindMount(c *Container, containerPath string) *specs.Mount { cleanedPath := filepath.Clean(containerPath) for _, m := range c.Config().Spec.Mounts { @@ -125,3 +147,20 @@ func findBindMount(c *Container, containerPath string) *specs.Mount { } return nil } + +/// isPathOnBindMount returns true if the specified containerPath is a subdir of any +// Mount's destination. +func isPathOnBindMount(c *Container, containerPath string) bool { + cleanedContainerPath := filepath.Clean(containerPath) + for _, m := range c.Config().Spec.Mounts { + if cleanedContainerPath == filepath.Clean(m.Destination) { + return true + } + for dest := m.Destination; dest != "/"; dest = filepath.Dir(dest) { + if cleanedContainerPath == dest { + return true + } + } + } + return false +} diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go index cc3f7928c..31d317bf8 100644 --- a/pkg/specgen/generate/container.go +++ b/pkg/specgen/generate/container.go @@ -203,20 +203,6 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat } s.Annotations = annotations - // workdir - if s.WorkDir == "" { - if newImage != nil { - workingDir, err := newImage.WorkingDir(ctx) - if err != nil { - return nil, err - } - s.WorkDir = workingDir - } - } - if s.WorkDir == "" { - s.WorkDir = "/" - } - if len(s.SeccompProfilePath) < 1 { p, err := libpod.DefaultSeccompPath() if err != nil { diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index 4f36744ca..1bc050b00 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -272,10 +272,18 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. if s.Entrypoint != nil { options = append(options, libpod.WithEntrypoint(s.Entrypoint)) } - // If the user did not set an workdir but the image did, ensure it is - // created. + // If the user did not specify a workdir on the CLI, let's extract it + // from the image. if s.WorkDir == "" && img != nil { options = append(options, libpod.WithCreateWorkingDir()) + wd, err := img.WorkingDir(ctx) + if err != nil { + return nil, err + } + s.WorkDir = wd + } + if s.WorkDir == "" { + s.WorkDir = "/" } if s.StopSignal != nil { options = append(options, libpod.WithStopSignal(*s.StopSignal)) diff --git a/test/e2e/run_working_dir_test.go b/test/e2e/run_working_dir_test.go index 7d8db361c..59538448e 100644 --- a/test/e2e/run_working_dir_test.go +++ b/test/e2e/run_working_dir_test.go @@ -2,7 +2,6 @@ package integration import ( "os" - "strings" . "github.com/containers/podman/v2/test/utils" . "github.com/onsi/ginkgo" @@ -41,12 +40,9 @@ var _ = Describe("Podman run", func() { }) It("podman run a container using non existing --workdir", func() { - if !strings.Contains(podmanTest.OCIRuntime, "crun") { - Skip("Test only works on crun") - } session := podmanTest.Podman([]string{"run", "--workdir", "/home/foobar", ALPINE, "pwd"}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(Equal(127)) + Expect(session.ExitCode()).To(Equal(126)) }) It("podman run a container on an image with a workdir", func() { diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 29dc95dc3..dcf1da370 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -589,4 +589,25 @@ json-file | f is "${lines[1]}" "$rand" "Container runs successfully despite warning" } +@test "podman run - check workdir" { + # Workdirs specified via the CLI are not created on the root FS. + run_podman 126 run --rm --workdir /i/do/not/exist $IMAGE pwd + # Note: remote error prepends an attach error. + is "$output" "Error: .*workdir \"/i/do/not/exist\" does not exist on container.*" + + testdir=$PODMAN_TMPDIR/volume + mkdir -p $testdir + randomcontent=$(random_string 10) + echo "$randomcontent" > $testdir/content + + # Workdir does not exist on the image but is volume mounted. + run_podman run --rm --workdir /IamNotOnTheImage -v $testdir:/IamNotOnTheImage $IMAGE cat content + is "$output" "$randomcontent" "cat random content" + + # Workdir does not exist on the image but is created by the runtime as it's + # a subdir of a volume. + run_podman run --rm --workdir /IamNotOntheImage -v $testdir/content:/IamNotOntheImage/foo $IMAGE cat foo + is "$output" "$randomcontent" "cat random content" +} + # vim: filetype=sh diff --git a/test/system/070-build.bats b/test/system/070-build.bats index 0e83a184b..bf9fa789c 100644 --- a/test/system/070-build.bats +++ b/test/system/070-build.bats @@ -145,10 +145,12 @@ EOF https_proxy=https-proxy-in-env-file EOF + # NOTE: it's important to not create the workdir. + # Podman will make sure to create a missing workdir + # if needed. See #9040. cat >$tmpdir/Containerfile < Date: Wed, 20 Jan 2021 14:30:42 +0100 Subject: Make generate systemd --new robust against double curly braces If the container create command contains an argument with double curly braces the golang template parsing can fail since it tries to interpret the value as variable. To fix this change the default delimiter for the internal template to `{{{{`. Fixes #9034 Signed-off-by: Paul Holzinger --- pkg/systemd/generate/common.go | 12 ++++---- pkg/systemd/generate/containers.go | 48 +++++++++++++++--------------- pkg/systemd/generate/containers_test.go | 39 +++++++++++++++++++++++++ pkg/systemd/generate/pods.go | 52 ++++++++++++++++----------------- pkg/systemd/generate/pods_test.go | 43 +++++++++++++++++++++++++++ test/e2e/generate_systemd_test.go | 21 +++++++++++++ 6 files changed, 159 insertions(+), 56 deletions(-) (limited to 'test') diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go index 8901298db..de6751a17 100644 --- a/pkg/systemd/generate/common.go +++ b/pkg/systemd/generate/common.go @@ -30,14 +30,14 @@ func validateRestartPolicy(restart string) error { return errors.Errorf("%s is not a valid restart policy", restart) } -const headerTemplate = `# {{.ServiceName}}.service -# autogenerated by Podman {{.PodmanVersion}} -{{- if .TimeStamp}} -# {{.TimeStamp}} -{{- end}} +const headerTemplate = `# {{{{.ServiceName}}}}.service +# autogenerated by Podman {{{{.PodmanVersion}}}} +{{{{- if .TimeStamp}}}} +# {{{{.TimeStamp}}}} +{{{{- end}}}} [Unit] -Description=Podman {{.ServiceName}}.service +Description=Podman {{{{.ServiceName}}}}.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index b64b2593c..5f52b0a77 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -72,22 +72,22 @@ type containerInfo struct { } const containerTemplate = headerTemplate + ` -{{- if .BoundToServices}} -BindsTo={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} -After={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} -{{- end}} +{{{{- if .BoundToServices}}}} +BindsTo={{{{- range $index, $value := .BoundToServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}} +After={{{{- range $index, $value := .BoundToServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}} +{{{{- end}}}} [Service] -Environment={{.EnvVariable}}=%n -Restart={{.RestartPolicy}} -TimeoutStopSec={{.TimeoutStopSec}} -{{- if .ExecStartPre}} -ExecStartPre={{.ExecStartPre}} -{{- end}} -ExecStart={{.ExecStart}} -ExecStop={{.ExecStop}} -ExecStopPost={{.ExecStopPost}} -PIDFile={{.PIDFile}} +Environment={{{{.EnvVariable}}}}=%n +Restart={{{{.RestartPolicy}}}} +TimeoutStopSec={{{{.TimeoutStopSec}}}} +{{{{- if .ExecStartPre}}}} +ExecStartPre={{{{.ExecStartPre}}}} +{{{{- end}}}} +ExecStart={{{{.ExecStart}}}} +ExecStop={{{{.ExecStop}}}} +ExecStopPost={{{{.ExecStopPost}}}} +PIDFile={{{{.PIDFile}}}} Type=forking [Install] @@ -173,9 +173,9 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst } info.EnvVariable = EnvVariable - info.ExecStart = "{{.Executable}} start {{.ContainerNameOrID}}" - info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}}" - info.ExecStopPost = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}}" + info.ExecStart = "{{{{.Executable}}}} start {{{{.ContainerNameOrID}}}}" + info.ExecStop = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.ContainerNameOrID}}}}" + info.ExecStopPost = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.ContainerNameOrID}}}}" // Assemble the ExecStart command when creating a new container. // @@ -209,8 +209,8 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst } startCommand = append(startCommand, "run", - "--conmon-pidfile", "{{.PIDFile}}", - "--cidfile", "{{.ContainerIDFile}}", + "--conmon-pidfile", "{{{{.PIDFile}}}}", + "--cidfile", "{{{{.ContainerIDFile}}}}", "--cgroups=no-conmon", ) // If the container is in a pod, make sure that the @@ -281,10 +281,10 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst startCommand = append(startCommand, remainingCmd...) startCommand = quoteArguments(startCommand) - info.ExecStartPre = "/bin/rm -f {{.PIDFile}} {{.ContainerIDFile}}" + info.ExecStartPre = "/bin/rm -f {{{{.PIDFile}}}} {{{{.ContainerIDFile}}}}" info.ExecStart = strings.Join(startCommand, " ") - info.ExecStop = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}stop --ignore --cidfile {{.ContainerIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}" - info.ExecStopPost = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}rm --ignore -f --cidfile {{.ContainerIDFile}}" + info.ExecStop = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}stop --ignore --cidfile {{{{.ContainerIDFile}}}} {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}}" + info.ExecStopPost = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}rm --ignore -f --cidfile {{{{.ContainerIDFile}}}}" } info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout @@ -307,7 +307,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst // generation. That's especially needed for embedding the PID and ID // files in other fields which will eventually get replaced in the 2nd // template execution. - templ, err := template.New("container_template").Parse(containerTemplate) + templ, err := template.New("container_template").Delims("{{{{", "}}}}").Parse(containerTemplate) if err != nil { return "", errors.Wrap(err, "error parsing systemd service template") } @@ -318,7 +318,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst } // Now parse the generated template (i.e., buf) and execute it. - templ, err = template.New("container_template").Parse(buf.String()) + templ, err = template.New("container_template").Delims("{{{{", "}}}}").Parse(buf.String()) if err != nil { return "", errors.Wrap(err, "error parsing systemd service template") } diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go index c8e65bfe3..96d95644b 100644 --- a/pkg/systemd/generate/containers_test.go +++ b/pkg/systemd/generate/containers_test.go @@ -329,6 +329,29 @@ Type=forking WantedBy=multi-user.target default.target ` + goodNewWithJournaldTag := `# jadda-jadda.service +# autogenerated by Podman CI + +[Unit] +Description=Podman jadda-jadda.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +TimeoutStopSec=70 +ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d --replace --name test --log-driver=journald --log-opt=tag={{.Name}} awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id +PIDFile=%t/jadda-jadda.pid +Type=forking + +[Install] +WantedBy=multi-user.target default.target +` tests := []struct { name string info containerInfo @@ -608,6 +631,22 @@ WantedBy=multi-user.target default.target true, false, }, + {"good with journald log tag (see #9034)", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + CreateCommand: []string{"I'll get stripped", "create", "--name", "test", "--log-driver=journald", "--log-opt=tag={{.Name}}", "awesome-image:latest"}, + EnvVariable: EnvVariable, + }, + goodNewWithJournaldTag, + true, + false, + }, } for _, tt := range tests { test := tt diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go index 7678a240f..c7e3aa955 100644 --- a/pkg/systemd/generate/pods.go +++ b/pkg/systemd/generate/pods.go @@ -72,23 +72,23 @@ type podInfo struct { ExecStopPost string } -const podTemplate = headerTemplate + `Requires={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} -Before={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} +const podTemplate = headerTemplate + `Requires={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}} +Before={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}} [Service] -Environment={{.EnvVariable}}=%n -Restart={{.RestartPolicy}} -TimeoutStopSec={{.TimeoutStopSec}} -{{- if .ExecStartPre1}} -ExecStartPre={{.ExecStartPre1}} -{{- end}} -{{- if .ExecStartPre2}} -ExecStartPre={{.ExecStartPre2}} -{{- end}} -ExecStart={{.ExecStart}} -ExecStop={{.ExecStop}} -ExecStopPost={{.ExecStopPost}} -PIDFile={{.PIDFile}} +Environment={{{{.EnvVariable}}}}=%n +Restart={{{{.RestartPolicy}}}} +TimeoutStopSec={{{{.TimeoutStopSec}}}} +{{{{- if .ExecStartPre1}}}} +ExecStartPre={{{{.ExecStartPre1}}}} +{{{{- end}}}} +{{{{- if .ExecStartPre2}}}} +ExecStartPre={{{{.ExecStartPre2}}}} +{{{{- end}}}} +ExecStart={{{{.ExecStart}}}} +ExecStop={{{{.ExecStop}}}} +ExecStopPost={{{{.ExecStopPost}}}} +PIDFile={{{{.PIDFile}}}} Type=forking [Install] @@ -236,9 +236,9 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) } info.EnvVariable = EnvVariable - info.ExecStart = "{{.Executable}} start {{.InfraNameOrID}}" - info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.InfraNameOrID}}" - info.ExecStopPost = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.InfraNameOrID}}" + info.ExecStart = "{{{{.Executable}}}} start {{{{.InfraNameOrID}}}}" + info.ExecStop = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.InfraNameOrID}}}}" + info.ExecStopPost = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.InfraNameOrID}}}}" // Assemble the ExecStart command when creating a new pod. // @@ -278,8 +278,8 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) startCommand = append(startCommand, podRootArgs...) startCommand = append(startCommand, []string{"pod", "create", - "--infra-conmon-pidfile", "{{.PIDFile}}", - "--pod-id-file", "{{.PodIDFile}}"}...) + "--infra-conmon-pidfile", "{{{{.PIDFile}}}}", + "--pod-id-file", "{{{{.PodIDFile}}}}"}...) // Presence check for certain flags/options. fs := pflag.NewFlagSet("args", pflag.ContinueOnError) @@ -308,11 +308,11 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) startCommand = append(startCommand, podCreateArgs...) startCommand = quoteArguments(startCommand) - info.ExecStartPre1 = "/bin/rm -f {{.PIDFile}} {{.PodIDFile}}" + info.ExecStartPre1 = "/bin/rm -f {{{{.PIDFile}}}} {{{{.PodIDFile}}}}" info.ExecStartPre2 = strings.Join(startCommand, " ") - info.ExecStart = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod start --pod-id-file {{.PodIDFile}}" - info.ExecStop = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod stop --ignore --pod-id-file {{.PodIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}" - info.ExecStopPost = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod rm --ignore -f --pod-id-file {{.PodIDFile}}" + info.ExecStart = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod start --pod-id-file {{{{.PodIDFile}}}}" + info.ExecStop = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod stop --ignore --pod-id-file {{{{.PodIDFile}}}} {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}}" + info.ExecStopPost = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod rm --ignore -f --pod-id-file {{{{.PodIDFile}}}}" } info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout @@ -334,7 +334,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) // generation. That's especially needed for embedding the PID and ID // files in other fields which will eventually get replaced in the 2nd // template execution. - templ, err := template.New("pod_template").Parse(podTemplate) + templ, err := template.New("pod_template").Delims("{{{{", "}}}}").Parse(podTemplate) if err != nil { return "", errors.Wrap(err, "error parsing systemd service template") } @@ -345,7 +345,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) } // Now parse the generated template (i.e., buf) and execute it. - templ, err = template.New("pod_template").Parse(buf.String()) + templ, err = template.New("pod_template").Delims("{{{{", "}}}}").Parse(buf.String()) if err != nil { return "", errors.Wrap(err, "error parsing systemd service template") } diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go index 1c6330160..2b430226b 100644 --- a/pkg/systemd/generate/pods_test.go +++ b/pkg/systemd/generate/pods_test.go @@ -139,6 +139,33 @@ ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod- PIDFile=%t/pod-123abc.pid Type=forking +[Install] +WantedBy=multi-user.target default.target +` + + podNewLabelWithCurlyBraces := `# pod-123abc.service +# autogenerated by Podman CI + +[Unit] +Description=Podman pod-123abc.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +Requires=container-1.service container-2.service +Before=container-1.service container-2.service + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=on-failure +TimeoutStopSec=70 +ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id +ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --name foo --label key={{someval}} --replace +ExecStart=/usr/bin/podman pod start --pod-id-file %t/pod-123abc.pod-id +ExecStop=/usr/bin/podman pod stop --ignore --pod-id-file %t/pod-123abc.pod-id -t 10 +ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-id +PIDFile=%t/pod-123abc.pid +Type=forking + [Install] WantedBy=multi-user.target default.target ` @@ -230,6 +257,22 @@ WantedBy=multi-user.target default.target true, false, }, + {"pod --new with double curly braces", + podInfo{ + Executable: "/usr/bin/podman", + ServiceName: "pod-123abc", + InfraNameOrID: "jadda-jadda-infra", + RestartPolicy: "on-failure", + PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + RequiredServices: []string{"container-1", "container-2"}, + CreateCommand: []string{"podman", "pod", "create", "--name", "foo", "--label", "key={{someval}}"}, + }, + podNewLabelWithCurlyBraces, + true, + false, + }, } for _, tt := range tests { diff --git a/test/e2e/generate_systemd_test.go b/test/e2e/generate_systemd_test.go index be9727591..606d756b0 100644 --- a/test/e2e/generate_systemd_test.go +++ b/test/e2e/generate_systemd_test.go @@ -355,4 +355,25 @@ var _ = Describe("Podman generate systemd", func() { Expect(session.ExitCode()).To(Equal(0)) Expect(session.IsJSONOutputValid()).To(BeTrue()) }) + + It("podman generate systemd --new create command with double curly braces", func() { + // Regression test for #9034 + session := podmanTest.Podman([]string{"create", "--name", "foo", "--log-driver=journald", "--log-opt=tag={{.Name}}", ALPINE}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"generate", "systemd", "--new", "foo"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(ContainSubstring(" --log-opt=tag={{.Name}} ")) + + session = podmanTest.Podman([]string{"pod", "create", "--name", "pod", "--label", "key={{someval}}"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"generate", "systemd", "--new", "pod"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(ContainSubstring(" --label key={{someval}}")) + }) }) -- cgit v1.2.3-54-g00ecf