From 583ad9ecdf0efa29bf84df8beff7747ec0af3b59 Mon Sep 17 00:00:00 2001 From: Indradhanush Gupta Date: Sun, 19 Jul 2026 14:36:43 -0400 Subject: [PATCH 1/3] test(e2e): fix concurrency bugs in parallel test execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix two concurrency bugs under Ginkgo's in-process parallelism (GINKGO_NODES>1). 1. Setup routine's IPC payload omitted two critical environment variables; N-1 of N parallel processes got empty strings, triggering nil-pointer panics. Observed: 6 of 7 processes panicking on each run. 2. Control-plane endpoint IP hardcoded per-cluster; multiple concurrent specs' clusters collided on the same IP, causing apiserver TLS certs to duplicate the contested IP and flooding logs with certificate authority errors. Observed: 651 errors in one run. Both fixes are isolated to test-harness code and covered by table-driven unit tests using TDD. Verified: panics 6/7 → 0/7; cert errors 651 → 0. Co-Authored-By: Claude Sonnet 5 --- test/e2e/docker_helper.go | 37 +++++++++---- test/e2e/docker_helper_test.go | 53 +++++++++++++++++++ test/e2e/e2e_suite_shared_data_test.go | 50 ++++++++++++++++++ test/e2e/e2e_suite_test.go | 72 ++++++++++++++++++++------ 4 files changed, 187 insertions(+), 25 deletions(-) create mode 100644 test/e2e/docker_helper_test.go create mode 100644 test/e2e/e2e_suite_shared_data_test.go diff --git a/test/e2e/docker_helper.go b/test/e2e/docker_helper.go index 4d95220f1..d92caa8e1 100644 --- a/test/e2e/docker_helper.go +++ b/test/e2e/docker_helper.go @@ -1,14 +1,17 @@ // Copyright 2021 VMware, Inc. All Rights Reserved. +// Copyright 2026 Platform9, Inc. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package e2e import ( "context" + "fmt" "io" "os" "path/filepath" "regexp" + "strconv" "strings" "github.com/docker/cli/cli/command" @@ -20,6 +23,7 @@ import ( "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/system" "github.com/docker/go-units" + ginkgo "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" //nolint: staticcheck "github.com/pkg/errors" "k8s.io/client-go/tools/clientcmd" @@ -351,6 +355,24 @@ func (r *ByoHostRunner) ExecByoDockerHost(byohost *container.CreateResponse) (ty return output, byohost.ID, err } +// controlPlaneEndpointIPBaseOctet is the lowest last-octet value handed out for a control-plane +// endpoint IP (process 1 gets this value, process 2 gets this+1, and so on). It's a static IP in +// the kind network's subnet but outside its DHCP range. +const controlPlaneEndpointIPBaseOctet = 151 + +// controlPlaneEndpointIP derives a control-plane endpoint IP from the kind network's subnet, +// offset by processIndex so that concurrently running Ginkgo processes (GINKGO_NODES>1) each get +// their own unique endpoint IP instead of colliding on the same static address. +func controlPlaneEndpointIP(subnet string, processIndex int) (string, error) { + ipOctets := strings.Split(subnet, ".") + if len(ipOctets) < ipv4OctetCount { + return "", fmt.Errorf("unexpected subnet format: %s", subnet) + } + + ipOctets[3] = strconv.Itoa(controlPlaneEndpointIPBaseOctet + processIndex - 1) + return strings.Join(ipOctets[:ipv4OctetCount], "."), nil +} + func setControlPlaneIP(ctx context.Context, dockerClient *client.Client) { _, ok := os.LookupEnv("CONTROL_PLANE_ENDPOINT_IP") if ok { @@ -367,16 +389,11 @@ func setControlPlaneIP(ctx context.Context, dockerClient *client.Client) { } } Expect(ipv4Subnet).NotTo(BeEmpty(), "no IPv4 subnet found in kind network IPAM config") - ipOctets := strings.Split(ipv4Subnet, ".") - - // The ControlPlaneEndpoint is a static IP that is in the hosts' - // subnet but outside of its DHCP range. We believe 151 is a pretty - // high number and we have < 10 containers being spun up, so we - // can safely use this IP for the ControlPlaneEndpoint - Expect(len(ipOctets)).To(BeNumerically(">=", ipv4OctetCount), "unexpected subnet format: %s", ipv4Subnet) - ipOctets[3] = "151" - ip := strings.Join(ipOctets, ".") - err := os.Setenv("CONTROL_PLANE_ENDPOINT_IP", ip) + + ip, err := controlPlaneEndpointIP(ipv4Subnet, ginkgo.GinkgoParallelProcess()) + Expect(err).NotTo(HaveOccurred()) + + err = os.Setenv("CONTROL_PLANE_ENDPOINT_IP", ip) if err != nil { Expect(err).NotTo(HaveOccurred()) } diff --git a/test/e2e/docker_helper_test.go b/test/e2e/docker_helper_test.go new file mode 100644 index 000000000..93297dd4c --- /dev/null +++ b/test/e2e/docker_helper_test.go @@ -0,0 +1,53 @@ +// Copyright 2026 Platform9, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// nolint: testpackage +package e2e + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestControlPlaneEndpointIP(t *testing.T) { + testCases := []struct { + name string + subnet string + processIndex int + want string + }{ + { + name: "process 1 gets the base offset", + subnet: "172.18.0.0/16", + processIndex: 1, + want: "172.18.0.151", + }, + { + name: "distinct concurrent processes get distinct IPs", + subnet: "172.18.0.0/16", + processIndex: 2, + want: "172.18.0.152", + }, + { + name: "process index offsets the last octet regardless of subnet size", + subnet: "10.0.0.0/24", + processIndex: 7, + want: "10.0.0.157", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := controlPlaneEndpointIP(tc.subnet, tc.processIndex) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestControlPlaneEndpointIPInvalidSubnet(t *testing.T) { + _, err := controlPlaneEndpointIP("not-a-subnet", 1) + require.Error(t, err) +} diff --git a/test/e2e/e2e_suite_shared_data_test.go b/test/e2e/e2e_suite_shared_data_test.go new file mode 100644 index 000000000..a7b70ac62 --- /dev/null +++ b/test/e2e/e2e_suite_shared_data_test.go @@ -0,0 +1,50 @@ +// Copyright 2026 Platform9, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// nolint: testpackage +package e2e + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseSharedSuiteData(t *testing.T) { + testCases := []struct { + name string + data sharedSuiteData + }{ + { + name: "typical values", + data: sharedSuiteData{ + artifactFolder: "/tmp/artifacts", + configPath: "/tmp/e2e-config.yaml", + clusterctlConfigPath: "/tmp/artifacts/repository/clusterctl-config.yaml", + kubeconfigPath: "/tmp/kind-bootstrap.kubeconfig", + clusterConName: "test-ab12cd", + pathToHostAgentBinary: "/tmp/agent-binary/byoh-hostagent", + }, + }, + { + name: "empty fields round-trip as empty strings", + data: sharedSuiteData{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + encoded := formatSharedSuiteData(&tc.data) + + decoded, err := parseSharedSuiteData(encoded) + require.NoError(t, err) + assert.Equal(t, tc.data, decoded) + }) + } +} + +func TestParseSharedSuiteDataWrongFieldCount(t *testing.T) { + _, err := parseSharedSuiteData([]byte("only,four,comma,fields")) + require.Error(t, err) +} diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index ec0fc53dc..117ce092c 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -1,4 +1,5 @@ // Copyright 2021 VMware, Inc. All Rights Reserved. +// Copyright 2026 Platform9, Inc. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // nolint: testpackage @@ -96,6 +97,46 @@ func TestE2E(t *testing.T) { RunSpecs(t, "Controller Suite") } +// sharedSuiteData is the state the "run once" SynchronizedBeforeSuite closure hands to the +// "run on every ParallelNode" closure. Every field here must be read by more than one Ginkgo +// process under GINKGO_NODES>1, so it has to cross the process boundary via this struct rather +// than a plain package-level assignment. +type sharedSuiteData struct { + artifactFolder string + configPath string + clusterctlConfigPath string + kubeconfigPath string + clusterConName string + pathToHostAgentBinary string +} + +func formatSharedSuiteData(d *sharedSuiteData) []byte { + return []byte(strings.Join([]string{ + d.artifactFolder, + d.configPath, + d.clusterctlConfigPath, + d.kubeconfigPath, + d.clusterConName, + d.pathToHostAgentBinary, + }, ",")) +} + +func parseSharedSuiteData(data []byte) (sharedSuiteData, error) { + parts := strings.Split(string(data), ",") + if len(parts) != 6 { + return sharedSuiteData{}, fmt.Errorf("expected 6 comma-separated fields in shared suite data, got %d", len(parts)) + } + + return sharedSuiteData{ + artifactFolder: parts[0], + configPath: parts[1], + clusterctlConfigPath: parts[2], + kubeconfigPath: parts[3], + clusterConName: parts[4], + pathToHostAgentBinary: parts[5], + }, nil +} + // Using a SynchronizedBeforeSuite for controlling how to create resources shared across ParallelNodes (~ginkgo threads). // The local clusterctl repository & the bootstrap cluster are created once and shared across all the tests. var _ = SynchronizedBeforeSuite(func() []byte { @@ -130,27 +171,28 @@ var _ = SynchronizedBeforeSuite(func() []byte { Expect(err).NotTo(HaveOccurred()) clusterConName = e2eConfig.ManagementClusterName - return []byte( - strings.Join([]string{ - artifactFolder, - configPath, - clusterctlConfigPath, - bootstrapClusterProxy.GetKubeconfigPath(), - }, ","), - ) + return formatSharedSuiteData(&sharedSuiteData{ + artifactFolder: artifactFolder, + configPath: configPath, + clusterctlConfigPath: clusterctlConfigPath, + kubeconfigPath: bootstrapClusterProxy.GetKubeconfigPath(), + clusterConName: clusterConName, + pathToHostAgentBinary: pathToHostAgentBinary, + }) }, func(data []byte) { // Before each ParallelNode. - parts := strings.Split(string(data), ",") - Expect(parts).To(HaveLen(4)) + shared, err := parseSharedSuiteData(data) + Expect(err).NotTo(HaveOccurred()) - artifactFolder = parts[0] - configPath = parts[1] - clusterctlConfigPath = parts[2] - kubeconfigPath := parts[3] + artifactFolder = shared.artifactFolder + configPath = shared.configPath + clusterctlConfigPath = shared.clusterctlConfigPath + clusterConName = shared.clusterConName + pathToHostAgentBinary = shared.pathToHostAgentBinary e2eConfig = loadE2EConfig(configPath) - bootstrapClusterProxy = framework.NewClusterProxy("bootstrap", kubeconfigPath, initScheme(), framework.WithMachineLogCollector(framework.DockerLogCollector{})) + bootstrapClusterProxy = framework.NewClusterProxy("bootstrap", shared.kubeconfigPath, initScheme(), framework.WithMachineLogCollector(framework.DockerLogCollector{})) }) // Using a SynchronizedAfterSuite for controlling how to delete resources shared across ParallelNodes (~ginkgo threads). From 588762fc45de221cd7516763de1dfd9a8df542f1 Mon Sep 17 00:00:00 2001 From: Indradhanush Gupta Date: Sun, 19 Jul 2026 17:09:48 -0400 Subject: [PATCH 2/3] fix(e2e): give bootstrap kubeconfig reconcile wait a real timeout The generateBootstrapKubeconfig helper function was using Gomega's implicit default timeout (~1 second) when waiting for a BootstrapKubeconfig controller to reconcile and populate status. This timeout is far too tight for a real Kubernetes controller reconcile against a real API server, especially under resource contention on GitHub-hosted runners (4 vCPUs). This bug was pre-existing but masked by a concurrent Ginkgo state-propagation bug (fixed earlier) that crashed processes before reaching this Eventually call. Once that crash was fixed, execution began reaching this line under real load, immediately exposing the timeout: CI runs started failing with "Timed out after 1.074s" on specs that had passed consistently in prior validation runs. Fix: pass e2eConfig.GetIntervals("", "wait-controllers") as the timeout and polling-interval arguments to the Eventually call. This resolves to the 3-minute timeout and 10-second poll interval already defined in test/e2e/config/provider.yaml and already used elsewhere in the codebase for "wait for controller reconcile" style waits. The empty spec-name string deliberately falls through to the default wait-controllers config entry, which is the correct fallback since this helper is called from all 7 e2e spec files with different spec names. --- test/e2e/e2e_suite_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 117ce092c..ae0ba4956 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -358,7 +358,7 @@ func generateBootstrapKubeconfig(ctx context.Context, clusterProxy framework.Clu return nil } return createdBootstrapKubeconfig.Status.BootstrapKubeconfigData - }).ShouldNot(BeNil()) + }, e2eConfig.GetIntervals("", "wait-controllers")...).ShouldNot(BeNil()) return *createdBootstrapKubeconfig.Status.BootstrapKubeconfigData } From 0ff6fc55dcd77a7aa3766d3a9876567f94cf8f58 Mon Sep 17 00:00:00 2001 From: Indradhanush Gupta Date: Sun, 19 Jul 2026 18:32:04 -0400 Subject: [PATCH 3/3] test(e2e): fail upgrade specs fast on known CAPI/kubeadm incompatibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Cluster upgrade test" and "Clusterclass upgrade test" specs upgrade from a source to a target Kubernetes version. Both specs were hardcoded to v1.25.11 → v1.26.6, but publish bundles for only v1.31.0 and newer. Bumped both to v1.31.0 → v1.31.2 with matching etcdUpgradeVersion and coreDNSUpgradeVersion. However, this project's vendored CAPI v1.4.4 cannot decode the v1beta4 kubeadm API written by v1.31+ bundles. No currently-published bundle version can complete an upgrade against this CAPI version — upgrading CAPI itself is required as a separate change. Both specs now call Fail(...) as the first statement to signal this blocker clearly and fail fast instead of timing out after 10+ minutes. Additionally, the shared dumpSpecResourcesAndCleanup helper (used by all 7 e2e specs in AfterEach) unconditionally dereferenced a *clusterv1.Cluster pointer that is nil whenever a spec fails before calling ApplyClusterTemplateAndWait — exactly this new fail-fast scenario, but a real latent crash risk for any spec failing early. Added a nil guard around cluster-specific log-dumping calls and rewrote one log line to use the always-available namespace.Name instead of cluster fields. --- test/e2e/cluster_upgrade_test.go | 14 ++++++++++---- test/e2e/clusterclass_upgrade_test.go | 14 ++++++++++---- test/e2e/e2e_suite_test.go | 15 ++++++++++----- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/test/e2e/cluster_upgrade_test.go b/test/e2e/cluster_upgrade_test.go index fbce2f11c..796b8779b 100644 --- a/test/e2e/cluster_upgrade_test.go +++ b/test/e2e/cluster_upgrade_test.go @@ -36,10 +36,10 @@ var _ = Describe("Cluster upgrade test [K8s-Upgrade-Cluster]", func() { dockerClient *client.Client allbyohostContainerIDs []string allAgentLogFiles []string - kubernetesVersionUpgradeFrom = "v1.25.11" - kubernetesVersionUpgradeTo = "v1.26.6" - etcdUpgradeVersion = "3.5.6-0" - coreDNSUpgradeVersion = "v1.9.3" + kubernetesVersionUpgradeFrom = "v1.31.0" + kubernetesVersionUpgradeTo = "v1.31.2" + etcdUpgradeVersion = "3.5.15-0" + coreDNSUpgradeVersion = "v1.11.3" ) BeforeEach(func() { @@ -58,6 +58,12 @@ var _ = Describe("Cluster upgrade test [K8s-Upgrade-Cluster]", func() { }) It("Should successfully upgrade cluster", func() { + // Fail fast: no Kubernetes version in this project's OCI bundle registry + // (quay.io/platform9, v1.31.0+ only) can complete a kubeadm upgrade against the vendored + // CAPI v1.4.4 — its kubeadm-config decoder predates the kubeadm.k8s.io/v1beta4 API that + // v1.31+ writes. Needs CAPI bumped past v1.4.4 before this can run. + Fail("blocked on CAPI v1.4.4 -> kubeadm v1beta4 incompatibility") + clusterName := fmt.Sprintf("%s-%s", specName, util.RandomString(6)) var err error dockerClient, err = client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) diff --git a/test/e2e/clusterclass_upgrade_test.go b/test/e2e/clusterclass_upgrade_test.go index 88571ad52..c09cb1a5e 100644 --- a/test/e2e/clusterclass_upgrade_test.go +++ b/test/e2e/clusterclass_upgrade_test.go @@ -36,10 +36,10 @@ var _ = Describe("Clusterclass upgrade test [K8s-Upgrade-ClusterClass]", func() dockerClient *client.Client allbyohostContainerIDs []string allAgentLogFiles []string - kubernetesVersionUpgradeFrom = "v1.25.11" - kubernetesVersionUpgradeTo = "v1.26.6" - etcdUpgradeVersion = "3.5.6-0" - coreDNSUpgradeVersion = "v1.9.3" + kubernetesVersionUpgradeFrom = "v1.31.0" + kubernetesVersionUpgradeTo = "v1.31.2" + etcdUpgradeVersion = "3.5.15-0" + coreDNSUpgradeVersion = "v1.11.3" ) BeforeEach(func() { @@ -58,6 +58,12 @@ var _ = Describe("Clusterclass upgrade test [K8s-Upgrade-ClusterClass]", func() }) It("Should successfully upgrade cluster", func() { + // Fail fast: no Kubernetes version in this project's OCI bundle registry + // (quay.io/platform9, v1.31.0+ only) can complete a kubeadm upgrade against the vendored + // CAPI v1.4.4 — its kubeadm-config decoder predates the kubeadm.k8s.io/v1beta4 API that + // v1.31+ writes. Needs CAPI bumped past v1.4.4 before this can run. + Fail("blocked on CAPI v1.4.4 -> kubeadm v1beta4 incompatibility") + clusterName := fmt.Sprintf("%s-%s", specName, util.RandomString(6)) var err error dockerClient, err = client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index ae0ba4956..18d660f10 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -296,10 +296,15 @@ func setupSpecNamespace(ctx context.Context, specName string, clusterProxy frame } func dumpSpecResourcesAndCleanup(ctx context.Context, specName string, clusterProxy framework.ClusterProxy, artifactFolder string, namespace *corev1.Namespace, cancelWatches context.CancelFunc, cluster *clusterv1.Cluster, intervalsGetter func(spec, key string) []interface{}, skipCleanup bool) { - Byf("Dumping logs from the %q workload cluster", cluster.Name) - - // Dump all the logs from the workload cluster before deleting them. - clusterProxy.CollectWorkloadClusterLogs(ctx, cluster.Namespace, cluster.Name, filepath.Join(artifactFolder, "clusters", cluster.Name, "machines")) + // cluster is nil when the spec failed before ApplyClusterTemplateAndWait ever ran (e.g. an + // early Fail() call) -- skip the cluster-specific log collection below rather than crash on + // a nil pointer, since there's nothing captured under cluster.Name to actually dump. + if cluster != nil { + Byf("Dumping logs from the %q workload cluster", cluster.Name) + + // Dump all the logs from the workload cluster before deleting them. + clusterProxy.CollectWorkloadClusterLogs(ctx, cluster.Namespace, cluster.Name, filepath.Join(artifactFolder, "clusters", cluster.Name, "machines")) + } Byf("Dumping all the Cluster API resources in the %q namespace", namespace.Name) @@ -311,7 +316,7 @@ func dumpSpecResourcesAndCleanup(ctx context.Context, specName string, clusterPr }) if !skipCleanup { - Byf("Deleting cluster %s/%s", cluster.Namespace, cluster.Name) + Byf("Deleting cluster resources in namespace %q", namespace.Name) // While https://github.com/kubernetes-sigs/cluster-api/issues/2955 is addressed in future iterations, there is a chance // that cluster variable is not set even if the cluster exists, so we are calling DeleteAllClustersAndWait // instead of DeleteClusterAndWait