Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Controller create/update handling called even if exception when applying finalizer #310

Closed
ppatierno opened this issue Jan 22, 2021 · 2 comments · Fixed by #321
Closed
Assignees

Comments

@ppatierno
Copy link
Contributor

ppatierno commented Jan 22, 2021

Due to a different problem that I have to figure out, I noticed the following possibly wrong behavior of the sdk ...
I apply the custom resource handled by my controller and the handleDispatch reaches the point where a finalizer is applied to the resource itself. At that point an exception happens (in my case a 404 NOT FOUND) and the handleExecution of the dispatcher is executed and the execution ends (without calling the createOrUpdateResource of my controller which is ok).
AFAIK the operator sdk has a retry logic so the event is handled again but at this point in the handleCreateOrUpdate the ControllerUtils.hasGivenFinalizer return true because the resource (is it in a cache of the operator sdk?) has the finalizer in the metadata even if the corresponding apply in the previous step was failed against the Kubernetes cluster.
Due to the fact that the finalizer is there, the operator sdk goes ahead handling the event calling the createOrUpdateResource of the controller.
I guess it's wrong, because applying the finalizer didn't end well actually due to a problem that could be of any type and could happen even in the controller.

@csviri
Copy link
Collaborator

csviri commented Jan 23, 2021

@ppatierno @metacosm I will take a look into this too. To be honest did not had this on mind when implementing this part, thus the scenario when adding a finalizer would fail.

@ppatierno
Copy link
Contributor Author

@csviri thanks! My PR works but not sure if this is the best solution as a newbie on this sdk.

@csviri csviri linked a pull request Jan 25, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment