Skip to content

Commit d765284

Browse files
gcs278claude
andcommitted
OCPBUGS-82586: Use upgrades.Skippable for Gateway API upgrade test skip logic
The Gateway API upgrade test was calling g.Skip() from Setup(), which runs inside a goroutine managed by the disruption framework. Since g.Skip() panics and Ginkgo can only recover panics inside leaf nodes, this caused unrecoverable panics on IPv6/dual-stack, OKD, and unsupported platform clusters. Implement the upgrades.Skippable interface with a Skip() method that the disruption framework calls before Setup, avoiding the goroutine panic. Refactor checkPlatformSupportAndGetCapabilities into shouldSkipGatewayAPITests (safe outside Ginkgo nodes) and getPlatformCapabilities (returns LB/DNS support). https://redhat.atlassian.net/browse/OCPBUGS-82586 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 1a93dad commit d765284

File tree

2 files changed

+69
-28
lines changed

2 files changed

+69
-28
lines changed

test/extended/router/gatewayapi_upgrade.go

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,33 @@ func (t *GatewayAPIUpgradeTest) DisplayName() string {
4040
return "[sig-network-edge][Feature:Router][apigroup:gateway.networking.k8s.io] Verify Gateway API functionality during upgrade"
4141
}
4242

43+
// Skip checks if this upgrade test should be skipped. This is called by the
44+
// disruption framework before Setup.
45+
func (t *GatewayAPIUpgradeTest) Skip(_ upgrades.UpgradeContext) bool {
46+
oc := exutil.NewCLIWithoutNamespace("gateway-api-upgrade-skip").AsAdmin()
47+
48+
skip, reason, err := shouldSkipGatewayAPITests(oc)
49+
if err != nil {
50+
e2e.Logf("Error checking skip conditions: %v", err)
51+
return false // don't skip, let Setup fail with a proper error
52+
}
53+
if skip {
54+
g.By(fmt.Sprintf("skipping test: %s", reason))
55+
return true
56+
}
57+
58+
// Skip on clusters missing OLM/Marketplace capabilities if not using NO-OLM mode
59+
if !isNoOLMFeatureGateEnabled(oc) {
60+
enabled, err := exutil.AllCapabilitiesEnabled(oc, olmCapabilities...)
61+
if err != nil || !enabled {
62+
g.By("skipping test: OLM/Marketplace capabilities are not enabled and GatewayAPIWithoutOLM is not enabled")
63+
return true
64+
}
65+
}
66+
67+
return false
68+
}
69+
4370
// Setup creates Gateway and HTTPRoute resources and tests connectivity
4471
func (t *GatewayAPIUpgradeTest) Setup(ctx context.Context, f *e2e.Framework) {
4572
g.By("Setting up Gateway API upgrade test")
@@ -49,17 +76,12 @@ func (t *GatewayAPIUpgradeTest) Setup(ctx context.Context, f *e2e.Framework) {
4976
t.gatewayName = "upgrade-test-gateway"
5077
t.routeName = "test-httproute"
5178

52-
// Check platform support and get capabilities (LoadBalancer, DNS)
53-
t.loadBalancerSupported, t.managedDNS = checkPlatformSupportAndGetCapabilities(t.oc)
79+
// Get platform capabilities (skip checks already handled by Skip())
80+
t.loadBalancerSupported, t.managedDNS = getPlatformCapabilities(t.oc)
5481

5582
g.By("Checking if GatewayAPIWithoutOLM feature gate is enabled before upgrade")
5683
t.startedWithNoOLM = isNoOLMFeatureGateEnabled(t.oc)
5784

58-
// Skip on clusters missing OLM/Marketplace capabilities if starting with OLM mode
59-
if !t.startedWithNoOLM {
60-
exutil.SkipIfMissingCapabilities(t.oc, olmCapabilities...)
61-
}
62-
6385
if t.startedWithNoOLM {
6486
e2e.Logf("Starting with GatewayAPIWithoutOLM enabled (NO-OLM mode)")
6587
} else {

test/extended/router/gatewayapicontroller.go

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,12 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat
124124
)
125125
g.BeforeEach(func() {
126126
// Check platform support and get capabilities (LoadBalancer, DNS)
127-
loadBalancerSupported, managedDNS = checkPlatformSupportAndGetCapabilities(oc)
127+
skip, reason, err := shouldSkipGatewayAPITests(oc)
128+
o.Expect(err).NotTo(o.HaveOccurred())
129+
if skip {
130+
g.Skip(reason)
131+
}
132+
loadBalancerSupported, managedDNS = getPlatformCapabilities(oc)
128133

129134
if !isNoOLMFeatureGateEnabled(oc) {
130135
// GatewayAPIController without GatewayAPIWithoutOLM featuregate
@@ -548,44 +553,58 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat
548553
})
549554
})
550555

551-
// checkPlatformSupportAndGetCapabilities verifies the platform is supported and returns
552-
// platform capabilities for LoadBalancer services and managed DNS.
553-
func checkPlatformSupportAndGetCapabilities(oc *exutil.CLI) (loadBalancerSupported bool, managedDNS bool) {
554-
// Skip on OKD since OSSM is not available as a community operator
556+
// shouldSkipGatewayAPITests checks if Gateway API tests should be skipped on this cluster.
557+
// It returns (skip bool, reason string, err error). An error indicates a failure to
558+
// determine skip status and should be treated as a test failure, not a skip.
559+
// This function avoids calling g.Skip() or o.Expect() so it is safe to call from
560+
// upgrade test Skip() methods that run outside of Ginkgo leaf nodes.
561+
func shouldSkipGatewayAPITests(oc *exutil.CLI) (bool, string, error) {
555562
// TODO: Determine if we can enable and start testing OKD with Sail Library
556563
isokd, err := isOKD(oc)
557-
o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get clusterversion to determine if release is OKD")
564+
if err != nil {
565+
return false, "", fmt.Errorf("failed to determine if release is OKD: %v", err)
566+
}
558567
if isokd {
559-
g.Skip("Skipping on OKD cluster as OSSM is not available as a community operator")
568+
return true, "Skipping on OKD cluster as OSSM is not available as a community operator", nil
560569
}
561570

562571
infra, err := oc.AdminConfigClient().ConfigV1().Infrastructures().Get(context.Background(), "cluster", metav1.GetOptions{})
563-
o.Expect(err).NotTo(o.HaveOccurred())
564-
o.Expect(infra).NotTo(o.BeNil())
572+
if err != nil {
573+
return false, "", fmt.Errorf("failed to get infrastructure: %v", err)
574+
}
565575

566576
o.Expect(infra.Status.PlatformStatus).NotTo(o.BeNil())
567577
platformType := infra.Status.PlatformStatus.Type
568-
o.Expect(platformType).NotTo(o.BeEmpty())
569578
switch platformType {
570579
case configv1.AWSPlatformType, configv1.AzurePlatformType, configv1.GCPPlatformType, configv1.IBMCloudPlatformType:
571-
// Cloud platforms with native LoadBalancer support
572-
loadBalancerSupported = true
573580
case configv1.VSpherePlatformType, configv1.BareMetalPlatformType, configv1.EquinixMetalPlatformType:
574-
// Platforms without native LoadBalancer support (may have MetalLB or similar)
575-
loadBalancerSupported = false
576581
default:
577-
g.Skip(fmt.Sprintf("Skipping on unsupported platform type %q", platformType))
582+
return true, fmt.Sprintf("Skipping on unsupported platform type %q", platformType), nil
578583
}
579584

580-
// Check if DNS is managed (has public or private zones configured)
581-
managedDNS = isDNSManaged(oc)
582-
583-
// Skip Gateway API tests on IPv6 or dual-stack clusters (any platform)
584585
if isIPv6OrDualStack(oc) {
585-
g.Skip("Skipping Gateway API tests on IPv6/dual-stack cluster")
586+
return true, "Skipping Gateway API tests on IPv6/dual-stack cluster", nil
586587
}
587588

588-
e2e.Logf("Platform: %s, LoadBalancer supported: %t, DNS managed: %t", platformType, loadBalancerSupported, managedDNS)
589+
return false, "", nil
590+
}
591+
592+
// getPlatformCapabilities returns platform capabilities for LoadBalancer services and managed DNS.
593+
func getPlatformCapabilities(oc *exutil.CLI) (loadBalancerSupported bool, managedDNS bool) {
594+
infra, err := oc.AdminConfigClient().ConfigV1().Infrastructures().Get(context.Background(), "cluster", metav1.GetOptions{})
595+
o.Expect(err).NotTo(o.HaveOccurred())
596+
o.Expect(infra.Status.PlatformStatus).NotTo(o.BeNil())
597+
598+
switch infra.Status.PlatformStatus.Type {
599+
case configv1.AWSPlatformType, configv1.AzurePlatformType, configv1.GCPPlatformType, configv1.IBMCloudPlatformType:
600+
loadBalancerSupported = true
601+
case configv1.VSpherePlatformType, configv1.BareMetalPlatformType, configv1.EquinixMetalPlatformType:
602+
loadBalancerSupported = false
603+
}
604+
605+
managedDNS = isDNSManaged(oc)
606+
607+
e2e.Logf("Platform: %s, LoadBalancer supported: %t, DNS managed: %t", infra.Status.PlatformStatus.Type, loadBalancerSupported, managedDNS)
589608
return loadBalancerSupported, managedDNS
590609
}
591610

0 commit comments

Comments
 (0)