From b61701acb38d64a3d2622ebb86ad794435e94e54 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 9 Jun 2021 13:55:03 +0200 Subject: container: ignore named hierarchies when looking up the container cgroup, ignore named hierarchies since containers running systemd as payload will create a sub-cgroup and move themselves there. Closes: https://github.com/containers/podman/issues/10602 Signed-off-by: Giuseppe Scrivano --- libpod/container.go | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'libpod') diff --git a/libpod/container.go b/libpod/container.go index bad91d54f..e14051f80 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -946,6 +946,12 @@ func (c *Container) cGroupPath() (string, error) { // is the libpod-specific one we're looking for. // // See #8397 on the need for the longest-path look up. + // + // And another workaround for containers running systemd as the payload. + // containers running systemd moves themselves into a child subgroup of + // the named systemd cgroup hierarchy. Ignore any named cgroups during + // the lookup. + // See #10602 for more details. procPath := fmt.Sprintf("/proc/%d/cgroup", c.state.PID) lines, err := ioutil.ReadFile(procPath) if err != nil { @@ -961,6 +967,10 @@ func (c *Container) cGroupPath() (string, error) { logrus.Debugf("Error parsing cgroup: expected 3 fields but got %d: %s", len(fields), procPath) continue } + // Ignore named cgroups like name=systemd. + if bytes.Contains(fields[1], []byte("=")) { + continue + } path := string(fields[2]) if len(path) > len(cgroupPath) { cgroupPath = path -- cgit v1.2.3-54-g00ecf From c85b6b3fe1b17716492d0fd6f2e13331a6ed8890 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Thu, 10 Jun 2021 12:27:09 +0000 Subject: Fix pre-checkpointing Unfortunately --pre-checkpointing never worked as intended and recent changes to runc have shown that it is broken. To create a pre-checkpoint CRIU expects the paths between the pre-checkpoints to be a relative path. If having a previous checkpoint it needs the be referenced like this: --prev-images-dir ../parent Unfortunately Podman was giving runc (and CRIU) an absolute path. Unfortunately, again, until March 2021 CRIU silently ignored if the path was not relative and switch back to normal checkpointing. This has been now fixed in CRIU and runc and running pre-checkpoint with the latest runc fails, because runc already sees that the path is absolute and returns an error. This commit fixes this by giving runc a relative path. This commit also fixes a second pre-checkpointing error which was just recently introduced. So summarizing: pre-checkpointing never worked correctly because CRIU ignored wrong parameters and recent changes broke it even more. Now both errors should be fixed. [NO TESTS NEEDED] Signed-off-by: Adrian Reber Signed-off-by: Adrian Reber --- libpod/container_internal.go | 3 ++- libpod/container_internal_linux.go | 5 +++-- libpod/oci_conmon_linux.go | 6 +++++- 3 files changed, 10 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 53b85a466..af7e97471 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -41,6 +41,7 @@ const ( // name of the directory holding the artifacts artifactsDir = "artifacts" execDirPermission = 0755 + preCheckpointDir = "pre-checkpoint" ) // rootFsSize gets the size of the container's root filesystem @@ -140,7 +141,7 @@ func (c *Container) CheckpointPath() string { // PreCheckpointPath returns the path to the directory containing the pre-checkpoint-images func (c *Container) PreCheckPointPath() string { - return filepath.Join(c.bundlePath(), "pre-checkpoint") + return filepath.Join(c.bundlePath(), preCheckpointDir) } // AttachSocketPath retrieves the path of the container's attach socket diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index df9e03e78..bf235e42c 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -907,14 +907,15 @@ func (c *Container) exportCheckpoint(options ContainerCheckpointOptions) error { includeFiles := []string{ "artifacts", "ctr.log", - metadata.CheckpointDirectory, metadata.ConfigDumpFile, metadata.SpecDumpFile, metadata.NetworkStatusFile, } if options.PreCheckPoint { - includeFiles[0] = "pre-checkpoint" + includeFiles = append(includeFiles, preCheckpointDir) + } else { + includeFiles = append(includeFiles, metadata.CheckpointDirectory) } // Get root file-system changes included in the checkpoint archive var addToTarFiles []string diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 3da49b85f..2914bd1a1 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -787,7 +787,11 @@ func (r *ConmonOCIRuntime) CheckpointContainer(ctr *Container, options Container args = append(args, "--pre-dump") } if !options.PreCheckPoint && options.WithPrevious { - args = append(args, "--parent-path", ctr.PreCheckPointPath()) + args = append( + args, + "--parent-path", + filepath.Join("..", preCheckpointDir), + ) } runtimeDir, err := util.GetRuntimeDir() if err != nil { -- cgit v1.2.3-54-g00ecf From c3f6ef63a27ae23f79995c4019d552ed4dfdd406 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 8 Jun 2021 13:44:49 +0200 Subject: logs: k8s-file: fix race Fix a race in the k8s-file logs driver. When "following" the logs, Podman will print the container's logs until the end. Previously, Podman logged until the state transitioned into something non-running which opened up a race with the container still running, possibly in the "stopping" state. To fix the race, log until we've seen the wait event for the specific container. In that case, conmon will have finished writing all logs to the file, and Podman will read it until EOF. Further tweak the integration tests for testing `logs -f` on a running container. Previously, the test only checked for one of two lines stating that there was a race. Indeed the race was in using `run --rm` where a log file may be removed before we could fully read it. Fixes: #10596 Signed-off-by: Valentin Rothberg --- libpod/container_log.go | 54 ++++++++++++++++++++++++++++++------------------- test/e2e/logs_test.go | 14 +++++++------ 2 files changed, 41 insertions(+), 27 deletions(-) (limited to 'libpod') diff --git a/libpod/container_log.go b/libpod/container_log.go index c207df819..a30e4f5cc 100644 --- a/libpod/container_log.go +++ b/libpod/container_log.go @@ -4,11 +4,10 @@ import ( "context" "fmt" "os" - "time" "github.com/containers/podman/v3/libpod/define" + "github.com/containers/podman/v3/libpod/events" "github.com/containers/podman/v3/libpod/logs" - "github.com/hpcloud/tail/watch" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -94,27 +93,40 @@ func (c *Container) readFromLogFile(ctx context.Context, options *logs.LogOption }() // Check if container is still running or paused if options.Follow { + state, err := c.State() + if err != nil || state != define.ContainerStateRunning { + // If the container isn't running or if we encountered + // an error getting its state, instruct the logger to + // read the file until EOF. + tailError := t.StopAtEOF() + if tailError != nil && fmt.Sprintf("%v", tailError) != "tail: stop at eof" { + logrus.Error(tailError) + } + if errors.Cause(err) != define.ErrNoSuchCtr { + logrus.Error(err) + } + return nil + } + + // The container is running, so we need to wait until the container exited go func() { - for { - state, err := c.State() - time.Sleep(watch.POLL_DURATION) - if err != nil { - tailError := t.StopAtEOF() - if tailError != nil && fmt.Sprintf("%v", tailError) != "tail: stop at eof" { - logrus.Error(tailError) - } - if errors.Cause(err) != define.ErrNoSuchCtr { - logrus.Error(err) - } - break - } - if state != define.ContainerStateRunning && state != define.ContainerStatePaused { - tailError := t.StopAtEOF() - if tailError != nil && fmt.Sprintf("%v", tailError) != "tail: stop at eof" { - logrus.Error(tailError) - } - break + eventChannel := make(chan *events.Event) + eventOptions := events.ReadOptions{ + EventChannel: eventChannel, + Filters: []string{"event=died", "container=" + c.ID()}, + Stream: true, + } + go func() { + if err := c.runtime.Events(ctx, eventOptions); err != nil { + logrus.Errorf("Error waiting for container to exit: %v", err) } + }() + // Now wait for the died event and signal to finish + // reading the log until EOF. + <-eventChannel + tailError := t.StopAtEOF() + if tailError != nil && fmt.Sprintf("%v", tailError) != "tail: stop at eof" { + logrus.Error(tailError) } }() } diff --git a/test/e2e/logs_test.go b/test/e2e/logs_test.go index 3051031a5..b7a5a6243 100644 --- a/test/e2e/logs_test.go +++ b/test/e2e/logs_test.go @@ -173,9 +173,9 @@ var _ = Describe("Podman logs", func() { }) It("streaming output: "+log, func() { - containerName := "logs-f-rm" + containerName := "logs-f" - logc := podmanTest.Podman([]string{"run", "--log-driver", log, "--rm", "--name", containerName, "-dt", ALPINE, "sh", "-c", "echo podman; sleep 1; echo podman"}) + logc := podmanTest.Podman([]string{"run", "--log-driver", log, "--name", containerName, "-dt", ALPINE, "sh", "-c", "echo podman-1; sleep 1; echo podman-2"}) logc.WaitWithDefaultTimeout() Expect(logc).To(Exit(0)) @@ -183,10 +183,8 @@ var _ = Describe("Podman logs", func() { results.WaitWithDefaultTimeout() Expect(results).To(Exit(0)) - // TODO: we should actually check for two podman lines, - // but as of 2020-06-17 there's a race condition in which - // 'logs -f' may not catch all output from a container - Expect(results.OutputToString()).To(ContainSubstring("podman")) + Expect(results.OutputToString()).To(ContainSubstring("podman-1")) + Expect(results.OutputToString()).To(ContainSubstring("podman-2")) // Container should now be terminatING or terminatED, but we // have no guarantee of which: 'logs -f' does not necessarily @@ -199,6 +197,10 @@ var _ = Describe("Podman logs", func() { } else { Expect(inspect.ErrorToString()).To(ContainSubstring("no such container")) } + + results = podmanTest.Podman([]string{"rm", "-f", containerName}) + results.WaitWithDefaultTimeout() + Expect(results).To(Exit(0)) }) It("follow output stopped container: "+log, func() { -- cgit v1.2.3-54-g00ecf From 8ba0c92e6a8da59417d7e96a6f2002bfe1a74f1f Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 8 Jun 2021 11:28:46 +0200 Subject: Improve systemd-resolved detection When 127.0.0.53 is the only nameserver in /etc/resolv.conf assume systemd-resolved is used. This is better because /etc/resolv.conf does not have to be symlinked to /run/systemd/resolve/stub-resolv.conf in order to use systemd-resolved. [NO TESTS NEEDED] Fixes: #10570 Signed-off-by: Paul Holzinger --- libpod/container_internal_linux.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index bf235e42c..be59e3b54 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -1649,22 +1649,20 @@ func (c *Container) generateResolvConf() (string, error) { } } - // Determine the endpoint for resolv.conf in case it is a symlink - resolvPath, err := filepath.EvalSymlinks(resolvConf) + contents, err := ioutil.ReadFile(resolvConf) // resolv.conf doesn't have to exists if err != nil && !os.IsNotExist(err) { return "", err } - // Determine if symlink points to any of the systemd-resolved files - if strings.HasPrefix(resolvPath, "/run/systemd/resolve/") { - resolvPath = "/run/systemd/resolve/resolv.conf" - } - - contents, err := ioutil.ReadFile(resolvPath) - // resolv.conf doesn't have to exists - if err != nil && !os.IsNotExist(err) { - return "", err + ns := resolvconf.GetNameservers(contents) + // check if systemd-resolved is used, assume it is used when 127.0.0.53 is the only nameserver + if len(ns) == 1 && ns[0] == "127.0.0.53" { + // read the actual resolv.conf file for systemd-resolved + contents, err = ioutil.ReadFile("/run/systemd/resolve/resolv.conf") + if err != nil { + return "", errors.Wrapf(err, "detected that systemd-resolved is in use, but could not locate real resolv.conf") + } } ipv6 := false -- cgit v1.2.3-54-g00ecf From 4f56f7f13335bfe3795526d7ba00e70dbdbdf380 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 11 Jun 2021 15:05:52 +0200 Subject: Fix network connect race with docker-compose Network connect/disconnect has to call the cni plugins when the network namespace is already configured. This is the case for `ContainerStateRunning` and `ContainerStateCreated`. This is important otherwise the network is not attached to this network namespace and libpod will throw errors like `network inspection mismatch...` This problem happened when using `docker-compose up` in attached mode. Signed-off-by: Paul Holzinger --- libpod/networking_linux.go | 4 ++-- test/compose/test-compose | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'libpod') diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 0e8a4f768..eb515c000 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -1068,7 +1068,7 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro } c.newNetworkEvent(events.NetworkDisconnect, netName) - if c.state.State != define.ContainerStateRunning { + if !c.ensureState(define.ContainerStateRunning, define.ContainerStateCreated) { return nil } @@ -1123,7 +1123,7 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e return err } c.newNetworkEvent(events.NetworkConnect, netName) - if c.state.State != define.ContainerStateRunning { + if !c.ensureState(define.ContainerStateRunning, define.ContainerStateCreated) { return nil } if c.state.NetNS == nil { diff --git a/test/compose/test-compose b/test/compose/test-compose index 46ca80321..a36eae942 100755 --- a/test/compose/test-compose +++ b/test/compose/test-compose @@ -183,6 +183,8 @@ function test_port() { fi echo "# cat $WORKDIR/server.log:" cat $WORKDIR/server.log + echo "# cat $logfile:" + cat $logfile return fi -- cgit v1.2.3-54-g00ecf