podman network reload add rootless support

Allow podman network reload to be run as rootless user. While it is
unlikely that the iptable rules are flushed inside the rootless cni
namespace, it could still happen. Also fix podman network reload --all
to ignore errors when a container does not have the bridge network mode,
e.g. slirp4netns.

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>

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: "",
-		},
 	}
 )
 

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)

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()

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)

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() {

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" {