Merge pull request #112184 from danwinship/kubelet-node-ip-annotation-cleanup

Delete the cloud node IP annotation if it is stale

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 {
-				if node.ObjectMeta.Annotations == nil {
+				node.ObjectMeta.Annotations = make(map[string]string)
-					node.ObjectMeta.Annotations = make(map[string]string)
-				}
-				node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr] = nodeIP.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 {

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)},
+				ObjectMeta: metav1.ObjectMeta{
-				Spec:       v1.NodeSpec{},
+					Name:        testKubeletHostname,
+					Annotations: testCase.existingAnnotations,
+				},
+				Spec: v1.NodeSpec{},
 				Status: v1.NodeStatus{
 					Addresses: []v1.NodeAddress{},
 				},