Gate nodes associated with a machine based on resolved provider id

go.sum

@@ -837,8 +837,6 @@ k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280 h1:+70TFaan3hfJzs+7VK2o+O
 k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280/go.mod h1:+Axhij7bCpeqhklhUTe3xmOn6bWxolyZEeyaFpjGtl4=
 k8s.io/utils v0.0.0-20221108210102-8e77b1f39fe2 h1:GfD9OzL11kvZN5iArC6oTS7RTj7oJOIfnislxYlqTj8=
 k8s.io/utils v0.0.0-20221108210102-8e77b1f39fe2/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
-knative.dev/pkg v0.0.0-20221123154742-05b694ec4d3a h1:mTDxXL+zRBMz7BcdM3WOgw9FVbmkIN/3cvEj4MeS8zI=
-knative.dev/pkg v0.0.0-20221123154742-05b694ec4d3a/go.mod h1:fckNBPf9bu5/p1RbnOhEauX7r+kfN1zSQupEVtkaYBs=
 knative.dev/pkg v0.0.0-20230502134655-db8a35330281 h1:9mN8O5XO68DKlkzEhFAShUx+O/I+TQR71vmTvYt8oF4=
 knative.dev/pkg v0.0.0-20230502134655-db8a35330281/go.mod h1:2qWPP9Gjh9Q7ETti+WRHnBnGCSCq+6q7m3p/nmUQviE=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=

pkg/controllers/machine/termination/controller.go

@@ -72,21 +72,23 @@ func (c *Controller) Finalize(ctx context.Context, machine *v1alpha5.Machine) (r
 	if !controllerutil.ContainsFinalizer(machine, v1alpha5.TerminationFinalizer) {
 		return reconcile.Result{}, nil
 	}
-	nodes, err := machineutil.AllNodesForMachine(ctx, c.kubeClient, machine)
-	if err != nil {
-		return reconcile.Result{}, err
-	}
-	for _, node := range nodes {
-		// We delete nodes to trigger the node finalization and deletion flow
-		if err = c.kubeClient.Delete(ctx, node); client.IgnoreNotFound(err) != nil {
+	// We only delete machines that have their .status.providerID resolved so that we don't delete
+	// nodes that aren't actually properly mapped to a machine that manages it
+	if machine.Status.ProviderID != "" {
+		nodes, err := machineutil.AllNodesForMachine(ctx, c.kubeClient, machine)
+		if err != nil {
 			return reconcile.Result{}, err
 		}
-	}
-	// We wait until all the nodes associated with this machine have completed their deletion before triggering the finalization of the machine
-	if len(nodes) > 0 {
-		return reconcile.Result{}, nil
-	}
-	if machine.Status.ProviderID != "" {
+		for _, node := range nodes {
+			// We delete nodes to trigger the node finalization and deletion flow
+			if err = c.kubeClient.Delete(ctx, node); client.IgnoreNotFound(err) != nil {
+				return reconcile.Result{}, err
+			}
+		}
+		// We wait until all the nodes associated with this machine have completed their deletion before triggering the finalization of the machine
+		if len(nodes) > 0 {
+			return reconcile.Result{}, nil
+		}
 		if err := c.cloudProvider.Delete(ctx, machine); cloudprovider.IgnoreMachineNotFoundError(err) != nil {
 			return reconcile.Result{}, fmt.Errorf("terminating cloudprovider instance, %w", err)
 		}

pkg/controllers/machine/termination/suite_test.go

@@ -21,6 +21,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/samber/lo"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -187,4 +188,22 @@ var _ = Describe("Termination", func() {
 		Expect(cloudProvider.DeleteCalls).To(HaveLen(0))
 		ExpectNotFound(ctx, env.Client, machine)
 	})
+	It("should not delete nodes without provider ids if the machine hasn't been launched yet", func() {
+		// Generate 10 nodes, none of which have a provider id
+		var nodes []*v1.Node
+		for i := 0; i < 10; i++ {
+			nodes = append(nodes, test.Node())
+		}
+		ExpectApplied(ctx, env.Client, lo.Map(nodes, func(n *v1.Node, _ int) client.Object { return n })...)
+		ExpectApplied(ctx, env.Client, provisioner, machine)
+
+		// Expect the machine to be gone
+		Expect(env.Client.Delete(ctx, machine)).To(Succeed())
+		ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(machine))
+
+		ExpectNotFound(ctx, env.Client, machine)
+		for _, node := range nodes {
+			ExpectExists(ctx, env.Client, node)
+		}
+	})
 })