✨ Implement deletion lifecycle hooks #3273
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/M
michaelgugino
force-pushed
the
machine-deletion-hooks-impl
branch
2 times, most recently
from
fcd4bc6 to
3d1d833
Compare
api/v1alpha3/machine_types.go
Outdated
|
|
||
| // PreDrainDeleteHookAnnotionPrefix annotation specifies the prefix we | ||
| // search each annotation for during the pre-drain.delete lifecycle hook | ||
| // to pause reconciliation. |
There was a problem hiding this comment.
| // to pause reconciliation. | |
| // to pause deletion. |
to avoid confusion with the pause reconciliation at cluster/object level
There was a problem hiding this comment.
We are in effect pausing reconciliation, not just deletion. Returning empty, nil results in the object not being reconciled further. Makes more sense to me anyway :)
|
+1 to wait for the proposal to merge, I know I haven't had any time to review it yet given. @ncdc also had some comments on the hooking design. |
Pretty much what I assumed to be the case. But I wanted to get the PR up so people can see what the implementation might look like. From the machine-controller perspective, this implementation is quite trivial. |
michaelgugino
force-pushed
the
machine-deletion-hooks-impl
branch
from
3d1d833 to
d5646a9
Compare
michaelgugino
force-pushed
the
machine-deletion-hooks-impl
branch
from
d5646a9 to
7f720c7
Compare
controllers/machine_controller.go
Outdated
| @@ -301,6 +307,12 @@ func (r *MachineReconciler) reconcileDelete(ctx context.Context, cluster *cluste | |||
| } | |||
| } | |||
|
|
|||
| // pre-term.delete lifecycle hook | |||
| // Return early without error, will requeue if/when the Hook is removed. | |||
There was a problem hiding this comment.
| // Return early without error, will requeue if/when the Hook is removed. | |
| // Return early without error, will requeue if/when the hook owner removes the annotation. |
michaelgugino
force-pushed
the
machine-deletion-hooks-impl
branch
from
7f720c7 to
a53412e
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
I haven't looked at the unit tests in a while - is it possible to add/update the tests to make sure hooks are handled appropriately? |
|
I'm not sure how easy it will be... if @michaelgugino takes a look and it doesn't seem possible to do it easily/quickly, I think I'm fine merging this as-is and following up with unit tests later if/when we are able to do a more comprehensive set around deleted Machines. |
|
/milestone v0.3.9 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vincepri 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 |
k8s-ci-robot
added
the
approved
What this PR does / why we need it:
Implements Deletion Lifecycle Hooks: #3132