diff --git a/pkg/ipamd/ipamd.go b/pkg/ipamd/ipamd.go index b57dec2a1b..588bc3870a 100644 --- a/pkg/ipamd/ipamd.go +++ b/pkg/ipamd/ipamd.go @@ -43,6 +43,7 @@ import ( "github.com/aws/amazon-vpc-cni-k8s/pkg/ipamd/datastore" "github.com/aws/amazon-vpc-cni-k8s/pkg/k8sapi" "github.com/aws/amazon-vpc-cni-k8s/pkg/networkutils" + "github.com/aws/amazon-vpc-cni-k8s/pkg/utils/cniutils" "github.com/aws/amazon-vpc-cni-k8s/pkg/utils/logger" "github.com/aws/amazon-vpc-cni-k8s/utils" "github.com/aws/amazon-vpc-cni-k8s/utils/prometheusmetrics" @@ -1455,8 +1456,8 @@ func (c *IPAMContext) eniIPPoolReconcile(ipPool []string, attachedENI awsutils.E attachedENIIPs := attachedENI.IPv4Addresses needEC2Reconcile := true // Here we can't trust attachedENI since the IMDS metadata can be stale. We need to check with EC2 API. - // +1 is for the primary IP of the ENI that is not added to the ipPool and not available for pods to use. - if 1+len(ipPool) != len(attachedENIIPs) { + // IPsSimilar will exclude primary IP of the ENI that is not added to the ipPool and not available for pods to use. + if !cniutils.IPsSimilar(ipPool, attachedENIIPs) { log.Warnf("Instance metadata does not match data store! ipPool: %v, metadata: %v", ipPool, attachedENIIPs) log.Debugf("We need to check the ENI status by calling the EC2 control plane.") // Call EC2 to verify IPs on this ENI @@ -1492,14 +1493,14 @@ func (c *IPAMContext) eniIPPoolReconcile(ipPool []string, attachedENI awsutils.E } } -func (c *IPAMContext) eniPrefixPoolReconcile(ipPool []string, attachedENI awsutils.ENIMetadata, eni string) { +func (c *IPAMContext) eniPrefixPoolReconcile(prefixPool []string, attachedENI awsutils.ENIMetadata, eni string) { attachedENIIPs := attachedENI.IPv4Prefixes needEC2Reconcile := true // Here we can't trust attachedENI since the IMDS metadata can be stale. We need to check with EC2 API. - log.Debugf("Found prefix pool count %d for eni %s\n", len(ipPool), eni) + log.Debugf("Found prefix pool count %d for eni %s\n", len(prefixPool), eni) - if len(ipPool) != len(attachedENIIPs) { - log.Warnf("Instance metadata does not match data store! ipPool: %v, metadata: %v", ipPool, attachedENIIPs) + if !cniutils.PrefixSimilar(prefixPool, attachedENIIPs) { + log.Warnf("Instance metadata does not match data store! ipPool: %v, metadata: %v", prefixPool, attachedENIIPs) log.Debugf("We need to check the ENI status by calling the EC2 control plane.") // Call EC2 to verify IPs on this ENI ec2Addresses, err := c.awsClient.GetIPv4PrefixesFromEC2(eni) @@ -1515,7 +1516,7 @@ func (c *IPAMContext) eniPrefixPoolReconcile(ipPool []string, attachedENI awsuti seenIPs := c.verifyAndAddPrefixesToDatastore(eni, attachedENIIPs, needEC2Reconcile) // Sweep phase, delete remaining Prefixes since they should not remain in the datastore - for _, existingIP := range ipPool { + for _, existingIP := range prefixPool { if seenIPs[existingIP] { continue } diff --git a/pkg/utils/cniutils/cni_utils.go b/pkg/utils/cniutils/cni_utils.go index 11337dbceb..bf5520d12a 100644 --- a/pkg/utils/cniutils/cni_utils.go +++ b/pkg/utils/cniutils/cni_utils.go @@ -13,6 +13,7 @@ import ( "github.com/aws/amazon-vpc-cni-k8s/pkg/netlinkwrapper" "github.com/aws/amazon-vpc-cni-k8s/pkg/procsyswrapper" "github.com/aws/amazon-vpc-cni-k8s/utils/imds" + "github.com/aws/aws-sdk-go/service/ec2" ) const ( @@ -145,3 +146,51 @@ func IsIptableTargetNotExist(err error) bool { } return e.IsNotExist() } + +// PrefixSimilar checks if prefix pool and eni prefix are equivalent. +func PrefixSimilar(prefixPool []string, eniPrefixes []*ec2.Ipv4PrefixSpecification) bool { + if len(prefixPool) != len(eniPrefixes) { + return false + } + + prefixPoolSet := make(map[string]struct{}, len(prefixPool)) + for _, ip := range prefixPool { + prefixPoolSet[ip] = struct{}{} + } + + for _, prefix := range eniPrefixes { + if prefix == nil || prefix.Ipv4Prefix == nil { + return false + } + if _, exists := prefixPoolSet[*prefix.Ipv4Prefix]; !exists { + return false + } + } + return true +} + +// IPsSimilar checks if ipPool and eniIPs are equivalent. +func IPsSimilar(ipPool []string, eniIPs []*ec2.NetworkInterfacePrivateIpAddress) bool { + // Here we do +1 in ipPool because eniIPs will also have primary IP which is not used by pods. + if len(ipPool)+1 != len(eniIPs) { + return false + } + + ipPoolSet := make(map[string]struct{}, len(ipPool)) + for _, ip := range ipPool { + ipPoolSet[ip] = struct{}{} + } + + for _, ip := range eniIPs { + if ip == nil || ip.PrivateIpAddress == nil || ip.Primary == nil { + return false + } + if *ip.Primary { + continue + } + if _, exists := ipPoolSet[*ip.PrivateIpAddress]; !exists { + return false + } + } + return true +} diff --git a/pkg/utils/cniutils/cni_utils_test.go b/pkg/utils/cniutils/cni_utils_test.go index 4b0e81ac03..46e063ac42 100644 --- a/pkg/utils/cniutils/cni_utils_test.go +++ b/pkg/utils/cniutils/cni_utils_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" current "github.com/containernetworking/cni/pkg/types/100" "github.com/stretchr/testify/assert" ) @@ -208,3 +209,133 @@ func Test_FindIPConfigsByIfaceIndex(t *testing.T) { }) } } + +func TestPrefixSimilar(t *testing.T) { + tests := []struct { + name string + prefixPool []string + eniPrefixes []*ec2.Ipv4PrefixSpecification + want bool + }{ + { + name: "Empty slices", + prefixPool: []string{}, + eniPrefixes: []*ec2.Ipv4PrefixSpecification{}, + want: true, + }, + { + name: "Different lengths", + prefixPool: []string{"192.168.1.0/24"}, + eniPrefixes: []*ec2.Ipv4PrefixSpecification{}, + want: false, + }, + { + name: "Equivalent prefixes", + prefixPool: []string{"192.168.1.0/24", "10.0.0.0/16"}, + eniPrefixes: []*ec2.Ipv4PrefixSpecification{ + {Ipv4Prefix: stringPtr("192.168.1.0/24")}, + {Ipv4Prefix: stringPtr("10.0.0.0/16")}, + }, + want: true, + }, + { + name: "Different prefixes", + prefixPool: []string{"192.168.1.0/24", "10.0.0.0/16"}, + eniPrefixes: []*ec2.Ipv4PrefixSpecification{ + {Ipv4Prefix: stringPtr("192.168.1.0/24")}, + {Ipv4Prefix: stringPtr("172.16.0.0/16")}, + }, + want: false, + }, + { + name: "Nil prefix", + prefixPool: []string{"192.168.1.0/24"}, + eniPrefixes: []*ec2.Ipv4PrefixSpecification{ + nil, + }, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := PrefixSimilar(tt.prefixPool, tt.eniPrefixes); got != tt.want { + t.Errorf("in test %s PrefixSimilar() = %v, want %v", tt.name, got, tt.want) + } + }) + } +} + +func TestIPsSimilar(t *testing.T) { + tests := []struct { + name string + ipPool []string + eniIPs []*ec2.NetworkInterfacePrivateIpAddress + want bool + }{ + { + name: "Empty IP pool", + ipPool: []string{}, + eniIPs: []*ec2.NetworkInterfacePrivateIpAddress{ + {PrivateIpAddress: stringPtr("10.0.0.1"), Primary: boolPtr(true)}, + }, + want: true, + }, + { + name: "Different lengths", + ipPool: []string{"192.168.1.1"}, + eniIPs: []*ec2.NetworkInterfacePrivateIpAddress{ + {PrivateIpAddress: stringPtr("10.0.0.1"), Primary: boolPtr(true)}, + {PrivateIpAddress: stringPtr("192.168.1.1"), Primary: boolPtr(false)}, + {PrivateIpAddress: stringPtr("192.168.1.2"), Primary: boolPtr(false)}, + }, + want: false, + }, + { + name: "Equivalent IPs", + ipPool: []string{"192.168.1.1", "10.0.0.2"}, + eniIPs: []*ec2.NetworkInterfacePrivateIpAddress{ + {PrivateIpAddress: stringPtr("10.0.0.1"), Primary: boolPtr(true)}, + {PrivateIpAddress: stringPtr("192.168.1.1"), Primary: boolPtr(false)}, + {PrivateIpAddress: stringPtr("10.0.0.2"), Primary: boolPtr(false)}, + }, + want: true, + }, + { + name: "Different IPs", + ipPool: []string{"192.168.1.1", "10.0.0.2"}, + eniIPs: []*ec2.NetworkInterfacePrivateIpAddress{ + {PrivateIpAddress: stringPtr("10.0.0.1"), Primary: boolPtr(true)}, + {PrivateIpAddress: stringPtr("192.168.1.1"), Primary: boolPtr(false)}, + {PrivateIpAddress: stringPtr("172.16.0.1"), Primary: boolPtr(false)}, + }, + want: false, + }, + { + name: "Nil IP", + ipPool: []string{"192.168.1.1"}, + eniIPs: []*ec2.NetworkInterfacePrivateIpAddress{ + {PrivateIpAddress: stringPtr("10.0.0.1"), Primary: boolPtr(true)}, + nil, + }, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IPsSimilar(tt.ipPool, tt.eniIPs); got != tt.want { + t.Errorf("in test %s IPsSimilar() = %v, want %v", tt.name, got, tt.want) + } + }) + } +} + +// Helper functions for creating pointers +func stringPtr(s string) *string { + return &s +} + +func boolPtr(b bool) *bool { + return &b +}