Skip to content

Commit 6a7a94e

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 6a7a94e

File tree

2 files changed

+73
-29
lines changed

2 files changed

+73
-29
lines changed

test/extended/router/gatewayapi_upgrade.go

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,34 @@ 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+
// Don't skip; let Setup fail with the real error.
51+
e2e.Logf("Failed to check Gateway API skip conditions: %v", err)
52+
return false
53+
}
54+
if skip {
55+
g.By(fmt.Sprintf("skipping test: %s", reason))
56+
return true
57+
}
58+
59+
// Skip on clusters missing OLM/Marketplace capabilities if not using NO-OLM mode
60+
if !isNoOLMFeatureGateEnabled(oc) {
61+
enabled, err := exutil.AllCapabilitiesEnabled(oc, olmCapabilities...)
62+
if err != nil || !enabled {
63+
g.By("skipping test: OLM/Marketplace capabilities are not enabled and GatewayAPIWithoutOLM is not enabled")
64+
return true
65+
}
66+
}
67+
68+
return false
69+
}
70+
4371
// Setup creates Gateway and HTTPRoute resources and tests connectivity
4472
func (t *GatewayAPIUpgradeTest) Setup(ctx context.Context, f *e2e.Framework) {
4573
g.By("Setting up Gateway API upgrade test")
@@ -49,17 +77,12 @@ func (t *GatewayAPIUpgradeTest) Setup(ctx context.Context, f *e2e.Framework) {
4977
t.gatewayName = "upgrade-test-gateway"
5078
t.routeName = "test-httproute"
5179

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

5583
g.By("Checking if GatewayAPIWithoutOLM feature gate is enabled before upgrade")
5684
t.startedWithNoOLM = isNoOLMFeatureGateEnabled(t.oc)
5785

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-
6386
if t.startedWithNoOLM {
6487
e2e.Logf("Starting with GatewayAPIWithoutOLM enabled (NO-OLM mode)")
6588
} else {

test/extended/router/gatewayapicontroller.go

Lines changed: 43 additions & 22 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,60 @@ 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

566-
o.Expect(infra.Status.PlatformStatus).NotTo(o.BeNil())
576+
if infra.Status.PlatformStatus == nil {
577+
return false, "", fmt.Errorf("infrastructure PlatformStatus is nil")
578+
}
567579
platformType := infra.Status.PlatformStatus.Type
568-
o.Expect(platformType).NotTo(o.BeEmpty())
569580
switch platformType {
570581
case configv1.AWSPlatformType, configv1.AzurePlatformType, configv1.GCPPlatformType, configv1.IBMCloudPlatformType:
571-
// Cloud platforms with native LoadBalancer support
572-
loadBalancerSupported = true
573582
case configv1.VSpherePlatformType, configv1.BareMetalPlatformType, configv1.EquinixMetalPlatformType:
574-
// Platforms without native LoadBalancer support (may have MetalLB or similar)
575-
loadBalancerSupported = false
576583
default:
577-
g.Skip(fmt.Sprintf("Skipping on unsupported platform type %q", platformType))
584+
return true, fmt.Sprintf("Skipping on unsupported platform type %q", platformType), nil
578585
}
579586

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)
584587
if isIPv6OrDualStack(oc) {
585-
g.Skip("Skipping Gateway API tests on IPv6/dual-stack cluster")
588+
return true, "Skipping Gateway API tests on IPv6/dual-stack cluster", nil
586589
}
587590

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

0 commit comments

Comments
 (0)