Merge pull request #11551 from Luap99/rootlessport-restart

fix restart always with rootlessport

6 files changed, 107 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/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
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)
 		}
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