Skip to content

NE-2292: Skip OLM tests when GatewayAPIWithoutOLM enabled #30896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rhamini3 wants to merge 2 commits into
base: main
Choose a base branch
from
+98 −56
Open

NE-2292: Skip OLM tests when GatewayAPIWithoutOLM enabled #30896

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e7f9b01
Skip OLM tests when GatewayAPIWithoutOLM enabled
rhamini3 Mar 17, 2026
3f7b082
updated the custom gatewayclass test to wait
rhamini3 Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 98 additions & 56 deletions test/extended/router/gatewayapicontroller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,57 +144,61 @@ 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) {
g.By("Waiting for the istiod pod to be deleted")
waitForIstiodPodDeletion(oc)
} else {
g.By("Deleting the Istio CR")

// Explicitly deleting the Istio CR should not strictly be
// necessary; the Istio CR has an owner reference on the
// gatewayclass, and so deleting the gatewayclass should cause
// the garbage collector to delete the Istio CR. However, it
// has been observed that the Istio CR sometimes does not get
// deleted, and so we have an explicit delete command here just
// in case. The --ignore-not-found option should prevent errors
// if garbage collection has already deleted the object.
o.Expect(oc.AsAdmin().WithoutNamespace().Run("delete").Args("--ignore-not-found=true", "istio", istioName).Execute()).Should(o.Succeed())

g.By("Waiting for the istiod pod to be deleted")
waitForIstiodPodDeletion(oc)

g.By("Deleting the OSSM Operator resources")

gvr := schema.GroupVersionResource{
Group: "operators.coreos.com",
Version: "v1",
Resource: "operators",
}
operator, err := oc.KubeFramework().DynamicClient.Resource(gvr).Get(context.Background(), serviceMeshOperatorName, metav1.GetOptions{})
o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get Operator %q", serviceMeshOperatorName)

g.By("Deleting the Istio CR")

// Explicitly deleting the Istio CR should not strictly be
// necessary; the Istio CR has an owner reference on the
// gatewayclass, and so deleting the gatewayclass should cause
// the garbage collector to delete the Istio CR. However, it
// has been observed that the Istio CR sometimes does not get
// deleted, and so we have an explicit delete command here just
// in case. The --ignore-not-found option should prevent errors
// if garbage collection has already deleted the object.
o.Expect(oc.AsAdmin().WithoutNamespace().Run("delete").Args("--ignore-not-found=true", "istio", istioName).Execute()).Should(o.Succeed())

g.By("Waiting for the istiod pod to be deleted")

o.Eventually(func(g o.Gomega) {
podsList, err := oc.AdminKubeClient().CoreV1().Pods(ingressNamespace).List(context.Background(), metav1.ListOptions{LabelSelector: "app=istiod"})
g.Expect(err).NotTo(o.HaveOccurred())
g.Expect(podsList.Items).Should(o.BeEmpty())
}).WithTimeout(10 * time.Minute).WithPolling(10 * time.Second).Should(o.Succeed())

g.By("Deleting the OSSM Operator resources")

gvr := schema.GroupVersionResource{
Group: "operators.coreos.com",
Version: "v1",
Resource: "operators",
}
operator, err := oc.KubeFramework().DynamicClient.Resource(gvr).Get(context.Background(), serviceMeshOperatorName, metav1.GetOptions{})
o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get Operator %q", serviceMeshOperatorName)

refs, ok, err := unstructured.NestedSlice(operator.Object, "status", "components", "refs")
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(ok).To(o.BeTrue(), "Failed to find status.components.refs in Operator %q", serviceMeshOperatorName)
restmapper := oc.AsAdmin().RESTMapper()
for _, ref := range refs {
ref := extractObjectReference(ref.(map[string]any))
mapping, err := restmapper.RESTMapping(ref.GroupVersionKind().GroupKind())
refs, ok, err := unstructured.NestedSlice(operator.Object, "status", "components", "refs")
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(ok).To(o.BeTrue(), "Failed to find status.components.refs in Operator %q", serviceMeshOperatorName)
Comment on lines +176 to +178
Copy link

@coderabbitai coderabbitai bot Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't require status.components.refs during teardown.

Line 178 hard-fails cleanup if the Operator exists but has not populated status.components.refs yet. That is a normal partial-install state, so AfterEach should log and continue to the final operator delete instead of asserting ok == true.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test/extended/router/gatewayapicontroller.go` around lines 176 - 178, The
teardown currently asserts that status.components.refs exists using
unstructured.NestedSlice (refs, ok, err := unstructured.NestedSlice(...)) and
o.Expect(ok).To(o.BeTrue()), which aborts cleanup if the Operator is present but
not yet populated; change the AfterEach logic to handle ok==false gracefully:
check the ok boolean after calling unstructured.NestedSlice, and if it is false,
log a warning message referencing the Operator name variable
serviceMeshOperatorName (and optionally the operator Object) and continue to the
subsequent operator delete rather than using o.Expect to fail; keep the error
check for err (o.Expect(err).NotTo(o.HaveOccurred())) but avoid asserting
presence of refs.

restmapper := oc.AsAdmin().RESTMapper()
for _, ref := range refs {
ref := extractObjectReference(ref.(map[string]any))
mapping, err := restmapper.RESTMapping(ref.GroupVersionKind().GroupKind())
o.Expect(err).NotTo(o.HaveOccurred())

e2e.Logf("Deleting %s %s/%s...", ref.Kind, ref.Namespace, ref.Name)
err = oc.KubeFramework().DynamicClient.Resource(mapping.Resource).Namespace(ref.Namespace).Delete(context.Background(), ref.Name, metav1.DeleteOptions{})
o.Expect(err).Should(o.Or(o.Not(o.HaveOccurred()), o.MatchError(apierrors.IsNotFound, "IsNotFound")), "Failed to delete %s %q: %v", ref.GroupVersionKind().Kind, ref.Name, err)
}

e2e.Logf("Deleting %s %s/%s...", ref.Kind, ref.Namespace, ref.Name)
err = oc.KubeFramework().DynamicClient.Resource(mapping.Resource).Namespace(ref.Namespace).Delete(context.Background(), ref.Name, metav1.DeleteOptions{})
o.Expect(err).Should(o.Or(o.Not(o.HaveOccurred()), o.MatchError(apierrors.IsNotFound, "IsNotFound")), "Failed to delete %s %q: %v", ref.GroupVersionKind().Kind, ref.Name, err)
}
o.Expect(oc.AsAdmin().WithoutNamespace().Run("delete").Args("operators", serviceMeshOperatorName).Execute()).Should(o.Succeed())

o.Expect(oc.AsAdmin().WithoutNamespace().Run("delete").Args("operators", serviceMeshOperatorName).Execute()).Should(o.Succeed())
}
}
})

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) {
g.Skip("Skip this test since it requires OLM resources")
}

//check the catalogSource
g.By("Check OLM catalogSource, subscription, CSV and Pod")
Expand Down Expand Up @@ -284,15 +288,21 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat
err = oc.AdminGatewayApiClient().GatewayV1().GatewayClasses().Delete(context.Background(), customGatewayClassName, metav1.DeleteOptions{})
o.Expect(err).NotTo(o.HaveOccurred())

_, err = oc.AdminGatewayApiClient().GatewayV1().GatewayClasses().Get(context.Background(), customGatewayClassName, metav1.GetOptions{})
o.Expect(err).To(o.HaveOccurred(), "The custom gatewayClass \"custom-gatewayclass\" has been sucessfully deleted")
// Wait for the GatewayClass to be fully deleted (finalizers processed)
o.Eventually(func() bool {
_, err := oc.AdminGatewayApiClient().GatewayV1().GatewayClasses().Get(context.Background(), customGatewayClassName, metav1.GetOptions{})
return apierrors.IsNotFound(err)
}).WithTimeout(1*time.Minute).WithPolling(2*time.Second).Should(o.BeTrue(), "custom-gatewayclass should be deleted")

g.By("check if default gatewayClass is accepted and ISTIO CR and pod are still available")
g.By("check if default gatewayClass is accepted")
defaultCheck := checkGatewayClass(oc, gatewayClassName)
o.Expect(defaultCheck).NotTo(o.HaveOccurred())

g.By("Confirm that ISTIO CR is created and in healthy state")
waitForIstioHealthy(oc)
if !isNoOLMFeatureGateEnabled(oc) {
g.By("Confirm that ISTIO CR is created and in healthy state")
waitForIstioHealthy(oc)
}
//TODO when FG is enabled check GWC conditions
})

g.It("Ensure LB, service, and dnsRecord are created for a Gateway object", func() {
Expand Down Expand Up @@ -338,7 +348,7 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat
_, gwerr := createAndCheckGateway(oc, gw, gatewayClassName, customDomain)
o.Expect(gwerr).NotTo(o.HaveOccurred(), "Failed to create Gateway")

// make sure the DNSRecord is ready to use.
// make sure the DNSRecord is ready to use
assertDNSRecordStatus(oc, gw)

g.By("Create the http route using the custom gateway")
Expand All @@ -354,6 +364,10 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat

g.It("Ensure GIE is enabled after creating an inferencePool CRD", func() {
defer markTestDone(oc, gieEnabled)
// TODO check the istiod pod as a common or check istio and istiod
if isNoOLMFeatureGateEnabled(oc) {
g.Skip("The test requires OLM dependencies, skipping")
}

errCheck := checkGatewayClass(oc, gatewayClassName)
o.Expect(errCheck).NotTo(o.HaveOccurred(), "GatewayClass %q was not installed and accepted", gatewayClassName)
Expand Down Expand Up @@ -410,15 +424,20 @@ 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)

// 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()
o.Expect(err).NotTo(o.HaveOccurred())
_, err = oc.AsAdmin().WithoutNamespace().Run("delete").Args("-n", ingressNamespace, "istio/"+istioName).Output()
o.Expect(err).NotTo(o.HaveOccurred())
if !isNoOLMFeatureGateEnabled(oc) {
// 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()
o.Expect(err).NotTo(o.HaveOccurred())
_, err = oc.AsAdmin().WithoutNamespace().Run("delete").Args("-n", ingressNamespace, "istio/"+istioName).Output()
o.Expect(err).NotTo(o.HaveOccurred())

g.By(fmt.Sprintf("Wait until the the istiod %s is automatically created successfully", istioName))
pollWaitIstioCreated(oc, ingressNamespace, istioName, istioOriginalCreatedTimestamp)
} else {
e2e.Logf("Not checking the Istio CR, due to NO OLM featuregate being enabled")
}

g.By(fmt.Sprintf("Wait until the the istiod %s is automatically created successfully", istioName))
pollWaitIstioCreated(oc, ingressNamespace, istioName, istioOriginalCreatedTimestamp)
})

g.It("Ensure gateway loadbalancer service and dnsrecords could be deleted and then get recreated [Serial]", func() {
Expand Down Expand Up @@ -483,6 +502,21 @@ func skipGatewayIfNonCloudPlatform(oc *exutil.CLI) {
}
}

func isNoOLMFeatureGateEnabled(oc *exutil.CLI) bool {
fgs, err := oc.AdminConfigClient().ConfigV1().FeatureGates().Get(context.TODO(), "cluster", metav1.GetOptions{})
o.Expect(err).NotTo(o.HaveOccurred(), "Error getting cluster FeatureGates.")
// Skip if the GatewayAPIWithoutOLM feature gate is enabled
for _, fg := range fgs.Status.FeatureGates {
for _, enabledFG := range fg.Enabled {
if enabledFG.Name == "GatewayAPIWithoutOLM" {
e2e.Logf("GatewayAPIWithoutOLM featuregate is enabled")
return true
}
}
}
return false
}
Comment on lines +505 to +518
Copy link

@coderabbitai coderabbitai bot Mar 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Current file context:\n'
sed -n '106,123p' test/extended/router/gatewayapicontroller.go

printf '\nHelper definition:\n'
sed -n '502,515p' test/extended/router/gatewayapicontroller.go

def=$(rg -n --type=go 'func\s+SkipIfMissingCapabilities\s*\(' || true)
if [[ -z "$def" ]]; then
  echo 'SkipIfMissingCapabilities definition not found' >&2
  exit 1
fi

printf '\nSkipIfMissingCapabilities definition:\n%s\n' "$def"
file=$(printf '%s\n' "$def" | head -n1 | cut -d: -f1)
line=$(printf '%s\n' "$def" | head -n1 | cut -d: -f2)
start=$(( line > 5 ? line - 5 : 1 ))
end=$(( line + 60 ))
sed -n "${start},${end}p" "$file"

printf '\nGatewayAPIWithoutOLM references inside %s:\n' "$file"
rg -n -C2 'GatewayAPIWithoutOLM' "$file" || true

Repository: openshift/origin

Length of output: 2342

The suite-level capability gate bypasses the feature-gate logic.

exutil.SkipIfMissingCapabilities is called unconditionally in BeforeEach and does not check the GatewayAPIWithoutOLM feature gate. This means a cluster with GatewayAPIWithoutOLM enabled but OLM/Marketplace capabilities removed will skip the entire suite before any of this code runs.

Wrap the capability skip with a feature-gate check:

Suggested fix 
-		// GatewayAPIController relies on OSSM OLM operator.
-		// Skipping on clusters which don't have capabilities required
-		// to install an OLM operator.
-		exutil.SkipIfMissingCapabilities(oc, requiredCapabilities...)
+		// GatewayAPIController only requires OLM capabilities when
+		// GatewayAPIWithoutOLM is not enabled.
+		if !isNoOLMFeatureGateEnabled(oc) {
+			exutil.SkipIfMissingCapabilities(oc, requiredCapabilities...)
+		}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test/extended/router/gatewayapicontroller.go` around lines 502 - 515, The
suite currently calls exutil.SkipIfMissingCapabilities unconditionally in the
BeforeEach, which bypasses the GatewayAPIWithoutOLM feature-gate check; modify
the BeforeEach so it first calls isNoOLMFeatureGateEnabled(oc) and only calls
exutil.SkipIfMissingCapabilities(...) when that function returns false. In
short: locate the BeforeEach that invokes exutil.SkipIfMissingCapabilities and
wrap that call in an if !isNoOLMFeatureGateEnabled(oc) {
exutil.SkipIfMissingCapabilities(...) } block so clusters with
GatewayAPIWithoutOLM enabled are not prematurely skipped.

Copy link
Contributor

@gcs278 gcs278 Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhamini3 ah yea I remember seeing this but I forgot about it - this one is a bit tricky. Eventually we'll remove the skip to support no-OLM clusters, but for this PR we should keep SkipIfMissingCapabilities since the no-OLM implementation isn't merged yet. Removing the skip now may cause test failures on no-OLM clusters.

You can add a

// GatewayAPIController relies on OSSM OLM operator.
// Skipping on clusters which don't have capabilities required
// to install an OLM operator.
// TODO: Remove skip for no-OLM clusters when GatewayAPIWithoutOLM implementation is ready
exutil.SkipIfMissingCapabilities(oc, requiredCapabilities...)

Copy link

@coderabbitai coderabbitai bot Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


func waitForIstioHealthy(oc *exutil.CLI) {
timeout := 20 * time.Minute
err := wait.PollUntilContextTimeout(context.Background(), 10*time.Second, timeout, false, func(context context.Context) (bool, error) {
Expand Down Expand Up @@ -1142,3 +1176,11 @@ func getSortedString(obj interface{}) string {
sort.Strings(objList)
return strings.Join(objList, " ")
}

func waitForIstiodPodDeletion(oc *exutil.CLI) {
o.Eventually(func(g o.Gomega) {
podsList, err := oc.AdminKubeClient().CoreV1().Pods(ingressNamespace).List(context.Background(), metav1.ListOptions{LabelSelector: "app=istiod"})
g.Expect(err).NotTo(o.HaveOccurred())
g.Expect(podsList.Items).Should(o.BeEmpty())
}).WithTimeout(10 * time.Minute).WithPolling(10 * time.Second).Should(o.Succeed())
}