From dbb40e8238c0d3f619317b2f28543155491be85d Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Thu, 25 Jun 2026 20:16:33 +0000 Subject: [PATCH] Bug OCPBUGS-92656: backport dual-stack IPFamilyPolicy and upstream test skips to 4.22 OTE Port dual-stack support changes from PR #466 (main/5.0 branch) to the release-4.22 OTE structure under cmd/cloud-controller-manager-aws-tests-ext/: - Add GetCloudConfig and IsDualStack helpers to e2e/helper.go for detecting dual-stack clusters from the cloud-config ConfigMap NodeIPFamilies key. - In createServiceNLB (e2e/loadbalancer.go), set IPFamilyPolicy to RequireDualStack when the cluster is configured for dual-stack, ensuring NLB services work in dualstack IPv6-primary environments. - In main.go, detect dual-stack before building specs and exclude upstream loadbalancer tests ([cloud-provider-aws-e2e] loadbalancer) when the cluster is dual-stack with primary IPv6, since upstream tests do not support dual-stack yet. Co-Authored-By: Claude Opus 4.6 --- .../e2e/helper.go | 97 +++++++++++++++++++ .../e2e/loadbalancer.go | 16 ++- .../main.go | 36 ++++++- 3 files changed, 144 insertions(+), 5 deletions(-) diff --git a/cmd/cloud-controller-manager-aws-tests-ext/e2e/helper.go b/cmd/cloud-controller-manager-aws-tests-ext/e2e/helper.go index b1bd409e2..11bbcb718 100644 --- a/cmd/cloud-controller-manager-aws-tests-ext/e2e/helper.go +++ b/cmd/cloud-controller-manager-aws-tests-ext/e2e/helper.go @@ -3,6 +3,7 @@ package e2e import ( "context" "fmt" + "regexp" "strings" "github.com/aws/aws-sdk-go-v2/aws" @@ -12,7 +13,9 @@ import ( elbv2 "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2" elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types" configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clientset "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/test/e2e/framework" ) @@ -194,3 +197,97 @@ func ec2IsNotFoundError(err error) bool { strings.Contains(errMsg, "InvalidGroupId.NotFound") || strings.Contains(errMsg, "InvalidGroup.Malformed") } + +// GetCloudConfig retrieves the CCM cloud-config ConfigMap. +// When cs is nil, a clientset is created from the current kubeconfig. +// This function must not call Ginkgo control-flow helpers (Skip, Fail, etc.) +// because it is also called from main.go outside a spec context. +func GetCloudConfig(ctx context.Context, cs clientset.Interface) (*v1.ConfigMap, error) { + var err error + if cs == nil { + restConfig, err := framework.LoadConfig() + if err != nil { + return nil, fmt.Errorf("failed to load kubeconfig: %w", err) + } + cs, err = clientset.NewForConfig(restConfig) + if err != nil { + return nil, fmt.Errorf("failed to create kube clientset: %w", err) + } + } + cm, err := cs.CoreV1().ConfigMaps(cloudConfigNamespace).Get(ctx, cloudConfigName, metav1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to get cloud-config ConfigMap: %w", err) + } + return cm, nil +} + +// isConfigPresentCloudConfig checks if a specific configuration key is present in the +// cloud-config data stored in the given ConfigMap. It searches all data entries for an +// INI-style key=value match. Values are split by comma to support multi-value configs +// e.g.: "ipFamilies = IPv4,IPv6" returns ["IPv4", "IPv6"], and +// "NLBSecurityGroupMode" = "Managed" returns ["Managed"]. +func isConfigPresentCloudConfig(cm *v1.ConfigMap, configKey string) (bool, []string, error) { + if cm == nil { + return false, nil, fmt.Errorf("ConfigMap is nil") + } + if configKey == "" { + return false, nil, fmt.Errorf("configKey is empty") + } + + pattern, err := regexp.Compile(`(?m)^\s*` + regexp.QuoteMeta(configKey) + `\s*=\s*(.*)$`) + if err != nil { + return false, nil, fmt.Errorf("failed to compile regex for key %q: %w", configKey, err) + } + + for dataKey, content := range cm.Data { + allMatches := pattern.FindAllStringSubmatch(content, -1) + if allMatches == nil { + continue + } + + var values []string + for _, matches := range allMatches { + rawValue := strings.TrimSpace(matches[1]) + if rawValue == "" { + continue + } + for _, p := range strings.Split(rawValue, ",") { + if v := strings.TrimSpace(p); v != "" { + values = append(values, v) + } + } + } + + framework.Logf("Found key %q in ConfigMap data key %q with values: %v", configKey, dataKey, values) + return true, values, nil + } + + framework.Logf("Key %q not found in ConfigMap %s/%s", configKey, cm.Namespace, cm.Name) + return false, nil, nil +} + +// IsDualStack checks the NodeIPFamilies key in the cloud-config ConfigMap. +// It returns (isDualStack, primaryIPv6, error) where isDualStack is true when +// both IPv4 and IPv6 are present, and primaryIPv6 is true when the first +// entry is IPv6 (e.g. NodeIPFamilies=ipv6 then NodeIPFamilies=ipv4). +// When NodeIPFamilies is absent, both booleans are false with no error. +func IsDualStack(cm *v1.ConfigMap) (bool, bool, error) { + found, values, err := isConfigPresentCloudConfig(cm, "NodeIPFamilies") + if err != nil { + return false, false, fmt.Errorf("failed to lookup up configuration NodeIPFamilies in cloud-config: %w", err) + } + if !found { + return false, false, nil + } + var hasIPv4, hasIPv6 bool + for _, ipFamily := range values { + switch strings.ToLower(ipFamily) { + case "ipv6": + hasIPv6 = true + case "ipv4": + hasIPv4 = true + } + } + primaryIPv6 := len(values) > 0 && strings.ToLower(values[0]) == "ipv6" + return hasIPv4 && hasIPv6, primaryIPv6, nil +} diff --git a/cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go b/cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go index 0bdfc8120..0d0554d1c 100644 --- a/cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go +++ b/cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go @@ -532,7 +532,21 @@ func createServiceNLB(ctx context.Context, cs clientset.Interface, ns *v1.Namesp }, } - _, err := jig.Client.CoreV1().Services(jig.Namespace).Create(ctx, svc, metav1.CreateOptions{}) + cloudCfg, err := GetCloudConfig(ctx, cs) + if err != nil { + return nil, nil, fmt.Errorf("failed to get cloud-config: %w", err) + } + isDualStackCluster, _, err := IsDualStack(cloudCfg) + if err != nil { + return nil, nil, fmt.Errorf("failed to detect dual-stack from cloud-config: %w", err) + } + if isDualStackCluster { + framework.Logf("Detected DualStack clusters, patching Service setting IPFamilyPolicy to %q", v1.IPFamilyPolicyRequireDualStack) + dualStack := v1.IPFamilyPolicyRequireDualStack + svc.Spec.IPFamilyPolicy = &dualStack + } + + _, err = jig.Client.CoreV1().Services(jig.Namespace).Create(ctx, svc, metav1.CreateOptions{}) framework.ExpectNoError(err, "failed to create LoadBalancer Service") By("waiting for AWS load balancer provisioning") diff --git a/cmd/cloud-controller-manager-aws-tests-ext/main.go b/cmd/cloud-controller-manager-aws-tests-ext/main.go index 5fd588f94..d17b2813f 100644 --- a/cmd/cloud-controller-manager-aws-tests-ext/main.go +++ b/cmd/cloud-controller-manager-aws-tests-ext/main.go @@ -20,13 +20,16 @@ import ( log "github.com/sirupsen/logrus" // Importing ginkgo tests from the CCM e2e package - _ "github.com/openshift/cluster-cloud-controller-manager-operator/cmd/cloud-controller-manager-aws-tests-ext/e2e" + ccme2e "github.com/openshift/cluster-cloud-controller-manager-operator/cmd/cloud-controller-manager-aws-tests-ext/e2e" _ "k8s.io/cloud-provider-aws/tests/e2e" ) var ( // testContext is the global test context that is used to store the test configuration. testContext = &framework.TestContext + + isDualStackCluster bool + isDualStackPrimaryIpv6 bool ) func main() { @@ -43,6 +46,20 @@ func main() { panic(fmt.Errorf("failed to initialize test framework: %w", err)) } + // Detect dual-stack from cloud-config before building specs. + // Upstream load balancer tests do not support dual-stack yet, so they + // must be excluded when the cluster is configured for dual-stack. + if cm, err := ccme2e.GetCloudConfig(context.TODO(), nil); err != nil { + log.Debugf("failed to get cloud-config for dual-stack detection: %v", err) + } else { + var dsErr error + isDualStackCluster, isDualStackPrimaryIpv6, dsErr = ccme2e.IsDualStack(cm) + if dsErr != nil { + log.Debugf("failed to evaluate dual-stack configuration, leaving default Service config: %v", dsErr) + } + log.Debugf("Dual-stack cluster detected: %v", isDualStackCluster) + } + // Build the extension test specs specs, err := g.BuildExtensionTestSpecsFromOpenShiftGinkgoSuite() if err != nil { @@ -53,11 +70,22 @@ func main() { // We need to filter to prevent adding ECR tests. // All upstream tests must be runnable on OpenShift, if issues are found, let's try to // fix in upstream to work well with OpenShift and cloud-provider-aws CI. - specs, err = specs.MustSelectAny([]extensiontests.SelectFunction{ - extensiontests.NameContains("[cloud-provider-aws-e2e] loadbalancer"), + specSelectors := []extensiontests.SelectFunction{ extensiontests.NameContains("[cloud-provider-aws-e2e] nodes"), extensiontests.NameContains("[cloud-provider-aws-e2e-openshift]"), - }) + } + // Exclude upstream load balancer tests on dual-stack clusters — upstream + // does not support dual-stack yet. When detection fails, the upstream LB + // tests are also excluded to avoid false positives. + // FIXME when upstream e2e supports Service Dual-stack scenarios: + // https://github.com/kubernetes/cloud-provider-aws/pull/1313 + // https://github.com/kubernetes/cloud-provider-aws/pull/1356 + if isDualStackCluster && isDualStackPrimaryIpv6 { + framework.Logf("Dual-stack cluster with Primary IPv6 detected, skipping test name that contains '[cloud-provider-aws-e2e] loadbalancer'") + } else { + specSelectors = append(specSelectors, extensiontests.NameContains("[cloud-provider-aws-e2e] loadbalancer")) + } + specs, err = specs.MustSelectAny(specSelectors) if err != nil { panic(fmt.Errorf("failed to select specs: %w", err)) }