Repository: kubermatic/machine-controller
Pull request #816 (2020-09-01 15:51:55)

Ignore Node when deleting by NodeRef if it doesn't exist anymore
**What this PR does / why we need it**:

There's a regression introduced in #813, which prevents the Machine object from being deleted if the Node referred by NodeRef is deleted.

**Optional Release Note**:
```release-note
Ignore Node when deleting by NodeRef if it doesn't exist anymore
```


kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/816#" title="Author self-approved">xmudrii</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubermatic/machine-controller/blob/master/OWNERS)~~ [xmudrii]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
kubermatic-bot: LGTM label has been added.  <details>Git tree hash: de4c9591c7de5ec21d58055dca25dc832e0f8a3c</details>
kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/816#" title="Author self-approved">xmudrii</a>*, *<a href="https://github.com/kubermatic/machine-controller/pull/816#pullrequestreview-479911077" title="Approved">xrstf</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubermatic/machine-controller/blob/master/OWNERS)~~ [xmudrii,xrstf]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/controller/machine/machine.go b/pkg/controller/machine/machine.go
index 25cdeecb..ac6dc701 100644
--- a/pkg/controller/machine/machine.go
+++ b/pkg/controller/machine/machine.go
@@ -614,16 +614,23 @@ func (r *Reconciler) deleteNodeForMachine(machine *clusterv1alpha1.Machine) erro
 	if machine.Status.NodeRef != nil {
 		objKey := ctrlruntimeclient.ObjectKey{Name: machine.Status.NodeRef.Name}
 		node := &corev1.Node{}
+		nodeFound := true
 		if err := r.client.Get(r.ctx, objKey, node); err != nil {
-			return fmt.Errorf("failed to get node %s: %v", machine.Status.NodeRef.Name, err)
-		}
-
-		if err := r.client.Delete(r.ctx, node); err != nil {
 			if !kerrors.IsNotFound(err) {
-				return err
+				return fmt.Errorf("failed to get node %s: %v", machine.Status.NodeRef.Name, err)
 			}
+			nodeFound = false
 			klog.V(2).Infof("node %q does not longer exist for machine %q", machine.Status.NodeRef.Name, machine.Spec.Name)
 		}
+
+		if nodeFound {
+			if err := r.client.Delete(r.ctx, node); err != nil {
+				if !kerrors.IsNotFound(err) {
+					return err
+				}
+				klog.V(2).Infof("node %q does not longer exist for machine %q", machine.Status.NodeRef.Name, machine.Spec.Name)
+			}
+		}
 	} else {
 		selector, err := labels.Parse(NodeOwnerLabelName + "=" + string(machine.UID))
 		if err != nil {


Test patch:
diff --git a/pkg/controller/machine/machine_test.go b/pkg/controller/machine/machine_test.go
index 24ba69cd..799f8d98 100644
--- a/pkg/controller/machine/machine_test.go
+++ b/pkg/controller/machine/machine_test.go
@@ -548,6 +548,25 @@ func TestControllerDeleteNodeForMachine(t *testing.T) {
 			err:              nil,
 			shouldDeleteNode: "",
 		},
+		{
+			name: "node referred by nodeRef doesn't exist",
+			machine: &clusterv1alpha1.Machine{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "machine-1",
+					Finalizers: []string{"machine-node-delete-finalizer"},
+				},
+				Status: clusterv1alpha1.MachineStatus{
+					NodeRef: &corev1.ObjectReference{Name: "node-1"},
+				},
+			},
+			nodes: []runtime.Object{&corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "node-0",
+				}},
+			},
+			err:              nil,
+			shouldDeleteNode: "",
+		},
 	}
 
 	for _, test := range tests {