kpt-config-sync icon indicating copy to clipboard operation
kpt-config-sync copied to clipboard

Published 20 hours ago • verified publisher icon GoogleContainerTools

Add R*Sync finalizer to delete managed resources

Open karlkfi opened this issue 3 years ago • 10 comments

Add R*Sync finalizer to delete managed resources

  • Add a new custom finalizer for both RootSync & RepoSync.
  • Convert RootSync to use explicit custom finalizer, instead of delegating to the kubernetes owned resources garbage collector. This was done for consistency with new RepoSync finalizer. This has the tradeoff that resources will not be deleted automatically if the reconciler-manager is deleted first, but it does mean that the cleanup for RepoSync and RootSync are the same. Both RepoSync & RootSync will delete managed resources before allowing themselves to be deleted. This should both reduce flakey test failures and conform better to user expectations, especially when using RSync repos to manage other RSync objects, and using depends-on to order multiple R*Syncs.
  • Update Reconciling condition before and after reconciling. Add "Initialization" and "Termination" reasons for the Reconciling condition. "Deployment" reason is still used while waiting for the Deployment to reconcile.
  • Use "Replicas: 0/1" as the default Reconciling condition message when Deployment is the reason. This avoids unnecessarily updating the message when the Deployment status has not changed.
  • Use Foreground deletion while cleaning up managed objects, to ensure they are fully deleted before deleting the R*Sync.
  • Modify fakeClient to handle Foreground deletion propagation policy.
  • Default to foreground deletion in the e2e test client wrapper. This should reduce flakey test failures.
  • Add errors for invalid enums in switch cases.
  • Reduce reconciler-manager watch scope with namespaced informers.
  • Add ResourceVersion handling in fakeClient and tests
  • Add Watch handling to fakeClient
  • Use Watch to wait for objects to be deleted in the resource-manager cleanup. Fixes: b/236892981

karlkfi avatar Aug 11 '22 23:08 karlkfi

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karlkfi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

google-oss-prow[bot] avatar Aug 11 '22 23:08 google-oss-prow[bot]

added wip until rebased on https://github.com/GoogleContainerTools/kpt-config-sync/pull/25

karlkfi avatar Aug 12 '22 21:08 karlkfi

added wip until rebased on https://github.com/GoogleContainerTools/kpt-config-sync/pull/25 One of the things to potentially do is use the draft PR

mikebz avatar Aug 13 '22 18:08 mikebz

One of the things to potentially do is use the draft PR

In the cli-utils repo adding WIP causes the WIP label to be added, which blocks merging. I guess that must be a k8s bot feature tho, not a Prow feature.

karlkfi avatar Aug 13 '22 19:08 karlkfi

/hold

karlkfi avatar Aug 13 '22 19:08 karlkfi

In the cli-utils repo adding WIP causes the WIP label to be added, which blocks merging. I guess that must be a k8s bot feature tho, not a Prow feature.

Yes it seems like Kubernetes processes were trying to be git provider agnostic. I like using the tool if the tool has a feature optimized for something. This is a minor example of course, but gives folks a visual aid and also removes the ambiguity around is it [wip] [WIP] wip: wip// wip!!!

mikebz avatar Aug 16 '22 14:08 mikebz

/unhold

karlkfi avatar Aug 22 '22 23:08 karlkfi

/retest

karlkfi avatar Aug 23 '22 01:08 karlkfi

@karlkfi: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
kpt-config-sync-presubmit-e2e-multi-repo 30e10bae90e4b69b89ad817efd9afe35a7990a57 link true /test kpt-config-sync-presubmit-e2e-multi-repo

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. I understand the commands that are listed here.

google-oss-prow[bot] avatar Aug 29 '22 19:08 google-oss-prow[bot]

/hold

Extracted most of this to https://github.com/GoogleContainerTools/kpt-config-sync/pull/77

Once merged, I'll rebase this one to be just the Finalizer changes.

karlkfi avatar Sep 02 '22 02:09 karlkfi

Replaced by https://github.com/GoogleContainerTools/kpt-config-sync/pull/498

karlkfi avatar Mar 28 '23 20:03 karlkfi