Update GetNodeAddressesFromNodeIP to take the unparsed annotation

And simplify the callers in node_controller.go to merge the common
code.
This commit is contained in:
Dan Winship 2023-03-06 09:02:19 -05:00
parent 7605163620
commit d6a11b7138
4 changed files with 29 additions and 72 deletions

View File

@ -20,7 +20,6 @@ import (
"context"
"errors"
"fmt"
"net"
"time"
v1 "k8s.io/api/core/v1"
@ -384,19 +383,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.GetNodeAddressesFromNodeIP(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
}
@ -533,16 +525,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.GetNodeAddressesFromNodeIP(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 != "" {
@ -749,18 +734,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)
}
nodeIP, err := nodeutil.ParseNodeIPAnnotation(providedIP)
if err != nil {
return nil, fmt.Errorf("failed to parse node IP %q for node %q: %v", providedIP, node.Name, err)
}
return nodeIP, nil
return nodeAddresses, err
}
// getInstanceTypeByProviderIDOrName will attempt to get the instance type of node using its providerID

View File

@ -21,6 +21,7 @@ import (
"net"
"k8s.io/api/core/v1"
nodeutil "k8s.io/component-helpers/node/util"
netutils "k8s.io/utils/net"
)
@ -84,11 +85,12 @@ func GetNodeAddressesFromNodeIPLegacy(nodeIP net.IP, cloudNodeAddresses []v1.Nod
}
// Otherwise the result is the same as for GetNodeAddressesFromNodeIP
return GetNodeAddressesFromNodeIP(nodeIP, cloudNodeAddresses)
return GetNodeAddressesFromNodeIP(nodeIP.String(), cloudNodeAddresses)
}
// GetNodeAddressesFromNodeIP filters the provided list of nodeAddresses to
// match the provided nodeIP. This is used for external cloud providers.
// 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.
@ -100,10 +102,10 @@ func GetNodeAddressesFromNodeIPLegacy(nodeIP net.IP, cloudNodeAddresses []v1.Nod
// 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(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
func GetNodeAddressesFromNodeIP(providedNodeIP string, cloudNodeAddresses []v1.NodeAddress) ([]v1.NodeAddress, error) {
nodeIP, err := nodeutil.ParseNodeIPAnnotation(providedNodeIP)
if err != nil {
return nil, fmt.Errorf("failed to parse node IP %q: %v", providedNodeIP, err)
}
// For every address supplied by the cloud provider that matches nodeIP, nodeIP is the enforced node address for

View File

@ -316,14 +316,14 @@ func TestGetNodeAddressesFromNodeIPLegacy(t *testing.T) {
func TestGetNodeAddressesFromNodeIP(t *testing.T) {
cases := []struct {
name string
nodeIP net.IP
nodeIP string
nodeAddresses []v1.NodeAddress
expectedAddresses []v1.NodeAddress
shouldError bool
}{
{
name: "A single InternalIP",
nodeIP: netutils.ParseIPSloppy("10.1.1.1"),
nodeIP: "10.1.1.1",
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
@ -336,7 +336,7 @@ func TestGetNodeAddressesFromNodeIP(t *testing.T) {
},
{
name: "NodeIP is external",
nodeIP: netutils.ParseIPSloppy("55.55.55.55"),
nodeIP: "55.55.55.55",
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
@ -352,7 +352,7 @@ func TestGetNodeAddressesFromNodeIP(t *testing.T) {
{
// Accommodating #45201 and #49202
name: "InternalIP and ExternalIP are the same",
nodeIP: netutils.ParseIPSloppy("55.55.55.55"),
nodeIP: "55.55.55.55",
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "44.44.44.44"},
{Type: v1.NodeExternalIP, Address: "44.44.44.44"},
@ -369,7 +369,7 @@ func TestGetNodeAddressesFromNodeIP(t *testing.T) {
},
{
name: "An Internal/ExternalIP, an Internal/ExternalDNS",
nodeIP: netutils.ParseIPSloppy("10.1.1.1"),
nodeIP: "10.1.1.1",
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
@ -388,7 +388,7 @@ func TestGetNodeAddressesFromNodeIP(t *testing.T) {
},
{
name: "An Internal with multiple internal IPs",
nodeIP: netutils.ParseIPSloppy("10.1.1.1"),
nodeIP: "10.1.1.1",
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeInternalIP, Address: "10.2.2.2"},
@ -405,7 +405,7 @@ func TestGetNodeAddressesFromNodeIP(t *testing.T) {
},
{
name: "An InternalIP that isn't valid: should error",
nodeIP: netutils.ParseIPSloppy("10.2.2.2"),
nodeIP: "10.2.2.2",
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
@ -416,7 +416,7 @@ func TestGetNodeAddressesFromNodeIP(t *testing.T) {
},
{
name: "Dual-stack cloud, with nodeIP, different IPv6 formats",
nodeIP: netutils.ParseIPSloppy("2600:1f14:1d4:d101::ba3d"),
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"},
@ -428,34 +428,6 @@ func TestGetNodeAddressesFromNodeIP(t *testing.T) {
},
shouldError: false,
},
{
name: "Dual-stack cloud, IPv4 first, no nodeIP",
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, no nodeIP",
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: "fc01:1234::5678"},
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
},
shouldError: false,
},
}
for _, tt := range cases {

View File

@ -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