Trigger node deletion when machine is deleted#1102
Conversation
gardener-prow
Bot
added
do-not-merge/needs-kind
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
gardener-prow
Bot
added
cla: no
gardener-prow
Bot
removed
the
do-not-merge/needs-kind
thiyyakat
left a comment
thiyyakat
left a comment
There was a problem hiding this comment.
Thanks for the changes. Just two comments. PTAL.
| if c.targetCoreClient != nil { | ||
| if nodeName := machine.Labels[v1alpha1.NodeLabelKey]; nodeName != "" { | ||
| if err := c.targetCoreClient.CoreV1().Nodes().Delete(context.Background(), nodeName, metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) { | ||
| klog.Errorf("Machine %q: failed to delete node %q: %v", machine.Name, nodeName, err) |
There was a problem hiding this comment.
Could you please rephrase this? Maybe: "failed to delete backing node %q of machine %q queued for deletion"? Please start error message with lowercase when possible.
| machine, err := c.getMachineFromNode(node.Name) | ||
| if err != nil { | ||
| if errors.Is(err, errNoMachineMatch) { | ||
| if node.DeletionTimestamp != nil { |
There was a problem hiding this comment.
Could you please update the docstring above since we are no longer ignoring such updates.
3 participants
What this PR does / why we need it:
"Creating machine on cloud provider"to"Creating machine on cloud provider. Waiting for node object to register", so the status accurately reflects which stage the machine is in.Which issue(s) this PR fixes:
Fixes #1044, #1064
Release note: