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

fix(ack): controllers only skip isUpToDate on deletion #1979

Conversation

wotolom
Copy link
Contributor

@wotolom wotolom commented Dec 15, 2023

Description of your changes

  • adapts the ACK controller template to only skip isUpToDate() on resource deletion (and not also postObserve())
  • for docdb.Cluster
    • adapt unit-test
    • in postObserve() add if meta.WasDeleted(cr) {...} around secretRef accessing to avoid MR deletion stuck on missing secret
  • also cleaned-up the no longer needed if meta.WasDeleted(cr) {return} in some isUpToDate()

Fixes #1978

I have:

  • Read and followed Crossplane's [contribution process].
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

unit-tests, manually (secretsmanager.Secret + docdb.Cluster)

Signed-off-by: Charel Baum (external expert on behalf of DB Netz AG) <charel.baum-extern@deutschebahn.com>
Copy link
Collaborator

@MisterMX MisterMX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you very much @wotolom for pointing out this issue and for providing the fix!

@MisterMX MisterMX merged commit 51d618c into crossplane-contrib:master Dec 18, 2023
9 checks passed
@wotolom wotolom deleted the ack-controller-template-do-not-skip-postobserve-on-deletion branch December 18, 2023 10:27
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.

Secret MR does not get deleted directly anymore
2 participants