NE-2779: Implement Gateway API management mode - #1547
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
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:
📝 WalkthroughWalkthroughAdds the 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
rikatz
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@rikatz: This pull request references NE-2779 which is a valid jira issue. 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
pkg/operator/controller/status/controller_test.go (1)
1198-1215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse testify assertions in the new tests.
The assertion block compares fields with
t.Errorf. The repository guidelines require testify for test assertions.assertalso reports all mismatches in one run instead of stopping at the first field. Add an assertion on the conditionTypeat the same time.As per coding guidelines: "Use testify/assert for assertions in tests (e.g., assert.NoError(t, err))".♻️ Proposed assertion refactor
actual := computeModeTransitionDegradedCondition(state) - if actual.Status != tc.expectStatus { - t.Errorf("expected status %q, got %q", tc.expectStatus, actual.Status) - } - if tc.expectReason != "" && actual.Reason != tc.expectReason { - t.Errorf("expected reason %q, got %q", tc.expectReason, actual.Reason) - } - if tc.expectMessage != "" && actual.Message != tc.expectMessage { - t.Errorf("expected message %q, got %q", tc.expectMessage, actual.Message) - } + assert.Equal(t, tc.expectStatus, actual.Status, "unexpected degraded condition status") + if tc.expectStatus != "" { + assert.Equal(t, configv1.OperatorDegraded, actual.Type, "unexpected condition type") + } + if tc.expectReason != "" { + assert.Equal(t, tc.expectReason, actual.Reason, "unexpected degraded condition reason") + } + if tc.expectMessage != "" { + assert.Equal(t, tc.expectMessage, actual.Message, "unexpected degraded condition message") + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/operator/controller/status/controller_test.go` around lines 1198 - 1215, Update the assertion block in the computeModeTransitionDegradedCondition table test to use testify/assert instead of t.Errorf, asserting Status, Reason, and Message as applicable and adding an assertion for the condition Type. Preserve conditional checks for optional expected reason and message values while allowing all assertion failures to be reported in one run.Source: Coding guidelines
pkg/operator/controller/gatewayapi/metrics_test.go (1)
36-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReset the shared metric state at the start of each test.
gatewayAPIManagementModeMetric,gatewayAPIInfoMetric,lastInfoGatewayAPIVersion, andlastInfoOSSMVersionare package-level state. These four tests do not reset that state, whileTestUpdateManagementModeMetrics_SteadyStateNoResetandTestUpdateManagementModeMetrics_VersionChangeResetsStaledo. The series-count assertions at Line 116, Line 140, and Line 166 therefore depend on execution order.Add one helper and call it from every test to make each test independent.
Based on learnings, flat assertions are kept for the single-case tests; this suggestion only adds setup isolation.♻️ Proposed helper for per-test isolation
// resetGatewayAPIMetrics clears the package-level metric state so that // each test starts from a known state. func resetGatewayAPIMetrics(t *testing.T) { t.Helper() gatewayAPIManagementModeMetric.Reset() gatewayAPIInfoMetric.Reset() infoMetricMu.Lock() lastInfoGatewayAPIVersion = "" lastInfoOSSMVersion = "" infoMetricMu.Unlock() }Then call it first in each test:
func TestUpdateManagementModeMetrics_ManagedAndCompliant(t *testing.T) { + resetGatewayAPIMetrics(t) managedCond := metav1.Condition{Also applies to: 89-118, 120-142, 144-168
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/operator/controller/gatewayapi/metrics_test.go` around lines 36 - 56, Reset shared gateway API metric state before every test in metrics_test.go. Add a resetGatewayAPIMetrics helper that resets gatewayAPIManagementModeMetric and gatewayAPIInfoMetric, then clears lastInfoGatewayAPIVersion and lastInfoOSSMVersion under infoMetricMu; call it as the first setup step in all four tests, including TestUpdateManagementModeMetrics_SteadyStateNoReset and TestUpdateManagementModeMetrics_VersionChangeResetsStale, so series-count assertions are execution-order independent.Source: Learnings
pkg/operator/controller/gatewayapi/controller.go (1)
262-310: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated transition-error handling.
The same five-line block that calls
SetTransitionStatewithInProgress: true,Target: snapshot.desiredMode, andError: errappears five times in this function. Every copy must stay identical. If one copy is later missed or changed, the status controller reports a stale or wrongProgressingcondition.Wrap the error path in a small helper.
♻️ Proposed helper to remove the duplication
+ // failTransition records the failed transition state and + // returns the error for the reconciler to retry. + failTransition := func(err error) (reconcile.Result, error) { + r.config.ModeAccessor.SetTransitionState(operatorcontroller.TransitionState{ + InProgress: true, + Target: snapshot.desiredMode, + Error: err, + }) + return reconcile.Result{}, err + } + if modeChanged { if err := r.reconcileAdmissionPolicyTransition(ctx, snapshot); err != nil { - r.config.ModeAccessor.SetTransitionState(operatorcontroller.TransitionState{ - InProgress: true, - Target: snapshot.desiredMode, - Error: err, - }) - return reconcile.Result{}, err + return failTransition(err) } } if err := r.reconcileIngressStatus(ctx, snapshot); err != nil { - r.config.ModeAccessor.SetTransitionState(operatorcontroller.TransitionState{ - InProgress: true, - Target: snapshot.desiredMode, - Error: err, - }) - return reconcile.Result{}, err + return failTransition(err) } if r.config.ModeAccessor.ShouldManageCRDs() { if err := r.ensureAdmissionPolicy(ctx); err != nil { - ... + return failTransition(err) } if err := r.ensureGatewayAPICRDs(ctx); err != nil { - ... + return failTransition(err) } if err := r.ensureGatewayAPIRBAC(ctx); err != nil { - ... + return failTransition(err) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/operator/controller/gatewayapi/controller.go` around lines 262 - 310, Extract the repeated transition-error handling in the reconcile function into a small local helper that accepts err, calls SetTransitionState with InProgress true, Target snapshot.desiredMode, and Error err, then returns the existing empty reconcile result and error. Replace every duplicated error block, including admission-policy, ingress-status, CRD, and RBAC paths, with the helper while preserving their current control flow.pkg/operator/controller/listenerset-status/controller.go (1)
193-198: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider clearing the ListenerSet metric when dependents become disallowed.
Reconcilereturns before anylistenerSetOnManagedGatewayMetric.DeleteLabelValuescall.Reconcileis the only writer and the only place that clears this gauge. After a transition to Unmanaged mode, gauges set while Managed stay at 1 indefinitely, so the metric reports ListenerSets on managed gateways that the operator no longer manages.If the metric should reflect only active management, reset it on the skip path.
♻️ Proposed change to reset the gauge when dependents are disallowed
if !r.modeAccessor.AllowDependents() { log.Info("Management mode does not allow dependent controllers, skipping reconciliation") + listenerSetOnManagedGatewayMetric.DeleteLabelValues(request.Namespace, request.Name) return reconcile.Result{}, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/operator/controller/listenerset-status/controller.go` around lines 193 - 198, Update the early management-mode skip path in reconciler.Reconcile to clear the ListenerSet gauge via listenerSetOnManagedGatewayMetric.DeleteLabelValues before returning when AllowDependents() is false, ensuring stale managed-mode metric labels are reset.pkg/operator/controller/gatewayapi/admission_policy_test.go (2)
94-171: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated reconciler fixture.
These four tests differ only in the
Infrastructurestatus and the assertions on the created policy. The scheme setup, fake client,FakeClientRecorder,FakeCache, andreconcilerconstruction are identical, roughly 45 duplicated lines each. Extract a helper such asnewAdmissionPolicyTestReconciler(t, infra *configv1.Infrastructure) (*reconciler, *testutil.FakeClientRecorder, client.Client), then drive the topology variants from a table.Also applies to: 176-252, 257-330, 336-412
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/operator/controller/gatewayapi/admission_policy_test.go` around lines 94 - 171, Extract the duplicated scheme, fake client, recorder, cache, and reconciler setup from the four admission policy tests into a helper such as newAdmissionPolicyTestReconciler, accepting the Infrastructure fixture and returning the reconciler, recorder, and client. Refactor the tests at TestReconcile_GateOn_Managed_CreatesVAP and the related topology variants to use this helper, then table-drive the differing Infrastructure statuses and policy assertions while preserving each test’s expected behavior.
95-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCheck the scheme registration errors.
configv1.Install,apiextensionsv1.AddToScheme,operatorv1alpha1.Install,rbacv1.AddToScheme, andadmissionregistrationv1.AddToSchemeall return an error. This file discards every one of them, in this block and in the later tests. If registration fails, the test fails later with an unrelated "no kind is registered" message. Wrap each call inrequire.NoError(t, ...).As per coding guidelines: "Never ignore error returns".
♻️ Proposed fix for the scheme setup
scheme := runtime.NewScheme() - configv1.Install(scheme) - apiextensionsv1.AddToScheme(scheme) - rbacv1.AddToScheme(scheme) - operatorv1alpha1.Install(scheme) - admissionregistrationv1.AddToScheme(scheme) + require.NoError(t, configv1.Install(scheme)) + require.NoError(t, apiextensionsv1.AddToScheme(scheme)) + require.NoError(t, rbacv1.AddToScheme(scheme)) + require.NoError(t, operatorv1alpha1.Install(scheme)) + require.NoError(t, admissionregistrationv1.AddToScheme(scheme))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/operator/controller/gatewayapi/admission_policy_test.go` around lines 95 - 100, Update every scheme-registration call in the test setup and later test blocks to wrap configv1.Install, apiextensionsv1.AddToScheme, operatorv1alpha1.Install, rbacv1.AddToScheme, and admissionregistration.AddToScheme with require.NoError(t, ...), ensuring registration failures fail immediately with the original error.Source: Path instructions
pkg/operator/controller/gateway-labeler/controller.go (1)
180-185: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winShared
ObjectListin the Ingress wake-up mapper. All three wake-up watches calloperatorcontroller.IngressWakeUpMapperwith a freshly allocated list value that the returned closure then reuses on every invocation. The shared root cause is theIngressWakeUpMappersignature inpkg/operator/controller/mode.go, which takes oneclient.ObjectListinstead of producing a list per call.
pkg/operator/controller/gateway-labeler/controller.go#L180-L185: afterIngressWakeUpMapperaccepts a per-call list, pass a factory that returns a new&gatewayapiv1.GatewayList{}.pkg/operator/controller/gateway-networkpolicy/controller.go#L65-L70: pass the same factory, keeping theclient.InNamespace(operatorcontroller.DefaultOperandNamespace)list option.pkg/operator/controller/gateway-status/controller.go#L143-L148: pass the same factory while continuing to list fromgatewaysCache.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/operator/controller/gateway-labeler/controller.go` around lines 180 - 185, Update IngressWakeUpMapper in pkg/operator/controller/mode.go to accept a list factory that creates a fresh client.ObjectList for each invocation, preventing reuse across wake-up events. In pkg/operator/controller/gateway-labeler/controller.go lines 180-185, pass a factory returning a new GatewayList; make the same factory change in pkg/operator/controller/gateway-networkpolicy/controller.go lines 65-70 while preserving the InNamespace(DefaultOperandNamespace) option, and in pkg/operator/controller/gateway-status/controller.go lines 143-148 while preserving gatewaysCache listing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/operator/controller/gatewayapi/admission_policy.go`:
- Around line 119-130: Update ensureValidatingAdmissionPolicyBinding and its
binding asset so validationActions explicitly contains Deny, or use a stable
comparison of managed fields that normalizes zero-value differences. Ensure
created bindings match the desired spec and do not trigger repeated updates
through admissionPolicyUpToDate-style DeepEqual checks.
In `@pkg/operator/controller/gatewayapi/ingress_status_test.go`:
- Around line 368-370: Guard both bundleVersionAnnotation assignments in the
test before writing to the CRD annotation map: initialize firstCRD.Annotations
when it is nil, matching the existing pattern in controller_test.go. Apply the
same nil-map protection to the assignment at the additional affected site while
preserving the current test values and behavior.
In `@pkg/operator/controller/gatewayapi/metrics.go`:
- Around line 42-44: Run gofmt on metrics.go to normalize the alignment of the
infoMetricMu, lastInfoGatewayAPIVersion, and lastInfoOSSMVersion declarations
and ensure gofmt -l reports no formatting issues.
In `@pkg/operator/controller/gatewayclass/controller.go`:
- Around line 793-801: The UninstallSail method currently treats a nil
sailInstaller as a successful no-op, allowing Unmanaged transitions to complete
while OLM-installed Istio remains running. Update UninstallSail to handle the
OLM path by stopping or scaling down the OLM-managed Istio, or return an error
when that cannot be performed; preserve the existing sailInstaller.Uninstall
flow for CIO-managed installations.
In `@pkg/operator/controller/mode.go`:
- Around line 32-58: Update IngressWakeUpMapper in
pkg/operator/controller/mode.go:32-58 to treat objList as a template by creating
a per-invocation copy with DeepCopyObject inside the returned closure, then list
and extract items from that copy. Leave
pkg/operator/controller/gateway-service-dns/controller.go:120-131 and
pkg/operator/controller/listenerset-status/controller.go:169-179 unchanged;
their list arguments require no direct change.
In `@pkg/operator/controller/status/controller.go`:
- Around line 618-621: Update the status controller’s reconciliation trigger
around getOperatorState so gatewayapi mode transitions refresh ClusterOperator
conditions while in progress. When ModeAccessor.GateEnabled() is true, watch
operatorv1alpha1.Ingress events or return a RequeueAfter from the status path
until the transition completes, while preserving the existing event handling and
steady-state result.
In `@test/e2e/gateway_api_mgmt_mode_test.go`:
- Around line 457-485: Register cleanup before each cluster-scoped mutation so
failures cannot leave shared Gateway API state altered. In
test/e2e/gateway_api_mgmt_mode_test.go:457-485, move the existing cleanup above
the transition to Unmanaged; at 492-515, add cleanup before the mode change to
restore ManagementMode to Managed and the CRD bundle-version annotation; at
416-441, register cleanup while still Unmanaged to remove the
test.openshift.io/unmanaged annotation.
- Around line 416-441: Update the CRD modification flow around the test
annotation in the gateway management test to register cleanup that removes
test.openshift.io/unmanaged from the same CRD before Managed mode is restored.
Ensure cleanup updates the CRD while Unmanaged mode remains active and does not
affect the existing successful modification assertion.
In `@test/e2e/gateway_api_test.go`:
- Line 18: Remove the unused operatorv1alpha1 import from the gateway API
end-to-end test imports; no other code changes are needed.
---
Nitpick comments:
In `@pkg/operator/controller/gateway-labeler/controller.go`:
- Around line 180-185: Update IngressWakeUpMapper in
pkg/operator/controller/mode.go to accept a list factory that creates a fresh
client.ObjectList for each invocation, preventing reuse across wake-up events.
In pkg/operator/controller/gateway-labeler/controller.go lines 180-185, pass a
factory returning a new GatewayList; make the same factory change in
pkg/operator/controller/gateway-networkpolicy/controller.go lines 65-70 while
preserving the InNamespace(DefaultOperandNamespace) option, and in
pkg/operator/controller/gateway-status/controller.go lines 143-148 while
preserving gatewaysCache listing.
In `@pkg/operator/controller/gatewayapi/admission_policy_test.go`:
- Around line 94-171: Extract the duplicated scheme, fake client, recorder,
cache, and reconciler setup from the four admission policy tests into a helper
such as newAdmissionPolicyTestReconciler, accepting the Infrastructure fixture
and returning the reconciler, recorder, and client. Refactor the tests at
TestReconcile_GateOn_Managed_CreatesVAP and the related topology variants to use
this helper, then table-drive the differing Infrastructure statuses and policy
assertions while preserving each test’s expected behavior.
- Around line 95-100: Update every scheme-registration call in the test setup
and later test blocks to wrap configv1.Install, apiextensionsv1.AddToScheme,
operatorv1alpha1.Install, rbacv1.AddToScheme, and
admissionregistration.AddToScheme with require.NoError(t, ...), ensuring
registration failures fail immediately with the original error.
In `@pkg/operator/controller/gatewayapi/controller.go`:
- Around line 262-310: Extract the repeated transition-error handling in the
reconcile function into a small local helper that accepts err, calls
SetTransitionState with InProgress true, Target snapshot.desiredMode, and Error
err, then returns the existing empty reconcile result and error. Replace every
duplicated error block, including admission-policy, ingress-status, CRD, and
RBAC paths, with the helper while preserving their current control flow.
In `@pkg/operator/controller/gatewayapi/metrics_test.go`:
- Around line 36-56: Reset shared gateway API metric state before every test in
metrics_test.go. Add a resetGatewayAPIMetrics helper that resets
gatewayAPIManagementModeMetric and gatewayAPIInfoMetric, then clears
lastInfoGatewayAPIVersion and lastInfoOSSMVersion under infoMetricMu; call it as
the first setup step in all four tests, including
TestUpdateManagementModeMetrics_SteadyStateNoReset and
TestUpdateManagementModeMetrics_VersionChangeResetsStale, so series-count
assertions are execution-order independent.
In `@pkg/operator/controller/listenerset-status/controller.go`:
- Around line 193-198: Update the early management-mode skip path in
reconciler.Reconcile to clear the ListenerSet gauge via
listenerSetOnManagedGatewayMetric.DeleteLabelValues before returning when
AllowDependents() is false, ensuring stale managed-mode metric labels are reset.
In `@pkg/operator/controller/status/controller_test.go`:
- Around line 1198-1215: Update the assertion block in the
computeModeTransitionDegradedCondition table test to use testify/assert instead
of t.Errorf, asserting Status, Reason, and Message as applicable and adding an
assertion for the condition Type. Preserve conditional checks for optional
expected reason and message values while allowing all assertion failures to be
reported in one run.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f6a5a62-8330-40c3-a9f9-d2062fa38dec
⛔ Files ignored due to path filters (36)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_kmsencryption.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/features.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/legacyfeaturegates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversionoperators.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_imagecontentsourcepolicies.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_10_etcd_01_etcdbackups.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_10_operator-lifecycle-manager_01_olms.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_30_cluster-api_01_clusterapis.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_50_ingress_02_ingresses.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/route/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/route/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/route/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (44)
Dockerfilecmd/ingress-operator/start.gogo.modhack/update-generated-crd.shhack/verify-generated-crd.shmanifests/00-cluster-role.yamlmanifests/00-custom-resource-definition-ingress.yamlmanifests/00-ingress.yamlmanifests/01-validating-admission-policy-binding.yamlmanifests/01-validating-admission-policy-ibm-cloud-managed.yamlmanifests/01-validating-admission-policy.yamlpkg/manifests/assets/gateway-api/validating-admission-policy-binding.yamlpkg/manifests/assets/gateway-api/validating-admission-policy.yamlpkg/manifests/manifests.gopkg/operator/client/client.gopkg/operator/controller/gateway-labeler/controller.gopkg/operator/controller/gateway-networkpolicy/controller.gopkg/operator/controller/gateway-service-dns/controller.gopkg/operator/controller/gateway-service-dns/controller_test.gopkg/operator/controller/gateway-status/controller.gopkg/operator/controller/gateway-status/controller_test.gopkg/operator/controller/gatewayapi/admission_policy.gopkg/operator/controller/gatewayapi/admission_policy_test.gopkg/operator/controller/gatewayapi/controller.gopkg/operator/controller/gatewayapi/controller_test.gopkg/operator/controller/gatewayapi/crds.gopkg/operator/controller/gatewayapi/ingress_status.gopkg/operator/controller/gatewayapi/ingress_status_test.gopkg/operator/controller/gatewayapi/metrics.gopkg/operator/controller/gatewayapi/metrics_test.gopkg/operator/controller/gatewayapi/mode.gopkg/operator/controller/gatewayapi/mode_test.gopkg/operator/controller/gatewayclass/controller.gopkg/operator/controller/gatewayclass/controller_test.gopkg/operator/controller/listenerset-status/controller.gopkg/operator/controller/listenerset-status/controller_test.gopkg/operator/controller/mode.gopkg/operator/controller/mode_test.gopkg/operator/controller/status/controller.gopkg/operator/controller/status/controller_test.gopkg/operator/operator.gotest/e2e/gateway_api_mgmt_mode_test.gotest/e2e/gateway_api_test.gotools/tools.go
c0b5c4a to
48124e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/operator/controller/mode.go (1)
37-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude the triggering Ingress in mapper error logs.
Keep the event object instead of discarding it. Add its resource type and name to both error logs. This identifies the Ingress event that caused the failed list or extraction.
Proposed change
-func IngressWakeUpMapper(cacheReader client.Reader, listFactory func() client.ObjectList, listOpts ...client.ListOption) handler.MapFunc { - return func(ctx context.Context, _ client.Object) []reconcile.Request { +func IngressWakeUpMapper(cacheReader client.Reader, listFactory func() client.ObjectList, listOpts ...client.ListOption) handler.MapFunc { + return func(ctx context.Context, ingress client.Object) []reconcile.Request { objList := listFactory() if err := cacheReader.List(ctx, objList, listOpts...); err != nil { - modeLog.Error(err, "Failed to list objects for Ingress wake-up") + modeLog.Error(err, "Failed to list objects for Ingress wake-up", "resource", "Ingress", "name", ingress.GetName()) return nil }As per coding guidelines, “Use structured logging via go-logr/logr with relevant context (namespace, name, resource type).”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/operator/controller/mode.go` around lines 37 - 45, Update the mapper callback around the discarded client.Object parameter to retain the triggering Ingress event, then include its resource type and name as structured fields in both modeLog.Error calls for list and extraction failures. Preserve the existing error messages and return behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/operator/controller/mode_test.go`:
- Around line 78-85: Update the concurrent mapper test so worker goroutines only
record whether mapper returns nil, without calling require.NotNil. Wait for all
workers via the existing WaitGroup, then perform the nil-result assertion from
the main test goroutine.
In `@pkg/operator/controller/status/controller.go`:
- Around line 288-294: Move the in-progress mode-transition requeue handling out
of the early return in the controller reconciliation flow so
computeOperatorProgressingCondition and computeOperatorDegradedCondition execute
and persist ClusterOperator status first. Preserve the existing transition log,
target field, five-second RequeueAfter value, and nil error, but return that
result only after the status update path completes.
---
Nitpick comments:
In `@pkg/operator/controller/mode.go`:
- Around line 37-45: Update the mapper callback around the discarded
client.Object parameter to retain the triggering Ingress event, then include its
resource type and name as structured fields in both modeLog.Error calls for list
and extraction failures. Preserve the existing error messages and return
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: eb378d61-b3ef-4cd1-8ee5-9d0329b494d4
📒 Files selected for processing (18)
go.modpkg/operator/controller/gateway-labeler/controller.gopkg/operator/controller/gateway-networkpolicy/controller.gopkg/operator/controller/gateway-service-dns/controller.gopkg/operator/controller/gateway-status/controller.gopkg/operator/controller/gatewayapi/admission_policy.gopkg/operator/controller/gatewayapi/controller.gopkg/operator/controller/gatewayapi/controller_test.gopkg/operator/controller/gatewayapi/ingress_status.gopkg/operator/controller/gatewayapi/ingress_status_test.gopkg/operator/controller/gatewayapi/metrics.gopkg/operator/controller/gatewayapi/mode_test.gopkg/operator/controller/listenerset-status/controller.gopkg/operator/controller/mode.gopkg/operator/controller/mode_test.gopkg/operator/controller/status/controller.gotest/e2e/gateway_api_mgmt_mode_test.gotest/e2e/gateway_api_test.go
💤 Files with no reviewable changes (1)
- test/e2e/gateway_api_test.go
🚧 Files skipped from review as they are similar to previous changes (10)
- pkg/operator/controller/gatewayapi/admission_policy.go
- test/e2e/gateway_api_mgmt_mode_test.go
- pkg/operator/controller/gateway-status/controller.go
- pkg/operator/controller/gateway-labeler/controller.go
- pkg/operator/controller/listenerset-status/controller.go
- pkg/operator/controller/gatewayapi/mode_test.go
- pkg/operator/controller/gatewayapi/metrics.go
- pkg/operator/controller/gatewayapi/ingress_status_test.go
- pkg/operator/controller/gateway-networkpolicy/controller.go
- pkg/operator/controller/gateway-service-dns/controller.go
dee3f38 to
3158b28
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/e2e/gateway_api_mgmt_mode_test.go`:
- Around line 516-532: The takeover cleanup must reliably restore Managed mode
instead of relying on stale ingress state. Update the cleanup’s Gateway API
restore block around kclient.Get/kclient.Update to use the existing retry
pattern from the unmanaged-mode cleanup, handling Get failures and retrying
Update conflicts until the cluster is confirmed Managed.
- Around line 327-366: In the t.Cleanup function, replace require.Eventually
with assert.Eventually so cleanup continues to the GatewayAPICRDsManaged
condition wait after a restoration failure. Capture the kclient.Get error in the
CRD annotation cleanup and log it with t.Logf instead of discarding it, while
preserving the existing annotation-removal flow.
- Around line 518-523: Extend the cleanup flow around the kclient.Delete call so
that after deleting the non-compliant CRD, it waits for crdName to be recreated
with a compliant bundleVersionAnnotation before returning. Reuse the existing
polling or eventually-wait helper used by this test suite, and preserve the
current behavior when the CRD is absent or already compliant.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 9ad58c57-64e7-4f31-bb30-38e23e9423cc
📒 Files selected for processing (3)
pkg/operator/controller/mode_test.gopkg/operator/controller/status/controller.gotest/e2e/gateway_api_mgmt_mode_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/operator/controller/mode_test.go
- pkg/operator/controller/status/controller.go
|
@coderabbitai are your comments above still valid? Also can you verify the whole implementation and see if everything is correct now? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="openshift/cluster-ingress-operator"
pr_number="1547"
printf '%s\n' '--- Pull request metadata ---'
gh pr view "$pr_number" --repo "$repo" --json url,state,headRefName,baseRefName,updatedAt,reviews \
--jq '{url, state, headRefName, baseRefName, updatedAt, reviews: [.reviews[] | {author: .author.login, state, submittedAt}]}'
printf '%s\n' '--- CodeRabbit pull request review comments ---'
gh api --paginate "repos/$repo/pulls/$pr_number/comments?per_page=100" \
--jq '.[] | select(.user.login == "coderabbitai") | {id, path, line, original_line, created_at, updated_at, body, html_url}'
printf '%s\n' '--- CodeRabbit issue comments ---'
gh api --paginate "repos/$repo/issues/$pr_number/comments?per_page=100" \
--jq '.[] | select(.user.login == "coderabbitai") | {id, created_at, updated_at, body, html_url}'Length of output: 1790 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="openshift/cluster-ingress-operator"
pr_number="1547"
printf '%s\n' '--- CodeRabbit review bodies ---'
gh api --paginate "repos/$repo/pulls/$pr_number/reviews?per_page=100" \
--jq '.[] | select(.user.login == "coderabbitai") | {id, state, submitted_at, body, html_url, commit_id}'
printf '%s\n' '--- Pull request review threads ---'
gh api graphql \
-F owner="${repo%%/*}" \
-F name="${repo#*/}" \
-F number="$pr_number" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
isOutdated
path
line
comments(first: 20) {
nodes {
author { login }
createdAt
body
url
}
}
}
}
}
}
}' \
--jq '.data.repository.pullRequest.reviewThreads.nodes
| map(select(any(.comments.nodes[]?; .author.login == "coderabbitai")))
| .[]'Length of output: 76138 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Current pull request changes ---'
git diff --name-status HEAD~1..HEAD 2>/dev/null || git status --short
git diff --stat
printf '%s\n' '--- Gateway API controller package outline ---'
ast-grep outline pkg/operator/controller/gatewayapi --items all --type function,method,struct --match 'reconcile|ensure|delete|Mode|Admission|Ingress|CRD' || true
printf '%s\n' '--- Admission-policy asset and reconciliation ---'
fd -t f -g '*validating-admission-policy*' pkg/manifests/assets --exec sh -c 'echo "== $1 =="; cat -n "$1"' sh {}
sed -n '1,260p' pkg/operator/controller/gatewayapi/admission_policy.go | nl -ba
rg -n -C4 'validationActions|ensureValidatingAdmissionPolicyBinding|admissionPolicyUpToDate|DeepEqual' \
pkg/operator/controller/gatewayapi pkg/manifests
printf '%s\n' '--- Management-mode e2e cleanup paths ---'
ast-grep outline test/e2e/gateway_api_mgmt_mode_test.go --items all --type function --match 'testGatewayAPIManagementMode|TestGatewayAPI' || true
rg -n -C12 't\.Cleanup\(|require\.Eventually\(|assert\.Eventually\(|test\.openshift\.io/unmanaged|takeover-blocked|bundleVersionAnnotation|kclient\.Delete' \
test/e2e/gateway_api_mgmt_mode_test.go
printf '%s\n' '--- Management-mode wiring and synchronization ---'
rg -n -C5 'NewModeAccessor|ModeAccessor|IngressWakeUpMapper|reconcileIngressStatus|reconcileAdmissionPolicyTransition|SetTransitionState|SetLastAppliedMode|ShouldManageCRDs|AllowDependents' \
pkg/operator cmd/ingress-operator/start.goLength of output: 10062 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/master)"
printf '%s\n' '--- Full pull request change summary ---'
printf 'merge-base: %s\n' "$base"
git diff --stat "$base"..HEAD
git diff --name-only "$base"..HEAD
printf '%s\n' '--- Admission-policy loader and stable comparison ---'
awk '{printf "%6d %s\n", NR, $0}' pkg/operator/controller/gatewayapi/admission_policy.go | sed -n '1,280p'
awk '{printf "%6d %s\n", NR, $0}' pkg/manifests/manifests.go | sed -n '1,260p'
printf '%s\n' '--- Full management-mode e2e suite: setup, cleanup, and takeover paths ---'
awk '{printf "%6d %s\n", NR, $0}' test/e2e/gateway_api_mgmt_mode_test.go | sed -n '280,680p'
printf '%s\n' '--- Gateway API transition and status paths ---'
awk '{printf "%6d %s\n", NR, $0}' pkg/operator/controller/gatewayapi/controller.go | sed -n '150,430p'
awk '{printf "%6d %s\n", NR, $0}' pkg/operator/controller/gatewayapi/ingress_status.go | sed -n '1,320p'
awk '{printf "%6d %s\n", NR, $0}' pkg/operator/controller/status/controller.go | sed -n '240,340p'
printf '%s\n' '--- Tests that cover the transition and cleanup behavior ---'
rg -n -C5 'validationActions|AdmissionPolicyBinding|ModeSnapshot|UnmanagedDeletesVAP|takeover|Cleanup|recreat|restore' \
pkg/operator/controller/gatewayapi/*_test.go test/e2e/gateway_api_mgmt_mode_test.goLength of output: 144705
One finding is no longer valid:
Two e2e cleanup findings are still valid:
I also checked the main implementation paths. The mode controller uses one Ingress snapshot per reconcile. It removes the VAP before it writes Unmanaged status. It blocks CRD ownership during a non-compliant takeover. It gates dependent controllers on the management state. These paths have focused unit tests. I cannot confirm that the implementation is fully correct while the two cleanup defects remain. I also could not run Go unit tests or e2e tests in this sandbox. The full review request is active. You are interacting with an AI system. |
3158b28 to
b671ce8
Compare
|
@coderabbitai can you analyze this PR again and verify if everything is fine? |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="openshift/cluster-ingress-operator"
pr="1547"
echo '--- Pull request revision ---'
gh pr view "$pr" --repo "$repo" --json state,updatedAt,headRefOid,baseRefOid,mergeable,reviewDecision \
--jq '{state, updatedAt, headRefOid, baseRefOid, mergeable, reviewDecision}'
echo '--- Changed files against merge base ---'
base="$(git merge-base HEAD origin/master 2>/dev/null || git merge-base HEAD origin/main)"
git diff --stat "$base"..HEAD
git diff --name-status "$base"..HEAD
echo '--- Current CodeRabbit review threads ---'
gh api graphql \
-F owner="openshift" -F name="cluster-ingress-operator" -F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
isOutdated
path
line
comments(first: 20) {
nodes { author { login } body url }
}
}
}
}
}
}' \
--jq '.data.repository.pullRequest.reviewThreads.nodes[]
| select(any(.comments.nodes[]?; .author.login == "coderabbitai"))
| {isResolved, isOutdated, path, line,
comments: [.comments.nodes[] | select(.author.login == "coderabbitai") | {body, url}]}'
echo '--- E2E test structure ---'
ast-grep outline test/e2e/gateway_api_mgmt_mode_test.go --items all --type function --view expanded
echo '--- Prior cleanup locations and related cleanup operations ---'
rg -n -C10 't\.Cleanup\(|require\.Eventually\(|assert\.Eventually\(|kclient\.(Get|Update|Delete)|GatewayAPICRDsManaged|managementMode|Managed|Unmanaged' \
test/e2e/gateway_api_mgmt_mode_test.go
echo '--- Gateway API controller structure ---'
ast-grep outline pkg/operator/controller/gatewayapi --items all --type function,method,struct --match 'reconcile|Reconcile|Transition|Ingress|Admission|CRD|Mode' --view expanded
echo '--- Transition, status, CRD, and admission-policy implementation ---'
rg -n -C6 'func \(r \*reconciler\) Reconcile|reconcileIngressStatus|reconcileAdmissionPolicyTransition|ShouldManageCRDs|SetTransitionState|SetLastAppliedMode|ensureValidating|deleteValidating|crdSpecCompliant|Compute' \
pkg/operator/controller/gatewayapi pkg/operator/controller/mode.go pkg/operator/controller/status/controller.go
echo '--- Critical test coverage ---'
rg -n -C3 'Cleanup|ModeSnapshot|UnmanagedDeletesVAP|Takeover|PartialPresence|ForbiddenIngress|ObservedGeneration|AdmissionPolicy|Transition' \
pkg/operator/controller/gatewayapi/*_test.go test/e2e/gateway_api_mgmt_mode_test.goLength of output: 198875
The first cleanup finding is addressed. The Unmanaged-mode cleanup now retries the mode update. It also uses One cleanup defect remains in _ = kclient.Delete(context.Background(), crd)The takeover cleanup ignores a CRD deletion failure. The following The cleanup should retry I found no additional correctness defects in the reviewed controller paths. The implementation now has focused coverage for mode snapshots, VAP deletion before Unmanaged status, takeover blocking, partial CRD presence, transition state, and gate-off behavior. I could not run Go unit tests or e2e tests in this sandbox. Therefore, I cannot give runtime verification until CI completes.
|
|
/test e2e-aws-operator |
|
/payload-job-with-prs e2e-aws-ovn-serial openshift/origin#31503 |
|
@rikatz: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/baf0cf60-958c-11f1-8ef9-db390a0f6457-0 |
|
/payload-job-with-prs e2e-gcp-ovn openshift/origin#31503 |
|
@rikatz: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d88faa50-958c-11f1-966f-44422d7a35b5-0 |
|
I want to review this one: |
|
/payload-job-with-prs periodic-ci-openshift-release-master-nightly-5.0-e2e-aws-ovn-techpreview-serial openshift/origin#31503 |
|
@rikatz: trigger 0 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command |
|
/payload-job-with-prs e2e-gcp-ovn-techpreview-serial openshift/origin#31503 |
|
@rikatz: trigger 3 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/285f79d0-b03f-11f1-8c1a-b20b3f98249d-0 |
|
/payload-job-with-prs periodic-ci-openshift-release-main-ci-5.1-e2e-aws-ovn-techpreview-serial openshift/origin#31503 |
|
@rikatz: trigger 3 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d04dfb00-b05b-11f1-98cf-283880109ab1-0 |
|
/retest-required |
3 similar comments
|
/retest-required |
|
/retest-required |
|
/retest-required |
|
/payload-job-with-prs periodic-ci-openshift-release-main-ci-5.1-e2e-aws-ovn-techpreview-serial openshift/origin#31503 |
|
@rikatz: trigger 3 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f8855a30-b0ee-11f1-960d-38047f9f7f45-0 |
|
/payload-job-with-prs periodic-ci-openshift-hypershift-release-5.1-periodics-e2e-aws-ovn-conformance-serial-techpreview openshift/origin#31503 |
|
@rikatz: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/08af3b10-b0ef-11f1-9eba-3d8162dbffe5-0 |
|
/payload-job-with-prs periodic-ci-openshift-hypershift-release-5.1-periodics-e2e-aws-ovn-conformance-serial-techpreview openshift/origin#31503 |
|
@rikatz: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/8b029f70-b0ff-11f1-93ba-b7e58c0d484f-0 |
|
origin tests are passing: |
|
/test help |
|
/verified by "periodic-ci-openshift-hypershift-release-5.1-periodics-e2e-aws-ovn-conformance-serial-techpreview", "e2e-vsphere-static-metallb-operator-gwapi-techpreview", "local origin test run", "local e2e test" /hold I will run a techpreview e2e test as well |
openshift-ci
Bot
added
the
do-not-merge/hold
openshift-ci-robot
added
the
verified
|
@rikatz: This PR has been marked as verified by 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. |
|
/test e2e-aws-operator-techpreview |
|
/verified by "TestGatewayAPI/testGatewayAPIManagementModeDefault","TestGatewayAPI/testGatewayAPIManagementModeMetrics", "TestGatewayAPI/testGatewayAPIManagementModeCRDCompliance", "TestGatewayAPI/testGatewayAPIManagementModeUnmanaged", "TestGatewayAPI/testGatewayAPIManagementModeTakeover", "[sig-network-edge][OCPFeatureGate:GatewayAPIManagementMode]" |
|
@rikatz: This PR has been marked as verified by 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. |
|
@rikatz just fyi your techpreview job looks good - #1592 is trying to fix this LB Subnet Permafail - it's unrelated to you. |
|
failed tests on techpreview job are not related with Gateway API Management mode |
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
@rikatz: The following test 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. |
059345c
into
openshift:master
5 participants
This change implements the new Gateway API management mode, allowing users to fully disable managed Gateway API from OCP, and also to come back to a managed mode.
Additionally this change exposes the Gateway API CRDs on the container image, allowing support engineers to provide a clear rollback path to managed mode in case it is required.