Skip to content

Commit

Permalink
Merge branch 'master' into dependabot/go_modules/test/agent/github.co…
Browse files Browse the repository at this point in the history 
…m/coreos/go-iptables-0.8.0
  • Loading branch information
@orsenthil
orsenthil committed Sep 23, 2024
2 parents 377f7ca + 4d1442a commit 1de2579
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 10 deletions.
15 changes: 8 additions & 7 deletions pkg/ipamd/ipamd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
Expand Down
49 changes: 49 additions & 0 deletions pkg/utils/cniutils/cni_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
}
131 changes: 131 additions & 0 deletions pkg/utils/cniutils/cni_utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion test/agent/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.22.3
require (
github.com/coreos/go-iptables v0.8.0
github.com/vishvananda/netlink v1.1.0
golang.org/x/sys v0.22.0
golang.org/x/sys v0.24.0
)

require github.com/vishvananda/netns v0.0.0-20191106174202-0a2b9b5464df // indirect
4 changes: 2 additions & 2 deletions test/agent/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ github.com/vishvananda/netlink v1.1.0/go.mod h1:cTgwzPIzzgDAYoQrMm0EdrjRUBkTqKYp
github.com/vishvananda/netns v0.0.0-20191106174202-0a2b9b5464df h1:OviZH7qLw/7ZovXvuNyL3XQl8UFofeikI1NW1Gypu7k=
github.com/vishvananda/netns v0.0.0-20191106174202-0a2b9b5464df/go.mod h1:JP3t17pCcGlemwknint6hfoeCVQrEMVwxRLRjXpq+BU=
golang.org/x/sys v0.0.0-20190606203320-7fc4e5ec1444/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI=
golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.24.0 h1:Twjiwq9dn6R1fQcyiK+wQyHWfaz/BJB+YIpzU/Cv3Xg=
golang.org/x/sys v0.24.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=

0 comments on commit 1de2579

Please sign in to comment.