From e23f1a68affdee29c1580d8b02596c7372814e85 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 1 Sep 2022 14:59:52 -0400 Subject: [PATCH] Delete the cloud node IP annotation if it is stale If you run "kubelet --cloud-provider X --node-ip Y", kubelet will set an annotation on the node, but previously, if you then ran just "kubelet --cloud-provider X" (or just "kubelet --node-ip Y"), it wouldn't delete the stale annotation. Fix that. --- pkg/kubelet/nodestatus/setters.go | 14 +++--- pkg/kubelet/nodestatus/setters_test.go | 64 +++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/pkg/kubelet/nodestatus/setters.go b/pkg/kubelet/nodestatus/setters.go index a205809846f..8a2ecaa6248 100644 --- a/pkg/kubelet/nodestatus/setters.go +++ b/pkg/kubelet/nodestatus/setters.go @@ -92,7 +92,7 @@ func NodeAddress(nodeIPs []net.IP, // typically Kubelet.nodeIPs klog.V(4).InfoS("Using secondary node IP", "IP", secondaryNodeIP.String()) } - if externalCloudProvider || cloud != nil { + if (externalCloudProvider || cloud != nil) && nodeIPSpecified { // Annotate the Node object with nodeIP for external cloud provider. // // We do this even when external CCM is not configured to cover a situation @@ -105,12 +105,14 @@ func NodeAddress(nodeIPs []net.IP, // typically Kubelet.nodeIPs // We do not add the annotation in the case where there is no cloud // controller at all, as we don't expect to migrate these clusters to use an // external CCM. - if nodeIPSpecified { - if node.ObjectMeta.Annotations == nil { - node.ObjectMeta.Annotations = make(map[string]string) - } - node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr] = nodeIP.String() + if node.ObjectMeta.Annotations == nil { + node.ObjectMeta.Annotations = make(map[string]string) } + node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr] = nodeIP.String() + } else if node.ObjectMeta.Annotations != nil { + // Clean up stale annotations if no longer using a cloud provider or + // no longer overriding node IP. + delete(node.ObjectMeta.Annotations, cloudproviderapi.AnnotationAlphaProvidedIPAddr) } if externalCloudProvider { diff --git a/pkg/kubelet/nodestatus/setters_test.go b/pkg/kubelet/nodestatus/setters_test.go index 4ada6c1aa46..007e4b9385e 100644 --- a/pkg/kubelet/nodestatus/setters_test.go +++ b/pkg/kubelet/nodestatus/setters_test.go @@ -69,6 +69,7 @@ func TestNodeAddress(t *testing.T) { cloudProviderType cloudProviderType nodeAddresses []v1.NodeAddress expectedAddresses []v1.NodeAddress + existingAnnotations map[string]string expectedAnnotations map[string]string shouldError bool }{ @@ -452,13 +453,72 @@ func TestNodeAddress(t *testing.T) { expectedAnnotations: map[string]string{}, shouldError: false, }, + { + name: "Stale nodeIP annotation is removed when not using cloud provider", + nodeIP: netutils.ParseIPSloppy("10.1.1.1"), + cloudProviderType: cloudProviderNone, + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + existingAnnotations: map[string]string{ + "alpha.kubernetes.io/provided-node-ip": "10.1.1.3", + }, + expectedAnnotations: map[string]string{}, + shouldError: false, + }, + { + name: "Stale nodeIP annotation is removed when using cloud provider but no --node-ip", + nodeIP: nil, + cloudProviderType: cloudProviderLegacy, + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + existingAnnotations: map[string]string{ + "alpha.kubernetes.io/provided-node-ip": "10.1.1.1", + }, + expectedAnnotations: map[string]string{}, + shouldError: false, + }, + { + name: "Incorrect nodeIP annotation is fixed", + nodeIP: netutils.ParseIPSloppy("10.1.1.1"), + cloudProviderType: cloudProviderExternal, + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + existingAnnotations: map[string]string{ + "alpha.kubernetes.io/provided-node-ip": "10.1.1.3", + }, + expectedAnnotations: map[string]string{ + "alpha.kubernetes.io/provided-node-ip": "10.1.1.1", + }, + shouldError: false, + }, } for _, testCase := range cases { t.Run(testCase.name, func(t *testing.T) { // testCase setup existingNode := &v1.Node{ - ObjectMeta: metav1.ObjectMeta{Name: testKubeletHostname, Annotations: make(map[string]string)}, - Spec: v1.NodeSpec{}, + ObjectMeta: metav1.ObjectMeta{ + Name: testKubeletHostname, + Annotations: testCase.existingAnnotations, + }, + Spec: v1.NodeSpec{}, Status: v1.NodeStatus{ Addresses: []v1.NodeAddress{}, },