From ee146a9ab0721ec6fe2c6e3092928c34a9b3b6d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 3 Nov 2021 15:59:23 +0100 Subject: [PATCH 1/3] Refactor remote socket path determination in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Separate the code that determines the directory and file prefix from the code that chooses and applies a UUID; we will make the second part more complex in a bit. Should not change behavior. Signed-off-by: Miloslav Trmač --- test/e2e/common_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index 6e1a62b997..456e2bab17 100644 --- a/test/e2e/common_test.go +++ b/test/e2e/common_test.go @@ -274,15 +274,15 @@ func PodmanTestCreateUtil(tempDir string, remote bool) *PodmanTestIntegration { } if remote { - uuid := stringid.GenerateNonCryptoID() + var pathPrefix string if !rootless.IsRootless() { - p.RemoteSocket = fmt.Sprintf("unix:/run/podman/podman-%s.sock", uuid) + pathPrefix = "/run/podman/podman" } else { runtimeDir := os.Getenv("XDG_RUNTIME_DIR") - socket := fmt.Sprintf("podman-%s.sock", uuid) - fqpath := filepath.Join(runtimeDir, socket) - p.RemoteSocket = fmt.Sprintf("unix:%s", fqpath) + pathPrefix = filepath.Join(runtimeDir, "podman") } + uuid := stringid.GenerateNonCryptoID() + p.RemoteSocket = fmt.Sprintf("unix:%s-%s.sock", pathPrefix, uuid) } // Setup registries.conf ENV variable From 90e74e794cc0c2c34112877821ed9ff0e7f51c28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 3 Nov 2021 16:37:49 +0100 Subject: [PATCH 2/3] Avoid collisions on RemoteSocket paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add lock files and re-generate the UUID if we are not a known-unique user of the socket path. Signed-off-by: Miloslav Trmač --- test/e2e/common_test.go | 22 ++++++++++++++++++++-- test/e2e/libpod_suite_remote_test.go | 6 ++++++ test/utils/utils.go | 1 + 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index 456e2bab17..bd744aa782 100644 --- a/test/e2e/common_test.go +++ b/test/e2e/common_test.go @@ -281,8 +281,26 @@ func PodmanTestCreateUtil(tempDir string, remote bool) *PodmanTestIntegration { runtimeDir := os.Getenv("XDG_RUNTIME_DIR") pathPrefix = filepath.Join(runtimeDir, "podman") } - uuid := stringid.GenerateNonCryptoID() - p.RemoteSocket = fmt.Sprintf("unix:%s-%s.sock", pathPrefix, uuid) + // We want to avoid collisions in socket paths, but using the + // socket directly for a collision check doesn’t work; bind(2) on AF_UNIX + // creates the file, and we need to pass a unique path now before the bind(2) + // happens. So, use a podman-%s.sock-lock empty file as a marker. + tries := 0 + for { + uuid := stringid.GenerateNonCryptoID() + lockPath := fmt.Sprintf("%s-%s.sock-lock", pathPrefix, uuid) + lockFile, err := os.OpenFile(lockPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0700) + if err == nil { + lockFile.Close() + p.RemoteSocketLock = lockPath + p.RemoteSocket = fmt.Sprintf("unix:%s-%s.sock", pathPrefix, uuid) + break + } + tries++ + if tries >= 1000 { + panic("Too many RemoteSocket collisions") + } + } } // Setup registries.conf ENV variable diff --git a/test/e2e/libpod_suite_remote_test.go b/test/e2e/libpod_suite_remote_test.go index d60383029f..4644e3748e 100644 --- a/test/e2e/libpod_suite_remote_test.go +++ b/test/e2e/libpod_suite_remote_test.go @@ -1,3 +1,4 @@ +//go:build remote // +build remote package integration @@ -143,6 +144,11 @@ func (p *PodmanTestIntegration) StopRemoteService() { if err := os.Remove(socket); err != nil { fmt.Println(err) } + if p.RemoteSocketLock != "" { + if err := os.Remove(p.RemoteSocketLock); err != nil { + fmt.Println(err) + } + } } //MakeOptions assembles all the podman main options diff --git a/test/utils/utils.go b/test/utils/utils.go index f41024072f..a73bdb8564 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -41,6 +41,7 @@ type PodmanTest struct { RemotePodmanBinary string RemoteSession *os.Process RemoteSocket string + RemoteSocketLock string // If not "", should be removed _after_ RemoteSocket is removed RemoteCommand *exec.Cmd ImageCacheDir string ImageCacheFS string From f6a3eddd2cbe6434c87cf1db04aaa56aac935e5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 29 Nov 2021 18:58:38 +0100 Subject: [PATCH 3/3] Don't initialize the global RNG with GinkgoRandomSeed() in e2e tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - It probably doesn't actually make a difference: in experiments, the github.com/containers/storage/pkg/stringid RNG initialization has been happening later - This makes the RNG caller-controlled (which we don't benefit from), but also the same on all nodes of multi-process Ginkgo execution. So, if it works at all, it may make collisions of random ID values more likely, and our tests are not robust against that. So don't go out of our way to make collisions more likely. Signed-off-by: Miloslav Trmač --- test/utils/utils.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/utils/utils.go b/test/utils/utils.go index a73bdb8564..1f5067950d 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -470,10 +470,6 @@ func Containerized() bool { return strings.Contains(string(b), "docker") } -func init() { - rand.Seed(GinkgoRandomSeed()) -} - var randomLetters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") // RandomString returns a string of given length composed of random characters