From 2d469e517dd0bf1ce5a8babbb76663abc369572c Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 18 Sep 2024 12:04:27 +0200 Subject: [PATCH 1/2] test/system: netns leak check for rootless as well This fixes the problem where even as root we check the netns files from root. But in order to catch any rootless bugs we must check the rootless files from $XDG_RUNTIME_DIR/netns. Signed-off-by: Paul Holzinger --- test/system/setup_suite.bash | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/test/system/setup_suite.bash b/test/system/setup_suite.bash index 8a920c614167..48a71c7fe4ae 100644 --- a/test/system/setup_suite.bash +++ b/test/system/setup_suite.bash @@ -37,7 +37,7 @@ function setup_suite() { touch "$BATS_SUITE_TMPDIR/all-tests-passed" # Track network namespaces, so we can check for leaks at test end - ip netns list > $BATS_SUITE_TMPDIR/netns-pre + check_netns_files > $BATS_SUITE_TMPDIR/netns-pre } # Run at the very end of all tests. Useful for cleanup of non-BATS tmpdirs. @@ -54,14 +54,30 @@ function teardown_suite() { fi # Network namespace leak check. List should match what we saw above. + # When they leak we indefinitely leak resources which is bad. echo - ip netns list > $BATS_SUITE_TMPDIR/netns-post + check_netns_files > $BATS_SUITE_TMPDIR/netns-post if ! diff -u $BATS_SUITE_TMPDIR/netns-{pre,post}; then echo - echo "^^^^^ Leaks found in /run/netns ^^^^^" + echo "^^^^^ Leaks found in $NETNS_DIR ^^^^^" exit_code=$((exit_code + 1)) fi fi return $exit_code } + +NETNS_DIR= +# List a files in the common netns dir that is used to bind the netns files. +function check_netns_files() { + if is_rootless; then + NETNS_DIR=$XDG_RUNTIME_DIR/netns + else + NETNS_DIR=/run/netns + fi + + # The dir may not exists which is fine + if [ -d "$NETNS_DIR" ]; then + ls -1 "$NETNS_DIR" + fi +} From 755a06aa441e79319cd00c39cf5144e427d192a6 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 18 Sep 2024 12:23:29 +0200 Subject: [PATCH 2/2] test/e2e: add netns leak check Like we do in system tests now check for netns leaks in e2e as well. Now because things run in parallel and this dir is shared we cannot test after each test only once per suite. This will be a PITA to debug if leaks happen as the netns files do not contain the container ID and are just random bytes (maybe we should change this?) Fixes #23715 Signed-off-by: Paul Holzinger --- test/e2e/common_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index d19306c533ee..61fed90b648e 100644 --- a/test/e2e/common_test.go +++ b/test/e2e/common_test.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "io/fs" "math/rand" "net" "net/url" @@ -138,10 +139,32 @@ const ( imageCacheDir = "imagecachedir" ) +var netnsFiles []fs.DirEntry + +func getNetnsDir() string { + if isRootless() { + var path string + if env, ok := os.LookupEnv("XDG_RUNTIME_DIR"); ok { + path = env + } else { + path = fmt.Sprintf("/run/user/%d", os.Getuid()) + } + return filepath.Join(path, "netns") + } + // root is hard coded to + return "/run/netns" +} + var _ = SynchronizedBeforeSuite(func() []byte { globalTmpDir, err := os.MkdirTemp("", "podman-e2e-") Expect(err).ToNot(HaveOccurred()) + netnsFiles, err = os.ReadDir(getNetnsDir()) + // dir might not exists which is fine + if !errors.Is(err, fs.ErrNotExist) { + Expect(err).ToNot(HaveOccurred()) + } + // make cache dir ImageCacheDir = filepath.Join(globalTmpDir, imageCacheDir) err = os.MkdirAll(ImageCacheDir, 0700) @@ -203,6 +226,13 @@ var _ = SynchronizedAfterSuite(func() { timingsFile = nil }, func() { + // perform a netns leak check after all tests run + newNetnsFiles, err := os.ReadDir(getNetnsDir()) + if !errors.Is(err, fs.ErrNotExist) { + Expect(err).ToNot(HaveOccurred()) + } + Expect(newNetnsFiles).To(ConsistOf(netnsFiles), "Netns files were leaked") + testTimings := make(testResultsSorted, 0, 2000) for i := 1; i <= GinkgoT().ParallelTotal(); i++ { f, err := os.Open(fmt.Sprintf("%s/timings-%d", LockTmpDir, i))