Skip to content

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

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

fix: stop requeueing ApisixConsumer forever after a successful delete (backport apache/apisix-ingress-controller#2850)#472
AlinsRan wants to merge 6 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 3 committers have signed the CLA.

✅ AlinsRan
❌ johannes-engler-mw
❌ alliasgher
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 Change StackReview 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: 0de4e883-fbff-41cd-8fd4-f56770c7896a

📥 Commits

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

📒 Files selected for processing (1)
  • internal/controller/apisixconsumer_controller_test.go

Included review availability: 2 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.


📝 Walkthrough

Walkthrough

The controller now logs successful provider deletion for missing ApisixConsumer objects. Tests cover successful deletion, provider delete failures, 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. Tests verify that a missing consumer is deleted from the provider without requeueing.
Deletion error handling
internal/controller/apisixconsumer_controller_test.go
Tests verify that provider deletion errors are returned and non-NotFound client errors prevent provider deletion.

Priority: ⬇️ Low

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

Merge Risk: ⚪ Minimal · up to f7ea9

The change adds logging and test coverage for successful ApisixConsumer deletion handling without introducing an identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title describes a requeue behavior fix, but the changes add a deletion log entry and tests for behavior that already exists. It does not accurately describe the main changeset. Update the title to describe the added deletion log and reconciliation tests, or include the requeue logic change in this pull request.
E2e Test Quality Review ⚠️ Warning Blocking: the new ApisixConsumer coverage is unit-only. internal/controller/apisixconsumer_controller_test.go uses a fake Kubernetes client, an interceptor, and recordingProvider; it does not ex… Add an E2E ApisixConsumer scenario that creates the resource and dependent route, deletes the consumer through Kubernetes, verifies the external provider/data-plane state, and checks the controller completes without repeated reconcile fai…
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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 PASS. The production change logs only utils.NamespacedName(ac) at internal/controller/apisixconsumer_controller.go:81; that helper emits only namespace and name, not the ApisixConsumer spec or i…
Full details: E2e Test Quality Review

Explanation

Blocking: the new ApisixConsumer coverage is unit-only. internal/controller/apisixconsumer_controller_test.go uses a fake Kubernetes client, an interceptor, and recordingProvider; it does not exercise the Kubernetes-to-controller-to-provider/APISIX flow. The only E2E file changed by this branch is for HTTPRoute, while the existing test/e2e/crds/v2/consumer.go coverage is unchanged and does not verify the new no-requeue path. The PR range also contains unrelated #471 HTTPRoute/provider changes across 12 other files, which violates the scope criterion. A stale comment claims recordingUpdater is shared, but no such declaration exists.

Resolution

Add an E2E ApisixConsumer scenario that creates the resource and dependent route, deletes the consumer through Kubernetes, verifies the external provider/data-plane state, and checks the controller completes without repeated reconcile failures or requeues. Keep the unit tests for provider and Get error handling. Remove the unrelated #471 commits/files from this PR or split them into a separate PR. Correct the shared-test-double comment to mention only recordingProvider.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch backport/apisixconsumer-delete-requeue

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.
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.

4 participants