From 32424d9a9be09be3887fec903d960c87f5aedf28 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 13 Sep 2021 16:25:05 +0200 Subject: remove rootlessport socket to prevent EADDRINUSE When we restart a container via podman restart or restart policy the rootlessport process fails with `address already in use` because the socketfile still exists. This is a regression and was introduced in commit abdedc31a25e. Signed-off-by: Paul Holzinger --- pkg/rootlessport/rootlessport_linux.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/rootlessport/rootlessport_linux.go b/pkg/rootlessport/rootlessport_linux.go index 730d91aa2..10d135e0b 100644 --- a/pkg/rootlessport/rootlessport_linux.go +++ b/pkg/rootlessport/rootlessport_linux.go @@ -218,6 +218,9 @@ outer: // we only need to have a socket to reload ports when we run under rootless cni if cfg.RootlessCNI { + socketfile := filepath.Join(socketDir, cfg.ContainerID) + // make sure to remove the file if it exists to prevent EADDRINUSE + _ = os.Remove(socketfile) // workaround to bypass the 108 char socket path limit // open the fd and use the path to the fd as bind argument fd, err := unix.Open(socketDir, unix.O_PATH, 0) @@ -229,6 +232,8 @@ outer: return err } err = unix.Close(fd) + // remove the socket file on exit + defer os.Remove(socketfile) if err != nil { logrus.Warnf("failed to close the socketDir fd: %v", err) } -- cgit v1.2.3-54-g00ecf From 6221f269a8936876034a342010b89bc71f8bbe95 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 13 Sep 2021 15:52:49 +0200 Subject: fix restart always with rootlessport When a container is automatically restarted due its restart policy and the container uses rootless cni networking with ports forwarded we have to start a new rootlessport process since it exits with conmon. Signed-off-by: Paul Holzinger --- libpod/container_internal.go | 9 +++ libpod/networking_linux.go | 1 + test/system/500-networking.bats | 119 ++++++++++++++++++++++++++-------------- 3 files changed, 88 insertions(+), 41 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 4d1a25541..18b80475b 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -293,6 +293,15 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (_ bool, retErr err } } + // setup rootlesskit port forwarder again since it dies when conmon exits + // we use rootlesskit port forwarder only as rootless and when bridge network is used + if rootless.IsRootless() && c.config.NetMode.IsBridge() && len(c.config.PortMappings) > 0 { + err := c.runtime.setupRootlessPortMappingViaRLK(c, c.state.NetNS.Path()) + if err != nil { + return false, err + } + } + if c.state.State == define.ContainerStateStopped { // Reinitialize the container if we need to if err := c.reinit(ctx, true); err != nil { diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 9aa6cab15..b0d4e0b2d 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -718,6 +718,7 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) error { // set up port forwarder for CNI-in-slirp4netns netnsPath := ctr.state.NetNS.Path() // TODO: support slirp4netns port forwarder as well + // make sure to fix this container.handleRestartPolicy() as well return r.setupRootlessPortMappingViaRLK(ctr, netnsPath) } return nil diff --git a/test/system/500-networking.bats b/test/system/500-networking.bats index ad5891dd9..bdedfae19 100644 --- a/test/system/500-networking.bats +++ b/test/system/500-networking.bats @@ -32,7 +32,6 @@ load helpers # Bind-mount this file with a different name to a container running httpd run_podman run -d --name myweb -p "$HOST_PORT:80" \ - --restart always \ -v $INDEX1:/var/www/index.txt:Z \ -w /var/www \ $IMAGE /bin/busybox-extras httpd -f -p 80 @@ -67,46 +66,6 @@ load helpers run_podman 125 port myweb 99/tcp is "$output" 'Error: failed to find published port "99/tcp"' - # Tests #10310: podman will restart slirp4netns on container restart - run_podman container inspect --format "{{.State.Pid}}" $cid - pid=$output - - # Kill the process; podman restart policy will bring up a new container. - # -9 is crucial: busybox httpd ignores all other signals. - kill -9 $pid - # Wait for process to exit - retries=30 - while kill -0 $pid; do - sleep 0.5 - retries=$((retries - 1)) - if [[ $retries -eq 0 ]]; then - die "Process $pid (container $cid) refused to die" - fi - done - - # Wait for container to restart - retries=20 - while :;do - run_podman container inspect --format "{{.State.Pid}}" myweb - # pid is 0 as long as the container is not running - if [[ $output -ne 0 ]]; then - if [[ $output == $pid ]]; then - die "This should never happen! Restarted container has same PID ($output) as killed one!" - fi - break - fi - sleep 0.5 - retries=$((retries - 1)) - if [[ $retries -eq 0 ]]; then - die "Timed out waiting for container to restart" - fi - done - - # Verify http contents again: curl from localhost - # Use retry since it can take a moment until the new container is ready - run curl --retry 2 -s $SERVER/index.txt - is "$output" "$random_1" "curl 127.0.0.1:/index.txt after restart" - # Clean up run_podman stop -t 1 myweb run_podman rm myweb @@ -476,4 +435,82 @@ load helpers run_podman network rm -f $netname $netname2 } +@test "podman network after restart" { + random_1=$(random_string 30) + + HOST_PORT=$(random_free_port) + SERVER=http://127.0.0.1:$HOST_PORT + + # Create a test file with random content + INDEX1=$PODMAN_TMPDIR/hello.txt + echo $random_1 > $INDEX1 + + local netname=testnet-$(random_string 10) + run_podman network create $netname + is "$output" ".*/cni/net.d/$netname.conflist" "output of 'network create'" + + for network in "slirp4netns" "$netname"; do + # Start container with the restart always policy + run_podman run -d --name myweb -p "$HOST_PORT:80" \ + --restart always \ + --network $network \ + -v $INDEX1:/var/www/index.txt:Z \ + -w /var/www \ + $IMAGE /bin/busybox-extras httpd -f -p 80 + cid=$output + + # Tests #10310: podman will restart slirp4netns on container restart + run_podman container inspect --format "{{.State.Pid}}" $cid + pid=$output + + # Kill the process; podman restart policy will bring up a new container. + # -9 is crucial: busybox httpd ignores all other signals. + kill -9 $pid + # Wait for process to exit + retries=30 + while kill -0 $pid; do + sleep 0.5 + retries=$((retries - 1)) + if [[ $retries -eq 0 ]]; then + die "Process $pid (container $cid) refused to die" + fi + done + + # Wait for container to restart + retries=20 + while :;do + run_podman container inspect --format "{{.State.Pid}}" $cid + # pid is 0 as long as the container is not running + if [[ $output -ne 0 ]]; then + if [[ $output == $pid ]]; then + die "This should never happen! Restarted container has same PID ($output) as killed one!" + fi + break + fi + sleep 0.5 + retries=$((retries - 1)) + if [[ $retries -eq 0 ]]; then + die "Timed out waiting for container to restart" + fi + done + + # Verify http contents again: curl from localhost + # Use retry since it can take a moment until the new container is ready + run curl --retry 2 -s $SERVER/index.txt + is "$output" "$random_1" "curl 127.0.0.1:/index.txt after auto restart" + + run_podman restart $cid + # Verify http contents again: curl from localhost + # Use retry since it can take a moment until the new container is ready + run curl --retry 2 -s $SERVER/index.txt + is "$output" "$random_1" "curl 127.0.0.1:/index.txt after podman restart" + + run_podman stop -t 0 $cid + run_podman rm -f $cid + done + + # Cleanup network + run_podman network rm $netname +} + # vim: filetype=sh -- cgit v1.2.3-54-g00ecf From d3f0f09ad94af1d67b4f7baa83036dc851edee18 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 14 Sep 2021 11:13:28 +0200 Subject: libpod: rootful close binded ports For rootful users ports are forwarded via iptables. To make sure no other process tries to use them, libpod will bind the ports and pass the fds to conmon. There seems to be race when a container is restarted because libpod tries to bind the port before the conmon process exited. The problem only hapens with the podman service because it keeps the connection open. Once we have the fd and passed it to conmon the podman service should close the connection. To verify run `sudo ss -tulpn` and check that only the conmon process keeps the port open. Previously you would also see the podman server process listed. Signed-off-by: Paul Holzinger --- libpod/oci_conmon_linux.go | 1 + libpod/oci_util.go | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index c00d83f95..831e89223 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -1140,6 +1140,7 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co if err != nil { return err } + filesToClose = append(filesToClose, ports...) // Leak the port we bound in the conmon process. These fd's won't be used // by the container and conmon will keep the ports busy so that another diff --git a/libpod/oci_util.go b/libpod/oci_util.go index 1cafd5863..f2843b09b 100644 --- a/libpod/oci_util.go +++ b/libpod/oci_util.go @@ -68,6 +68,12 @@ func bindPorts(ports []ocicni.PortMapping) ([]*os.File, error) { return nil, errors.Wrapf(err, "cannot get file for UDP socket") } files = append(files, f) + // close the listener + // note that this does not affect the fd, see the godoc for server.File() + err = server.Close() + if err != nil { + logrus.Warnf("failed to close connection: %v", err) + } case "tcp": var ( @@ -96,6 +102,13 @@ func bindPorts(ports []ocicni.PortMapping) ([]*os.File, error) { return nil, errors.Wrapf(err, "cannot get file for TCP socket") } files = append(files, f) + // close the listener + // note that this does not affect the fd, see the godoc for server.File() + err = server.Close() + if err != nil { + logrus.Warnf("failed to close connection: %v", err) + } + case "sctp": if !notifySCTP { notifySCTP = true -- cgit v1.2.3-54-g00ecf