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

Ensure finalizers are re-reconciled in CAPV #2357

Closed
killianmuldoon opened this issue Sep 14, 2023 · 10 comments
Closed

Ensure finalizers are re-reconciled in CAPV #2357

killianmuldoon opened this issue Sep 14, 2023 · 10 comments
Assignees
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@killianmuldoon
Copy link
Contributor

killianmuldoon commented Sep 14, 2023

This is a tracking issue for similar work in upstream Cluster API

CAPV should add tests to ensure that finalizers added to objects can be correctly re-added if they are incorrectly removed. The desired state of an object without a deletionTimestamp is that it has the correct finalizers in place.

This can be tested in a similar way to the OwnerReference end-to-end tests. The test should remove all finalizers and then force objects to re-reconcile. The objects should have the same finalizers in place after reconciliation as before.

An early prototype of this test in upstream CAPI is #7614 (files), but the underlying code has moved since the PoC.

Note: it might be better to make this change in CAPI first so we can import utils from upstream.

@adityabhatia
Copy link
Contributor

/assign

@adityabhatia
Copy link
Contributor

adityabhatia commented Oct 5, 2023

Depends on kubernetes-sigs/cluster-api#9471 and thereafter updating CAPI dependencies in CAPV, to the next upcoming release.

@killianmuldoon
Copy link
Contributor Author

We should backport those test utils so we can get this in CAPI v1.5.3.

@killianmuldoon
Copy link
Contributor Author

@adityabhatia - this should be unblocked in #2463

@sbueringer
Copy link
Member

@adityabhatia Just in case you had time to work on it, any updates? :)

@adityabhatia
Copy link
Contributor

@adityabhatia Just in case you had time to work on it, any updates? :)

Thanks, this got lost between the years :) This is already worked upon, but it would require some changes to the CAPI framework upstream, captured in this feature request and implemented here

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 8, 2024
@chrischdi
Copy link
Member

@sbueringer / @adityabhatia : can we close this since the following PR was merged:

@adityabhatia
Copy link
Contributor

This issue should have been closed with the PR, but it was tagged incorrectly. 😄

/close

@k8s-ci-robot
Copy link
Contributor

@adityabhatia: Closing this issue.

In response to this:

This issue should have been closed with the PR, but it was tagged incorrectly. 😄

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

6 participants