Vendor latest c/common and fix tests

This vendors the latest c/common version, including making Pasta
the default rootless network provider. That broke a number of
tests, which have been fixed as part of this PR.

Also includes a change to network stats logic, which simplifies
the code a bit and makes it actually work with Pasta.

Signed-off-by: Matt Heon <mheon@redhat.com>

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/checkpoint-restore/go-criu/v7 v7.0.0
 	github.com/containernetworking/plugins v1.4.0
 	github.com/containers/buildah v1.34.1-0.20240201124221-b850c711ff5c
-	github.com/containers/common v0.57.1-0.20240229151045-1c65e0de241a
+	github.com/containers/common v0.57.1-0.20240229165734-cec09922602e
 	github.com/containers/conmon v2.0.20+incompatible
 	github.com/containers/gvisor-tap-vsock v0.7.3
 	github.com/containers/image/v5 v5.29.3-0.20240227090231-5bef5e1e1506

go.sum

@@ -76,8 +76,8 @@ github.com/containernetworking/plugins v1.4.0 h1:+w22VPYgk7nQHw7KT92lsRmuToHvb7w
 github.com/containernetworking/plugins v1.4.0/go.mod h1:UYhcOyjefnrQvKvmmyEKsUA+M9Nfn7tqULPpH0Pkcj0=
 github.com/containers/buildah v1.34.1-0.20240201124221-b850c711ff5c h1:r+1vFyTAoXptJrsPsnOMI3G0jm4+BCfXAcIyuA33lzo=
 github.com/containers/buildah v1.34.1-0.20240201124221-b850c711ff5c/go.mod h1:Hw4qo2URFpWvZ2tjLstoQMpNC6+gR4PtxQefvV/UKaA=
-github.com/containers/common v0.57.1-0.20240229151045-1c65e0de241a h1:N/AOv7bQBTD/+inld7Qpc2WzxtpbsPgPDB/gg7JGQKA=
-github.com/containers/common v0.57.1-0.20240229151045-1c65e0de241a/go.mod h1:8irlyBcVooYx0F+YmoY7PQPAIgdJvCj17bvL7PqeaxI=
+github.com/containers/common v0.57.1-0.20240229165734-cec09922602e h1:TPgCd6bWFyliJxCXEiCI1LnbB3kBUkpx1dw51ngDjWI=
+github.com/containers/common v0.57.1-0.20240229165734-cec09922602e/go.mod h1:8irlyBcVooYx0F+YmoY7PQPAIgdJvCj17bvL7PqeaxI=
 github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
 github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
 github.com/containers/gvisor-tap-vsock v0.7.3 h1:yORnf15sP+sLFhxLNLgmB5/lOhldn9dRMHx/tmYtSOQ=

libpod/networking_linux.go

@@ -191,7 +191,7 @@ func getContainerNetNS(ctr *Container) (string, *Container, error) {
 func getContainerNetIO(ctr *Container) (map[string]define.ContainerNetworkStats, error) {
 	perNetworkStats := make(map[string]define.ContainerNetworkStats)
 
-	netNSPath, otherCtr, netPathErr := getContainerNetNS(ctr)
+	netNSPath, _, netPathErr := getContainerNetNS(ctr)
 	if netPathErr != nil {
 		return nil, netPathErr
 	}
@@ -201,30 +201,28 @@ func getContainerNetIO(ctr *Container) (map[string]define.ContainerNetworkStats,
 		return nil, nil
 	}
 
-	netMode := ctr.config.NetMode
-	netStatus := ctr.getNetworkStatus()
-	if otherCtr != nil {
-		netMode = otherCtr.config.NetMode
-		netStatus = otherCtr.getNetworkStatus()
-	}
-	if netMode.IsSlirp4netns() {
-		// create a fake status with correct interface name for the logic below
-		netStatus = map[string]types.StatusBlock{
-			"slirp4netns": {
-				Interfaces: map[string]types.NetInterface{"tap0": {}},
-			},
-		}
-	}
 	err := ns.WithNetNSPath(netNSPath, func(_ ns.NetNS) error {
-		for _, status := range netStatus {
-			for dev := range status.Interfaces {
-				link, err := netlink.LinkByName(dev)
+		links, err := netlink.LinkList()
 		if err != nil {
-					return err
+			return fmt.Errorf("retrieving all network interfaces: %w", err)
 		}
-				stats := link.Attrs().Statistics
-				if stats != nil {
-					newStats := define.ContainerNetworkStats{
+		for _, link := range links {
+			attributes := link.Attrs()
+			if attributes.Flags&net.FlagLoopback != 0 {
+				continue
+			}
+
+			if attributes.Statistics != nil {
+				perNetworkStats[attributes.Name] = getNetStatsFromNetlinkStats(attributes.Statistics)
+			}
+		}
+		return nil
+	})
+	return perNetworkStats, err
+}
+
+func getNetStatsFromNetlinkStats(stats *netlink.LinkStatistics) define.ContainerNetworkStats {
+	return define.ContainerNetworkStats{
 		RxBytes:   stats.RxBytes,
 		RxDropped: stats.RxDropped,
 		RxErrors:  stats.RxErrors,
@@ -234,14 +232,6 @@ func getContainerNetIO(ctr *Container) (map[string]define.ContainerNetworkStats,
 		TxErrors:  stats.TxErrors,
 		TxPackets: stats.TxPackets,
 	}
-
-					perNetworkStats[dev] = newStats
-				}
-			}
-		}
-		return nil
-	})
-	return perNetworkStats, err
 }
 
 // joinedNetworkNSPath returns netns path and bool if netns was set

test/apiv2/20-containers.at

@@ -69,7 +69,7 @@ t GET libpod/containers/json?all=true 200 \
   .[0].IsInfra=false
 
 # Test compat API for Network Settings (.Network is N/A when rootless)
-network_expect="Networks.slirp4netns.NetworkID=slirp4netns"
+network_expect="Networks.pasta.NetworkID=pasta"
 if root; then
     network_expect="Networks.podman.NetworkID=podman"
 fi

test/e2e/unshare_test.go

@@ -9,14 +9,6 @@ import (
 	. "github.com/onsi/gomega/gexec"
 )
 
-// podman unshare --rootless-netns leaks the process by design.
-// Running a container will cause the cleanup to kick in when this container gets stopped.
-func cleanupRootlessSlirp4netns(p *PodmanTestIntegration) {
-	session := p.Podman([]string{"run", "--network", "bridge", ALPINE, "true"})
-	session.WaitWithDefaultTimeout()
-	Expect(session).Should(ExitCleanly())
-}
-
 var _ = Describe("Podman unshare", func() {
 	BeforeEach(func() {
 		if _, err := os.Stat("/proc/self/uid_map"); err != nil {
@@ -37,15 +29,6 @@ var _ = Describe("Podman unshare", func() {
 		Expect(session.OutputToString()).ToNot(ContainSubstring(userNS))
 	})
 
-	It("podman unshare --rootless-netns", func() {
-		SkipIfRemote("podman-remote unshare is not supported")
-		defer cleanupRootlessSlirp4netns(podmanTest)
-		session := podmanTest.Podman([]string{"unshare", "--rootless-netns", "ip", "addr"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).To(ContainSubstring("tap0"))
-	})
-
 	It("podman unshare exit codes", func() {
 		SkipIfRemote("podman-remote unshare is not supported")
 		session := podmanTest.Podman([]string{"unshare", "false"})

test/system/250-systemd.bats

@@ -5,6 +5,7 @@
 
 load helpers
 load helpers.systemd
+load helpers.network
 
 SERVICE_NAME="podman_test_$(random_string)"
 
@@ -294,7 +295,7 @@ LISTEN_FDNAMES=listen_fdnames" | sort)
 }
 
 # https://github.com/containers/podman/issues/13153
-@test "podman rootless-netns slirp4netns process should be in different cgroup" {
+@test "podman rootless-netns pasta processes should be in different cgroup" {
     is_rootless || skip "only meaningful for rootless"
 
     cname=$(random_string)
@@ -314,9 +315,11 @@ LISTEN_FDNAMES=listen_fdnames" | sort)
     # stop systemd container
     service_cleanup
 
+    pasta_iface=$(default_ifname)
+
     # now check that the rootless netns slirp4netns process is still alive and working
     run_podman unshare --rootless-netns ip addr
-    is "$output" ".*tap0.*" "slirp4netns interface exists in the netns"
+    is "$output" ".*$pasta_iface.*" "pasta interface exists in the netns"
     run_podman exec $cname2 nslookup google.com
 
     run_podman rm -f -t0 $cname2

test/system/252-quadlet.bats

@@ -144,6 +144,8 @@ function remove_secret() {
 }
 
 @test "quadlet - basic" {
+    # Network=none is to work around a Pasta bug, can be removed once a patched Pasta is available.
+    # Ref https://github.com/containers/podman/pull/21563#issuecomment-1965145324
     local quadlet_file=$PODMAN_TMPDIR/basic_$(random_string).container
     cat > $quadlet_file <<EOF
 [Container]
@@ -151,6 +153,7 @@ Image=$IMAGE
 Exec=sh -c "echo STARTED CONTAINER; echo "READY=1" | socat -u STDIN unix-sendto:\$NOTIFY_SOCKET; sleep inf"
 Notify=yes
 LogDriver=passthrough
+Network=none
 EOF
 
     # FIXME: Temporary until podman fully removes cgroupsv1 support; see #21431

test/system/270-socket-activation.bats

@@ -4,8 +4,17 @@
 #
 
 load helpers
+load helpers.registry
 load helpers.systemd
 
+function setup_file() {
+    # We have to stop the background registry here. These tests kill the podman pause
+    # process which means commands after that are in a new one and when the cleanup
+    # later tries to stop the registry container it will be in the wrong ns and can fail.
+    # https://github.com/containers/podman/pull/21563#issuecomment-1960047648
+    stop_registry
+}
+
 SERVICE_NAME="podman_test_$(random_string)"
 
 SERVICE_SOCK_ADDR="/run/podman/$SERVICE_NAME.sock"

test/system/500-networking.bats

@@ -857,7 +857,7 @@ EOF
     cid=${output}
     run_podman inspect --format '{{ .NetworkSettings.Networks }}' $cid
     if is_rootless; then
-        is "$output" "map\[slirp4netns:.*" "NeworkSettings should contain one network named slirp4netns"
+        is "$output" "map\[pasta:.*" "NeworkSettings should contain one network named pasta"
     else
         is "$output" "map\[podman:.*" "NeworkSettings should contain one network named podman"
     fi

test/system/505-networking-pasta.bats

@@ -18,21 +18,6 @@ function setup() {
     XFER_FILE="${PODMAN_TMPDIR}/pasta.bin"
 }
 
-function default_ifname() {
-    local ip_ver="${1}"
-
-    local expr='[.[] | select(.dst == "default").dev] | .[0]'
-    ip -j -"${ip_ver}" route show | jq -rM "${expr}"
-}
-
-function default_addr() {
-    local ip_ver="${1}"
-    local ifname="${2:-$(default_ifname "${ip_ver}")}"
-
-    local expr='.[0] | .addr_info[0].local'
-    ip -j -"${ip_ver}" addr show "${ifname}" | jq -rM "${expr}"
-}
-
 # _set_opt() - meta-helper for pasta_test_do.
 #
 # Sets an option, but panics if option is already set (e.g. UDP+TCP, IPv4/v6)
@@ -789,3 +774,14 @@ EOF
     CONTAINERS_CONF_OVERRIDE=$containersconf run_podman run --net=pasta:--ns-mac-addr,"$mac2" $IMAGE ip link show myname
     assert "$output" =~ "$mac2" "mac address from cli is set on custom interface"
 }
+
+### Rootless unshare testins
+
+@test "Podman unshare --rootless-netns with Pasta" {
+    skip_if_remote "unshare is local-only"
+
+    pasta_iface=$(default_ifname)
+
+    run_podman unshare --rootless-netns ip addr
+    is "$output" ".*${pasta_iface}.*"
+}

test/system/550-pause-process.bats

@@ -4,8 +4,17 @@
 #
 
 load helpers
+load helpers.registry
 load helpers.sig-proxy
 
+function setup_file() {
+    # We have to stop the background registry here. These tests kill the podman pause
+    # process which means commands after that are in a new one and when the cleanup
+    # later tries to stop the registry container it will be in the wrong ns and can fail.
+    # https://github.com/containers/podman/pull/21563#issuecomment-1960047648
+    stop_registry
+}
+
 function _check_pause_process() {
     pause_pid=
     if [[ -z "$pause_pid_file" ]]; then

test/system/helpers.network.bash

@@ -369,3 +369,20 @@ function tcp_port_probe() {
 
     : | nc "${address}" "${1}"
 }
+
+### Pasta Helpers ##############################################################
+
+function default_ifname() {
+    local ip_ver="${1}"
+
+    local expr='[.[] | select(.dst == "default").dev] | .[0]'
+    ip -j -"${ip_ver}" route show | jq -rM "${expr}"
+}
+
+function default_addr() {
+    local ip_ver="${1}"
+    local ifname="${2:-$(default_ifname "${ip_ver}")}"
+
+    local expr='.[0] | .addr_info[0].local'
+    ip -j -"${ip_ver}" addr show "${ifname}" | jq -rM "${expr}"
+}

vendor/github.com/containers/common/libimage/events.go

@@ -18,6 +18,8 @@ const (
 	EventTypeUnknown EventType = iota
 	// EventTypeImagePull represents an image pull.
 	EventTypeImagePull
+	// EventTypeImagePullError represents an image pull failed.
+	EventTypeImagePullError
 	// EventTypeImagePush represents an image push.
 	EventTypeImagePush
 	// EventTypeImageRemove represents an image removal.
@@ -46,6 +48,8 @@ type Event struct {
 	Time time.Time
 	// Type of the event.
 	Type EventType
+	// Error in case of failure.
+	Error error
 }
 
 // writeEvent writes the specified event to the Runtime's event channel.  The

vendor/github.com/containers/common/libimage/pull.go

@@ -53,9 +53,16 @@ type PullOptions struct {
 // The error is storage.ErrImageUnknown iff the pull policy is set to "never"
 // and no local image has been found.  This allows for an easier integration
 // into some users of this package (e.g., Buildah).
-func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullPolicy, options *PullOptions) ([]*Image, error) {
+func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullPolicy, options *PullOptions) (_ []*Image, pullError error) {
 	logrus.Debugf("Pulling image %s (policy: %s)", name, pullPolicy)
-
+	if r.eventChannel != nil {
+		defer func() {
+			if pullError != nil {
+				// Note that we use the input name here to preserve the transport data.
+				r.writeEvent(&Event{Name: name, Time: time.Now(), Type: EventTypeImagePullError, Error: pullError})
+			}
+		}()
+	}
 	if options == nil {
 		options = &PullOptions{}
 	}
@@ -150,28 +157,25 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP
 		options.Variant = r.systemContext.VariantChoice
 	}
 
-	var (
-		pulledImages []string
-		pullError    error
-	)
+	var pulledImages []string
 
 	// Dispatch the copy operation.
 	switch ref.Transport().Name() {
 	// DOCKER REGISTRY
 	case registryTransport.Transport.Name():
-		pulledImages, pullError = r.copyFromRegistry(ctx, ref, possiblyUnqualifiedName, pullPolicy, options)
+		pulledImages, err = r.copyFromRegistry(ctx, ref, possiblyUnqualifiedName, pullPolicy, options)
 
 	// DOCKER ARCHIVE
 	case dockerArchiveTransport.Transport.Name():
-		pulledImages, pullError = r.copyFromDockerArchive(ctx, ref, &options.CopyOptions)
+		pulledImages, err = r.copyFromDockerArchive(ctx, ref, &options.CopyOptions)
 
 	// ALL OTHER TRANSPORTS
 	default:
-		pulledImages, pullError = r.copyFromDefault(ctx, ref, &options.CopyOptions)
+		pulledImages, err = r.copyFromDefault(ctx, ref, &options.CopyOptions)
 	}
 
-	if pullError != nil {
-		return nil, pullError
+	if err != nil {
+		return nil, err
 	}
 
 	localImages := []*Image{}

vendor/github.com/containers/common/libnetwork/internal/rootlessnetns/netns_linux.go

@@ -8,9 +8,9 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
-	"syscall"
 
 	"github.com/containernetworking/plugins/pkg/ns"
+	"github.com/containers/common/libnetwork/pasta"
 	"github.com/containers/common/libnetwork/resolvconf"
 	"github.com/containers/common/libnetwork/slirp4netns"
 	"github.com/containers/common/pkg/config"
@@ -31,8 +31,8 @@ const (
 	// refCountFile file name for the ref count file
 	refCountFile = "ref-count"
 
-	// rootlessNetNsSilrp4netnsPidFile is the name of the rootless netns slirp4netns pid file
-	rootlessNetNsSilrp4netnsPidFile = "rootless-netns-slirp4netns.pid"
+	// rootlessNetNsConnPidFile is the name of the rootless netns slirp4netns/pasta pid file
+	rootlessNetNsConnPidFile = "rootless-netns-conn.pid"
 
 	// persistentCNIDir is the directory where the CNI files are stored
 	persistentCNIDir = "/var/lib/cni"
@@ -113,7 +113,14 @@ func (n *Netns) getOrCreateNetns() (ns.NetNS, bool, error) {
 	if err != nil {
 		return nil, false, wrapError("create netns", err)
 	}
+	switch strings.ToLower(n.config.Network.DefaultRootlessNetworkCmd) {
+	case "", slirp4netns.BinaryName:
 		err = n.setupSlirp4netns(nsPath)
+	case pasta.BinaryName:
+		err = n.setupPasta(nsPath)
+	default:
+		err = fmt.Errorf("invalid rootless network command %q", n.config.Network.DefaultRootlessNetworkCmd)
+	}
 	return netns, true, err
 }
 
@@ -133,8 +140,8 @@ func (n *Netns) cleanup() error {
 	if err := netns.UnmountNS(nsPath); err != nil {
 		multiErr = multierror.Append(multiErr, err)
 	}
-	if err := n.cleanupSlirp4netns(); err != nil {
-		multiErr = multierror.Append(multiErr, wrapError("kill slirp4netns", err))
+	if err := n.cleanupRootlessNetns(); err != nil {
+		multiErr = multierror.Append(multiErr, wrapError("kill network process", err))
 	}
 	if err := os.RemoveAll(n.dir); err != nil {
 		multiErr = multierror.Append(multiErr, wrapError("remove rootless netns dir", err))
@@ -143,6 +150,53 @@ func (n *Netns) cleanup() error {
 	return multiErr.ErrorOrNil()
 }
 
+func (n *Netns) setupPasta(nsPath string) error {
+	pidPath := n.getPath(rootlessNetNsConnPidFile)
+
+	pastaOpts := pasta.SetupOptions{
+		Config:       n.config,
+		Netns:        nsPath,
+		ExtraOptions: []string{"--pid", pidPath},
+	}
+	if err := pasta.Setup(&pastaOpts); err != nil {
+		return fmt.Errorf("setting up Pasta: %w", err)
+	}
+
+	if systemd.RunsOnSystemd() {
+		// Treat these as fatal - if pasta failed to write a PID file something is probably wrong.
+		pidfile, err := os.ReadFile(pidPath)
+		if err != nil {
+			return fmt.Errorf("unable to open pasta PID file: %w", err)
+		}
+		pid, err := strconv.Atoi(strings.TrimSpace(string(pidfile)))
+		if err != nil {
+			return fmt.Errorf("unable to decode pasta PID: %w", err)
+		}
+
+		if err := systemd.MoveRootlessNetnsSlirpProcessToUserSlice(pid); err != nil {
+			// only log this, it is not fatal but can lead to issues when running podman inside systemd units
+			logrus.Errorf("failed to move the rootless netns pasta process to the systemd user.slice: %v", err)
+		}
+	}
+
+	if err := resolvconf.New(&resolvconf.Params{
+		Path: n.getPath(resolvConfName),
+		// fake the netns since we want to filter localhost
+		Namespaces: []specs.LinuxNamespace{
+			{Type: specs.NetworkNamespace},
+		},
+		// TODO: Need a way to determine if there is a valid v6 address on any
+		// external interface of the system.
+		IPv6Enabled:     false,
+		KeepHostServers: true,
+		Nameservers:     []string{},
+	}); err != nil {
+		return wrapError("create resolv.conf", err)
+	}
+
+	return nil
+}
+
 func (n *Netns) setupSlirp4netns(nsPath string) error {
 	res, err := slirp4netns.Setup(&slirp4netns.SetupOptions{
 		Config:      n.config,
@@ -155,7 +209,7 @@ func (n *Netns) setupSlirp4netns(nsPath string) error {
 	// create pid file for the slirp4netns process
 	// this is need to kill the process in the cleanup
 	pid := strconv.Itoa(res.Pid)
-	err = os.WriteFile(n.getPath(rootlessNetNsSilrp4netnsPidFile), []byte(pid), 0o600)
+	err = os.WriteFile(n.getPath(rootlessNetNsConnPidFile), []byte(pid), 0o600)
 	if err != nil {
 		return wrapError("write slirp4netns pid file", err)
 	}
@@ -190,15 +244,18 @@ func (n *Netns) setupSlirp4netns(nsPath string) error {
 	return nil
 }
 
-func (n *Netns) cleanupSlirp4netns() error {
-	pidFile := n.getPath(rootlessNetNsSilrp4netnsPidFile)
+func (n *Netns) cleanupRootlessNetns() error {
+	pidFile := n.getPath(rootlessNetNsConnPidFile)
 	b, err := os.ReadFile(pidFile)
 	if err == nil {
 		var i int
-		i, err = strconv.Atoi(string(b))
+		i, err = strconv.Atoi(strings.TrimSpace(string(b)))
 		if err == nil {
 			// kill the slirp process so we do not leak it
-			err = syscall.Kill(i, syscall.SIGTERM)
+			err = unix.Kill(i, unix.SIGTERM)
+			if err == unix.ESRCH {
+				err = nil
+			}
 		}
 	}
 	return err

vendor/github.com/containers/common/pkg/config/containers.conf

@@ -384,9 +384,9 @@ default_sysctls = [
 
 
 # Configure which rootless network program to use by default. Valid options are
-# `slirp4netns` (default) and `pasta`.
+# `slirp4netns` and `pasta` (default).
 #
-#default_rootless_network_cmd = "slirp4netns"
+#default_rootless_network_cmd = "pasta"
 
 # Path to the directory where network configuration files are located.
 # For the CNI backend the default is "/etc/cni/net.d" as root

vendor/github.com/containers/common/pkg/config/default.go

@@ -257,7 +257,7 @@ func defaultConfig() (*Config, error) {
 			DefaultNetwork:            "podman",
 			DefaultSubnet:             DefaultSubnet,
 			DefaultSubnetPools:        DefaultSubnetPools,
-			DefaultRootlessNetworkCmd: "slirp4netns",
+			DefaultRootlessNetworkCmd: "pasta",
 			DNSBindPort:               0,
 			CNIPluginDirs:             attributedstring.NewSlice(DefaultCNIPluginDirs),
 			NetavarkPluginDirs:        attributedstring.NewSlice(DefaultNetavarkPluginDirs),

vendor/modules.txt

@@ -171,7 +171,7 @@ github.com/containers/buildah/pkg/sshagent
 github.com/containers/buildah/pkg/util
 github.com/containers/buildah/pkg/volumes
 github.com/containers/buildah/util
-# github.com/containers/common v0.57.1-0.20240229151045-1c65e0de241a
+# github.com/containers/common v0.57.1-0.20240229165734-cec09922602e
 ## explicit; go 1.20
 github.com/containers/common/internal
 github.com/containers/common/internal/attributedstring