From 4c1c4c082a88a3da950e10883db33c4052cd9d03 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Thu, 8 Feb 2024 09:21:41 -0500 Subject: [PATCH 1/2] 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 --- go.mod | 2 +- go.sum | 4 +- libpod/networking_linux.go | 60 ++++++-------- test/apiv2/20-containers.at | 2 +- test/e2e/unshare_test.go | 17 ---- test/system/250-systemd.bats | 7 +- test/system/252-quadlet.bats | 3 + test/system/270-socket-activation.bats | 9 +++ test/system/500-networking.bats | 2 +- test/system/505-networking-pasta.bats | 26 +++--- test/system/550-pause-process.bats | 9 +++ test/system/helpers.network.bash | 17 ++++ .../containers/common/libimage/events.go | 4 + .../containers/common/libimage/pull.go | 26 +++--- .../internal/rootlessnetns/netns_linux.go | 79 ++++++++++++++++--- .../common/pkg/config/containers.conf | 4 +- .../containers/common/pkg/config/default.go | 2 +- vendor/modules.txt | 2 +- 18 files changed, 175 insertions(+), 100 deletions(-) diff --git a/go.mod b/go.mod index 7eb7e157bc..af9856b044 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 78d5090cbd..0887950a9b 100644 --- a/go.sum +++ b/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= diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 10e0ec53e4..a8a057d991 100644 --- a/libpod/networking_linux.go +++ b/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,42 +201,19 @@ 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) - if err != nil { - return err - } - stats := link.Attrs().Statistics - if stats != nil { - newStats := define.ContainerNetworkStats{ - RxBytes: stats.RxBytes, - RxDropped: stats.RxDropped, - RxErrors: stats.RxErrors, - RxPackets: stats.RxPackets, - TxBytes: stats.TxBytes, - TxDropped: stats.TxDropped, - TxErrors: stats.TxErrors, - TxPackets: stats.TxPackets, - } + links, err := netlink.LinkList() + if err != nil { + return fmt.Errorf("retrieving all network interfaces: %w", err) + } + for _, link := range links { + attributes := link.Attrs() + if attributes.Flags&net.FlagLoopback != 0 { + continue + } - perNetworkStats[dev] = newStats - } + if attributes.Statistics != nil { + perNetworkStats[attributes.Name] = getNetStatsFromNetlinkStats(attributes.Statistics) } } return nil @@ -244,6 +221,19 @@ func getContainerNetIO(ctr *Container) (map[string]define.ContainerNetworkStats, return perNetworkStats, err } +func getNetStatsFromNetlinkStats(stats *netlink.LinkStatistics) define.ContainerNetworkStats { + return define.ContainerNetworkStats{ + RxBytes: stats.RxBytes, + RxDropped: stats.RxDropped, + RxErrors: stats.RxErrors, + RxPackets: stats.RxPackets, + TxBytes: stats.TxBytes, + TxDropped: stats.TxDropped, + TxErrors: stats.TxErrors, + TxPackets: stats.TxPackets, + } +} + // joinedNetworkNSPath returns netns path and bool if netns was set func (c *Container) joinedNetworkNSPath() (string, bool) { for _, namespace := range c.config.Spec.Linux.Namespaces { diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index c30f848256..6e3ef6fd5d 100644 --- a/test/apiv2/20-containers.at +++ b/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 diff --git a/test/e2e/unshare_test.go b/test/e2e/unshare_test.go index 0b4c115158..b004fe2800 100644 --- a/test/e2e/unshare_test.go +++ b/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"}) diff --git a/test/system/250-systemd.bats b/test/system/250-systemd.bats index 1d590224f8..81573b7063 100644 --- a/test/system/250-systemd.bats +++ b/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 diff --git a/test/system/252-quadlet.bats b/test/system/252-quadlet.bats index 1dda5518d2..bf1f7450e7 100644 --- a/test/system/252-quadlet.bats +++ b/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 < Date: Thu, 29 Feb 2024 13:53:36 -0500 Subject: [PATCH 2/2] Fix events by fully adding the new PullError event Signed-off-by: Matt Heon --- libpod/events/config.go | 4 ++++ libpod/events/events.go | 2 ++ libpod/events/journal_linux.go | 6 ++++++ libpod/runtime.go | 19 ++++++++++--------- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/libpod/events/config.go b/libpod/events/config.go index c67f0daae2..deb8f9e4f1 100644 --- a/libpod/events/config.go +++ b/libpod/events/config.go @@ -41,6 +41,8 @@ type Event struct { Type Type // Health status of the current container HealthStatus string `json:"health_status,omitempty"` + // Error code for certain events involving errors. + Error error `json:"error,omitempty"` Details } @@ -170,6 +172,8 @@ const ( Prune Status = "prune" // Pull ... Pull Status = "pull" + // PullError is an error pulling an image + PullError Status = "pull-error" // Push ... Push Status = "push" // Refresh indicates that the system refreshed the state after a diff --git a/libpod/events/events.go b/libpod/events/events.go index 18f5314691..11a1785b3d 100644 --- a/libpod/events/events.go +++ b/libpod/events/events.go @@ -194,6 +194,8 @@ func StringToStatus(name string) (Status, error) { return Prune, nil case Pull.String(): return Pull, nil + case PullError.String(): + return PullError, nil case Push.String(): return Push, nil case Refresh.String(): diff --git a/libpod/events/journal_linux.go b/libpod/events/journal_linux.go index 273c5307dc..e31c63c6b7 100644 --- a/libpod/events/journal_linux.go +++ b/libpod/events/journal_linux.go @@ -43,6 +43,9 @@ func (e EventJournalD) Write(ee Event) error { case Image: m["PODMAN_NAME"] = ee.Name m["PODMAN_ID"] = ee.ID + if ee.Error != nil { + m["ERROR"] = ee.Error.Error() + } case Container, Pod: m["PODMAN_IMAGE"] = ee.Image m["PODMAN_NAME"] = ee.Name @@ -228,6 +231,9 @@ func newEventFromJournalEntry(entry *sdjournal.JournalEntry) (*Event, error) { newEvent.Network = entry.Fields["PODMAN_NETWORK_NAME"] case Image: newEvent.ID = entry.Fields["PODMAN_ID"] + if _, ok := entry.Fields["ERROR"]; ok { + newEvent.Error = errors.New(entry.Fields["ERROR"]) + } } return &newEvent, nil } diff --git a/libpod/runtime.go b/libpod/runtime.go index 50b2c75eb9..d430f3a421 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -696,15 +696,16 @@ func (r *Runtime) GetConfig() (*config.Config, error) { // libimageEventsMap translates a libimage event type to a libpod event status. var libimageEventsMap = map[libimage.EventType]events.Status{ - libimage.EventTypeImagePull: events.Pull, - libimage.EventTypeImagePush: events.Push, - libimage.EventTypeImageRemove: events.Remove, - libimage.EventTypeImageLoad: events.LoadFromArchive, - libimage.EventTypeImageSave: events.Save, - libimage.EventTypeImageTag: events.Tag, - libimage.EventTypeImageUntag: events.Untag, - libimage.EventTypeImageMount: events.Mount, - libimage.EventTypeImageUnmount: events.Unmount, + libimage.EventTypeImagePull: events.Pull, + libimage.EventTypeImagePullError: events.PullError, + libimage.EventTypeImagePush: events.Push, + libimage.EventTypeImageRemove: events.Remove, + libimage.EventTypeImageLoad: events.LoadFromArchive, + libimage.EventTypeImageSave: events.Save, + libimage.EventTypeImageTag: events.Tag, + libimage.EventTypeImageUntag: events.Untag, + libimage.EventTypeImageMount: events.Mount, + libimage.EventTypeImageUnmount: events.Unmount, } // libimageEvents spawns a goroutine which will listen for events on