Skip to content

Commit

Permalink
fix: handle comma-separated provided IPs
Browse files Browse the repository at this point in the history
  • Loading branch information
@mainred
mainred authored and k8s-infra-cherrypick-robot committed Mar 20, 2024
1 parent e96edd0 commit c7f7810
Show file tree
Hide file tree
Showing 2 changed files with 193 additions and 16 deletions.
39 changes: 23 additions & 16 deletions pkg/nodemanager/nodemanager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,10 +302,8 @@ func (cnc *CloudNodeController) updateNodeAddress(ctx context.Context, node *v1.
}
// If nodeIP was suggested by user, ensure that
// it can be found in the cloud as well (consistent with the behaviour in kubelet)
if nodeIP, ok := ensureNodeProvidedIPExists(node, nodeAddresses); ok {
if nodeIP == nil {
return fmt.Errorf("specified Node IP %s not found in cloudprovider for node %q", nodeAddresses, node.Name)
}
if existNodeIPs, ok := ensureNodeProvidedIPsExists(node, nodeAddresses); !ok {
return fmt.Errorf("not all specified Node IP %s found in cloudprovider for node %q, existing Node IPs are %s ", nodeAddresses, node.Name, existNodeIPs)
}
if !nodeAddressesChangeDetected(node.Status.Addresses, nodeAddresses) {
return nil
Expand Down Expand Up @@ -468,10 +466,8 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(ctx context.Co

// If user provided an IP address, ensure that IP address is found
// in the cloud provider before removing the taint on the node
if nodeIP, ok := ensureNodeProvidedIPExists(node, nodeAddresses); ok {
if nodeIP == nil {
return nil, errors.New("failed to find kubelet node IP from cloud provider")
}
if _, ok := ensureNodeProvidedIPsExists(node, nodeAddresses); !ok {
return nil, errors.New("failed to find kubelet node IP from cloud provider")
}

if instanceType, err := cnc.getInstanceTypeByName(ctx, node); err != nil {
Expand Down Expand Up @@ -602,19 +598,30 @@ func nodeAddressesChangeDetected(addressSet1, addressSet2 []v1.NodeAddress) bool
return false
}

func ensureNodeProvidedIPExists(node *v1.Node, nodeAddresses []v1.NodeAddress) (*v1.NodeAddress, bool) {
var nodeIP *v1.NodeAddress
nodeIPExists := false
if providedIP, ok := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr]; ok {
nodeIPExists = true
for i := range nodeAddresses {
// Ensure all provided node ip addresses are found in the cloud provider, otherwise return false
// When there's no provided node ip addresses, it will return true.
func ensureNodeProvidedIPsExists(node *v1.Node, nodeAddresses []v1.NodeAddress) ([]v1.NodeAddress, bool) {
providedIPStr, ok := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr]
if !ok || len(providedIPStr) == 0 {
return []v1.NodeAddress{}, true
}
var existNodeIPs []v1.NodeAddress
nodeIPsExists := false
providedIPs := strings.Split(providedIPStr, ",")
for i := range nodeAddresses {
for _, providedIP := range providedIPs {
providedIP = strings.TrimSpace(providedIP)
if nodeAddresses[i].Address == providedIP {
nodeIP = &nodeAddresses[i]
existNodeIPs = append(existNodeIPs, nodeAddresses[i])
break
}
}
}
return nodeIP, nodeIPExists
if len(existNodeIPs) == len(providedIPs) {
nodeIPsExists = true
}

return existNodeIPs, nodeIPsExists
}

func (cnc *CloudNodeController) getInstanceTypeByName(ctx context.Context, node *v1.Node) (string, error) {
Expand Down
170 changes: 170 additions & 0 deletions pkg/nodemanager/nodemanager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1338,3 +1338,173 @@ func TestNodeProviderIDNotSet(t *testing.T) {
// Node update should fail
assert.Equal(t, 0, len(fnh.UpdatedNodes), "Node was updated (unexpected)")
}

func Test_ensureNodeProvidedIPsExists(t *testing.T) {
testcases := []struct {
name string
node *v1.Node
nodeAddresses []v1.NodeAddress
expectedExistingNodeIPs []v1.NodeAddress
nodeIPsExists bool
}{
{
name: "return true when there's provide node ip address",
node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "node0",
Labels: map[string]string{},
Annotations: map[string]string{},
},
},
nodeAddresses: []v1.NodeAddress{
{
Address: "10.0.0.1",
},
},
expectedExistingNodeIPs: []v1.NodeAddress{},
nodeIPsExists: true,
},
{
name: "return true when all provided IPv4 IP address are found",
node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "node0",
Labels: map[string]string{},
Annotations: map[string]string{
cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1",
},
},
},
nodeAddresses: []v1.NodeAddress{
{
Address: "10.0.0.1",
},
},
expectedExistingNodeIPs: []v1.NodeAddress{{Address: "10.0.0.1"}},
nodeIPsExists: true,
},
{
name: "return true when all provided dual stack IP addresses are found",
node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "node0",
Labels: map[string]string{},
Annotations: map[string]string{
cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1,fd47:c915:f8a8:e63d::5",
},
},
},
nodeAddresses: []v1.NodeAddress{
{
Address: "10.0.0.1",
},
{
Address: "fd47:c915:f8a8:e63d::5",
},
},
expectedExistingNodeIPs: []v1.NodeAddress{
{
Address: "10.0.0.1",
},
{
Address: "fd47:c915:f8a8:e63d::5",
},
},
nodeIPsExists: true,
},
{
name: "return true when all provided dual stack IP addresses are found but joined with extra space",
node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "node0",
Labels: map[string]string{},
Annotations: map[string]string{
cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1, fd47:c915:f8a8:e63d::5",
},
},
},
nodeAddresses: []v1.NodeAddress{
{
Address: "10.0.0.1",
},
{
Address: "fd47:c915:f8a8:e63d::5",
},
},
expectedExistingNodeIPs: []v1.NodeAddress{
{
Address: "10.0.0.1",
},
{
Address: "fd47:c915:f8a8:e63d::5",
},
},
nodeIPsExists: true,
},
{
name: "return false when not all ip addresses are found for provided dual stack IP addresses",
node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "node0",
Labels: map[string]string{},
Annotations: map[string]string{
cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1,fd47:c915:f8a8:e63d::5",
},
},
},
nodeAddresses: []v1.NodeAddress{
{
Address: "10.0.0.1",
},
},
expectedExistingNodeIPs: []v1.NodeAddress{
{
Address: "10.0.0.1",
},
},
nodeIPsExists: false,
},
{
name: "return false when wrong ip addresses are provided for provided dual stack IP addresses",
node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "node0",
Labels: map[string]string{},
Annotations: map[string]string{
cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1,fd47:c915:f8a8:e63d::10",
},
},
},
nodeAddresses: []v1.NodeAddress{
{
Address: "10.0.0.1",
},
{
Address: "fd47:c915:f8a8:e63d::5",
},
},
expectedExistingNodeIPs: []v1.NodeAddress{
{
Address: "10.0.0.1",
},
},
nodeIPsExists: false,
},
}

for _, test := range testcases {
t.Run(test.name, func(t *testing.T) {

actualNodeIP, actualNodeIPsExists := ensureNodeProvidedIPsExists(test.node, test.nodeAddresses)

if !reflect.DeepEqual(actualNodeIP, test.expectedExistingNodeIPs) {
t.Logf("Actual existing node IPs: %v", actualNodeIP)
t.Logf("Expected existing node IPs: %v", test.expectedExistingNodeIPs)
t.Errorf("Actual existing node IP does not match expected existing node IP")
}
if actualNodeIPsExists != test.nodeIPsExists {
t.Errorf("all node ip addresses exist result mismatch, got: %t, wanted: %t", actualNodeIPsExists, test.nodeIPsExists)
}
})
}
}

0 comments on commit c7f7810

Please sign in to comment.