Skip to content

Commit 73d03cd

Browse files
gcs278claude
andcommitted
OCPBUGS-82586 NE-2292: 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). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1a93dad commit 73d03cd

File tree

2 files changed

+65
-30
lines changed

2 files changed

+65
-30
lines changed

test/extended/router/gatewayapi_upgrade.go

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,28 @@ 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+
if skip, reason := shouldSkipGatewayAPITests(oc); skip {
49+
g.By(fmt.Sprintf("skipping test: %s", reason))
50+
return true
51+
}
52+
53+
// Skip on clusters missing OLM/Marketplace capabilities if not using NO-OLM mode
54+
if !isNoOLMFeatureGateEnabled(oc) {
55+
enabled, err := exutil.AllCapabilitiesEnabled(oc, olmCapabilities...)
56+
if err != nil || !enabled {
57+
g.By("skipping test: OLM/Marketplace capabilities are not enabled and GatewayAPIWithoutOLM is not enabled")
58+
return true
59+
}
60+
}
61+
62+
return false
63+
}
64+
4365
// Setup creates Gateway and HTTPRoute resources and tests connectivity
4466
func (t *GatewayAPIUpgradeTest) Setup(ctx context.Context, f *e2e.Framework) {
4567
g.By("Setting up Gateway API upgrade test")
@@ -49,17 +71,12 @@ func (t *GatewayAPIUpgradeTest) Setup(ctx context.Context, f *e2e.Framework) {
4971
t.gatewayName = "upgrade-test-gateway"
5072
t.routeName = "test-httproute"
5173

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

5577
g.By("Checking if GatewayAPIWithoutOLM feature gate is enabled before upgrade")
5678
t.startedWithNoOLM = isNoOLMFeatureGateEnabled(t.oc)
5779

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

test/extended/router/gatewayapicontroller.go

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,10 @@ 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+
if skip, reason := shouldSkipGatewayAPITests(oc); skip {
128+
g.Skip(reason)
129+
}
130+
loadBalancerSupported, managedDNS = getPlatformCapabilities(oc)
128131

129132
if !isNoOLMFeatureGateEnabled(oc) {
130133
// GatewayAPIController without GatewayAPIWithoutOLM featuregate
@@ -548,47 +551,62 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat
548551
})
549552
})
550553

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
555-
// TODO: Determine if we can enable and start testing OKD with Sail Library
554+
// shouldSkipGatewayAPITests checks if Gateway API tests should be skipped on this cluster.
555+
// It returns true and a reason string if the tests should be skipped.
556+
// This function avoids calling g.Skip() or o.Expect() so it is safe to call from
557+
// upgrade test Skip() methods that run outside of Ginkgo leaf nodes.
558+
func shouldSkipGatewayAPITests(oc *exutil.CLI) (bool, string) {
556559
isokd, err := isOKD(oc)
557-
o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get clusterversion to determine if release is OKD")
560+
if err != nil {
561+
return true, fmt.Sprintf("Failed to determine if release is OKD: %v", err)
562+
}
558563
if isokd {
559-
g.Skip("Skipping on OKD cluster as OSSM is not available as a community operator")
564+
return true, "Skipping on OKD cluster as OSSM is not available as a community operator"
560565
}
561566

562567
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())
568+
if err != nil {
569+
return true, fmt.Sprintf("Failed to get infrastructure: %v", err)
570+
}
571+
if infra == nil || infra.Status.PlatformStatus == nil {
572+
return true, "Infrastructure or PlatformStatus is nil"
573+
}
565574

566-
o.Expect(infra.Status.PlatformStatus).NotTo(o.BeNil())
567575
platformType := infra.Status.PlatformStatus.Type
568-
o.Expect(platformType).NotTo(o.BeEmpty())
569576
switch platformType {
570577
case configv1.AWSPlatformType, configv1.AzurePlatformType, configv1.GCPPlatformType, configv1.IBMCloudPlatformType:
571-
// Cloud platforms with native LoadBalancer support
572-
loadBalancerSupported = true
573578
case configv1.VSpherePlatformType, configv1.BareMetalPlatformType, configv1.EquinixMetalPlatformType:
574-
// Platforms without native LoadBalancer support (may have MetalLB or similar)
575-
loadBalancerSupported = false
576579
default:
577-
g.Skip(fmt.Sprintf("Skipping on unsupported platform type %q", platformType))
580+
return true, fmt.Sprintf("Skipping on unsupported platform type %q", platformType)
578581
}
579582

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)
584583
if isIPv6OrDualStack(oc) {
585-
g.Skip("Skipping Gateway API tests on IPv6/dual-stack cluster")
584+
return true, "Skipping Gateway API tests on IPv6/dual-stack cluster"
586585
}
587586

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

609+
592610
// isDNSManaged checks if the cluster has DNS zones configured (public or private).
593611
// On platforms like vSphere without external DNS, DNS records cannot be managed.
594612
func isDNSManaged(oc *exutil.CLI) bool {

0 commit comments

Comments
 (0)