From 18e9ae59ca0800789ea6cfef3a27516801a7e1a5 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 11 Nov 2021 15:35:11 +0100 Subject: Fix network mode in play kube We need to use the config network mode when no network mode was set. To do so we have to keep the nsmode empty, MakeContainer() will use the correct network mode from the config when needed. Fixes #12248 Signed-off-by: Paul Holzinger Signed-off-by: Matthew Heon --- test/e2e/config/containers-netns2.conf | 3 +++ test/e2e/play_kube_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 test/e2e/config/containers-netns2.conf (limited to 'test') diff --git a/test/e2e/config/containers-netns2.conf b/test/e2e/config/containers-netns2.conf new file mode 100644 index 000000000..1ffd100f5 --- /dev/null +++ b/test/e2e/config/containers-netns2.conf @@ -0,0 +1,3 @@ +[containers] + +netns = "bridge" diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index f38350413..c9899c0bc 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -2759,6 +2759,34 @@ invalid kube kind Expect(exists).To(Exit(0)) }) + It("podman play kube use network mode from config", func() { + confPath, err := filepath.Abs("config/containers-netns2.conf") + Expect(err).ToNot(HaveOccurred()) + os.Setenv("CONTAINERS_CONF", confPath) + defer os.Unsetenv("CONTAINERS_CONF") + if IsRemote() { + podmanTest.RestartRemoteService() + } + + pod := getPod() + err = generateKubeYaml("pod", pod, kubeYaml) + Expect(err).To(BeNil()) + + kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + podInspect := podmanTest.Podman([]string{"pod", "inspect", pod.Name, "--format", "{{.InfraContainerID}}"}) + podInspect.WaitWithDefaultTimeout() + Expect(podInspect).To(Exit(0)) + infraID := podInspect.OutputToString() + + inspect := podmanTest.Podman([]string{"inspect", "--format", "{{.HostConfig.NetworkMode}}", infraID}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).To(Exit(0)) + Expect(inspect.OutputToString()).To(Equal("bridge")) + }) + Describe("verify environment variables", func() { var maxLength int BeforeEach(func() { -- cgit v1.2.3-54-g00ecf From 4ed07fb67d11a875f10173a2a20e9149157b77ce Mon Sep 17 00:00:00 2001 From: Michael Scherer Date: Fri, 12 Nov 2021 12:56:57 +0100 Subject: Always create working directory when using compat API Docker/Moby always create the working directory, and some tools rely on that behavior (example, woodpecker/drone). Fixes #11842 Signed-off-by: Michael Scherer Signed-off-by: Matthew Heon --- pkg/api/handlers/compat/containers_create.go | 2 ++ pkg/specgen/generate/container_create.go | 3 +++ pkg/specgen/specgen.go | 4 ++++ test/python/docker/compat/test_containers.py | 14 ++++++++++++++ 4 files changed, 23 insertions(+) (limited to 'test') diff --git a/pkg/api/handlers/compat/containers_create.go b/pkg/api/handlers/compat/containers_create.go index 94d20a04a..1e175d664 100644 --- a/pkg/api/handlers/compat/containers_create.go +++ b/pkg/api/handlers/compat/containers_create.go @@ -86,6 +86,8 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "fill out specgen")) return } + // moby always create the working directory + sg.CreateWorkingDir = true ic := abi.ContainerEngine{Libpod: runtime} report, err := ic.ContainerCreate(r.Context(), sg) diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index f82b2a3c6..fc66145df 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -333,6 +333,9 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. if s.WorkDir == "" { s.WorkDir = "/" } + if s.CreateWorkingDir { + options = append(options, libpod.WithCreateWorkingDir()) + } if s.StopSignal != nil { options = append(options, libpod.WithStopSignal(*s.StopSignal)) } diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go index 0c30c498a..50ef3f197 100644 --- a/pkg/specgen/specgen.go +++ b/pkg/specgen/specgen.go @@ -264,6 +264,10 @@ type ContainerStorageConfig struct { // If unset, the default, /, will be used. // Optional. WorkDir string `json:"work_dir,omitempty"` + // Create the working directory if it doesn't exist. + // If unset, it doesn't create it. + // Optional. + CreateWorkingDir bool `json:"create_working_dir,omitempty"` // RootfsPropagation is the rootfs propagation mode for the container. // If not set, the default of rslave will be used. // Optional. diff --git a/test/python/docker/compat/test_containers.py b/test/python/docker/compat/test_containers.py index 1ad1e7f15..e6f7d992d 100644 --- a/test/python/docker/compat/test_containers.py +++ b/test/python/docker/compat/test_containers.py @@ -251,3 +251,17 @@ class TestContainers(unittest.TestCase): ctr.start() ret, out = ctr.exec_run(["stat", "-c", "%u:%g", "/workspace"]) self.assertEqual(out.rstrip(), b'1042:1043', "UID/GID set in dockerfile") + + + def test_non_existant_workdir(self): + dockerfile = (B'FROM quay.io/libpod/alpine:latest\n' + B'USER root\n' + B'WORKDIR /workspace/scratch\n' + B'RUN touch test') + img: Image + img, out = self.client.images.build(fileobj=io.BytesIO(dockerfile)) + ctr: Container = self.client.containers.create(image=img.id, detach=True, command="top", + volumes=["test_non_existant_workdir:/workspace"]) + ctr.start() + ret, out = ctr.exec_run(["stat", "/workspace/scratch/test"]) + self.assertEqual(ret, 0, "Working directory created if it doesn't exist") -- cgit v1.2.3-54-g00ecf From 76ada2c0119430d10f5aad423120b4b082d3570d Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Thu, 28 Oct 2021 08:54:47 -0400 Subject: Error logs --follow if events-backend != journald, event-logger=journald Fixes: https://github.com/containers/podman/issues/11255 Signed-off-by: Daniel J Walsh --- libpod/container_log_linux.go | 12 ++++++------ test/system/090-events.bats | 11 +++++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) (limited to 'test') diff --git a/libpod/container_log_linux.go b/libpod/container_log_linux.go index b6b780bab..73a081c75 100644 --- a/libpod/container_log_linux.go +++ b/libpod/container_log_linux.go @@ -44,6 +44,12 @@ func (c *Container) initializeJournal(ctx context.Context) error { } func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOptions, logChannel chan *logs.LogLine) error { + // We need the container's events in the same journal to guarantee + // consistency, see #10323. + if options.Follow && c.runtime.config.Engine.EventsLogger != "journald" { + return errors.Errorf("using --follow with the journald --log-driver but without the journald --events-backend (%s) is not supported", c.runtime.config.Engine.EventsLogger) + } + journal, err := sdjournal.NewJournal() if err != nil { return err @@ -105,12 +111,6 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption return errors.Wrap(cursorError, "initial journal cursor") } - // We need the container's events in the same journal to guarantee - // consistency, see #10323. - if options.Follow && c.runtime.config.Engine.EventsLogger != "journald" { - return errors.Errorf("using --follow with the journald --log-driver but without the journald --events-backend (%s) is not supported", c.runtime.config.Engine.EventsLogger) - } - options.WaitGroup.Add(1) go func() { defer func() { diff --git a/test/system/090-events.bats b/test/system/090-events.bats index 22edaeee9..f8a7a87c8 100644 --- a/test/system/090-events.bats +++ b/test/system/090-events.bats @@ -85,6 +85,17 @@ function _events_disjunctive_filters() { _events_disjunctive_filters --events-backend=journald } +@test "events with file backend and journald logdriver with --follow failure" { + skip_if_remote "remote does not support --events-backend" + skip_if_journald_unavailable "system does not support journald events" + run_podman --events-backend=file run --log-driver=journald --name=test $IMAGE echo hi + is "$output" "hi" "Should support events-backend=file" + + run_podman 125 --events-backend=file logs --follow test + is "$output" "Error: using --follow with the journald --log-driver but without the journald --events-backend (file) is not supported" "Should fail with reasonable error message when events-backend and events-logger do not match" + +} + @test "events with disjunctive filters - default" { _events_disjunctive_filters "" } -- cgit v1.2.3-54-g00ecf From 08bcd60c8f4147e4817d3e4e488e3394dbb29c64 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Fri, 12 Nov 2021 11:15:18 -0500 Subject: journald logs: keep reading until the journal's end When reading logs from the journal, keep going after the container exits, in case it gets restarted. Events logged to the journal via the normal paths don't include CONTAINER_ID_FULL, so don't bother adding it to the "history" event we use to force at least one entry for the container to show up in the log. Signed-off-by: Nalin Dahyabhai --- libpod/container_log_linux.go | 14 ++++++++++---- test/system/035-logs.bats | 21 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) (limited to 'test') diff --git a/libpod/container_log_linux.go b/libpod/container_log_linux.go index 73a081c75..90da8053d 100644 --- a/libpod/container_log_linux.go +++ b/libpod/container_log_linux.go @@ -37,9 +37,11 @@ func (c *Container) initializeJournal(ctx context.Context) error { m := make(map[string]string) m["SYSLOG_IDENTIFIER"] = "podman" m["PODMAN_ID"] = c.ID() - m["CONTAINER_ID_FULL"] = c.ID() history := events.History m["PODMAN_EVENT"] = history.String() + container := events.Container + m["PODMAN_TYPE"] = container.String() + m["PODMAN_TIME"] = time.Now().Format(time.RFC3339Nano) return journal.Send("", journal.PriInfo, m) } @@ -95,6 +97,7 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption // exponential backoff. var cursor string var cursorError error + var containerCouldBeLogging bool for i := 1; i <= 3; i++ { cursor, cursorError = journal.GetCursor() hundreds := 1 @@ -173,7 +176,7 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption doTailFunc() } // Unless we follow, quit. - if !options.Follow { + if !options.Follow || !containerCouldBeLogging { return } // Sleep until something's happening on the journal. @@ -202,11 +205,14 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption logrus.Errorf("Failed to translate event: %v", err) return } - if status == events.Exited { + switch status { + case events.History, events.Init, events.Start, events.Restart: + containerCouldBeLogging = true + case events.Exited: + containerCouldBeLogging = false if doTail { doTailFunc() } - return } continue } diff --git a/test/system/035-logs.bats b/test/system/035-logs.bats index 76ce12b81..d8bc0f27d 100644 --- a/test/system/035-logs.bats +++ b/test/system/035-logs.bats @@ -76,6 +76,27 @@ ${cid[0]} d" "Sequential output from logs" _log_test_multi journald } +function _log_test_restarted() { + run_podman run --log-driver=$1 --name logtest $IMAGE sh -c 'start=0; if test -s log; then start=`tail -n 1 log`; fi; seq `expr $start + 1` `expr $start + 10` | tee -a log' + run_podman start -a logtest + logfile=$(mktemp -p ${PODMAN_TMPDIR} logfileXXXXXXXX) + $PODMAN $_PODMAN_TEST_OPTS logs -f logtest > $logfile + expected=$(mktemp -p ${PODMAN_TMPDIR} expectedXXXXXXXX) + seq 1 20 > $expected + diff -u ${expected} ${logfile} +} + +@test "podman logs restarted - k8s-file" { + _log_test_restarted k8s-file +} + +@test "podman logs restarted journald" { + # We can't use journald on RHEL as rootless: rhbz#1895105 + skip_if_journald_unavailable + + _log_test_restarted journald +} + @test "podman logs - journald log driver requires journald events backend" { skip_if_remote "remote does not support --events-backend" # We can't use journald on RHEL as rootless: rhbz#1895105 -- cgit v1.2.3-54-g00ecf From db6b9131ef1a14cd19cdd52425fb60d90862f05e Mon Sep 17 00:00:00 2001 From: Aditya Rajan Date: Mon, 15 Nov 2021 14:39:26 +0530 Subject: secret: honor custom target for secrets with run Honor custom `target` if specified while running or creating containers with secret `type=mount`. Example: `podman run -it --secret token,type=mount,target=TOKEN ubi8/ubi:latest bash` Signed-off-by: Aditya Rajan --- libpod/container.go | 2 ++ libpod/container_internal_linux.go | 11 +++++++- pkg/specgen/generate/container_create.go | 1 + pkg/specgen/specgen.go | 1 + pkg/specgenutil/specgen.go | 5 ++-- test/e2e/run_test.go | 48 +++++++++++++++++++++++++++++--- 6 files changed, 60 insertions(+), 8 deletions(-) (limited to 'test') diff --git a/libpod/container.go b/libpod/container.go index a4bbb5dd0..8bbe02b58 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -253,6 +253,8 @@ type ContainerSecret struct { GID uint32 // Mode is the mode of the secret file Mode uint32 + // Secret target inside container + Target string } // ContainerNetworkDescriptions describes the relationship between the CNI diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 89869e2f5..1c85339c7 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -1777,8 +1777,17 @@ rootless=%d return errors.Wrapf(err, "error creating secrets mount") } for _, secret := range c.Secrets() { + secretFileName := secret.Name + base := "/run/secrets" + if secret.Target != "" { + secretFileName = secret.Target + //If absolute path for target given remove base. + if filepath.IsAbs(secretFileName) { + base = "" + } + } src := filepath.Join(c.config.SecretsPath, secret.Name) - dest := filepath.Join("/run/secrets", secret.Name) + dest := filepath.Join(base, secretFileName) c.state.BindMounts[dest] = src } } diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index fc66145df..4003567e9 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -429,6 +429,7 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. UID: s.UID, GID: s.GID, Mode: s.Mode, + Target: s.Target, }) } options = append(options, libpod.WithSecrets(secrs)) diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go index 50ef3f197..70518c073 100644 --- a/pkg/specgen/specgen.go +++ b/pkg/specgen/specgen.go @@ -513,6 +513,7 @@ type SpecGenerator struct { type Secret struct { Source string + Target string UID uint32 GID uint32 Mode uint32 diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go index 8007e5d8e..eba173a81 100644 --- a/pkg/specgenutil/specgen.go +++ b/pkg/specgenutil/specgen.go @@ -861,6 +861,7 @@ func parseSecrets(secrets []string) ([]specgen.Secret, map[string]string, error) if len(split) == 1 { mountSecret := specgen.Secret{ Source: val, + Target: target, UID: uid, GID: gid, Mode: mode, @@ -926,11 +927,9 @@ func parseSecrets(secrets []string) ([]specgen.Secret, map[string]string, error) return nil, nil, errors.Wrapf(secretParseError, "no source found %s", val) } if secretType == "mount" { - if target != "" { - return nil, nil, errors.Wrapf(secretParseError, "target option is invalid for mounted secrets") - } mountSecret := specgen.Secret{ Source: source, + Target: target, UID: uid, GID: gid, Mode: mode, diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index a5a0aad8b..e21b1dcf3 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -1670,6 +1670,50 @@ WORKDIR /madethis`, BB) }) + It("podman run --secret source=mysecret,type=mount with target", func() { + secretsString := "somesecretdata" + secretFilePath := filepath.Join(podmanTest.TempDir, "secret") + err := ioutil.WriteFile(secretFilePath, []byte(secretsString), 0755) + Expect(err).To(BeNil()) + + session := podmanTest.Podman([]string{"secret", "create", "mysecret_target", secretFilePath}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + session = podmanTest.Podman([]string{"run", "--secret", "source=mysecret_target,type=mount,target=hello", "--name", "secr_target", ALPINE, "cat", "/run/secrets/hello"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(Equal(secretsString)) + + session = podmanTest.Podman([]string{"inspect", "secr_target", "--format", " {{(index .Config.Secrets 0).Name}}"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(ContainSubstring("mysecret_target")) + + }) + + It("podman run --secret source=mysecret,type=mount with target at /tmp", func() { + secretsString := "somesecretdata" + secretFilePath := filepath.Join(podmanTest.TempDir, "secret") + err := ioutil.WriteFile(secretFilePath, []byte(secretsString), 0755) + Expect(err).To(BeNil()) + + session := podmanTest.Podman([]string{"secret", "create", "mysecret_target2", secretFilePath}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + session = podmanTest.Podman([]string{"run", "--secret", "source=mysecret_target2,type=mount,target=/tmp/hello", "--name", "secr_target2", ALPINE, "cat", "/tmp/hello"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(Equal(secretsString)) + + session = podmanTest.Podman([]string{"inspect", "secr_target2", "--format", " {{(index .Config.Secrets 0).Name}}"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(ContainSubstring("mysecret_target2")) + + }) + It("podman run --secret source=mysecret,type=env", func() { secretsString := "somesecretdata" secretFilePath := filepath.Join(podmanTest.TempDir, "secret") @@ -1695,10 +1739,6 @@ WORKDIR /madethis`, BB) session := podmanTest.Podman([]string{"secret", "create", "mysecret", secretFilePath}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - // target with mount type should fail - session = podmanTest.Podman([]string{"run", "--secret", "source=mysecret,type=mount,target=anotherplace", "--name", "secr", ALPINE, "cat", "/run/secrets/mysecret"}) - session.WaitWithDefaultTimeout() - Expect(session).To(ExitWithError()) session = podmanTest.Podman([]string{"run", "--secret", "source=mysecret,type=env,target=anotherplace", "--name", "secr", ALPINE, "printenv", "anotherplace"}) session.WaitWithDefaultTimeout() -- cgit v1.2.3-54-g00ecf From 6d9c1d8fdc0c4ec81d978c6f358d4b304a5a9aef Mon Sep 17 00:00:00 2001 From: Hironori Shiina Date: Wed, 27 Oct 2021 09:26:58 -0400 Subject: Set flags to test 'logs -f' with journald driver `logs -f` with `journald` is supported only when `journald` events backend is used. To pass system tests using `logs -f` in an environment where `events_logger` is not set to `journald` in `containers.conf`, this fix sets `--events-backend` or `--log-driver` temporally. Signed-off-by: Hironori Shiina Signed-off-by: Matthew Heon --- test/system/035-logs.bats | 28 +++++++++++++++++++++++----- test/system/130-kill.bats | 15 ++++++++++++++- 2 files changed, 37 insertions(+), 6 deletions(-) (limited to 'test') diff --git a/test/system/035-logs.bats b/test/system/035-logs.bats index d8bc0f27d..d9ec8423b 100644 --- a/test/system/035-logs.bats +++ b/test/system/035-logs.bats @@ -30,6 +30,17 @@ load helpers run_podman rm $cid } +function _additional_events_backend() { + local driver=$1 + # Since PR#10431, 'logs -f' with journald driver is only supported with journald events backend. + if [[ $driver = "journald" ]]; then + run_podman info --format '{{.Host.EventLogger}}' >/dev/null + if [[ $output != "journald" ]]; then + echo "--events-backend journald" + fi + fi +} + function _log_test_multi() { local driver=$1 @@ -42,10 +53,12 @@ function _log_test_multi() { etc='.*' fi + local events_backend=$(_additional_events_backend $driver) + # Simple helper to make the container starts, below, easier to read local -a cid doit() { - run_podman run --log-driver=$driver --rm -d --name "$1" $IMAGE sh -c "$2"; + run_podman ${events_backend} run --log-driver=$driver --rm -d --name "$1" $IMAGE sh -c "$2"; cid+=($(echo "${output:0:12}")) } @@ -57,7 +70,7 @@ function _log_test_multi() { doit c1 "echo a;sleep 10;echo d;sleep 3" doit c2 "sleep 1;echo b;sleep 2;echo c;sleep 3" - run_podman logs -f c1 c2 + run_podman ${events_backend} logs -f c1 c2 is "$output" \ "${cid[0]} a$etc ${cid[1]} b$etc @@ -208,15 +221,20 @@ function _log_test_follow() { contentA=$(random_string) contentB=$(random_string) contentC=$(random_string) + local events_backend=$(_additional_events_backend $driver) + + if [[ -n "${events_backend}" ]]; then + skip_if_remote "remote does not support --events-backend" + fi # Note: it seems we need at least three log lines to hit #11461. - run_podman run --log-driver=$driver --name $cname $IMAGE sh -c "echo $contentA; echo $contentB; echo $contentC" - run_podman logs -f $cname + run_podman ${events_backend} run --log-driver=$driver --name $cname $IMAGE sh -c "echo $contentA; echo $contentB; echo $contentC" + run_podman ${events_backend} logs -f $cname is "$output" "$contentA $contentB $contentC" "logs -f on exitted container works" - run_podman rm -f $cname + run_podman ${events_backend} rm -f $cname } @test "podman logs - --follow k8s-file" { diff --git a/test/system/130-kill.bats b/test/system/130-kill.bats index d85f0a6a9..1ff3a7b61 100644 --- a/test/system/130-kill.bats +++ b/test/system/130-kill.bats @@ -6,9 +6,22 @@ load helpers @test "podman kill - test signal handling in containers" { + + # Prepare for 'logs -f' + run_podman info --format '{{.Host.LogDriver}}' + log_driver=$output + run_podman info --format '{{.Host.EventLogger}}' + event_logger=$output + opt_log_driver= + if [ $log_driver = "journald" ] && [ $event_logger != "journald" ]; then + # Since PR#10431, 'logs -f' with journald driver is only supported with journald events backend. + # Set '--log driver' temporally because remote doesn't support '--events-backend'. + opt_log_driver="--log-driver k8s-file" + fi + # Start a container that will handle all signals by emitting 'got: N' local -a signals=(1 2 3 4 5 6 8 10 12 13 14 15 16 20 21 22 23 24 25 26 64) - run_podman run -d $IMAGE sh -c \ + run_podman run -d ${opt_log_driver} $IMAGE sh -c \ "for i in ${signals[*]}; do trap \"echo got: \$i\" \$i; done; echo READY; while ! test -e /stop; do sleep 0.05; done; -- cgit v1.2.3-54-g00ecf From 905eabf0bef95d5202483464779680f01a3cc1a1 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 24 Nov 2021 10:35:07 +0100 Subject: libpod: improve heuristic to detect cgroup improve the heuristic to detect the scope that was created for the container. This is necessary with systemd running as PID 1, since it moves itself to a different sub-cgroup, thus stats would not account for other processes in the same container. Closes: https://github.com/containers/podman/issues/12400 Signed-off-by: Giuseppe Scrivano Signed-off-by: Matthew Heon --- libpod/container.go | 25 +++++++++++++++++++++++++ libpod/container_internal_linux.go | 2 +- test/e2e/systemd_test.go | 5 +++++ 3 files changed, 31 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/libpod/container.go b/libpod/container.go index 8bbe02b58..b83420ee7 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -6,10 +6,12 @@ import ( "io/ioutil" "net" "os" + "strings" "time" "github.com/containernetworking/cni/pkg/types" cnitypes "github.com/containernetworking/cni/pkg/types/current" + "github.com/containers/common/pkg/config" "github.com/containers/common/pkg/secrets" "github.com/containers/image/v5/manifest" "github.com/containers/podman/v3/libpod/define" @@ -1001,6 +1003,29 @@ func (c *Container) cGroupPath() (string, error) { return "", errors.Errorf("could not find any cgroup in %q", procPath) } + cgroupManager := c.CgroupManager() + switch { + case c.config.CgroupsMode == cgroupSplit: + name := fmt.Sprintf("/libpod-payload-%s/", c.ID()) + if index := strings.LastIndex(cgroupPath, name); index >= 0 { + return cgroupPath[:index+len(name)-1], nil + } + case cgroupManager == config.CgroupfsCgroupsManager: + name := fmt.Sprintf("/libpod-%s/", c.ID()) + if index := strings.LastIndex(cgroupPath, name); index >= 0 { + return cgroupPath[:index+len(name)-1], nil + } + case cgroupManager == config.SystemdCgroupsManager: + // When running under systemd, try to detect the scope that was requested + // to be created. It improves the heuristic since we report the first + // cgroup that was created instead of the cgroup where PID 1 might have + // moved to. + name := fmt.Sprintf("/libpod-%s.scope/", c.ID()) + if index := strings.LastIndex(cgroupPath, name); index >= 0 { + return cgroupPath[:index+len(name)-1], nil + } + } + return cgroupPath, nil } diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 1c85339c7..8bd433427 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -2512,7 +2512,7 @@ func (c *Container) getOCICgroupPath() (string, error) { if err != nil { return "", err } - return filepath.Join(selfCgroup, "container"), nil + return filepath.Join(selfCgroup, fmt.Sprintf("libpod-payload-%s", c.ID())), nil case cgroupManager == config.SystemdCgroupsManager: // When the OCI runtime is set to use Systemd as a cgroup manager, it // expects cgroups to be passed as follows: diff --git a/test/e2e/systemd_test.go b/test/e2e/systemd_test.go index 7212f8843..bb2cd11f2 100644 --- a/test/e2e/systemd_test.go +++ b/test/e2e/systemd_test.go @@ -109,6 +109,11 @@ WantedBy=default.target stats := podmanTest.Podman([]string{"stats", "--no-stream", ctrName}) stats.WaitWithDefaultTimeout() Expect(stats).Should(Exit(0)) + + cgroupPath := podmanTest.Podman([]string{"inspect", "--format='{{.State.CgroupPath}}'", ctrName}) + cgroupPath.WaitWithDefaultTimeout() + Expect(cgroupPath).Should(Exit(0)) + Expect(result.OutputToString()).To(Not(ContainSubstring("init.scope"))) }) It("podman create container with systemd entrypoint triggers systemd mode", func() { -- cgit v1.2.3-54-g00ecf From 741d98fadea94d4042922cb6edabb468f11a4eb3 Mon Sep 17 00:00:00 2001 From: Matej Vasek Date: Sun, 28 Nov 2021 19:02:15 +0100 Subject: fix: error reporting for archive endpoint Returning 500 when copying to read-only destination. Signed-off-by: Matej Vasek --- pkg/api/handlers/compat/containers_archive.go | 4 +++- test/python/docker/compat/test_containers.py | 14 +++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/pkg/api/handlers/compat/containers_archive.go b/pkg/api/handlers/compat/containers_archive.go index cda23a399..54cbe01e9 100644 --- a/pkg/api/handlers/compat/containers_archive.go +++ b/pkg/api/handlers/compat/containers_archive.go @@ -133,8 +133,10 @@ func handlePut(w http.ResponseWriter, r *http.Request, decoder *schema.Decoder, return } - w.WriteHeader(http.StatusOK) if err := copyFunc(); err != nil { logrus.Error(err.Error()) + utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err) + return } + w.WriteHeader(http.StatusOK) } diff --git a/test/python/docker/compat/test_containers.py b/test/python/docker/compat/test_containers.py index e6f7d992d..d6eacd560 100644 --- a/test/python/docker/compat/test_containers.py +++ b/test/python/docker/compat/test_containers.py @@ -8,6 +8,7 @@ from typing import IO, Optional from docker import DockerClient, errors from docker.models.containers import Container from docker.models.images import Image +from docker.models.volumes import Volume from test.python.docker import Podman from test.python.docker.compat import common, constant @@ -207,9 +208,14 @@ class TestContainers(unittest.TestCase): def test_copy_to_container(self): ctr: Optional[Container] = None + vol: Optional[Volume] = None try: test_file_content = b"Hello World!" - ctr = self.client.containers.create(image="alpine", detach=True, command="top") + vol = self.client.volumes.create("test-volume") + ctr = self.client.containers.create(image="alpine", + detach=True, + command="top", + volumes=["test-volume:/test-volume-read-only:ro"]) ctr.start() buff: IO[bytes] = io.BytesIO() @@ -234,10 +240,16 @@ class TestContainers(unittest.TestCase): ret, out = ctr.exec_run(["cat", "/tmp/a.txt"]) self.assertEqual(ret, 0) self.assertEqual(out.rstrip(), test_file_content, "Content of copied file") + + buff.seek(0) + with self.assertRaises(errors.APIError): + ctr.put_archive("/test-volume-read-only/", buff) finally: if ctr is not None: ctr.stop() ctr.remove() + if vol is not None: + vol.remove(force=True) def test_mount_preexisting_dir(self): dockerfile = (B'FROM quay.io/libpod/alpine:latest\n' -- cgit v1.2.3-54-g00ecf From 7b8e66de0b020ff197112f598387c23415054512 Mon Sep 17 00:00:00 2001 From: Matej Vasek Date: Sun, 28 Nov 2021 20:34:07 +0100 Subject: fix: parsing of HostConfig.Mounts for container create Signed-off-by: Matej Vasek --- cmd/podman/common/create_opts.go | 4 +++- test/python/docker/compat/test_containers.py | 25 ++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go index 50d7c446d..9accae327 100644 --- a/cmd/podman/common/create_opts.go +++ b/cmd/podman/common/create_opts.go @@ -102,7 +102,9 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, rtc *c addField(&builder, "type", string(m.Type)) addField(&builder, "source", m.Source) addField(&builder, "target", m.Target) - addField(&builder, "ro", strconv.FormatBool(m.ReadOnly)) + if m.ReadOnly { + addField(&builder, "ro", "true") + } addField(&builder, "consistency", string(m.Consistency)) // Map any specialized mount options that intersect between *Options and cli options switch m.Type { diff --git a/test/python/docker/compat/test_containers.py b/test/python/docker/compat/test_containers.py index d6eacd560..d14c09fc1 100644 --- a/test/python/docker/compat/test_containers.py +++ b/test/python/docker/compat/test_containers.py @@ -3,12 +3,13 @@ import subprocess import sys import time import unittest -from typing import IO, Optional +from typing import IO, Optional, List from docker import DockerClient, errors from docker.models.containers import Container from docker.models.images import Image from docker.models.volumes import Volume +from docker.types import Mount from test.python.docker import Podman from test.python.docker.compat import common, constant @@ -277,3 +278,25 @@ class TestContainers(unittest.TestCase): ctr.start() ret, out = ctr.exec_run(["stat", "/workspace/scratch/test"]) self.assertEqual(ret, 0, "Working directory created if it doesn't exist") + + def test_mount_rw_by_default(self): + ctr: Optional[Container] = None + vol: Optional[Volume] = None + try: + vol = self.client.volumes.create("test-volume") + ctr = self.client.containers.create(image="alpine", + detach=True, + command="top", + mounts=[Mount(target="/vol-mnt", + source="test-volume", + type='volume', + read_only=False)]) + ctr_inspect = self.client.api.inspect_container(ctr.id) + binds: List[str] = ctr_inspect["HostConfig"]["Binds"] + self.assertEqual(len(binds), 1) + self.assertEqual(binds[0], 'test-volume:/vol-mnt:rw,rprivate,nosuid,nodev,rbind') + finally: + if ctr is not None: + ctr.remove() + if vol is not None: + vol.remove(force=True) -- cgit v1.2.3-54-g00ecf From 39318f177965b3fa49cb4343c6a8c224836febd6 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 30 Nov 2021 17:39:05 +0100 Subject: top: parse ps(1) args correctly The arguments of ps(1) should be shlexed. Fixes: #12452 Signed-off-by: Valentin Rothberg --- libpod/container_top_linux.go | 18 +++++++++++++++++- test/e2e/top_test.go | 5 +++++ 2 files changed, 22 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/libpod/container_top_linux.go b/libpod/container_top_linux.go index 0d4cba85e..d4f4ddfc1 100644 --- a/libpod/container_top_linux.go +++ b/libpod/container_top_linux.go @@ -4,6 +4,7 @@ package libpod import ( "bufio" + "fmt" "os" "strconv" "strings" @@ -11,6 +12,7 @@ import ( "github.com/containers/podman/v3/libpod/define" "github.com/containers/podman/v3/pkg/rootless" "github.com/containers/psgo" + "github.com/google/shlex" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -51,7 +53,21 @@ func (c *Container) Top(descriptors []string) ([]string, error) { return nil, psgoErr } - output, err = c.execPS(descriptors) + // Note that the descriptors to ps(1) must be shlexed (see #12452). + psDescriptors := []string{} + for _, d := range descriptors { + shSplit, err := shlex.Split(d) + if err != nil { + return nil, fmt.Errorf("parsing ps args: %v", err) + } + for _, s := range shSplit { + if s != "" { + psDescriptors = append(psDescriptors, s) + } + } + } + + output, err = c.execPS(psDescriptors) if err != nil { return nil, errors.Wrapf(err, "error executing ps(1) in the container") } diff --git a/test/e2e/top_test.go b/test/e2e/top_test.go index 93c4f3f12..1a71ebf33 100644 --- a/test/e2e/top_test.go +++ b/test/e2e/top_test.go @@ -101,6 +101,11 @@ var _ = Describe("Podman top", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) Expect(len(result.OutputToStringArray())).To(BeNumerically(">", 1)) + + result = podmanTest.Podman([]string{"top", session.OutputToString(), "ax -o args"}) + result.WaitWithDefaultTimeout() + Expect(result).Should(Exit(0)) + Expect(result.OutputToStringArray()).To(Equal([]string{"COMMAND", "top -d 2"})) }) It("podman top with comma-separated options", func() { -- cgit v1.2.3-54-g00ecf From d9527638845d6eb756885e858a7c27670b8ca687 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 2 Dec 2021 13:25:54 +0100 Subject: compat: images/json Do not list manifest lists. Docker doesn't either. Fixes: #12453 Signed-off-by: Valentin Rothberg --- pkg/api/handlers/compat/images.go | 12 +++++++++--- test/apiv2/10-images.at | 7 +++++++ 2 files changed, 16 insertions(+), 3 deletions(-) (limited to 'test') diff --git a/pkg/api/handlers/compat/images.go b/pkg/api/handlers/compat/images.go index 0b7ba8bee..5c73cb0bd 100644 --- a/pkg/api/handlers/compat/images.go +++ b/pkg/api/handlers/compat/images.go @@ -393,14 +393,20 @@ func GetImages(w http.ResponseWriter, r *http.Request) { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Failed get images")) return } - var summaries = make([]*entities.ImageSummary, len(images)) - for j, img := range images { + + summaries := make([]*entities.ImageSummary, 0, len(images)) + for _, img := range images { + // If the image is a manifest list, extract as much as we can. + if isML, _ := img.IsManifestList(r.Context()); isML { + continue + } + is, err := handlers.ImageToImageSummary(img) if err != nil { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Failed transform image summaries")) return } - summaries[j] = is + summaries = append(summaries, is) } utils.WriteResponse(w, http.StatusOK, summaries) } diff --git a/test/apiv2/10-images.at b/test/apiv2/10-images.at index d3fde9f9d..ec32ffb59 100644 --- a/test/apiv2/10-images.at +++ b/test/apiv2/10-images.at @@ -10,6 +10,13 @@ t GET libpod/images/json 200 \ .[0].Id~[0-9a-f]\\{64\\} iid=$(jq -r '.[0].Id' <<<"$output") +# Create an empty manifest and make sure it is not listed +# in the compat endpoint. +t GET images/json 200 length=1 +podman manifest create foo +t GET images/json 200 length=1 +t GET libpod/images/json 200 length=2 + t GET libpod/images/$iid/exists 204 t GET libpod/images/$PODMAN_TEST_IMAGE_NAME/exists 204 t GET libpod/images/${iid}abcdef/exists 404 \ -- cgit v1.2.3-54-g00ecf