Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi card support - Prevent route override for primary ENI across multi-cards ENAs #1396

Merged
merged 6 commits into from
Mar 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/aws/amazon-vpc-cni-k8s
go 1.14

require (
github.com/aws/aws-sdk-go v1.35.27
github.com/aws/aws-sdk-go v1.37.23
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 // indirect
github.com/containernetworking/cni v0.8.0
github.com/containernetworking/plugins v0.9.0
Expand Down Expand Up @@ -37,7 +37,7 @@ require (
go.uber.org/zap v1.15.0
golang.org/x/lint v0.0.0-20201208152925-83fdc39ff7b5 // indirect
golang.org/x/mod v0.4.0 // indirect
golang.org/x/net v0.0.0-20201021035429-f5854403a974
golang.org/x/net v0.0.0-20201110031124-69a78807bb2b
golang.org/x/sys v0.0.0-20201117170446-d9b008d0a637
golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 // indirect
golang.org/x/tools v0.0.0-20210113180300-f96436850f18 // indirect
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdko
github.com/alexflint/go-filemutex v0.0.0-20171022225611-72bdc8eae2ae/go.mod h1:CgnQgUtFrFz9mxFNtED3jI5tLDjKlOM+oUF/sTk6ps0=
github.com/aws/aws-sdk-go v1.35.27 h1:F0dUW+kouzchjt4X6kYfYMw1YtQPkA4pihpCDqQMrq8=
github.com/aws/aws-sdk-go v1.35.27/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k=
github.com/aws/aws-sdk-go v1.37.23 h1:bO80NcSmRv52w+GFpBegoLdlP/Z0OwUqQ9bbeCLCy/0=
github.com/aws/aws-sdk-go v1.37.23/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 h1:xJ4a3vCFaGF/jqvzLMYoU8P317H5OQ+Via4RmuPwCS0=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
github.com/buger/jsonparser v0.0.0-20180808090653-f4dd9f5a6b44/go.mod h1:bbYlZJ7hK1yFx9hf58LP0zeX7UjIGs20ufpu3evjr+s=
Expand Down Expand Up @@ -69,6 +71,7 @@ github.com/golang/protobuf v1.4.0-rc.1.0.20200221234624-67d41d38c208/go.mod h1:x
github.com/golang/protobuf v1.4.0-rc.2/go.mod h1:LlEzMj4AhA7rCAGe4KMBDvJI+AwstrUpVNzEA03Pprs=
github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0/go.mod h1:WU3c8KckQ9AFe+yFwt9sWVRKCVIyN9cPHBJSNnbL67w=
github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvqG2KuDX0=
github.com/golang/protobuf v1.4.2 h1:+Z5KGCizgyZCbGh1KZqA0fcLLkwbsjIzS4aV2v7wJX0=
github.com/golang/protobuf v1.4.2/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI=
github.com/google/btree v1.0.0 h1:0udJVsspx3VBr5FwtLhQQtuAsVc79tTq0ocGIPAU6qo=
github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
Expand Down Expand Up @@ -231,6 +234,7 @@ golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLL
golang.org/x/net v0.0.0-20201006153459-a7d1128ccaa0/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
golang.org/x/net v0.0.0-20201021035429-f5854403a974 h1:IX6qOQeG5uLjB/hjjwjedwfjND0hgjPMMyO1RoIXQNI=
golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand Down Expand Up @@ -307,6 +311,7 @@ google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ
google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM=
google.golang.org/protobuf v1.20.1-0.20200309200217-e05f789c0967/go.mod h1:A+miEFZTKqfCUM6K7xSMQL9OKL/b6hQv+e19PK+JZNE=
google.golang.org/protobuf v1.21.0/go.mod h1:47Nbq4nVaFHyn7ilMalzfO3qCViNmqZ2kzikPIcrTAo=
google.golang.org/protobuf v1.23.0 h1:4MY060fB1DLGMB/7MBTLnwQUY6+F09GEiz6SsrNqyzM=
google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU=
gopkg.in/airbrake/gobrake.v2 v2.0.9/go.mod h1:/h5ZAUhDkGaJfjzjKLSjv6zCL6O0LLBxU4K+aSYdM/U=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand All @@ -330,6 +335,7 @@ gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU=
gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
Expand Down
54 changes: 40 additions & 14 deletions pkg/awsutils/awsutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,16 @@ type APIs interface {

// WaitForENIAndIPsAttached waits until the ENI has been attached and the secondary IPs have been added
WaitForENIAndIPsAttached(eni string, wantedSecondaryIPs int) (ENIMetadata, error)

//SetCNIunmanaged ENI
SetCNIUnmanagedENIs(eniID []string) error
jayanthvn marked this conversation as resolved.
Show resolved Hide resolved

//isCNIUnmanagedENI
IsCNIUnmanagedENI(eniID string) bool

//RefreshSGIDs
RefreshSGIDs(mac string) error

}

// EC2InstanceMetadataCache caches instance metadata
Expand All @@ -171,6 +181,7 @@ type EC2InstanceMetadataCache struct {
region string
unmanagedENIs StringSet
useCustomNetworking bool
cniunmanagedENIs StringSet

imds TypedIMDS
ec2SVC ec2wrapper.EC2
Expand Down Expand Up @@ -219,6 +230,7 @@ type DescribeAllENIsResult struct {
TagMap map[string]TagMap
TrunkENI string
EFAENIs map[string]bool
MultiCardENIIDs []string
jayanthvn marked this conversation as resolved.
Show resolved Hide resolved
}

// msSince returns milliseconds since start.
Expand Down Expand Up @@ -397,17 +409,7 @@ func (cache *EC2InstanceMetadataCache) initWithEC2Metadata(ctx context.Context)
return err
}
log.Debugf("Found subnet-id: %s ", cache.subnetID)

// retrieve security groups
err = cache.refreshSGIDs(mac)
if err != nil {
return err
}

// Refresh security groups and VPC CIDR blocks in the background
// Ignoring errors since we will retry in 30s
go wait.Forever(func() { _ = cache.refreshSGIDs(mac) }, 30*time.Second)


// We use the ctx here for testing, since we spawn go-routines above which will run forever.
select {
case <-ctx.Done():
Expand All @@ -417,8 +419,8 @@ func (cache *EC2InstanceMetadataCache) initWithEC2Metadata(ctx context.Context)
return nil
}

// refreshSGIDs retrieves security groups
func (cache *EC2InstanceMetadataCache) refreshSGIDs(mac string) error {
// RefreshSGIDs retrieves security groups
func (cache *EC2InstanceMetadataCache) RefreshSGIDs(mac string) error {
ctx := context.TODO()

sgIDs, err := cache.imds.GetSecurityGroupIDs(ctx, mac)
Expand Down Expand Up @@ -457,7 +459,8 @@ func (cache *EC2InstanceMetadataCache) refreshSGIDs(mac string) error {
newENIs := StringSet{}
newENIs.Set(eniIDs)

filteredENIs := newENIs.Difference(&cache.unmanagedENIs)
tempfilteredENIs := newENIs.Difference(&cache.cniunmanagedENIs)
filteredENIs := tempfilteredENIs.Difference(&cache.unmanagedENIs)

sgIDsPtrs := aws.StringSlice(sgIDs)
// This will update SG for managed ENIs created by EKS.
Expand Down Expand Up @@ -1034,13 +1037,19 @@ func (cache *EC2InstanceMetadataCache) DescribeAllENIs() (DescribeAllENIsResult,

// Collect ENI response into ENI metadata and tags.
var trunkENI string
var multiCardENIIDs []string
efaENIs := make(map[string]bool, 0)
tagMap := make(map[string]TagMap, len(ec2Response.NetworkInterfaces))
for _, ec2res := range ec2Response.NetworkInterfaces {
log.Infof("Got network cardindex %v for ENI %v", aws.Int64Value(ec2res.Attachment.NetworkCardIndex), aws.StringValue(ec2res.NetworkInterfaceId))
if ec2res.Attachment != nil && aws.Int64Value(ec2res.Attachment.DeviceIndex) == 0 && !aws.BoolValue(ec2res.Attachment.DeleteOnTermination) {
log.Warn("Primary ENI will not get deleted when node terminates because 'delete_on_termination' is set to false")
}
eniID := aws.StringValue(ec2res.NetworkInterfaceId)
if aws.Int64Value(ec2res.Attachment.NetworkCardIndex) > 0 {
multiCardENIIDs = append(multiCardENIIDs, eniID)
}

eniMetadata := eniMap[eniID]
interfaceType := aws.StringValue(ec2res.InterfaceType)

Expand All @@ -1065,6 +1074,7 @@ func (cache *EC2InstanceMetadataCache) DescribeAllENIs() (DescribeAllENIsResult,
TagMap: tagMap,
TrunkENI: trunkENI,
EFAENIs: efaENIs,
MultiCardENIIDs: multiCardENIIDs,
}, nil
}

Expand Down Expand Up @@ -1507,3 +1517,19 @@ func (cache *EC2InstanceMetadataCache) getENIsFromPaginatedDescribeNetworkInterf
}
return innerErr
}

//SetCNIUnmanagedENIs Set unmanaged ENI set
func (cache *EC2InstanceMetadataCache) SetCNIUnmanagedENIs(eniID []string) error {
if len(eniID) != 0 {
cache.cniunmanagedENIs.Set(eniID)
}
return nil
}

//IsCNIUnmanagedENI returns if the eni is unmanaged
func (cache *EC2InstanceMetadataCache) IsCNIUnmanagedENI(eniID string) bool {
if len(eniID) != 0 {
return cache.cniunmanagedENIs.Has(eniID)
}
return false
}
10 changes: 3 additions & 7 deletions pkg/awsutils/awsutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,6 @@ func TestInitWithEC2metadata(t *testing.T) {
defer ctrl.Finish()
mockMetadata := testMetadata(nil)

mockEC2.EXPECT().ModifyNetworkInterfaceAttributeWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil)

ins := &EC2InstanceMetadataCache{imds: TypedIMDS{mockMetadata}, ec2SVC: mockEC2}
err := ins.initWithEC2Metadata(ctx)
if assert.NoError(t, err) {
Expand All @@ -120,7 +118,6 @@ func TestInitWithEC2metadata(t *testing.T) {
assert.Equal(t, ins.instanceID, instanceID)
assert.Equal(t, ins.primaryENImac, primaryMAC)
assert.Equal(t, ins.primaryENI, primaryeniID)
assert.Equal(t, len(ins.securityGroups.SortedList()), 2)
assert.Equal(t, subnetID, ins.subnetID)
}
}
Expand All @@ -132,8 +129,6 @@ func TestInitWithEC2metadataErr(t *testing.T) {
ctrl, mockEC2 := setup(t)
defer ctrl.Finish()

mockEC2.EXPECT().ModifyNetworkInterfaceAttributeWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil)

var keys []string
for k := range testMetadata(nil) {
keys = append(keys, k)
Expand Down Expand Up @@ -275,6 +270,9 @@ func TestDescribeAllENIs(t *testing.T) {
TagSet: []*ec2.Tag{
{Key: aws.String("foo"), Value: aws.String("foo-value")},
},
Attachment: &ec2.NetworkInterfaceAttachment{
NetworkCardIndex: aws.Int64(0),
},
}},
}

Expand Down Expand Up @@ -313,8 +311,6 @@ func TestTagEni(t *testing.T) {
defer ctrl.Finish()
mockMetadata := testMetadata(nil)

mockEC2.EXPECT().ModifyNetworkInterfaceAttributeWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil)

ins := &EC2InstanceMetadataCache{imds: TypedIMDS{mockMetadata}, ec2SVC: mockEC2}

err := ins.initWithEC2Metadata(ctx)
Expand Down
47 changes: 44 additions & 3 deletions pkg/awsutils/mocks/awsutils_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions pkg/ec2wrapper/mocks/ec2wrapper_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 24 additions & 1 deletion pkg/ipamd/ipamd.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,17 @@ func New(k8sapiClient kubernetes.Interface, eniConfig *eniconfig.ENIConfigContro
if err != nil {
return nil, err
}

mac := c.awsClient.GetPrimaryENImac()
// retrieve security groups
err = c.awsClient.RefreshSGIDs(mac)
jayanthvn marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}

// Refresh security groups and VPC CIDR blocks in the background
// Ignoring errors since we will retry in 30s
go wait.Forever(func() { _ = c.awsClient.RefreshSGIDs(mac) }, 30*time.Second)
return c, nil
}

Expand Down Expand Up @@ -347,15 +358,21 @@ func (c *IPAMContext) nodeInit() error {
return errors.New("ipamd init: failed to retrieve attached ENIs info")
}
log.Debugf("DescribeAllENIs success: ENIs: %d, tagged: %d", len(metadataResult.ENIMetadata), len(metadataResult.TagMap))
c.awsClient.SetCNIUnmanagedENIs(metadataResult.MultiCardENIIDs)
c.setUnmanagedENIs(metadataResult.TagMap)
enis := c.filterUnmanagedENIs(metadataResult.ENIMetadata)

for _, eni := range enis {
log.Debugf("Discovered ENI %s, trying to set it up", eni.ENIID)
// Retry ENI sync
if c.awsClient.IsCNIUnmanagedENI(eni.ENIID) {
log.Infof("Skipping ENI %s since it is not on network card 0", eni.ENIID)
continue
}
retry := 0
for {
retry++

if err = c.setupENI(eni.ENIID, eni, eni.ENIID == metadataResult.TrunkENI, metadataResult.EFAENIs[eni.ENIID]); err == nil {
log.Infof("ENI %s set up.", eni.ENIID)
break
Expand Down Expand Up @@ -969,6 +986,7 @@ func (c *IPAMContext) nodeIPPoolReconcile(interval time.Duration) {
// Just copy values of the EFA set
efaENIs = metadataResult.EFAENIs
c.setUnmanagedENIs(metadataResult.TagMap)
c.awsClient.SetCNIUnmanagedENIs(metadataResult.MultiCardENIIDs)
attachedENIs = c.filterUnmanagedENIs(metadataResult.ENIMetadata)
}

Expand Down Expand Up @@ -1196,7 +1214,12 @@ func (c *IPAMContext) filterUnmanagedENIs(enis []awsutils.ENIMetadata) []awsutil
log.Debugf("Skipping ENI %s: tagged with %s", eni.ENIID, eniNoManageTagKey)
numFiltered++
continue
} else if c.awsClient.IsCNIUnmanagedENI(eni.ENIID) {
log.Debugf("Skipping ENI %s: since on non-zero network card", eni.ENIID)
numFiltered++
continue
}

ret = append(ret, eni)
}
c.unmanagedENI = numFiltered
Expand Down Expand Up @@ -1320,4 +1343,4 @@ func (c *IPAMContext) SetNodeLabel(key, value string) error {
// GetPod returns the pod matching the name and namespace
func (c *IPAMContext) GetPod(podName, namespace string) (*v1.Pod, error) {
return c.k8sClient.CoreV1().Pods(namespace).Get(podName, metav1.GetOptions{})
}
}
Loading