[OCPCLOUD-1350] Implement machine pre-drain and pre-terminate deletion hooks #945
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
JoelSpeed
force-pushed
the
machine-deletion-hooks
branch
from
47fd6e9 to
c23a2e7
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexander-demichev 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 |
openshift-ci
bot
added
the
approved
JoelSpeed
force-pushed
the
machine-deletion-hooks
branch
2 times, most recently
from
e6f4449 to
ce1a6cc
Compare
| @@ -39,6 +40,8 @@ import ( | |||
| "sigs.k8s.io/controller-runtime/pkg/envtest" | |||
| ) | |||
|
|
|||
| const alphabet = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" | |||
There was a problem hiding this comment.
just out of curiosity, are we limited by an English alphabet here?
There was a problem hiding this comment.
The validation on the field I had down as [a-zA-Z]+, do you think that is too restrictive? The proposal suggests a CamelCase string and something easily computer readable, hence sticking to basic ascii.
pkg/webhooks/machine_webhook_test.go
Outdated
| }, | ||
| updateAfterDelete: true, | ||
| updateMachine: func(m *machinev1.Machine) { | ||
| m.Annotations = map[string]string{} |
There was a problem hiding this comment.
does this update remove the lifecycle hook?
JoelSpeed
force-pushed
the
machine-deletion-hooks
branch
3 times, most recently
from
3b945d4 to
5428012
Compare
JoelSpeed
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
JoelSpeed
force-pushed
the
machine-deletion-hooks
branch
from
5428012 to
73cbce6
Compare
JoelSpeed
force-pushed
the
machine-deletion-hooks
branch
from
73cbce6 to
45c1e52
Compare
|
/lgtm |
|
@JoelSpeed: The following tests failed, say
Full PR test history. Your PR dashboard. 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. |
This PR implements the feature part of openshift/enhancements#862.
This does not include the conditions part of the enhancement which will follow up in a later PR.
This will ensure that when Machine Deletion Hooks are in play, that the machine controller behaves as described in the enhancement by either blocking the drain or blocking the termination of the Machine.
Something I realised while implementing this is that we should only honour the pre-drain hooks when we are actually attempting a drain. There are certain scenarios where pre-drain hooks wouldn't make sense, eg, when a node doesn't actually exist. So that is included within this PR and can be seen in the unit tests included.
This is blocked on the API PR being merged and the api dependency commit being updated