NE-2292: Skip OLM tests when GatewayAPIWithoutOLM enabled#30896
NE-2292: Skip OLM tests when GatewayAPIWithoutOLM enabled#30896rhamini3 wants to merge 2 commits intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@rhamini3: This pull request references NE-2292 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds GatewayAPIWithoutOLM feature-gate detection and branches test AfterEach cleanup accordingly: when enabled, waits for istiod pod deletion and skips OLM-specific checks; when disabled, performs explicit Istio CR deletion, waits for istiod deletion, removes OSSM operator resources, and deletes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
|
/testwith openshift/origin/main/e2e-gcp-ovn-techpreview openshift/cluster-ingress-operator#1354 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/extended/router/gatewayapicontroller.go`:
- Around line 197-200: The skipped branches call g.Skip before markTestDone
runs, preventing test completion bookkeeping (so checkAllTestsDone stays false);
update the two isNoOLMFeatureGateEnabled(oc) skip locations to invoke
markTestDone(...) with the same arguments used in AfterEach (including names
like ossmAndOLMResourcesCreated and gieEnabled / the relevant testNames)
immediately before calling g.Skip, or otherwise call markTestDone and return
instead of skipping early; ensure you reference the same markTestDone signature
used elsewhere so AfterEach/checkAllTestsDone logic remains consistent.
- Around line 173-190: The cleanup assumes the Operator always exists; make it
idempotent by treating apierrors.IsNotFound as success: when calling
oc.KubeFramework().DynamicClient.Resource(gvr).Get(...) and when deleting each
ref via DynamicClient.Resource(...).Namespace(...).Delete(...), check if err !=
nil and if apierrors.IsNotFound(err) then skip/continue instead of failing the
test; update the o.Expect/O.Expect(...).Should(...) assertions around Get and
Delete (and the final
oc.AsAdmin().WithoutNamespace().Run("delete").Args("operators",
serviceMeshOperatorName).Execute()) to accept IsNotFound as a non-fatal outcome
so removal already completed does not cause flakes.
- Around line 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.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 84600770-32da-4c28-a6b5-2b3a1050ab57
📒 Files selected for processing (1)
test/extended/router/gatewayapicontroller.go
| 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 | ||
| } |
There was a problem hiding this comment.
🧩 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" || trueRepository: 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.
There was a problem hiding this comment.
@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...)
There was a problem hiding this comment.
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!
|
Scheduling required tests: |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
test/extended/router/gatewayapicontroller.go (2)
196-201:⚠️ Potential issue | 🟠 MajorRegister
markTestDonebeforeg.Skip.Line 198 and Line 364 skip before the defer is registered. In no-OLM mode those annotation keys are never written,
checkAllTestsDone()stays false, and the ordered suite never reaches its final cleanup.Minimal fix
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") } - defer markTestDone(oc, ossmAndOLMResourcesCreated) ... }) 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") } - defer markTestDone(oc, gieEnabled) ... })Also applies to: 362-367
🤖 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 196 - 201, The defer markTestDone(oc, ossmAndOLMResourcesCreated) must be registered before any early-exit skips: move the defer call to immediately after entering the test (before the isNoOLMFeatureGateEnabled check and any g.Skip), so that markTestDone always runs even when g.Skip is executed; update both occurrences in the test (the one around lines with isNoOLMFeatureGateEnabled(oc) and the other at 362-367) to ensure markTestDone is deferred prior to calling g.Skip so checkAllTestsDone() can observe the annotation.
173-190:⚠️ Potential issue | 🟠 MajorKeep operator teardown idempotent.
Line 173 and Line 190 still treat an already-removed
Operatoras fatal, and Line 178 still requiresstatus.components.refseven after a partial install.AfterEachcan still fail after successful or partial cleanup.Minimal hardening
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()) - 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) - } - - o.Expect(oc.AsAdmin().WithoutNamespace().Run("delete").Args("operators", serviceMeshOperatorName).Execute()).Should(o.Succeed()) + if apierrors.IsNotFound(err) { + e2e.Logf("Operator %q already removed", serviceMeshOperatorName) + } else { + 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()) + if ok { + 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) + } + } + } + + err = oc.AsAdmin().WithoutNamespace().Run("delete").Args("--ignore-not-found=true", "operators", serviceMeshOperatorName).Execute() + o.Expect(err).NotTo(o.HaveOccurred())🤖 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 173 - 190, The teardown treats a missing Operator or missing/empty status.components.refs as fatal; make it idempotent by allowing NotFound or absent refs: when calling oc.KubeFramework().DynamicClient.Resource(gvr).Get(...) (operator) handle and ignore apierrors.IsNotFound instead of failing, and when reading unstructured.NestedSlice(..., "status", "components", "refs") treat ok==false or empty slice as a no-op instead of failing; in the loop that deletes refs (extractObjectReference, restmapper.RESTMapping, and DynamicClient.Resource(...).Delete) accept NotFound errors as successful and continue, and finally make the oc.AsAdmin().WithoutNamespace().Run("delete").Args("operators", serviceMeshOperatorName).Execute() call tolerant of NotFound/failure so repeated AfterEach runs do not error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/extended/router/gatewayapicontroller.go`:
- Around line 196-201: The defer markTestDone(oc, ossmAndOLMResourcesCreated)
must be registered before any early-exit skips: move the defer call to
immediately after entering the test (before the isNoOLMFeatureGateEnabled check
and any g.Skip), so that markTestDone always runs even when g.Skip is executed;
update both occurrences in the test (the one around lines with
isNoOLMFeatureGateEnabled(oc) and the other at 362-367) to ensure markTestDone
is deferred prior to calling g.Skip so checkAllTestsDone() can observe the
annotation.
- Around line 173-190: The teardown treats a missing Operator or missing/empty
status.components.refs as fatal; make it idempotent by allowing NotFound or
absent refs: when calling
oc.KubeFramework().DynamicClient.Resource(gvr).Get(...) (operator) handle and
ignore apierrors.IsNotFound instead of failing, and when reading
unstructured.NestedSlice(..., "status", "components", "refs") treat ok==false or
empty slice as a no-op instead of failing; in the loop that deletes refs
(extractObjectReference, restmapper.RESTMapping, and
DynamicClient.Resource(...).Delete) accept NotFound errors as successful and
continue, and finally make the
oc.AsAdmin().WithoutNamespace().Run("delete").Args("operators",
serviceMeshOperatorName).Execute() call tolerant of NotFound/failure so repeated
AfterEach runs do not error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e8236ea-35d2-4f32-b5f7-845bdbf874a5
📒 Files selected for processing (1)
test/extended/router/gatewayapicontroller.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/extended/router/gatewayapicontroller.go (1)
173-174:⚠️ Potential issue | 🟠 MajorMake
AfterEachoperator cleanup idempotent (still flakes on NotFound).Line 173 and Line 190 still treat an already-removed
Operatoras a hard failure. That can fail cleanup even when teardown already succeeded via reconciliation or prior deletes.Suggested fix
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) + if apierrors.IsNotFound(err) { + e2e.Logf("Operator %q already deleted; skipping operator-specific cleanup", serviceMeshOperatorName) + return + } + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get Operator %q", serviceMeshOperatorName) @@ - o.Expect(oc.AsAdmin().WithoutNamespace().Run("delete").Args("operators", serviceMeshOperatorName).Execute()).Should(o.Succeed()) + err = oc.AsAdmin().WithoutNamespace().Run("delete").Args("--ignore-not-found=true", "operators", serviceMeshOperatorName).Execute() + o.Expect(err).NotTo(o.HaveOccurred())As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
Also applies to: 190-190
🤖 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 173 - 174, The cleanup currently treats already-removed Operator as a hard failure when calling oc.KubeFramework().DynamicClient.Resource(gvr).Get (and the subsequent Delete), so make the AfterEach idempotent by checking for Kubernetes NotFound errors and ignoring them; use the API errors helper (e.g., errors.IsNotFound(err) from k8s.io/apimachinery/pkg/api/errors) to only fail the test for errors other than NotFound when calling Resource(gvr).Get and when invoking Resource(gvr).Delete, and otherwise proceed quietly if the Operator is already gone.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/extended/router/gatewayapicontroller.go`:
- Around line 173-174: The cleanup currently treats already-removed Operator as
a hard failure when calling oc.KubeFramework().DynamicClient.Resource(gvr).Get
(and the subsequent Delete), so make the AfterEach idempotent by checking for
Kubernetes NotFound errors and ignoring them; use the API errors helper (e.g.,
errors.IsNotFound(err) from k8s.io/apimachinery/pkg/api/errors) to only fail the
test for errors other than NotFound when calling Resource(gvr).Get and when
invoking Resource(gvr).Delete, and otherwise proceed quietly if the Operator is
already gone.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9fb8e866-d4ca-47db-977e-d26be61f8d4a
📒 Files selected for processing (1)
test/extended/router/gatewayapicontroller.go
Update gatewayAPIController tests to skip certain tests with OLM dependencies when GatewayAPIWithoutOLM FeatureGate is enabled This will unblock openshift/cluster-ingress-operator#1354 by skipping any tests that require OLM capabilities without causing failures in the origin testing. JIRA link: https://redhat.atlassian.net/browse/NE-2292
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/extended/router/gatewayapicontroller.go (1)
173-174:⚠️ Potential issue | 🟠 MajorMake the operator cleanup idempotent.
Line 173 and Line 190 still treat
IsNotFoundas a hard failure. InAfterEach, that turns an already-clean cluster or a partially-created install into a flaky suite failure.Suggested fix
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) + if apierrors.IsNotFound(err) { + return + } + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get Operator %q", serviceMeshOperatorName) @@ - o.Expect(oc.AsAdmin().WithoutNamespace().Run("delete").Args("operators", serviceMeshOperatorName).Execute()).Should(o.Succeed()) + err = oc.AsAdmin().WithoutNamespace().Run("delete").Args("--ignore-not-found=true", "operators", serviceMeshOperatorName).Execute() + o.Expect(err).NotTo(o.HaveOccurred())Also applies to: 190-190
🤖 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 173 - 174, The operator cleanup should be idempotent: update the logic around oc.KubeFramework().DynamicClient.Resource(gvr).Get(...) and any subsequent delete calls that currently treat k8s API errors.IsNotFound as a test failure (occurring at the serviceMeshOperatorName lookup in AfterEach and the similar check at the other occurrence) so that a NotFound error is ignored instead of failing the test; specifically, change the Expect(err).NotTo(HaveOccurred(...)) around the Get for serviceMeshOperatorName and any delete-confirmation checks to allow errors.IsNotFound(err) and skip deletion/verification when not found, while still failing on other error types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/extended/router/gatewayapicontroller.go`:
- Around line 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.
---
Duplicate comments:
In `@test/extended/router/gatewayapicontroller.go`:
- Around line 173-174: The operator cleanup should be idempotent: update the
logic around oc.KubeFramework().DynamicClient.Resource(gvr).Get(...) and any
subsequent delete calls that currently treat k8s API errors.IsNotFound as a test
failure (occurring at the serviceMeshOperatorName lookup in AfterEach and the
similar check at the other occurrence) so that a NotFound error is ignored
instead of failing the test; specifically, change the
Expect(err).NotTo(HaveOccurred(...)) around the Get for serviceMeshOperatorName
and any delete-confirmation checks to allow errors.IsNotFound(err) and skip
deletion/verification when not found, while still failing on other error types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e398dc6-e147-422f-8cb6-0841aea06f0b
📒 Files selected for processing (1)
test/extended/router/gatewayapicontroller.go
| 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) |
There was a problem hiding this comment.
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.
|
/testwith openshift/origin/main/e2e-gcp-ovn-techpreview openshift/cluster-ingress-operator#1354 |
|
@rhamini3: This pull request references NE-2292 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
rhamini3
changed the title
|
This PR makes sense as a prerequisite to getting openshift/cluster-ingress-operator#1354 merged, so tech preview jobs won't break when we merge. /approve I'll add LGTM I see passing status of |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gcs278, rhamini3 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
Scheduling required tests: |
| @@ -287,12 +291,15 @@ var _ = g.Describe("[sig-network-edge][OCPFeatureGate:GatewayAPIController][Feat | |||
| _, err = oc.AdminGatewayApiClient().GatewayV1().GatewayClasses().Get(context.Background(), customGatewayClassName, metav1.GetOptions{}) | |||
There was a problem hiding this comment.
@rhamini3 we have to wait now - there is a finalizer on the GatewayClass so it doesn't delete immediately, this should fix it:
| _, err = oc.AdminGatewayApiClient().GatewayV1().GatewayClasses().Get(context.Background(), customGatewayClassName, metav1.GetOptions{}) | |
| // 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") | |
i have the suggested fix here: #30896 (comment) |
|
/testwith openshift/origin/main/e2e-gcp-ovn-techpreview openshift/cluster-ingress-operator#1354 |
|
Scheduling required tests: |
|
@rhamini3: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Update gatewayAPIController tests to skip certain tests with OLM dependencies when
GatewayAPIWithoutOLM FeatureGate is enabled This will unblock
openshift/cluster-ingress-operator#1354 by skipping any
tests that require OLM capabilities without causing failures in the
origin testing.