fix: stop requeueing ApisixConsumer forever after a successful delete - #2850
Open
alliasgher wants to merge 1 commit into
Open
fix: stop requeueing ApisixConsumer forever after a successful delete#2850alliasgher wants to merge 1 commit into
alliasgher wants to merge 1 commit into
Conversation
When the ApisixConsumer is gone, Reconcile synthesizes a stub, calls Provider.Delete and then falls out of the IsNotFound block without returning. Execution reaches the trailing return, which hands back the captured NotFound error, so controller-runtime treats the reconcile as failed and requeues it with exponential backoff even though the delete succeeded. Return after the successful delete, matching every other reconciler. Fixes apache#2849
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of change:
What this PR does / why we need it:
Fixes #2849
When an
ApisixConsumeris deleted,Reconcilegets a NotFound from the cache, synthesizes a minimal object and callsProvider.Delete. On the success path there is no return, so execution falls out of theIsNotFoundblock to the trailingreturn ctrl.Result{}, err, which hands back the captured NotFound error. controller-runtime sees a non-nil error, so it requeues the key indefinitely with exponential backoff even though the delete already succeeded. That matches the increasing intervals in the report.The fix is to return after a successful delete, which is what every other reconciler already does.
The issue asks whether other controllers have the same problem. I checked all of them: there are 15
Provider.Deletecall sites across 13 files ininternal/controller, and every one is followed byreturn ctrl.Result{}, nilexceptapisixconsumer_controller.go. So this is the only affected reconciler and no wider change is needed.Pre-submission checklist:
Tests
internal/controller/apisixconsumer_controller_test.goadds three cases:TestApisixConsumerReconcile_DeletedObjectDoesNotRequeueis the regression guard. It asserts the reconcile succeeds, returns an emptyctrl.Result, and that the provider received the right key. With only the two production lines reverted it fails withapisixconsumers.apisix.apache.org "gone" not found, so it is not vacuous.TestApisixConsumerReconcile_DeleteErrorIsReturnedproves a provider failure still propagates rather than being swallowed.TestApisixConsumerReconcile_NonNotFoundGetErrorIsReturnedusesinterceptor.Funcsto return an internal error fromGetand asserts it is returned and does not trigger a spurious provider delete.Verified locally:
go build ./...cleango test $(go list ./... | grep -v '/test/') -count=1all passgo vet ./internal/controller/andgofmt -lcleangolangci-lint run ./internal/controller/...reports no issues in the changed filesI did not run the envtest-backed
make testor the e2e and conformance suites. This change touches no CRD, manifest, or API surface.