fix: Gate nodes associated with a machine based on resolved .status.providerID

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

@@ -86,7 +86,7 @@ func (c *Controller) Finalize(ctx context.Context, machine *v1alpha5.Machine) (r
 	if len(nodes) > 0 {
 		return reconcile.Result{}, nil
 	}
-	if machine.Status.ProviderID != "" {
+	if machine.Status.ProviderID != "" || machine.Annotations[v1alpha5.MachineLinkedAnnotationKey] != "" {
 		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"
@@ -90,6 +91,9 @@ var _ = Describe("Termination", func() {
 				Labels: map[string]string{
 					v1alpha5.ProvisionerNameLabelKey: provisioner.Name,
 				},
+				Finalizers: []string{
+					v1alpha5.TerminationFinalizer,
+				},
 			},
 			Spec: v1alpha5.MachineSpec{
 				Resources: v1alpha5.ResourceRequirements{
@@ -153,6 +157,26 @@ var _ = Describe("Termination", func() {
 		_, err = cloudProvider.Get(ctx, machine.Status.ProviderID)
 		Expect(cloudprovider.IsMachineNotFoundError(err)).To(BeTrue())
 	})
+	It("should delete the Node if the Machine is linked but doesn't have its providerID resolved yet", func() {
+		node := test.MachineLinkedNode(machine)
+
+		machine.Annotations = lo.Assign(machine.Annotations, map[string]string{v1alpha5.MachineLinkedAnnotationKey: machine.Status.ProviderID})
+		machine.Status.ProviderID = ""
+		ExpectApplied(ctx, env.Client, provisioner, machine, node)
+
+		// Expect the machine to be gone
+		Expect(env.Client.Delete(ctx, machine)).To(Succeed())
+		ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(machine)) // triggers the node deletion
+		ExpectFinalizersRemoved(ctx, env.Client, node)
+		ExpectNotFound(ctx, env.Client, node)
+
+		ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(machine)) // now all nodes are gone so machine deletion continues
+		ExpectNotFound(ctx, env.Client, machine, node)
+
+		// Expect the machine to be gone from the cloudprovider
+		_, err := cloudProvider.Get(ctx, machine.Annotations[v1alpha5.MachineLinkedAnnotationKey])
+		Expect(cloudprovider.IsMachineNotFoundError(err)).To(BeTrue())
+	})
 	It("should not delete the Machine until all the Nodes are removed", func() {
 		ExpectApplied(ctx, env.Client, provisioner, machine)
 		ExpectReconcileSucceeded(ctx, machineController, client.ObjectKeyFromObject(machine))
@@ -178,6 +202,7 @@ var _ = Describe("Termination", func() {
 		ExpectNotFound(ctx, env.Client, machine)
 	})
 	It("should not call Delete() on the CloudProvider if the machine hasn't been launched yet", func() {
+		machine.Status.ProviderID = ""
 		ExpectApplied(ctx, env.Client, provisioner, machine)
 
 		// Expect the machine to be gone
@@ -187,4 +212,23 @@ 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)
+		}
+	})
 })

pkg/utils/machine/machine.go

@@ -125,19 +125,28 @@ func NodeForMachine(ctx context.Context, c client.Client, machine *v1alpha5.Mach
 	if err != nil {
 		return nil, err
 	}
+	// If the providerID is defined, use that value; else, use the machine linked annotation if it's on the machine
+	providerID := lo.Ternary(machine.Status.ProviderID != "", machine.Status.ProviderID, machine.Annotations[v1alpha5.MachineLinkedAnnotationKey])
 	if len(nodes) > 1 {
-		return nil, &DuplicateNodeError{ProviderID: machine.Status.ProviderID}
+		return nil, &DuplicateNodeError{ProviderID: providerID}
 	}
 	if len(nodes) == 0 {
-		return nil, &NodeNotFoundError{ProviderID: machine.Status.ProviderID}
+		return nil, &NodeNotFoundError{ProviderID: providerID}
 	}
 	return nodes[0], nil
 }
 
 // AllNodesForMachine is a helper function that takes a v1alpha5.Machine and finds ALL matching v1.Nodes by their providerID
+// If the providerID is not resolved for a Machine, then no Nodes will map to it
 func AllNodesForMachine(ctx context.Context, c client.Client, machine *v1alpha5.Machine) ([]*v1.Node, error) {
+	// If the providerID is defined, use that value; else, use the machine linked annotation if it's on the machine
+	providerID := lo.Ternary(machine.Status.ProviderID != "", machine.Status.ProviderID, machine.Annotations[v1alpha5.MachineLinkedAnnotationKey])
+	// Machines that have no resolved providerID have no nodes mapped to them
+	if providerID == "" {
+		return nil, nil
+	}
 	nodeList := v1.NodeList{}
-	if err := c.List(ctx, &nodeList, client.MatchingFields{"spec.providerID": machine.Status.ProviderID}); err != nil {
+	if err := c.List(ctx, &nodeList, client.MatchingFields{"spec.providerID": providerID}); err != nil {
 		return nil, fmt.Errorf("listing nodes, %w", err)
 	}
 	return lo.ToSlicePtr(nodeList.Items), nil