From ef42f54aca619476caf889fe47632eb88dfffbd3 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Tue, 13 Jun 2023 11:17:22 -0600 Subject: [PATCH] e2e: GetSafeIPAddress() replaces GetRandomIPAddress For tests that use '--ip XX', random IP allocation is not working well. Switch instead to a deterministic algorithm with CPU affinity and a fudge factor for CNI. Signed-off-by: Ed Santiago --- test/e2e/checkpoint_image_test.go | 2 +- test/e2e/checkpoint_test.go | 2 +- test/e2e/common_test.go | 40 ++++++++++++++++++------------- test/e2e/create_staticip_test.go | 4 ++-- test/e2e/pod_create_test.go | 6 ++--- test/e2e/run_staticip_test.go | 8 +++---- 6 files changed, 35 insertions(+), 27 deletions(-) diff --git a/test/e2e/checkpoint_image_test.go b/test/e2e/checkpoint_image_test.go index ab3dc72908..3334fda11b 100644 --- a/test/e2e/checkpoint_image_test.go +++ b/test/e2e/checkpoint_image_test.go @@ -49,7 +49,7 @@ var _ = Describe("Podman checkpoint", func() { localRunString := []string{ "run", "-d", - "--ip", GetRandomIPAddress(), + "--ip", GetSafeIPAddress(), "--name", containerName, ALPINE, "top", diff --git a/test/e2e/checkpoint_test.go b/test/e2e/checkpoint_test.go index e713667721..5218ff8a2a 100644 --- a/test/e2e/checkpoint_test.go +++ b/test/e2e/checkpoint_test.go @@ -23,7 +23,7 @@ import ( func getRunString(input []string) []string { // CRIU does not work with seccomp correctly on RHEL7 : seccomp=unconfined - runString := []string{"run", "--security-opt", "seccomp=unconfined", "-d", "--ip", GetRandomIPAddress()} + runString := []string{"run", "--security-opt", "seccomp=unconfined", "-d", "--ip", GetSafeIPAddress()} return append(runString, input...) } diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index 12b2b0e40f..66464d5fd6 100644 --- a/test/e2e/common_test.go +++ b/test/e2e/common_test.go @@ -103,15 +103,19 @@ func TestLibpod(t *testing.T) { } var ( - tempdir string - err error - podmanTest *PodmanTestIntegration + tempdir string + err error + podmanTest *PodmanTestIntegration + safeIPOctets [2]uint8 _ = BeforeEach(func() { tempdir, err = CreateTempDirInTempDir() Expect(err).ToNot(HaveOccurred()) podmanTest = PodmanTestCreate(tempdir) podmanTest.Setup() + // see GetSafeIPAddress() below + safeIPOctets[0] = uint8(GinkgoT().ParallelProcess()) + 128 + safeIPOctets[1] = 2 }) _ = AfterEach(func() { @@ -479,19 +483,23 @@ func GetPortLock(port string) *lockfile.LockFile { return lock } -// GetRandomIPAddress returns a random IP address to avoid IP -// collisions during parallel tests -func GetRandomIPAddress() string { - // To avoid IP collisions of initialize random seed for random IP addresses - rng := rand.New(rand.NewSource(time.Now().UnixNano())) - - nProcs := GinkgoT().ParallelTotal() - myProc := GinkgoT().ParallelProcess() - 1 - - // 10.88.255 is unlikely to be used by CNI or netavark. - // Last octet .0 - .254, and will be unique to this ginkgo process. - ip4 := strconv.Itoa(rng.Intn((255-nProcs)/nProcs)*nProcs + myProc) - return "10.88.255." + ip4 +// GetSafeIPAddress returns a sequentially allocated IP address that _should_ +// be safe and unique across parallel tasks +// +// Used by tests which want to use "--ip SOMETHING-SAFE". Picking at random +// just doesn't work: we get occasional collisions. Our current approach +// allocates a /24 subnet for each ginkgo process, starting at .128.x, see +// BeforeEach() above. Unfortunately, CNI remembers each address assigned +// and assigns by default -- so other parallel jobs may +// get IPs in our block. The +10 leaves a gap for that. (Netavark works +// differently, allocating sequentially from .0.0, hence our .128.x). +// This heuristic will fail if run in parallel on >127 processors or if +// one test calls us more than 25 times or if some other test runs more +// than ten networked containers at the same time as any test that +// relies on GetSafeIPAddress(). I'm finding it hard to care. +func GetSafeIPAddress() string { + safeIPOctets[1] += 10 + return fmt.Sprintf("10.88.%d.%d", safeIPOctets[0], safeIPOctets[1]) } // RunTopContainer runs a simple container in the background that diff --git a/test/e2e/create_staticip_test.go b/test/e2e/create_staticip_test.go index 01bd0323e3..e623c18e26 100644 --- a/test/e2e/create_staticip_test.go +++ b/test/e2e/create_staticip_test.go @@ -29,7 +29,7 @@ var _ = Describe("Podman create with --ip flag", func() { }) It("Podman create with specified static IP has correct IP", func() { - ip := GetRandomIPAddress() + ip := GetSafeIPAddress() result := podmanTest.Podman([]string{"create", "--name", "test", "--ip", ip, ALPINE, "ip", "addr"}) result.WaitWithDefaultTimeout() // Rootless static ip assignment without network should error @@ -47,7 +47,7 @@ var _ = Describe("Podman create with --ip flag", func() { It("Podman create two containers with the same IP", func() { SkipIfRootless("--ip not supported without network in rootless mode") - ip := GetRandomIPAddress() + ip := GetSafeIPAddress() result := podmanTest.Podman([]string{"create", "--log-driver", "k8s-file", "--name", "test1", "--ip", ip, ALPINE, "sleep", "999"}) result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go index 7c6da34ab0..e725eea1ca 100644 --- a/test/e2e/pod_create_test.go +++ b/test/e2e/pod_create_test.go @@ -213,7 +213,7 @@ var _ = Describe("Podman pod create", func() { It("podman create pod with IP address", func() { name := "test" - ip := GetRandomIPAddress() + ip := GetSafeIPAddress() podCreate := podmanTest.Podman([]string{"pod", "create", "--ip", ip, "--name", name}) podCreate.WaitWithDefaultTimeout() // Rootless should error without network @@ -232,7 +232,7 @@ var _ = Describe("Podman pod create", func() { SkipIfRootless("Rootless does not support --ip without network") podName := "test" ctrName := "testCtr" - ip := GetRandomIPAddress() + ip := GetSafeIPAddress() podCreate := podmanTest.Podman([]string{"pod", "create", "--ip", ip, "--name", podName}) podCreate.WaitWithDefaultTimeout() Expect(podCreate).Should(Exit(0)) @@ -248,7 +248,7 @@ var _ = Describe("Podman pod create", func() { It("podman create pod with IP address and no infra should fail", func() { name := "test" - ip := GetRandomIPAddress() + ip := GetSafeIPAddress() podCreate := podmanTest.Podman([]string{"pod", "create", "--ip", ip, "--name", name, "--infra=false"}) podCreate.WaitWithDefaultTimeout() Expect(podCreate).Should(Exit(125)) diff --git a/test/e2e/run_staticip_test.go b/test/e2e/run_staticip_test.go index a103453dcc..4a8f875384 100644 --- a/test/e2e/run_staticip_test.go +++ b/test/e2e/run_staticip_test.go @@ -37,7 +37,7 @@ var _ = Describe("Podman run with --ip flag", func() { }) It("Podman run with specified static IP has correct IP", func() { - ip := GetRandomIPAddress() + ip := GetSafeIPAddress() result := podmanTest.Podman([]string{"run", "--ip", ip, ALPINE, "ip", "addr"}) result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) @@ -59,7 +59,7 @@ var _ = Describe("Podman run with --ip flag", func() { }) It("Podman run with --network bridge:ip=", func() { - ip := GetRandomIPAddress() + ip := GetSafeIPAddress() result := podmanTest.Podman([]string{"run", "--network", "bridge:ip=" + ip, ALPINE, "ip", "addr"}) result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) @@ -67,7 +67,7 @@ var _ = Describe("Podman run with --ip flag", func() { }) It("Podman run with --network net:ip=,mac=,interface_name=", func() { - ip := GetRandomIPAddress() + ip := GetSafeIPAddress() mac := "44:33:22:11:00:99" intName := "myeth" result := podmanTest.Podman([]string{"run", "--network", "bridge:ip=" + ip + ",mac=" + mac + ",interface_name=" + intName, ALPINE, "ip", "addr"}) @@ -79,7 +79,7 @@ var _ = Describe("Podman run with --ip flag", func() { }) It("Podman run two containers with the same IP", func() { - ip := GetRandomIPAddress() + ip := GetSafeIPAddress() result := podmanTest.Podman([]string{"run", "-d", "--name", "nginx", "--ip", ip, NGINX_IMAGE}) result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0))