Merge pull request #10356 from Luap99/network-reload-rootless

podman network reload add rootless support

6 files changed, 54 insertions, 26 deletions

diff --git a/cmd/podman/networks/reload.go b/cmd/podman/networks/reload.go
index 8f2fbf011..035e56a07 100644
--- a/cmd/podman/networks/reload.go
+++ b/cmd/podman/networks/reload.go
@@ -26,9 +26,6 @@ var (
 		Example: `podman network reload --latest
   podman network reload 3c13ef6dd843
   podman network reload test1 test2`,
-		Annotations: map[string]string{
-			registry.ParentNSRequired: "",
-		},
 	}
 )
 
diff --git a/libpod/define/errors.go b/libpod/define/errors.go
index 64c652eec..81bf5f69c 100644
--- a/libpod/define/errors.go
+++ b/libpod/define/errors.go
@@ -179,6 +179,9 @@ var (
 	// ErrNoNetwork indicates that a container has no net namespace, like network=none
 	ErrNoNetwork = errors.New("container has no network namespace")
 
+	// ErrNetworkModeInvalid indicates that a container has the wrong network mode for an operation
+	ErrNetworkModeInvalid = errors.New("invalid network mode")
+
 	// ErrSetSecurityAttribute indicates that a request to set a container's security attribute
 	// was not possible.
 	ErrSetSecurityAttribute = fmt.Errorf("%w: unable to assign security attribute", ErrOCIRuntime)
diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index cfed5a1f2..7a86e8c0c 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -23,6 +23,7 @@ import (
 	"github.com/containers/podman/v3/libpod/events"
 	"github.com/containers/podman/v3/libpod/network"
 	"github.com/containers/podman/v3/pkg/errorhandling"
+	"github.com/containers/podman/v3/pkg/namespaces"
 	"github.com/containers/podman/v3/pkg/netns"
 	"github.com/containers/podman/v3/pkg/resolvconf"
 	"github.com/containers/podman/v3/pkg/rootless"
@@ -757,6 +758,15 @@ func getContainerNetNS(ctr *Container) (string, error) {
 	return "", nil
 }
 
+// isBridgeNetMode checks if the given network mode is bridge.
+// It returns nil when it is set to bridge and an error otherwise.
+func isBridgeNetMode(n namespaces.NetworkMode) error {
+	if !n.IsBridge() {
+		return errors.Wrapf(define.ErrNetworkModeInvalid, "%q is not supported", n)
+	}
+	return nil
+}
+
 // Reload only works with containers with a configured network.
 // It will tear down, and then reconfigure, the network of the container.
 // This is mainly used when a reload of firewall rules wipes out existing
@@ -770,8 +780,8 @@ func (r *Runtime) reloadContainerNetwork(ctr *Container) ([]*cnitypes.Result, er
 	if ctr.state.NetNS == nil {
 		return nil, errors.Wrapf(define.ErrCtrStateInvalid, "container %s network is not configured, refusing to reload", ctr.ID())
 	}
-	if rootless.IsRootless() || ctr.config.NetMode.IsSlirp4netns() {
-		return nil, errors.Wrapf(define.ErrRootless, "network reload only supported for root containers")
+	if err := isBridgeNetMode(ctr.config.NetMode); err != nil {
+		return nil, err
 	}
 
 	logrus.Infof("Going to reload container %s network", ctr.ID())
@@ -1025,8 +1035,8 @@ func (w *logrusDebugWriter) Write(p []byte) (int, error) {
 // NetworkDisconnect removes a container from the network
 func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) error {
 	// only the bridge mode supports cni networks
-	if !c.config.NetMode.IsBridge() {
-		return errors.Errorf("network mode %q is not supported", c.config.NetMode)
+	if err := isBridgeNetMode(c.config.NetMode); err != nil {
+		return err
 	}
 
 	networks, err := c.networksByNameIndex()
@@ -1086,8 +1096,8 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro
 // ConnectNetwork connects a container to a given network
 func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) error {
 	// only the bridge mode supports cni networks
-	if !c.config.NetMode.IsBridge() {
-		return errors.Errorf("network mode %q is not supported", c.config.NetMode)
+	if err := isBridgeNetMode(c.config.NetMode); err != nil {
+		return err
 	}
 
 	networks, err := c.networksByNameIndex()
diff --git a/pkg/domain/infra/abi/network.go b/pkg/domain/infra/abi/network.go
index 1a833332c..33ab280e5 100644
--- a/pkg/domain/infra/abi/network.go
+++ b/pkg/domain/infra/abi/network.go
@@ -71,7 +71,9 @@ func (ic *ContainerEngine) NetworkReload(ctx context.Context, names []string, op
 		report := new(entities.NetworkReloadReport)
 		report.Id = ctr.ID()
 		report.Err = ctr.ReloadNetwork()
-		if options.All && errors.Cause(report.Err) == define.ErrCtrStateInvalid {
+		// ignore errors for invalid ctr state and network mode when --all is used
+		if options.All && (errors.Cause(report.Err) == define.ErrCtrStateInvalid ||
+			errors.Cause(report.Err) == define.ErrNetworkModeInvalid) {
 			continue
 		}
 		reports = append(reports, report)
diff --git a/test/e2e/network_connect_disconnect_test.go b/test/e2e/network_connect_disconnect_test.go
index 6974c7614..c82aacbe4 100644
--- a/test/e2e/network_connect_disconnect_test.go
+++ b/test/e2e/network_connect_disconnect_test.go
@@ -66,7 +66,7 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		con := podmanTest.Podman([]string{"network", "disconnect", netName, "test"})
 		con.WaitWithDefaultTimeout()
 		Expect(con.ExitCode()).ToNot(BeZero())
-		Expect(con.ErrorToString()).To(ContainSubstring(`network mode "slirp4netns" is not supported`))
+		Expect(con.ErrorToString()).To(ContainSubstring(`"slirp4netns" is not supported: invalid network mode`))
 	})
 
 	It("podman network disconnect", func() {
@@ -132,7 +132,7 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		con := podmanTest.Podman([]string{"network", "connect", netName, "test"})
 		con.WaitWithDefaultTimeout()
 		Expect(con.ExitCode()).ToNot(BeZero())
-		Expect(con.ErrorToString()).To(ContainSubstring(`network mode "slirp4netns" is not supported`))
+		Expect(con.ErrorToString()).To(ContainSubstring(`"slirp4netns" is not supported: invalid network mode`))
 	})
 
 	It("podman connect on a container that already is connected to the network should error", func() {
diff --git a/test/system/500-networking.bats b/test/system/500-networking.bats
index 34220829a..f3478fa2f 100644
--- a/test/system/500-networking.bats
+++ b/test/system/500-networking.bats
@@ -215,7 +215,6 @@ load helpers
 
 @test "podman network reload" {
     skip_if_remote "podman network reload does not have remote support"
-    skip_if_rootless "podman network reload does not work rootless"
 
     random_1=$(random_string 30)
     HOST_PORT=12345
@@ -225,29 +224,42 @@ load helpers
     INDEX1=$PODMAN_TMPDIR/hello.txt
     echo $random_1 > $INDEX1
 
+    # use default network for root
+    local netname=podman
+    # for rootless we have to create a custom network since there is no default network
+    if is_rootless; then
+        netname=testnet-$(random_string 10)
+        run_podman network create $netname
+        is "$output" ".*/cni/net.d/$netname.conflist" "output of 'network create'"
+    fi
+
     # Bind-mount this file with a different name to a container running httpd
     run_podman run -d --name myweb -p "$HOST_PORT:80" \
-	       -v $INDEX1:/var/www/index.txt \
-	       -w /var/www \
-	       $IMAGE /bin/busybox-extras httpd -f -p 80
+            --network $netname \
+            -v $INDEX1:/var/www/index.txt \
+            -w /var/www \
+            $IMAGE /bin/busybox-extras httpd -f -p 80
     cid=$output
 
-    run_podman inspect $cid --format "{{.NetworkSettings.IPAddress}}"
+    run_podman inspect $cid --format "{{(index .NetworkSettings.Networks \"$netname\").IPAddress}}"
     ip="$output"
-    run_podman inspect $cid --format "{{.NetworkSettings.MacAddress}}"
+    run_podman inspect $cid --format "{{(index .NetworkSettings.Networks \"$netname\").MacAddress}}"
     mac="$output"
 
     # Verify http contents: curl from localhost
     run curl -s $SERVER/index.txt
     is "$output" "$random_1" "curl 127.0.0.1:/index.txt"
 
-    # flush the CNI iptables here
-    run iptables -t nat -F CNI-HOSTPORT-DNAT
+    # rootless cannot modify iptables
+    if ! is_rootless; then
+        # flush the CNI iptables here
+        run iptables -t nat -F CNI-HOSTPORT-DNAT
 
-    # check that we cannot curl (timeout after 5 sec)
-    run timeout 5 curl -s $SERVER/index.txt
-    if [ "$status" -ne 124 ]; then
-	die "curl did not timeout, status code: $status"
+        # check that we cannot curl (timeout after 5 sec)
+        run timeout 5 curl -s $SERVER/index.txt
+        if [ "$status" -ne 124 ]; then
+	        die "curl did not timeout, status code: $status"
+        fi
     fi
 
     # reload the network to recreate the iptables rules
@@ -255,9 +267,9 @@ load helpers
     is "$output" "$cid" "Output does not match container ID"
 
     # check that we still have the same mac and ip
-    run_podman inspect $cid --format "{{.NetworkSettings.IPAddress}}"
+    run_podman inspect $cid --format "{{(index .NetworkSettings.Networks \"$netname\").IPAddress}}"
     is "$output" "$ip" "IP address changed after podman network reload"
-    run_podman inspect $cid --format "{{.NetworkSettings.MacAddress}}"
+    run_podman inspect $cid --format "{{(index .NetworkSettings.Networks \"$netname\").MacAddress}}"
     is "$output" "$mac" "MAC address changed after podman network reload"
 
     # check that we can still curl
@@ -275,6 +287,10 @@ load helpers
 
     # cleanup the container
     run_podman rm -f $cid
+
+    if is_rootless; then
+        run_podman network rm -f $netname
+    fi
 }
 
 @test "podman rootless cni adds /usr/sbin to PATH" {