From 3ee2f27e5bb301bf70b71f928dbb4dcbd3fc5f2e Mon Sep 17 00:00:00 2001
From: Antonio Ojea <aojea@google.com>
Date: Fri, 6 Oct 2023 14:01:28 +0000
Subject: [PATCH] kubelet: cloud-provider external addresses

Kubelet, if using cloud provider external, initializes temporary
the node addresses using the non-cloud provider logic, until the
cloud provider overrides it.

This behavior has undesired consequences if the cloud-provider addresses
are different than the original ones, specially for hostNetwork pods,
that inherit these addresses from the Node.

Since some cloud-providers depend on this behavior, in order to keep
backward compatibility, assume that the specifying addresses via
the node-ip flags means that the intent is to keep the existing
behavior to temporary initialize the addresses.

If the node-ips are the unspecified addresses or are not set, then
wait for the external cloud provider to set the node addresses.

Change-Id: I3a3895f9b830769f9658e6a03f058c914c438a09
Signed-off-by: Antonio Ojea <aojea@google.com>
---
 pkg/kubelet/nodestatus/setters.go      | 10 +++++++++-
 pkg/kubelet/nodestatus/setters_test.go | 17 ++++++++++++++++-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/pkg/kubelet/nodestatus/setters.go b/pkg/kubelet/nodestatus/setters.go
index bf3f6e05a29..8515e0a7bef 100644
--- a/pkg/kubelet/nodestatus/setters.go
+++ b/pkg/kubelet/nodestatus/setters.go
@@ -123,11 +123,19 @@ func NodeAddress(nodeIPs []net.IP, // typically Kubelet.nodeIPs
 		if externalCloudProvider {
 			// If --cloud-provider=external and node address is already set,
 			// then we return early because provider set addresses should take precedence.
-			// Otherwise, we try to look up the node IP and let the cloud provider override it later
+			// Otherwise, we try to use the node IP defined via flags and let the cloud provider override it later
 			// This should alleviate a lot of the bootstrapping issues with out-of-tree providers
 			if len(node.Status.Addresses) > 0 {
 				return nil
 			}
+			// If nodeIPs are not specified wait for the external cloud-provider to set the node addresses.
+			// Otherwise uses them on the assumption that the installer/administrator has the previous knowledge
+			// required to ensure the external cloud provider will use the same addresses to avoid the issues explained
+			// in https://github.com/kubernetes/kubernetes/issues/120720.
+			// We are already hinting the external cloud provider via the annotation AnnotationAlphaProvidedIPAddr.
+			if !nodeIPSpecified {
+				return nil
+			}
 		}
 		if cloud != nil {
 			cloudNodeAddresses, err := nodeAddressesFunc()
diff --git a/pkg/kubelet/nodestatus/setters_test.go b/pkg/kubelet/nodestatus/setters_test.go
index 3e2aef43e0a..28033f1c787 100644
--- a/pkg/kubelet/nodestatus/setters_test.go
+++ b/pkg/kubelet/nodestatus/setters_test.go
@@ -223,7 +223,7 @@ func TestNodeAddress(t *testing.T) {
 			shouldError:      false,
 		},
 		{
-			name:              "cloud provider is external",
+			name:              "cloud provider is external and nodeIP specified",
 			nodeIP:            netutils.ParseIPSloppy("10.0.0.1"),
 			nodeAddresses:     []v1.NodeAddress{},
 			cloudProviderType: cloudProviderExternal,
@@ -233,6 +233,21 @@ func TestNodeAddress(t *testing.T) {
 			},
 			shouldError: false,
 		},
+		{
+			name:              "cloud provider is external and nodeIP unspecified",
+			nodeIP:            netutils.ParseIPSloppy("::"),
+			nodeAddresses:     []v1.NodeAddress{},
+			cloudProviderType: cloudProviderExternal,
+			expectedAddresses: []v1.NodeAddress{},
+			shouldError:       false,
+		},
+		{
+			name:              "cloud provider is external and no nodeIP",
+			nodeAddresses:     []v1.NodeAddress{},
+			cloudProviderType: cloudProviderExternal,
+			expectedAddresses: []v1.NodeAddress{},
+			shouldError:       false,
+		},
 		{
 			name: "cloud doesn't report hostname, no override, detected hostname mismatch",
 			nodeAddresses: []v1.NodeAddress{