Skip to content

fix: stop requeueing ApisixConsumer forever after a successful delete (backport apache/apisix-ingress-controller#2850) - #472

Open
AlinsRan wants to merge 7 commits into
masterfrom
backport/apisixconsumer-delete-requeue
Open

AlinsRan wants to merge 7 commits into
masterfrom
backport/apisixconsumer-delete-requeue

Conversation

@AlinsRan

@AlinsRan AlinsRan commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Backports apache/apisix-ingress-controller#2850 and #2854, and adds the status updater stub the other backports in flight need.

ApisixConsumerReconciler.Reconcile already returns after a successful delete here, so the only behavior change is the log line that upstream added alongside it. The rest is test scaffolding: recordingProvider, the provider stub the reconciler tests use, plus its coverage of the delete path.

recordingUpdater is added beside it. Upstream keeps that one in gateway_controller_publishservice_test.go, which this repository has not backported, and pulling in that whole file to get a nine-line stub is not worth it.

This is a prerequisite for the backports of apache/apisix-ingress-controller#2859 and #2860, whose tests use both stubs. Those two branches are built on this one so their CI is green before it merges; their diffs collapse to their own changes once this lands.

Summary by CodeRabbit

  • Bug Fixes

    • Improved reconciliation of deleted consumers so completed deletions do not trigger unnecessary retries.
    • Provider deletion errors are now surfaced for clearer troubleshooting.
    • Unexpected errors when retrieving consumer resources are propagated without attempting provider deletion.
    • Added informational logging when a consumer is successfully removed from the provider after its Kubernetes object is no longer found.
  • Tests

    • Expanded coverage for consumer deletion and error-handling scenarios.

alliasgher and others added 3 commits September 9, 2026 12:52
The reconciler tests being backported need a status.Updater stub as well.
Upstream keeps it in gateway_controller_publishservice_test.go, which this
repository has not backported, so it lives beside recordingProvider instead of
pulling in an unrelated test file.
@CLAassistant

CLAassistant commented Sep 9, 2026 •

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 4 committers have signed the CLA.

✅ AlinsRan
❌ johannes-engler-mw
❌ alliasgher
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 0a3e35df-4c0c-4f3c-9f42-72ea59923b71
📥 Commits

Reviewing files that changed from the base of the PR and between f7ea9e6 and b500639.

📒 Files selected for processing: 2
  • internal/controller/apisixconsumer_controller.go
  • internal/controller/apisixconsumer_controller_test.go

Included review availability: This review used your included allowance. 4 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

📝 Walkthrough

The controller logs successful provider deletion when an ApisixConsumer is missing from Kubernetes. Tests cover successful deletion, provider deletion errors, and non-NotFound client errors.

Changes: ApisixConsumer deletion reconciliation

Layer / File(s) Summary
Deleted consumer reconciliation
internal/controller/apisixconsumer_controller.go, internal/controller/apisixconsumer_controller_test.go
The controller logs successful provider deletion. A test checks that reconciliation deletes the provider configuration without requeueing.
Deletion error handling
internal/controller/apisixconsumer_controller_test.go
Tests check that provider deletion errors are returned and that non-NotFound client errors do not delete provider configuration.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: shreemaan-abhishek

Merge Risk: ⚪ Minimal · up to b5006

The change adds a success log for provider deletion and tests deletion and error handling. No material merge-blocking risk is evident from the supplied context.

Pre-merge checks: Passed 5 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
E2e Test Quality Review Warning Blocking: the PR adds only unit tests for the deleted-object path. internal/controller/apisixconsumer_controller_test.go:246-313 uses a fake Kubernetes client, an interceptor, and `recordingProvider… Add an E2E ApisixConsumer scenario that creates the consumer and dependent route, deletes the consumer through Kubernetes, verifies provider/data-plane removal, and verifies the controller completes without repeated reconcile failures or re…
Passed Passed checks: 5 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the ApisixConsumer deletion and requeue behavior addressed by the pull request. It is concise and relevant to the delete-path changes and tests.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Security Check Passed No security issue is introduced. The authoritative diff adds one production log at internal/controller/apisixconsumer_controller.go:81; utils.NamespacedName returns only namespace and name (internal/u…
Full details: E2e Test Quality Review

Explanation

Blocking: the PR adds only unit tests for the deleted-object path. internal/controller/apisixconsumer_controller_test.go:246-313 uses a fake Kubernetes client, an interceptor, and recordingProvider; it does not exercise the Kubernetes-to-controller-to-provider/APISIX flow or prove that a live delete does not cause repeated reconciles. The existing test/e2e/crds/v2/consumer.go delete scenarios are unchanged and verify authentication/data-plane removal only. The PR diff now contains only the relevant controller and unit-test files, so the prior unrelated-scope and stale-comment findings are resolved.

Resolution

Add an E2E ApisixConsumer scenario that creates the consumer and dependent route, deletes the consumer through Kubernetes, verifies provider/data-plane removal, and verifies the controller completes without repeated reconcile failures or requeues. Keep the unit tests for provider-delete and Kubernetes Get errors.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

conformance test report - apisix mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-09-09T08:47:09Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
  contact:
  - https://github.com/apache/apisix-ingress-controller/issues
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    result: partial
    skippedTests:
    - GRPCRouteListenerHostnameMatching
    statistics:
      Failed: 0
      Passed: 14
      Skipped: 1
  extended:
    result: success
    statistics:
      Failed: 0
      Passed: 1
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
  name: GATEWAY-GRPC
  summary: Core tests partially succeeded with 1 test skips. Extended tests succeeded.
- core:
    result: partial
    skippedTests:
    - TLSRouteHostnameIntersection
    - TLSRouteInvalidBackendRefNonexistent
    - TLSRouteInvalidBackendRefUnknownKind
    - TLSRouteSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 16
      Skipped: 4
  extended:
    result: partial
    skippedTests:
    - TLSRouteTerminateSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 3
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - TLSRouteModeTerminate
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
    - TLSRouteModeMixed
  name: GATEWAY-TLS
  summary: Core tests partially succeeded with 4 test skips. Extended tests partially
    succeeded with 1 test skips.
- core:
    result: partial
    skippedTests:
    - HTTPRouteHTTPSListener
    - HTTPRouteInvalidBackendRefUnknownKind
    - HTTPRouteInvalidCrossNamespaceBackendRef
    - HTTPRouteInvalidNonExistentBackendRef
    - HTTPRouteListenerHostnameMatching
    - HTTPRouteMultipleGateways
    - HTTPRouteNoBackendRefs
    statistics:
      Failed: 0
      Passed: 30
      Skipped: 7
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - BackendTLSPolicy
    - BackendTLSPolicySANValidation
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRoute303RedirectStatusCode
    - HTTPRoute307RedirectStatusCode
    - HTTPRoute308RedirectStatusCode
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteCORS
    - HTTPRouteNamedRouteRule
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
    - HTTPRouteRetry
    - HTTPRouteRetryBackendTimeout
    - HTTPRouteRetryConnectionError
    - ListenerSet
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 7 test skips. Extended tests partially
    succeeded with 1 test skips.
succeededProvisionalTests:
- GatewayOptionalAddressValue

@github-actions

github-actions Bot commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

conformance test report - apisix-standalone mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-09-09T08:46:55Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
  contact:
  - https://github.com/apache/apisix-ingress-controller/issues
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    result: partial
    skippedTests:
    - TLSRouteHostnameIntersection
    - TLSRouteInvalidBackendRefNonexistent
    - TLSRouteInvalidBackendRefUnknownKind
    - TLSRouteSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 16
      Skipped: 4
  extended:
    result: partial
    skippedTests:
    - TLSRouteTerminateSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 3
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - TLSRouteModeTerminate
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
    - TLSRouteModeMixed
  name: GATEWAY-TLS
  summary: Core tests partially succeeded with 4 test skips. Extended tests partially
    succeeded with 1 test skips.
- core:
    result: partial
    skippedTests:
    - HTTPRouteHTTPSListener
    - HTTPRouteInvalidBackendRefUnknownKind
    - HTTPRouteInvalidCrossNamespaceBackendRef
    - HTTPRouteInvalidNonExistentBackendRef
    - HTTPRouteListenerHostnameMatching
    - HTTPRouteMultipleGateways
    - HTTPRouteNoBackendRefs
    statistics:
      Failed: 0
      Passed: 30
      Skipped: 7
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - BackendTLSPolicy
    - BackendTLSPolicySANValidation
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRoute303RedirectStatusCode
    - HTTPRoute307RedirectStatusCode
    - HTTPRoute308RedirectStatusCode
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteCORS
    - HTTPRouteNamedRouteRule
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
    - HTTPRouteRetry
    - HTTPRouteRetryBackendTimeout
    - HTTPRouteRetryConnectionError
    - ListenerSet
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 7 test skips. Extended tests partially
    succeeded with 1 test skips.
- core:
    result: partial
    skippedTests:
    - GRPCRouteListenerHostnameMatching
    statistics:
      Failed: 0
      Passed: 14
      Skipped: 1
  extended:
    result: success
    statistics:
      Failed: 0
      Passed: 1
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
  name: GATEWAY-GRPC
  summary: Core tests partially succeeded with 1 test skips. Extended tests succeeded.
succeededProvisionalTests:
- GatewayOptionalAddressValue

The unused linter flags recordingUpdater because nothing in this file constructs
a reconciler with an Updater. It is here for the backports in flight that do.
Suppress it with the reason spelled out, to be dropped once one of them lands.
@github-actions

github-actions Bot commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2026-09-09T09:04:43Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
  contact:
  - https://github.com/apache/apisix-ingress-controller/issues
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    failedTests:
    - GatewayModifyListeners
    - HTTPRouteMultipleGateways
    - HTTPRouteNoBackendRefs
    result: failure
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 3
      Passed: 33
      Skipped: 1
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - BackendTLSPolicy
    - BackendTLSPolicySANValidation
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRoute303RedirectStatusCode
    - HTTPRoute307RedirectStatusCode
    - HTTPRoute308RedirectStatusCode
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteCORS
    - HTTPRouteNamedRouteRule
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
    - HTTPRouteRetry
    - HTTPRouteRetryBackendTimeout
    - HTTPRouteRetryConnectionError
    - ListenerSet
  name: GATEWAY-HTTP
  summary: Core tests failed with 3 test failures. Extended tests partially succeeded
    with 1 test skips.
- core:
    failedTests:
    - GatewayModifyListeners
    result: failure
    statistics:
      Failed: 1
      Passed: 14
      Skipped: 0
  extended:
    result: success
    statistics:
      Failed: 0
      Passed: 1
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
  name: GATEWAY-GRPC
  summary: Core tests failed with 1 test failures. Extended tests succeeded.
- core:
    failedTests:
    - GatewayModifyListeners
    - TLSRouteHostnameIntersection
    - TLSRouteInvalidBackendRefNonexistent
    - TLSRouteInvalidBackendRefUnknownKind
    - TLSRouteSimpleSameNamespace
    result: failure
    statistics:
      Failed: 5
      Passed: 15
      Skipped: 0
  extended:
    failedTests:
    - TLSRouteTerminateSimpleSameNamespace
    result: failure
    statistics:
      Failed: 1
      Passed: 3
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - TLSRouteModeTerminate
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
    - TLSRouteModeMixed
  name: GATEWAY-TLS
  summary: Core tests failed with 5 test failures. Extended tests failed with 1 test
    failures.
succeededProvisionalTests:
- GatewayOptionalAddressValue

#471 merged after this branch was cut and brought its own recordingProvider in
httproute_controller_test.go, so the cherry-picked copy here was a second
declaration in the same package. Git merged both files without conflict; the
package simply stopped compiling.

Drop the copy and use the one on master. recordingUpdater goes too: nothing in
this file constructs a reconciler with an Updater, which is why it needed a
nolint in the first place, and the backport that does need it adds it beside the
provider stub instead.
@AlinsRan

Copy link
Copy Markdown
Contributor Author

@copilot Please merge the latest main branch and resolve conflicts

…umer-delete-requeue

# Conflicts:
#	internal/controller/apisixconsumer_controller_test.go

Co-authored-by: AlinsRan <79972061+AlinsRan@users.noreply.github.com>

Copilot AI commented Oct 14, 2026

Copy link
Copy Markdown

@copilot Please merge the latest main branch and resolve conflicts

Merged the latest master into this branch and resolved the test conflict, preserving both sets of coverage. Merge commit: b500639.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants