fix: Handle providerID changing in cluster state

pkg/apis/v1alpha5/labels.go

@@ -56,12 +56,6 @@ const (
 	TerminationFinalizer = Group + "/termination"
 )
 
-// Tags for infrastructure resources deployed into Cloud Provider's accounts
-const (
-	DiscoveryTagKey = Group + "/discovery"
-	ManagedByTagKey = Group + "/managed-by"
-)
-
 var (
 	// RestrictedLabelDomains are either prohibited by the kubelet or reserved by karpenter
 	RestrictedLabelDomains = sets.NewString(

pkg/controllers/state/cluster.go

@@ -165,13 +165,17 @@ func (c *Cluster) UpdateNode(ctx context.Context, node *v1.Node) error {
 	return nil
 }
 
-func (c *Cluster) DeleteNode(nodeName string) {
+func (c *Cluster) DeleteNode(name string) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	if id := c.nameToProviderID[nodeName]; id != "" {
+	c.cleanupNode(name)
+}
+
+func (c *Cluster) cleanupNode(name string) {
+	if id, ok := c.nameToProviderID[name]; ok {
 		delete(c.nodes, id)
-		delete(c.nameToProviderID, nodeName)
+		delete(c.nameToProviderID, name)
 		c.RecordConsolidationChange()
 	}
 }
@@ -250,6 +254,12 @@ func (c *Cluster) newStateFromNode(ctx context.Context, node *v1.Node, oldNode *
 	); err != nil {
 		return nil, err
 	}
+	// Cleanup the old node with its old providerID if its providerID changes
+	// This can happen since nodes don't get created with providerIDs. Rather, CCM picks up the
+	// created node and injects the providerID into the spec.providerID
+	if id, ok := c.nameToProviderID[node.Name]; ok && id != node.Spec.ProviderID {
+		c.cleanupNode(node.Name)
+	}
 	c.triggerConsolidationOnChange(oldNode, n)
 	return n, nil
 }

pkg/controllers/state/suite_test.go

@@ -612,6 +612,22 @@ var _ = Describe("Node Resource Level", func() {
 		time.Sleep(time.Second * 6) // past 10s, node should no longer be nominated
 		Expect(ExpectStateNodeExists(node).Nominated()).To(BeFalse())
 	})
+	It("should handle a node changing from no providerID to registering a providerID", func() {
+		node := test.Node()
+		ExpectApplied(ctx, env.Client, node)
+		ExpectReconcileSucceeded(ctx, nodeController, client.ObjectKeyFromObject(node))
+
+		ExpectStateNodeCount("==", 1)
+		ExpectStateNodeExists(node)
+
+		// Change the providerID; this mocks CCM adding the providerID onto the node after registration
+		node.Spec.ProviderID = fmt.Sprintf("fake://%s", node.Name)
+		ExpectApplied(ctx, env.Client, node)
+		ExpectReconcileSucceeded(ctx, nodeController, client.ObjectKeyFromObject(node))
+
+		ExpectStateNodeCount("==", 1)
+		ExpectStateNodeExists(node)
+	})
 })
 
 var _ = Describe("Pod Anti-Affinity", func() {
@@ -802,6 +818,16 @@ var _ = Describe("Provisioner Spec Updates", func() {
 	})
 })
 
+func ExpectStateNodeCount(comparator string, count int) int {
+	c := 0
+	cluster.ForEachNode(func(n *state.Node) bool {
+		c++
+		return true
+	})
+	ExpectWithOffset(1, count).To(BeNumerically(comparator, count))
+	return c
+}
+
 func ExpectStateNodeExistsWithOffset(offset int, node *v1.Node) *state.Node {
 	var ret *state.Node
 	cluster.ForEachNode(func(n *state.Node) bool {