Multi card support - Prevent route override for primary ENI across mu…

…lti-cards ENAs (#1396)

* Multi card support

* Updated filterunmanaged ENIs to account for both unmanaged and CNI unmanaged

* Missed commiting this

* Fixed infinite retries

* Debug logs for CIDR not null scenarios

* pr review

go.mod

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

go.sum

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

pkg/awsutils/awsutils.go

@@ -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
+
+	//isCNIUnmanagedENI
+	IsCNIUnmanagedENI(eniID string) bool
+
+	//RefreshSGIDs
+	RefreshSGIDs(mac string) error 
+
 }
 
 // EC2InstanceMetadataCache caches instance metadata
@@ -171,6 +181,7 @@ type EC2InstanceMetadataCache struct {
 	region              string
 	unmanagedENIs       StringSet
 	useCustomNetworking bool
+	cniunmanagedENIs    StringSet
 
 	imds   TypedIMDS
 	ec2SVC ec2wrapper.EC2
@@ -219,6 +230,7 @@ type DescribeAllENIsResult struct {
 	TagMap      map[string]TagMap
 	TrunkENI    string
 	EFAENIs     map[string]bool
+	MultiCardENIIDs []string
 }
 
 // msSince returns milliseconds since start.
@@ -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():
@@ -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)
@@ -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.
@@ -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)
 
@@ -1065,6 +1074,7 @@ func (cache *EC2InstanceMetadataCache) DescribeAllENIs() (DescribeAllENIsResult,
 		TagMap:      tagMap,
 		TrunkENI:    trunkENI,
 		EFAENIs:     efaENIs,
+		MultiCardENIIDs: multiCardENIIDs,
 	}, nil
 }
 
@@ -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
+}

pkg/awsutils/awsutils_test.go

@@ -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) {
@@ -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)
 	}
 }
@@ -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)
@@ -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),
+			},
 		}},
 	}
 
@@ -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)

pkg/ipamd/ipamd.go

@@ -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)
+	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
 }
 
@@ -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
@@ -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)
 	}
 
@@ -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
@@ -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{})
-}
+}