diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index 68af6aaba7b..4c4fb27b244 100644 --- a/cmd/kubelet/app/server.go +++ b/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{ diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index bdd459988a6..0d08585be67 100644 --- a/pkg/features/kube_features.go +++ b/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 diff --git a/pkg/kubelet/nodestatus/setters.go b/pkg/kubelet/nodestatus/setters.go index 687e8212403..e6460f14d9f 100644 --- a/pkg/kubelet/nodestatus/setters.go +++ b/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 } diff --git a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go index 878ca1194a6..ace70dd0aaf 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go +++ b/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 + + providedNodeIP, exists := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr] + if exists { + nodeAddresses, err = cloudnodeutil.GetNodeAddressesFromNodeIP(providedNodeIP, nodeAddresses, utilfeature.DefaultFeatureGate.Enabled(features.CloudDualStackNodeIPs)) } - nodeIP := netutils.ParseIPSloppy(providedIP) - if nodeIP == nil { - return nil, fmt.Errorf("failed to parse node IP %q for node %q", providedIP, node.Name) - } - - return nodeIP, nil + return nodeAddresses, err } // getInstanceTypeByProviderIDOrName will attempt to get the instance type of node using its providerID diff --git a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller_test.go b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller_test.go index 723756242aa..9bb65f693d1 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller_test.go +++ b/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" diff --git a/staging/src/k8s.io/cloud-provider/node/helpers/address.go b/staging/src/k8s.io/cloud-provider/node/helpers/address.go index 6eb44a90feb..41112cdaeb3 100644 --- a/staging/src/k8s.io/cloud-provider/node/helpers/address.go +++ b/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 !matched { + 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 len(enforcedNodeAddresses) == 0 { - 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}) diff --git a/staging/src/k8s.io/cloud-provider/node/helpers/address_test.go b/staging/src/k8s.io/cloud-provider/node/helpers/address_test.go index eccb4a75b81..cf208df1a28 100644 --- a/staging/src/k8s.io/cloud-provider/node/helpers/address_test.go +++ b/staging/src/k8s.io/cloud-provider/node/helpers/address_test.go @@ -94,7 +94,7 @@ func TestAddToNodeAddresses(t *testing.T) { } } -func TestPreferNodeIP(t *testing.T) { +func TestGetNodeAddressesFromNodeIPLegacy(t *testing.T) { cases := []struct { name string nodeIP net.IP @@ -301,13 +301,264 @@ func TestPreferNodeIP(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { - got, err := PreferNodeIP(tt.nodeIP, tt.nodeAddresses) + got, err := GetNodeAddressesFromNodeIPLegacy(tt.nodeIP, tt.nodeAddresses) if (err != nil) != tt.shouldError { - t.Errorf("PreferNodeIP() error = %v, wantErr %v", err, tt.shouldError) + t.Errorf("GetNodeAddressesFromNodeIPLegacy() error = %v, wantErr %v", err, tt.shouldError) return } if !reflect.DeepEqual(got, tt.expectedAddresses) { - t.Errorf("PreferNodeIP() = %v, want %v", got, tt.expectedAddresses) + t.Errorf("GetNodeAddressesFromNodeIPLegacy() = %v, want %v", got, tt.expectedAddresses) + } + }) + } +} + +func TestGetNodeAddressesFromNodeIP(t *testing.T) { + cases := []struct { + name string + nodeIP string + nodeAddresses []v1.NodeAddress + expectedAddresses []v1.NodeAddress + shouldError bool + }{ + { + name: "A single InternalIP", + nodeIP: "10.1.1.1", + 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}, + }, + shouldError: false, + }, + { + name: "NodeIP is external", + nodeIP: "55.55.55.55", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + { + // Accommodating #45201 and #49202 + name: "InternalIP and ExternalIP are the same", + nodeIP: "55.55.55.55", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "44.44.44.44"}, + {Type: v1.NodeExternalIP, Address: "44.44.44.44"}, + {Type: v1.NodeInternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + { + name: "An Internal/ExternalIP, an Internal/ExternalDNS", + nodeIP: "10.1.1.1", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeInternalDNS, Address: "ip-10-1-1-1.us-west-2.compute.internal"}, + {Type: v1.NodeExternalDNS, Address: "ec2-55-55-55-55.us-west-2.compute.amazonaws.com"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeInternalDNS, Address: "ip-10-1-1-1.us-west-2.compute.internal"}, + {Type: v1.NodeExternalDNS, Address: "ec2-55-55-55-55.us-west-2.compute.amazonaws.com"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + { + name: "An Internal with multiple internal IPs", + nodeIP: "10.1.1.1", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeInternalIP, Address: "10.2.2.2"}, + {Type: v1.NodeInternalIP, Address: "10.3.3.3"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + { + name: "An InternalIP that isn't valid: should error", + nodeIP: "10.2.2.2", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + expectedAddresses: nil, + shouldError: true, + }, + { + name: "Dual-stack cloud, with nodeIP, different IPv6 formats", + nodeIP: "2600:1f14:1d4:d101::ba3d", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeInternalIP, Address: "2600:1f14:1d4:d101:0:0:0:ba3d"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "2600:1f14:1d4:d101:0:0:0:ba3d"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + { + name: "Single-stack cloud, dual-stack request", + nodeIP: "10.1.1.1,fc01:1234::5678", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: true, + }, + { + name: "Dual-stack cloud, IPv4 first, IPv4-primary request", + nodeIP: "10.1.1.1,fc01:1234::5678", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeInternalIP, Address: "fc01:1234::5678"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeInternalIP, Address: "fc01:1234::5678"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + { + name: "Dual-stack cloud, IPv6 first, IPv4-primary request", + nodeIP: "10.1.1.1,fc01:1234::5678", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "fc01:1234::5678"}, + {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.NodeInternalIP, Address: "fc01:1234::5678"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + { + name: "Dual-stack cloud, dual-stack request, multiple IPs", + nodeIP: "10.1.1.1,fc01:1234::5678", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeInternalIP, Address: "10.1.1.2"}, + {Type: v1.NodeInternalIP, Address: "fc01:1234::1234"}, + {Type: v1.NodeInternalIP, Address: "fc01:1234::5678"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + // additional IPs of the same type are removed, as in the + // single-stack case. + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeInternalIP, Address: "fc01:1234::5678"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + { + name: "Dual-stack cloud, dual-stack request, extra ExternalIP", + nodeIP: "10.1.1.1,fc01:1234::5678", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeInternalIP, Address: "fc01:1234::1234"}, + {Type: v1.NodeInternalIP, Address: "fc01:1234::5678"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + // The ExternalIP is preserved, since no ExternalIP was matched + // by --node-ip. + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeInternalIP, Address: "fc01:1234::5678"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + { + name: "Dual-stack cloud, dual-stack request, multiple ExternalIPs", + nodeIP: "fc01:1234::5678,10.1.1.1", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeInternalIP, Address: "fc01:1234::5678"}, + {Type: v1.NodeExternalIP, Address: "2001:db1::1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + // The ExternalIPs are preserved, since no ExternalIP was matched + // by --node-ip. + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "fc01:1234::5678"}, + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeExternalIP, Address: "2001:db1::1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + { + name: "Dual-stack cloud, dual-stack request, mixed InternalIP/ExternalIP match", + nodeIP: "55.55.55.55,fc01:1234::5678", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeInternalIP, Address: "fc01:1234::5678"}, + {Type: v1.NodeExternalIP, Address: "2001:db1::1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + // Since the IPv4 --node-ip value matched an ExternalIP, that + // filters out the IPv6 ExternalIP. Since the IPv6 --node-ip value + // matched in InternalIP, that filters out the IPv4 InternalIP + // value. + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeInternalIP, Address: "fc01:1234::5678"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + got, err := GetNodeAddressesFromNodeIP(tt.nodeIP, tt.nodeAddresses, true) + if (err != nil) != tt.shouldError { + t.Errorf("GetNodeAddressesFromNodeIP() error = %v, wantErr %v", err, tt.shouldError) + return + } + if !reflect.DeepEqual(got, tt.expectedAddresses) { + t.Errorf("GetNodeAddressesFromNodeIP() = %v, want %v", got, tt.expectedAddresses) } }) } diff --git a/staging/src/k8s.io/component-helpers/node/util/ips.go b/staging/src/k8s.io/component-helpers/node/util/ips.go new file mode 100644 index 00000000000..ff306a3dc3c --- /dev/null +++ b/staging/src/k8s.io/component-helpers/node/util/ips.go @@ -0,0 +1,82 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "fmt" + "net" + "strings" + + "k8s.io/klog/v2" + netutils "k8s.io/utils/net" +) + +const ( + cloudProviderNone = "" + cloudProviderExternal = "external" +) + +// parseNodeIP implements ParseNodeIPArgument and ParseNodeIPAnnotation +func parseNodeIP(nodeIP string, allowDual, sloppy bool) ([]net.IP, error) { + var nodeIPs []net.IP + if nodeIP != "" || !sloppy { + for _, ip := range strings.Split(nodeIP, ",") { + if sloppy { + ip = strings.TrimSpace(ip) + } + parsedNodeIP := netutils.ParseIPSloppy(ip) + if parsedNodeIP == nil { + if sloppy { + klog.InfoS("Could not parse node IP. Ignoring", "IP", ip) + } else { + return nil, fmt.Errorf("could not parse %q", ip) + } + } else { + nodeIPs = append(nodeIPs, parsedNodeIP) + } + } + } + + if len(nodeIPs) > 2 || (len(nodeIPs) == 2 && netutils.IsIPv6(nodeIPs[0]) == netutils.IsIPv6(nodeIPs[1])) { + return nil, fmt.Errorf("must contain either a single IP or a dual-stack pair of IPs") + } else if len(nodeIPs) == 2 && !allowDual { + return nil, fmt.Errorf("dual-stack not supported in this configuration") + } else if len(nodeIPs) == 2 && (nodeIPs[0].IsUnspecified() || nodeIPs[1].IsUnspecified()) { + return nil, fmt.Errorf("dual-stack node IP cannot include '0.0.0.0' or '::'") + } + + return nodeIPs, nil +} + +// ParseNodeIPArgument parses kubelet's --node-ip argument. If nodeIP contains invalid +// values, they will be logged and ignored. Dual-stack node IPs are allowed if +// cloudProvider is unset, or if it is `"external"` and allowCloudDualStack is true. +func ParseNodeIPArgument(nodeIP, cloudProvider string, allowCloudDualStack bool) ([]net.IP, error) { + var allowDualStack bool + if (cloudProvider == cloudProviderNone) || (cloudProvider == cloudProviderExternal && allowCloudDualStack) { + allowDualStack = true + } + return parseNodeIP(nodeIP, allowDualStack, true) +} + +// ParseNodeIPAnnotation parses the `alpha.kubernetes.io/provided-node-ip` annotation, +// which can be either a single IP address or (if allowDualStack is true) a +// comma-separated pair of IP addresses. Unlike with ParseNodeIPArgument, invalid values +// are considered an error. +func ParseNodeIPAnnotation(nodeIP string, allowDualStack bool) ([]net.IP, error) { + return parseNodeIP(nodeIP, allowDualStack, false) +} diff --git a/staging/src/k8s.io/component-helpers/node/util/ips_test.go b/staging/src/k8s.io/component-helpers/node/util/ips_test.go new file mode 100644 index 00000000000..0d3586ef582 --- /dev/null +++ b/staging/src/k8s.io/component-helpers/node/util/ips_test.go @@ -0,0 +1,374 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "fmt" + "net" + "reflect" + "strings" + "testing" + + netutils "k8s.io/utils/net" +) + +func TestParseNodeIPArgument(t *testing.T) { + testCases := []struct { + desc string + in string + out []net.IP + err string + ssErr string + }{ + { + desc: "empty --node-ip", + in: "", + out: nil, + }, + { + desc: "just whitespace (ignored)", + in: " ", + out: nil, + }, + { + desc: "garbage (ignored)", + in: "blah", + out: nil, + }, + { + desc: "single IPv4", + in: "1.2.3.4", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + }, + }, + { + desc: "single IPv4 with whitespace", + in: " 1.2.3.4 ", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + }, + }, + { + desc: "single IPv4 non-canonical", + in: "01.2.3.004", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + }, + }, + { + desc: "single IPv4 invalid (ignored)", + in: "1.2.3", + out: nil, + }, + { + desc: "single IPv4 CIDR (ignored)", + in: "1.2.3.0/24", + out: nil, + }, + { + desc: "single IPv4 unspecified", + in: "0.0.0.0", + out: []net.IP{ + net.IPv4zero, + }, + }, + { + desc: "single IPv4 plus ignored garbage", + in: "1.2.3.4,not-an-IPv6-address", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + }, + }, + { + desc: "single IPv6", + in: "abcd::ef01", + out: []net.IP{ + netutils.ParseIPSloppy("abcd::ef01"), + }, + }, + { + desc: "single IPv6 non-canonical", + in: "abcd:0abc:00ab:0000:0000::1", + out: []net.IP{ + netutils.ParseIPSloppy("abcd:abc:ab::1"), + }, + }, + { + desc: "simple dual-stack", + in: "1.2.3.4,abcd::ef01", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + netutils.ParseIPSloppy("abcd::ef01"), + }, + ssErr: "not supported in this configuration", + }, + { + desc: "dual-stack with whitespace", + in: "abcd::ef01 , 1.2.3.4", + out: []net.IP{ + netutils.ParseIPSloppy("abcd::ef01"), + netutils.ParseIPSloppy("1.2.3.4"), + }, + ssErr: "not supported in this configuration", + }, + { + desc: "double IPv4", + in: "1.2.3.4,5.6.7.8", + err: "either a single IP or a dual-stack pair of IPs", + }, + { + desc: "double IPv6", + in: "abcd::1,abcd::2", + err: "either a single IP or a dual-stack pair of IPs", + }, + { + desc: "dual-stack with unspecified", + in: "1.2.3.4,::", + err: "cannot include '0.0.0.0' or '::'", + ssErr: "not supported in this configuration", + }, + { + desc: "dual-stack with unspecified", + in: "0.0.0.0,abcd::1", + err: "cannot include '0.0.0.0' or '::'", + ssErr: "not supported in this configuration", + }, + { + desc: "dual-stack plus ignored garbage", + in: "abcd::ef01 , 1.2.3.4, something else", + out: []net.IP{ + netutils.ParseIPSloppy("abcd::ef01"), + netutils.ParseIPSloppy("1.2.3.4"), + }, + ssErr: "not supported in this configuration", + }, + { + desc: "triple stack!", + in: "1.2.3.4,abcd::1,5.6.7.8", + err: "either a single IP or a dual-stack pair of IPs", + }, + } + + configurations := []struct { + cloudProvider string + allowCloudDualStack bool + dualStackSupported bool + }{ + {cloudProviderNone, false, true}, + {cloudProviderNone, true, true}, + {cloudProviderExternal, false, false}, + {cloudProviderExternal, true, true}, + {"gce", false, false}, + {"gce", true, false}, + } + + for _, tc := range testCases { + for _, conf := range configurations { + desc := fmt.Sprintf("%s, cloudProvider=%q, allowCloudDualStack=%v", tc.desc, conf.cloudProvider, conf.allowCloudDualStack) + t.Run(desc, func(t *testing.T) { + parsed, err := ParseNodeIPArgument(tc.in, conf.cloudProvider, conf.allowCloudDualStack) + + expectedOut := tc.out + expectedErr := tc.err + + if !conf.dualStackSupported { + if len(tc.out) == 2 { + expectedOut = nil + } + if tc.ssErr != "" { + expectedErr = tc.ssErr + } + } + + if !reflect.DeepEqual(parsed, expectedOut) { + t.Errorf("expected %#v, got %#v", expectedOut, parsed) + } + if err != nil { + if expectedErr == "" { + t.Errorf("unexpected error %v", err) + } else if !strings.Contains(err.Error(), expectedErr) { + t.Errorf("expected error with %q, got %v", expectedErr, err) + } + } else if expectedErr != "" { + t.Errorf("expected error with %q, got no error", expectedErr) + } + }) + } + } +} + +func TestParseNodeIPAnnotation(t *testing.T) { + testCases := []struct { + desc string + in string + out []net.IP + err string + ssErr string + }{ + { + desc: "empty --node-ip", + in: "", + err: "could not parse", + }, + { + desc: "just whitespace", + in: " ", + err: "could not parse", + }, + { + desc: "garbage", + in: "blah", + err: "could not parse", + }, + { + desc: "single IPv4", + in: "1.2.3.4", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + }, + }, + { + desc: "single IPv4 with whitespace", + in: " 1.2.3.4 ", + err: "could not parse", + }, + { + desc: "single IPv4 non-canonical", + in: "01.2.3.004", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + }, + }, + { + desc: "single IPv4 invalid", + in: "1.2.3", + err: "could not parse", + }, + { + desc: "single IPv4 CIDR", + in: "1.2.3.0/24", + err: "could not parse", + }, + { + desc: "single IPv4 unspecified", + in: "0.0.0.0", + out: []net.IP{ + net.IPv4zero, + }, + }, + { + desc: "single IPv4 plus garbage", + in: "1.2.3.4,not-an-IPv6-address", + err: "could not parse", + }, + { + desc: "single IPv6", + in: "abcd::ef01", + out: []net.IP{ + netutils.ParseIPSloppy("abcd::ef01"), + }, + }, + { + desc: "single IPv6 non-canonical", + in: "abcd:0abc:00ab:0000:0000::1", + out: []net.IP{ + netutils.ParseIPSloppy("abcd:abc:ab::1"), + }, + }, + { + desc: "simple dual-stack", + in: "1.2.3.4,abcd::ef01", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + netutils.ParseIPSloppy("abcd::ef01"), + }, + ssErr: "not supported in this configuration", + }, + { + desc: "dual-stack with whitespace", + in: "abcd::ef01 , 1.2.3.4", + err: "could not parse", + }, + { + desc: "double IPv4", + in: "1.2.3.4,5.6.7.8", + err: "either a single IP or a dual-stack pair of IPs", + }, + { + desc: "double IPv6", + in: "abcd::1,abcd::2", + err: "either a single IP or a dual-stack pair of IPs", + }, + { + desc: "dual-stack with unspecified", + in: "1.2.3.4,::", + err: "cannot include '0.0.0.0' or '::'", + ssErr: "not supported in this configuration", + }, + { + desc: "dual-stack with unspecified", + in: "0.0.0.0,abcd::1", + err: "cannot include '0.0.0.0' or '::'", + ssErr: "not supported in this configuration", + }, + { + desc: "dual-stack plus garbage", + in: "abcd::ef01 , 1.2.3.4, something else", + err: "could not parse", + }, + { + desc: "triple stack!", + in: "1.2.3.4,abcd::1,5.6.7.8", + err: "either a single IP or a dual-stack pair of IPs", + }, + } + + for _, tc := range testCases { + for _, allowDualStack := range []bool{false, true} { + desc := fmt.Sprintf("%s, allowDualStack=%v", tc.desc, allowDualStack) + t.Run(desc, func(t *testing.T) { + parsed, err := ParseNodeIPAnnotation(tc.in, allowDualStack) + + expectedOut := tc.out + expectedErr := tc.err + + if !allowDualStack { + if len(tc.out) == 2 { + expectedOut = nil + } + if tc.ssErr != "" { + expectedErr = tc.ssErr + } + } + + if !reflect.DeepEqual(parsed, expectedOut) { + t.Errorf("expected %#v, got %#v", expectedOut, parsed) + } + if err != nil { + if expectedErr == "" { + t.Errorf("unexpected error %v", err) + } else if !strings.Contains(err.Error(), expectedErr) { + t.Errorf("expected error with %q, got %v", expectedErr, err) + } + } else if expectedErr != "" { + t.Errorf("expected error with %q, got no error", expectedErr) + } + }) + } + } +} diff --git a/staging/src/k8s.io/controller-manager/pkg/features/kube_features.go b/staging/src/k8s.io/controller-manager/pkg/features/kube_features.go index fad02729503..8864be325e1 100644 --- a/staging/src/k8s.io/controller-manager/pkg/features/kube_features.go +++ b/staging/src/k8s.io/controller-manager/pkg/features/kube_features.go @@ -32,6 +32,19 @@ const ( // of code conflicts because changes are more likely to be scattered // across the file. + // owner: @nckturner + // kep: http://kep.k8s.io/2699 + // alpha: v1.27 + // Enable webhook in cloud controller manager + CloudControllerManagerWebhook featuregate.Feature = "CloudControllerManagerWebhook" + + // owner: @danwinship + // alpha: v1.27 + // + // Enables dual-stack values in the + // `alpha.kubernetes.io/provided-node-ip` annotation + CloudDualStackNodeIPs featuregate.Feature = "CloudDualStackNodeIPs" + // owner: @alexanderConstantinescu // kep: http://kep.k8s.io/3458 // beta: v1.27 @@ -39,12 +52,6 @@ const ( // Enables less load balancer re-configurations by the service controller // (KCCM) as an effect of changing node state. StableLoadBalancerNodeSet featuregate.Feature = "StableLoadBalancerNodeSet" - - // owner: @nckturner - // kep: http://kep.k8s.io/2699 - // alpha: v1.27 - // Enable webhook in cloud controller manager - CloudControllerManagerWebhook featuregate.Feature = "CloudControllerManagerWebhook" ) func SetupCurrentKubernetesSpecificFeatureGates(featuregates featuregate.MutableFeatureGate) error { @@ -54,6 +61,7 @@ func SetupCurrentKubernetesSpecificFeatureGates(featuregates featuregate.Mutable // cloudPublicFeatureGates consists of cloud-specific feature keys. // To add a new feature, define a key for it at k8s.io/api/pkg/features and add it here. var cloudPublicFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ - StableLoadBalancerNodeSet: {Default: true, PreRelease: featuregate.Beta}, CloudControllerManagerWebhook: {Default: false, PreRelease: featuregate.Alpha}, + CloudDualStackNodeIPs: {Default: false, PreRelease: featuregate.Alpha}, + StableLoadBalancerNodeSet: {Default: true, PreRelease: featuregate.Beta}, } diff --git a/staging/src/k8s.io/legacy-cloud-providers/go.mod b/staging/src/k8s.io/legacy-cloud-providers/go.mod index 023c21c920c..5db30abca9a 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/go.mod +++ b/staging/src/k8s.io/legacy-cloud-providers/go.mod @@ -85,6 +85,7 @@ require ( gopkg.in/warnings.v0 v0.1.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect + k8s.io/component-helpers v0.0.0 // indirect k8s.io/kube-openapi v0.0.0-20230308215209-15aac26d736a // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect