diff --git a/pkg/controllers/state/cluster.go b/pkg/controllers/state/cluster.go index 45e91b72bb..9d6da19562 100644 --- a/pkg/controllers/state/cluster.go +++ b/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 := c.nameToProviderID[name]; id != "" { 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 } diff --git a/pkg/controllers/state/suite_test.go b/pkg/controllers/state/suite_test.go index ea6949be07..bcc23df9fd 100644 --- a/pkg/controllers/state/suite_test.go +++ b/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 {