🐛 Machine Controller should try to retrieve node on delete #11032
Conversation
k8s-ci-robot
added
area/machine
|
Looking into tests if the change makes sense |
Consider this scenario: * Machine is created * InfraMachine is created * MachineHealthCheck is set to 10 minutes * [10 minutes pass] * Machine is marked as needing remediation * Machine and InfraMachine goes into deletion flow * Node finally joins the cluster, say 10 minutes + few seconds * InfraMachine is still waiting for VM to be deleted * Machine keeps retrying to delete, but `nodeRef` was never set * Machine, InfraMachine go away (finalizer removed) * Node object sticks around in the cluster This changset allows the Machine controller to look into the cluster during deletion flow if the Node eventually got created before the infrastructure provider was able to fully delete the machine. Signed-off-by: Vince Prignano <[email protected]>
vincepri
force-pushed
the
machine-node-deletion-fallback
branch
from
f6d09ad
to
9517bba
Compare
| g.Expect(err).ToNot(HaveOccurred()) | ||
| if tc.expectNodeDeletion { | ||
| n := &corev1.Node{} | ||
| g.Expect(fakeClient.Get(context.Background(), client.ObjectKeyFromObject(node), n)).NotTo(Succeed()) |
There was a problem hiding this comment.
| g.Expect(fakeClient.Get(context.Background(), client.ObjectKeyFromObject(node), n)).NotTo(Succeed()) | |
| g.Expect(apierrors.IsNotFound(fakeClient.Get(context.Background(), client.ObjectKeyFromObject(node), n))).To(BeTrue()) |
nit
| } | ||
|
|
||
| cluster := testCluster.DeepCopy() | ||
| if tc.clusterDeleted { |
| Name: "test", | ||
| Namespace: metav1.NamespaceDefault, | ||
| Labels: map[string]string{ | ||
| clusterv1.MachineControlPlaneLabel: "", |
There was a problem hiding this comment.
Should also have
clusterv1.ClusterNameLabel: "test-cluster",
(just noticed while diff'ing to figure out what the difference between the two Machines is)
|
/lgtm |
|
/cherry-pick release-1.8 |
|
/cherry-pick release-1.7 |
|
@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.8 in a new PR and assign it to you. In response to this:
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-sigs/prow repository. |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 0d575840e42e4eb1686a6d581cfbaf1ea0996557
|
|
@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.7 in a new PR and assign it to you. In response to this:
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-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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
|
@sbueringer: new pull request created: #11042 In response to this:
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-sigs/prow repository. |
|
@sbueringer: new pull request created: #11043 In response to this:
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-sigs/prow repository. |
Consider this scenario:
nodeRefwas never setThis changset allows the Machine controller to look into the cluster during deletion flow if the Node eventually got created before the infrastructure provider was able to fully delete the machine.
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
/area machine