diff --git a/test/extended/router/gatewayapi_upgrade.go b/test/extended/router/gatewayapi_upgrade.go index 0498d09f3393..6ca6e04d3e70 100644 --- a/test/extended/router/gatewayapi_upgrade.go +++ b/test/extended/router/gatewayapi_upgrade.go @@ -30,6 +30,7 @@ type GatewayAPIUpgradeTest struct { startedWithNoOLM bool // tracks if GatewayAPIWithoutOLM was enabled at start loadBalancerSupported bool managedDNS bool + precheckErr error // error from Skip() to surface in Setup() } func (t *GatewayAPIUpgradeTest) Name() string { @@ -40,25 +41,48 @@ func (t *GatewayAPIUpgradeTest) DisplayName() string { return "[sig-network-edge][Feature:Router][apigroup:gateway.networking.k8s.io] Verify Gateway API functionality during upgrade" } +// Skip checks if this upgrade test should be skipped. This is called by the +// disruption framework before Setup. +func (t *GatewayAPIUpgradeTest) Skip(_ upgrades.UpgradeContext) bool { + oc := exutil.NewCLIForMonitorTest("gateway-api-upgrade-skip").AsAdmin() + + t.precheckErr = nil + noOLM, err := isNoOLMFeatureGateEnabled(oc) + if err != nil { + t.precheckErr = fmt.Errorf("failed to check GatewayAPIWithoutOLM feature gate: %w", err) + return false + } + + skip, reason, err := shouldSkipGatewayAPITests(oc, noOLM) + if err != nil { + t.precheckErr = fmt.Errorf("failed to check Gateway API skip conditions: %w", err) + return false + } + if skip { + g.By(fmt.Sprintf("skipping test: %s", reason)) + return true + } + + return false +} + // Setup creates Gateway and HTTPRoute resources and tests connectivity func (t *GatewayAPIUpgradeTest) Setup(ctx context.Context, f *e2e.Framework) { g.By("Setting up Gateway API upgrade test") + o.Expect(t.precheckErr).NotTo(o.HaveOccurred(), "Skip() precheck failed: could not determine if Gateway API upgrade test should run") t.oc = exutil.NewCLIWithFramework(f).AsAdmin() t.namespace = f.Namespace.Name t.gatewayName = "upgrade-test-gateway" t.routeName = "test-httproute" - // Check platform support and get capabilities (LoadBalancer, DNS) - t.loadBalancerSupported, t.managedDNS = checkPlatformSupportAndGetCapabilities(t.oc) + // Get platform capabilities (skip checks already handled by Skip()) + t.loadBalancerSupported, t.managedDNS = getPlatformCapabilities(t.oc) g.By("Checking if GatewayAPIWithoutOLM feature gate is enabled before upgrade") - t.startedWithNoOLM = isNoOLMFeatureGateEnabled(t.oc) - - // Skip on clusters missing OLM/Marketplace capabilities if starting with OLM mode - if !t.startedWithNoOLM { - exutil.SkipIfMissingCapabilities(t.oc, olmCapabilities...) - } + var noOLMErr error + t.startedWithNoOLM, noOLMErr = isNoOLMFeatureGateEnabled(t.oc) + o.Expect(noOLMErr).NotTo(o.HaveOccurred()) if t.startedWithNoOLM { e2e.Logf("Starting with GatewayAPIWithoutOLM enabled (NO-OLM mode)") @@ -135,7 +159,8 @@ func (t *GatewayAPIUpgradeTest) Test(ctx context.Context, f *e2e.Framework, done o.Expect(err).NotTo(o.HaveOccurred(), "Gateway should remain programmed") g.By("Checking if GatewayAPIWithoutOLM feature gate is enabled after upgrade") - endsWithNoOLM := isNoOLMFeatureGateEnabled(t.oc) + endsWithNoOLM, err := isNoOLMFeatureGateEnabled(t.oc) + o.Expect(err).NotTo(o.HaveOccurred()) // Determine if migration happened: started with OLM, ended with NO-OLM migrationOccurred := !t.startedWithNoOLM && endsWithNoOLM diff --git a/test/extended/router/gatewayapicontroller.go b/test/extended/router/gatewayapicontroller.go index ea46af401d1b..95574f2f21d3 100644 --- a/test/extended/router/gatewayapicontroller.go +++ b/test/extended/router/gatewayapicontroller.go @@ -115,6 +115,7 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat infPoolCRD = "https://raw.githubusercontent.com/kubernetes-sigs/gateway-api-inference-extension/main/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml" managedDNS bool loadBalancerSupported bool + noOLM bool ) const ( @@ -123,16 +124,16 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat openshiftOperatorsNamespace = "openshift-operators" ) g.BeforeEach(func() { - // Check platform support and get capabilities (LoadBalancer, DNS) - loadBalancerSupported, managedDNS = checkPlatformSupportAndGetCapabilities(oc) + noOLM, err = isNoOLMFeatureGateEnabled(oc) + o.Expect(err).NotTo(o.HaveOccurred()) - if !isNoOLMFeatureGateEnabled(oc) { - // GatewayAPIController without GatewayAPIWithoutOLM featuregate - // relies on OSSM OLM operator. - // Skipping on clusters which don't have capabilities required - // to install an OLM operator. - exutil.SkipIfMissingCapabilities(oc, olmCapabilities...) + // Check platform support, capabilities, and skip conditions + skip, reason, err := shouldSkipGatewayAPITests(oc, noOLM) + o.Expect(err).NotTo(o.HaveOccurred()) + if skip { + g.Skip(reason) } + loadBalancerSupported, managedDNS = getPlatformCapabilities(oc) // create the default gatewayClass gatewayClass := buildGatewayClass(gatewayClassName, gatewayClassControllerName) _, err = oc.AdminGatewayApiClient().GatewayV1().GatewayClasses().Create(context.TODO(), gatewayClass, metav1.CreateOptions{}) @@ -157,7 +158,7 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat if err := oc.AdminGatewayApiClient().GatewayV1().GatewayClasses().Delete(context.Background(), gatewayClassName, metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) { e2e.Failf("Failed to delete GatewayClass %q", gatewayClassName) } - if isNoOLMFeatureGateEnabled(oc) { + if noOLM { g.By("Waiting for the istiod pod to be deleted") waitForIstiodPodDeletion(oc) } else { @@ -283,7 +284,7 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat g.It("Ensure OSSM and OLM related resources are created after creating GatewayClass", func() { defer markTestDone(oc, ossmAndOLMResourcesCreated) // these will fail since no OLM Resources will be available - if isNoOLMFeatureGateEnabled(oc) { + if noOLM { g.Skip("Skip this test since it requires OLM resources") } @@ -313,7 +314,7 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat errCheck := checkGatewayClassCondition(oc, customGatewayClassName, string(gatewayapiv1.GatewayClassConditionStatusAccepted), metav1.ConditionTrue) o.Expect(errCheck).NotTo(o.HaveOccurred(), "GatewayClass %q was not installed and accepted", gwc.Name) - if isNoOLMFeatureGateEnabled(oc) { + if noOLM { g.By("Check the GatewayClass conditions") errCheck = checkGatewayClassCondition(oc, customGatewayClassName, gatewayClassControllerInstalledConditionType, metav1.ConditionTrue) o.Expect(errCheck).NotTo(o.HaveOccurred(), "GatewayClass %q does not have the ControllerInstalled condition", customGatewayClassName) @@ -340,7 +341,7 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat defaultCheck := checkGatewayClassCondition(oc, gatewayClassName, string(gatewayapiv1.GatewayClassConditionStatusAccepted), metav1.ConditionTrue) o.Expect(defaultCheck).NotTo(o.HaveOccurred()) - if !isNoOLMFeatureGateEnabled(oc) { + if !noOLM { g.By("Confirm that ISTIO CR is created and in healthy state") waitForIstioHealthy(oc, 20*time.Minute) } @@ -482,7 +483,7 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat g.By(fmt.Sprintf("Wait until the istiod deployment in %s namespace is automatically created successfully", ingressNamespace)) pollWaitDeploymentCreated(oc, ingressNamespace, istiodDeployment, deployment.CreationTimestamp) - if !isNoOLMFeatureGateEnabled(oc) { + if !noOLM { // delete the istio and check if it is restored g.By(fmt.Sprintf("Try to delete the istio %s", istioName)) istioOriginalCreatedTimestamp, err := oc.AsAdmin().WithoutNamespace().Run("get").Args("-n", ingressNamespace, "istio/"+istioName, `-o=jsonpath={.metadata.creationTimestamp}`).Output() @@ -548,44 +549,80 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat }) }) -// checkPlatformSupportAndGetCapabilities verifies the platform is supported and returns -// platform capabilities for LoadBalancer services and managed DNS. -func checkPlatformSupportAndGetCapabilities(oc *exutil.CLI) (loadBalancerSupported bool, managedDNS bool) { - // Skip on OKD since OSSM is not available as a community operator +// shouldSkipGatewayAPITests checks if Gateway API tests should be skipped on this cluster. +// It returns (skip bool, reason string, err error). An error indicates a failure to +// determine skip status and should be treated as a test failure, not a skip. +// This function avoids calling g.Skip() or o.Expect() so it is safe to call from +// upgrade test Skip() methods that run outside of Ginkgo leaf nodes. +func shouldSkipGatewayAPITests(oc *exutil.CLI, noOLM bool) (bool, string, error) { // TODO: Determine if we can enable and start testing OKD with Sail Library isokd, err := isOKD(oc) - o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get clusterversion to determine if release is OKD") + if err != nil { + return false, "", fmt.Errorf("failed to determine if release is OKD: %v", err) + } if isokd { - g.Skip("Skipping on OKD cluster as OSSM is not available as a community operator") + return true, "Skipping on OKD cluster as OSSM is not available as a community operator", nil } infra, err := oc.AdminConfigClient().ConfigV1().Infrastructures().Get(context.Background(), "cluster", metav1.GetOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) - o.Expect(infra).NotTo(o.BeNil()) + if err != nil { + return false, "", fmt.Errorf("failed to get infrastructure: %v", err) + } - o.Expect(infra.Status.PlatformStatus).NotTo(o.BeNil()) + if infra.Status.PlatformStatus == nil { + return false, "", fmt.Errorf("infrastructure PlatformStatus is nil") + } platformType := infra.Status.PlatformStatus.Type - o.Expect(platformType).NotTo(o.BeEmpty()) switch platformType { + case configv1.AWSPlatformType, + configv1.AzurePlatformType, + configv1.GCPPlatformType, + configv1.IBMCloudPlatformType, + configv1.VSpherePlatformType, + configv1.BareMetalPlatformType, + configv1.EquinixMetalPlatformType: + // supported + default: + return true, fmt.Sprintf("Skipping on unsupported platform type %q", platformType), nil + } + + ipv6, err := isIPv6OrDualStack(oc) + if err != nil { + return false, "", fmt.Errorf("failed to check IPv6/dual-stack: %v", err) + } + if ipv6 { + return true, "Skipping Gateway API tests on IPv6/dual-stack cluster", nil + } + + if !noOLM { + enabled, err := exutil.AllCapabilitiesEnabled(oc, olmCapabilities...) + if err != nil { + return false, "", fmt.Errorf("failed to check OLM capabilities: %v", err) + } + if !enabled { + return true, "Skipping: OLM/Marketplace capabilities are not enabled and GatewayAPIWithoutOLM is not enabled", nil + } + } + + return false, "", nil +} + +// getPlatformCapabilities returns platform capabilities for LoadBalancer services and managed DNS. +func getPlatformCapabilities(oc *exutil.CLI) (loadBalancerSupported bool, managedDNS bool) { + infra, err := oc.AdminConfigClient().ConfigV1().Infrastructures().Get(context.Background(), "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(infra.Status.PlatformStatus).NotTo(o.BeNil()) + + switch infra.Status.PlatformStatus.Type { case configv1.AWSPlatformType, configv1.AzurePlatformType, configv1.GCPPlatformType, configv1.IBMCloudPlatformType: - // Cloud platforms with native LoadBalancer support loadBalancerSupported = true case configv1.VSpherePlatformType, configv1.BareMetalPlatformType, configv1.EquinixMetalPlatformType: - // Platforms without native LoadBalancer support (may have MetalLB or similar) loadBalancerSupported = false - default: - g.Skip(fmt.Sprintf("Skipping on unsupported platform type %q", platformType)) } - // Check if DNS is managed (has public or private zones configured) managedDNS = isDNSManaged(oc) - // Skip Gateway API tests on IPv6 or dual-stack clusters (any platform) - if isIPv6OrDualStack(oc) { - g.Skip("Skipping Gateway API tests on IPv6/dual-stack cluster") - } - - e2e.Logf("Platform: %s, LoadBalancer supported: %t, DNS managed: %t", platformType, loadBalancerSupported, managedDNS) + e2e.Logf("Platform: %s, LoadBalancer supported: %t, DNS managed: %t", infra.Status.PlatformStatus.Type, loadBalancerSupported, managedDNS) return loadBalancerSupported, managedDNS } @@ -599,30 +636,34 @@ func isDNSManaged(oc *exutil.CLI) bool { // isIPv6OrDualStack checks if the cluster is using IPv6 or dual-stack networking. // Returns true if any ServiceNetwork CIDR is IPv6 (indicates IPv6-only or dual-stack). -func isIPv6OrDualStack(oc *exutil.CLI) bool { +func isIPv6OrDualStack(oc *exutil.CLI) (bool, error) { networkConfig, err := oc.AdminOperatorClient().OperatorV1().Networks().Get(context.Background(), "cluster", metav1.GetOptions{}) - o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get network config") + if err != nil { + return false, fmt.Errorf("failed to get network config: %v", err) + } for _, cidr := range networkConfig.Spec.ServiceNetwork { if utilnet.IsIPv6CIDRString(cidr) { - return true + return true, nil } } - return false + return false, nil } -func isNoOLMFeatureGateEnabled(oc *exutil.CLI) bool { +func isNoOLMFeatureGateEnabled(oc *exutil.CLI) (bool, error) { fgs, err := oc.AdminConfigClient().ConfigV1().FeatureGates().Get(context.TODO(), "cluster", metav1.GetOptions{}) - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting cluster FeatureGates.") + if err != nil { + return false, fmt.Errorf("failed to get cluster FeatureGates: %v", err) + } for _, fg := range fgs.Status.FeatureGates { for _, enabledFG := range fg.Enabled { if enabledFG.Name == "GatewayAPIWithoutOLM" { e2e.Logf("GatewayAPIWithoutOLM featuregate is enabled") - return true + return true, nil } } } - return false + return false, nil } func waitForIstioHealthy(oc *exutil.CLI, timeout time.Duration) {