Merge pull request kubernetes#116305 from danwinship/cloud-node-ips

KEP-3705 cloud dual-stack --node-ip

cmd/kubelet/app/server.go

@@ -1120,24 +1120,9 @@ func RunKubelet(kubeServer *options.KubeletServer, kubeDeps *kubelet.Dependencie
 	// Setup event recorder if required.
 	makeEventRecorder(kubeDeps, nodeName)
 
-	var nodeIPs []net.IP
-	if kubeServer.NodeIP != "" {
-		for _, ip := range strings.Split(kubeServer.NodeIP, ",") {
-			parsedNodeIP := netutils.ParseIPSloppy(strings.TrimSpace(ip))
-			if parsedNodeIP == nil {
-				klog.InfoS("Could not parse --node-ip ignoring", "IP", ip)
-			} else {
-				nodeIPs = append(nodeIPs, parsedNodeIP)
-			}
-		}
-	}
-
-	if len(nodeIPs) > 2 || (len(nodeIPs) == 2 && netutils.IsIPv6(nodeIPs[0]) == netutils.IsIPv6(nodeIPs[1])) {
-		return fmt.Errorf("bad --node-ip %q; must contain either a single IP or a dual-stack pair of IPs", kubeServer.NodeIP)
-	} else if len(nodeIPs) == 2 && kubeServer.CloudProvider != "" {
-		return fmt.Errorf("dual-stack --node-ip %q not supported when using a cloud provider", kubeServer.NodeIP)
-	} else if len(nodeIPs) == 2 && (nodeIPs[0].IsUnspecified() || nodeIPs[1].IsUnspecified()) {
-		return fmt.Errorf("dual-stack --node-ip %q cannot include '0.0.0.0' or '::'", kubeServer.NodeIP)
+	nodeIPs, err := nodeutil.ParseNodeIPArgument(kubeServer.NodeIP, kubeServer.CloudProvider, utilfeature.DefaultFeatureGate.Enabled(features.CloudDualStackNodeIPs))
+	if err != nil {
+		return fmt.Errorf("bad --node-ip %q: %v", kubeServer.NodeIP, err)
 	}
 
 	capabilities.Initialize(capabilities.Capabilities{

pkg/features/kube_features.go

@@ -61,6 +61,12 @@ const (
 	// beta: v1.4
 	AppArmor featuregate.Feature = "AppArmor"
 
+	// owner: @danwinship
+	// alpha: v1.27
+	//
+	// Enables dual-stack --node-ip in kubelet with external cloud providers
+	CloudDualStackNodeIPs featuregate.Feature = "CloudDualStackNodeIPs"
+
 	// owner: @szuecs
 	// alpha: v1.12
 	//
@@ -926,6 +932,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	AppArmor: {Default: true, PreRelease: featuregate.Beta},
 
+	CloudDualStackNodeIPs: {Default: false, PreRelease: featuregate.Alpha},
+
 	CPUCFSQuotaPeriod: {Default: false, PreRelease: featuregate.Alpha},
 
 	CPUManager: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // GA in 1.26

pkg/kubelet/nodestatus/setters.go

@@ -131,7 +131,7 @@ func NodeAddress(nodeIPs []net.IP, // typically Kubelet.nodeIPs
 				return err
 			}
 
-			nodeAddresses, err := cloudprovidernodeutil.PreferNodeIP(nodeIP, cloudNodeAddresses)
+			nodeAddresses, err := cloudprovidernodeutil.GetNodeAddressesFromNodeIPLegacy(nodeIP, cloudNodeAddresses)
 			if err != nil {
 				return err
 			}

staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"net"
 	"time"
 
 	v1 "k8s.io/api/core/v1"
@@ -30,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	coreinformers "k8s.io/client-go/informers/core/v1"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -44,8 +44,8 @@ import (
 	cloudnodeutil "k8s.io/cloud-provider/node/helpers"
 	controllersmetrics "k8s.io/component-base/metrics/prometheus/controllers"
 	nodeutil "k8s.io/component-helpers/node/util"
+	"k8s.io/controller-manager/pkg/features"
 	"k8s.io/klog/v2"
-	netutils "k8s.io/utils/net"
 )
 
 // labelReconcileInfo lists Node labels to reconcile, and how to reconcile them.
@@ -385,19 +385,12 @@ func (cnc *CloudNodeController) updateNodeAddress(ctx context.Context, node *v1.
 		}
 	}
 	// If kubelet provided a node IP, prefer it in the node address list
-	nodeIP, err := getNodeProvidedIP(node)
+	nodeAddresses, err := updateNodeAddressesFromNodeIP(node, nodeAddresses)
 	if err != nil {
-		klog.Errorf("Failed to get preferred node IP for node %q: %v", node.Name, err)
+		klog.Errorf("Failed to update node addresses for node %q: %v", node.Name, err)
 		return
 	}
 
-	if nodeIP != nil {
-		nodeAddresses, err = cloudnodeutil.PreferNodeIP(nodeIP, nodeAddresses)
-		if err != nil {
-			klog.Errorf("Failed to update node addresses for node %q: %v", node.Name, err)
-			return
-		}
-	}
 	if !nodeAddressesChangeDetected(node.Status.Addresses, nodeAddresses) {
 		return
 	}
@@ -534,16 +527,9 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(
 	// If kubelet annotated the node with a node IP, ensure that it is valid
 	// and can be applied to the discovered node addresses before removing
 	// the taint on the node.
-	nodeIP, err := getNodeProvidedIP(node)
+	_, err := updateNodeAddressesFromNodeIP(node, instanceMeta.NodeAddresses)
 	if err != nil {
-		return nil, err
-	}
-
-	if nodeIP != nil {
-		_, err := cloudnodeutil.PreferNodeIP(nodeIP, instanceMeta.NodeAddresses)
-		if err != nil {
-			return nil, fmt.Errorf("provided node ip for node %q is not valid: %w", node.Name, err)
-		}
+		return nil, fmt.Errorf("provided node ip for node %q is not valid: %w", node.Name, err)
 	}
 
 	if instanceMeta.InstanceType != "" {
@@ -750,18 +736,15 @@ func nodeAddressesChangeDetected(addressSet1, addressSet2 []v1.NodeAddress) bool
 	return false
 }
 
-func getNodeProvidedIP(node *v1.Node) (net.IP, error) {
-	providedIP, ok := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr]
-	if !ok {
-		return nil, nil
-	}
+func updateNodeAddressesFromNodeIP(node *v1.Node, nodeAddresses []v1.NodeAddress) ([]v1.NodeAddress, error) {
+	var err error
 
-	nodeIP := netutils.ParseIPSloppy(providedIP)
-	if nodeIP == nil {
-		return nil, fmt.Errorf("failed to parse node IP %q for node %q", providedIP, node.Name)
+	providedNodeIP, exists := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr]
+	if exists {
+		nodeAddresses, err = cloudnodeutil.GetNodeAddressesFromNodeIP(providedNodeIP, nodeAddresses, utilfeature.DefaultFeatureGate.Enabled(features.CloudDualStackNodeIPs))
 	}
 
-	return nodeIP, nil
+	return nodeAddresses, err
 }
 
 // getInstanceTypeByProviderIDOrName will attempt to get the instance type of node using its providerID

staging/src/k8s.io/cloud-provider/controllers/node/node_controller_test.go

@@ -38,6 +38,7 @@ import (
 	cloudprovider "k8s.io/cloud-provider"
 	cloudproviderapi "k8s.io/cloud-provider/api"
 	fakecloud "k8s.io/cloud-provider/fake"
+	_ "k8s.io/controller-manager/pkg/features/register"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"

staging/src/k8s.io/cloud-provider/node/helpers/address.go

@@ -21,6 +21,7 @@ import (
 	"net"
 
 	"k8s.io/api/core/v1"
+	nodeutil "k8s.io/component-helpers/node/util"
 	netutils "k8s.io/utils/net"
 )
 
@@ -41,8 +42,8 @@ func AddToNodeAddresses(addresses *[]v1.NodeAddress, addAddresses ...v1.NodeAddr
 	}
 }
 
-// PreferNodeIP filters node addresses to prefer a specific node IP or address
-// family.
+// GetNodeAddressesFromNodeIPLegacy filters node addresses to prefer a specific node IP or
+// address family. This function is used only with legacy cloud providers.
 //
 // If nodeIP is either '0.0.0.0' or '::' it is taken to represent any address of
 // that address family: IPv4 or IPv6. i.e. if nodeIP is '0.0.0.0' we will return
@@ -55,7 +56,7 @@ func AddToNodeAddresses(addresses *[]v1.NodeAddress, addAddresses ...v1.NodeAddr
 //   - If nodeIP matches an address of a particular type (internal or external),
 //     that will be the *only* address of that type returned.
 //   - All remaining addresses are listed after.
-func PreferNodeIP(nodeIP net.IP, cloudNodeAddresses []v1.NodeAddress) ([]v1.NodeAddress, error) {
+func GetNodeAddressesFromNodeIPLegacy(nodeIP net.IP, cloudNodeAddresses []v1.NodeAddress) ([]v1.NodeAddress, error) {
 	// If nodeIP is unset, just use the addresses provided by the cloud provider as-is
 	if nodeIP == nil {
 		return cloudNodeAddresses, nil
@@ -83,26 +84,58 @@ func PreferNodeIP(nodeIP net.IP, cloudNodeAddresses []v1.NodeAddress) ([]v1.Node
 		return sortedAddresses, nil
 	}
 
-	// For every address supplied by the cloud provider that matches nodeIP, nodeIP is the enforced node address for
-	// that address Type (like InternalIP and ExternalIP), meaning other addresses of the same Type are discarded.
-	// See #61921 for more information: some cloud providers may supply secondary IPs, so nodeIP serves as a way to
-	// ensure that the correct IPs show up on a Node object.
-	enforcedNodeAddresses := []v1.NodeAddress{}
+	// Otherwise the result is the same as for GetNodeAddressesFromNodeIP
+	return GetNodeAddressesFromNodeIP(nodeIP.String(), cloudNodeAddresses, false)
+}
+
+// GetNodeAddressesFromNodeIP filters the provided list of nodeAddresses to match the
+// providedNodeIP from the Node annotation (which is assumed to be non-empty). This is
+// used for external cloud providers.
+//
+// It will return node addresses filtered such that:
+//   - Any address matching nodeIP will be listed first.
+//   - If nodeIP matches an address of a particular type (internal or external),
+//     that will be the *only* address of that type returned.
+//   - All remaining addresses are listed after.
+//
+// (This does not have the same behavior with `0.0.0.0` and `::` as
+// GetNodeAddressesFromNodeIPLegacy, because that case never occurs for external cloud
+// providers, because kubelet does not set the `provided-node-ip` annotation in that
+// case.)
+func GetNodeAddressesFromNodeIP(providedNodeIP string, cloudNodeAddresses []v1.NodeAddress, allowDualStack bool) ([]v1.NodeAddress, error) {
+	nodeIPs, err := nodeutil.ParseNodeIPAnnotation(providedNodeIP, allowDualStack)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse node IP %q: %v", providedNodeIP, err)
+	}
 
+	enforcedNodeAddresses := []v1.NodeAddress{}
 	nodeIPTypes := make(map[v1.NodeAddressType]bool)
-	for _, nodeAddress := range cloudNodeAddresses {
-		if netutils.ParseIPSloppy(nodeAddress.Address).Equal(nodeIP) {
-			enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: nodeAddress.Type, Address: nodeAddress.Address})
-			nodeIPTypes[nodeAddress.Type] = true
+
+	for _, nodeIP := range nodeIPs {
+		// For every address supplied by the cloud provider that matches nodeIP,
+		// nodeIP is the enforced node address for that address Type (like
+		// InternalIP and ExternalIP), meaning other addresses of the same Type
+		// are discarded. See #61921 for more information: some cloud providers
+		// may supply secondary IPs, so nodeIP serves as a way to ensure that the
+		// correct IPs show up on a Node object.
+
+		matched := false
+		for _, nodeAddress := range cloudNodeAddresses {
+			if netutils.ParseIPSloppy(nodeAddress.Address).Equal(nodeIP) {
+				enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: nodeAddress.Type, Address: nodeAddress.Address})
+				nodeIPTypes[nodeAddress.Type] = true
+				matched = true
+			}
 		}
-	}
 
-	// nodeIP must be among the addresses supplied by the cloud provider
-	if len(enforcedNodeAddresses) == 0 {
-		return nil, fmt.Errorf("failed to get node address from cloud provider that matches ip: %v", nodeIP)
+		// nodeIP must be among the addresses supplied by the cloud provider
+		if !matched {
+			return nil, fmt.Errorf("failed to get node address from cloud provider that matches ip: %v", nodeIP)
+		}
 	}
 
-	// nodeIP was found, now use all other addresses supplied by the cloud provider NOT of the same Type as nodeIP.
+	// Now use all other addresses supplied by the cloud provider NOT of the same Type
+	// as any nodeIP.
 	for _, nodeAddress := range cloudNodeAddresses {
 		if !nodeIPTypes[nodeAddress.Type] {
 			enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: nodeAddress.Type, Address: nodeAddress.Address})