From eb5e76d1ba12045d1d6aad1d49f131b9cc4f2916 Mon Sep 17 00:00:00 2001 From: M00nF1sh Date: Fri, 4 Jun 2021 15:36:05 -0700 Subject: [PATCH] Refine ENI Tagging logic 1. All ENIs created by IPAM-D will be tagged with all desired tags on creation. 2. All ENIs managed by IPAM-D will be tagged with all desired tags if not already tagged with these tags. Other tags on ENI will be kept as it is. 1. Trunk ENI is excluded, as it's lifecycle is managed by vpc-resource-controller. 2. Since we tag ENIs on ENI creation, this backfill logic will only trigger for below cases: * ENIs created by previous versions of ENI and the desired tag set changed. * ENIs attached to instances from external source without the node.k8s.amazonaws.com/no_manage tag. 3. The desired tag set is: * node.k8s.amazonaws.com/instance_id: * cluster.k8s.amazonaws.com/name: if CLUSTER_NAME envVar is specified. * additional tags specified if ADDITIONAL_ENI_TAGS envVar is specified. --- pkg/awsutils/awsutils.go | 243 ++--- pkg/awsutils/awsutils_test.go | 990 ++++++++++++++++---- pkg/awsutils/mocks/awsutils_mocks.go | 14 + pkg/ipamd/ipamd.go | 29 +- pkg/ipamd/ipamd_test.go | 2 + test/agent/cmd/networking/tester/network.go | 2 +- 6 files changed, 992 insertions(+), 288 deletions(-) diff --git a/pkg/awsutils/awsutils.go b/pkg/awsutils/awsutils.go index 2c80ebe750..3f2df962ff 100644 --- a/pkg/awsutils/awsutils.go +++ b/pkg/awsutils/awsutils.go @@ -111,6 +111,9 @@ type APIs interface { // FreeENI detaches ENI interface and deletes it FreeENI(eniName string) error + // TagENI Tags ENI with current tags to contain expected tags. + TagENI(eniID string, currentTags map[string]string) error + // GetAttachedENIs retrieves eni information from instance metadata service GetAttachedENIs() (eniList []ENIMetadata, err error) @@ -169,19 +172,23 @@ type APIs interface { // EC2InstanceMetadataCache caches instance metadata type EC2InstanceMetadataCache struct { // metadata info - securityGroups StringSet - subnetID string - localIPv4 net.IP - instanceID string - instanceType string - primaryENI string - primaryENImac string - availabilityZone string - region string + securityGroups StringSet + subnetID string + localIPv4 net.IP + instanceID string + instanceType string + primaryENI string + primaryENImac string + availabilityZone string + region string + unmanagedENIs StringSet useCustomNetworking bool cniunmanagedENIs StringSet + clusterName string + additionalENITags map[string]string + imds TypedIMDS ec2SVC ec2wrapper.EC2 } @@ -328,6 +335,8 @@ func New(useCustomNetworking bool) (*EC2InstanceMetadataCache, error) { cache := &EC2InstanceMetadataCache{} cache.imds = TypedIMDS{instrumentedIMDS{ec2Metadata}} + cache.clusterName = os.Getenv(clusterNameEnvVar) + cache.additionalENITags = loadAdditionalENITags() region, err := ec2Metadata.Region() if err != nil { @@ -627,9 +636,6 @@ func (cache *EC2InstanceMetadataCache) AllocENI(useCustomCfg bool, sg []*string, return "", errors.Wrap(err, "AllocENI: error attaching ENI") } - // Once the ENI is attached, tag it. - cache.tagENI(eniID, maxENIBackoffDelay) - // Also change the ENI's attribute so that the ENI will be deleted when the instance is deleted. attributeInput := &ec2.ModifyNetworkInterfaceAttributeInput{ Attachment: &ec2.NetworkInterfaceAttachmentChanges{ @@ -682,25 +688,24 @@ func (cache *EC2InstanceMetadataCache) attachENI(eniID string) (string, error) { // return ENI id, error func (cache *EC2InstanceMetadataCache) createENI(useCustomCfg bool, sg []*string, subnet string) (string, error) { eniDescription := eniDescriptionPrefix + cache.instanceID - //Tag on create with create TS - tags := []*ec2.Tag{ - { - Key: aws.String(eniCreatedAtTagKey), - Value: aws.String(time.Now().Format(time.RFC3339)), - }, + tags := map[string]string{ + eniCreatedAtTagKey: time.Now().Format(time.RFC3339), } - resourceType := "network-interface" - tagspec := []*ec2.TagSpecification{ + for key, value := range cache.buildENITags() { + tags[key] = value + } + tagSpec := []*ec2.TagSpecification{ { - ResourceType: &resourceType, - Tags: tags, + ResourceType: aws.String(ec2.ResourceTypeNetworkInterface), + Tags: convertTagsToSDKTags(tags), }, } + input := &ec2.CreateNetworkInterfaceInput{ Description: aws.String(eniDescription), Groups: aws.StringSlice(cache.securityGroups.SortedList()), SubnetId: aws.String(cache.subnetID), - TagSpecifications: tagspec, + TagSpecifications: tagSpec, } if useCustomCfg { @@ -715,11 +720,8 @@ func (cache *EC2InstanceMetadataCache) createENI(useCustomCfg bool, sg []*string log.Info("Using same config as the primary interface for the new ENI") } - var sgs []string - for i := range input.Groups { - sgs = append(sgs, *input.Groups[i]) - } - log.Infof("Creating ENI with security groups: %v in subnet: %s", sgs, *input.SubnetId) + log.Infof("Creating ENI with security groups: %v in subnet: %s", aws.StringValueSlice(input.Groups), aws.StringValue(input.SubnetId)) + start := time.Now() result, err := cache.ec2SVC.CreateNetworkInterfaceWithContext(context.Background(), input) awsAPILatency.WithLabelValues("CreateNetworkInterface", fmt.Sprint(err != nil), awsReqStatus(err)).Observe(msSince(start)) @@ -732,47 +734,43 @@ func (cache *EC2InstanceMetadataCache) createENI(useCustomCfg bool, sg []*string return aws.StringValue(result.NetworkInterface.NetworkInterfaceId), nil } -func (cache *EC2InstanceMetadataCache) tagENI(eniID string, maxBackoffDelay time.Duration) { - // Tag the ENI with "node.k8s.amazonaws.com/instance_id=" - tags := []*ec2.Tag{ - { - Key: aws.String(eniNodeTagKey), - Value: aws.String(cache.instanceID), - }, +// buildENITags computes the desired AWS Tags for eni +func (cache *EC2InstanceMetadataCache) buildENITags() map[string]string { + tags := map[string]string{ + eniNodeTagKey: cache.instanceID, } - // If the CLUSTER_NAME env var is present, + // If clusterName is provided, // tag the ENI with "cluster.k8s.amazonaws.com/name=" - clusterName := os.Getenv(clusterNameEnvVar) - if clusterName != "" { - tags = append(tags, &ec2.Tag{ - Key: aws.String(eniClusterTagKey), - Value: aws.String(clusterName), - }) + if cache.clusterName != "" { + tags[eniClusterTagKey] = cache.clusterName + } + for key, value := range cache.additionalENITags { + tags[key] = value } + return tags +} - //additionalEniTags for adding additional tags on ENI - additionalEniTags := os.Getenv(additionalEniTagsEnvVar) - if additionalEniTags != "" { - tagsMap, err := parseAdditionalEniTagsMap(additionalEniTags) - if err != nil { - log.Warnf("Failed to add additional tags to the newly created ENI %s: %v", eniID, err) +func (cache *EC2InstanceMetadataCache) TagENI(eniID string, currentTags map[string]string) error { + tagChanges := make(map[string]string) + for tagKey, tagValue := range cache.buildENITags() { + if currentTagValue, ok := currentTags[tagKey]; !ok || currentTagValue != tagValue { + tagChanges[tagKey] = tagValue } - tags = mapToTags(tagsMap, tags) } - - for _, tag := range tags { - log.Debugf("Trying to tag newly created ENI: key=%s, value=%s", aws.StringValue(tag.Key), aws.StringValue(tag.Value)) + if len(tagChanges) == 0 { + return nil } input := &ec2.CreateTagsInput{ Resources: []*string{ aws.String(eniID), }, - Tags: tags, + Tags: convertTagsToSDKTags(tagChanges), } - _ = retry.NWithBackoff(retry.NewSimpleBackoff(500*time.Millisecond, maxBackoffDelay, 0.3, 2), 5, func() error { + log.Debugf("Tagging ENI %s with missing tags: %v", eniID, tagChanges) + return retry.NWithBackoff(retry.NewSimpleBackoff(500*time.Millisecond, maxENIBackoffDelay, 0.3, 2), 5, func() error { start := time.Now() _, err := cache.ec2SVC.CreateTagsWithContext(context.Background(), input) awsAPILatency.WithLabelValues("CreateTags", fmt.Sprint(err != nil), awsReqStatus(err)).Observe(msSince(start)) @@ -786,38 +784,6 @@ func (cache *EC2InstanceMetadataCache) tagENI(eniID string, maxBackoffDelay time }) } -//parseAdditionalEniTagsMap will create a map for additional tags -func parseAdditionalEniTagsMap(additionalEniTags string) (map[string]string, error) { - var additionalEniTagsMap map[string]string - - // If duplicate keys exist, the value of the key will be the value of latter key. - err := json.Unmarshal([]byte(additionalEniTags), &additionalEniTagsMap) - if err != nil { - log.Errorf("Invalid format for ADDITIONAL_ENI_TAGS. Expected a json hash: %v", err) - } - return additionalEniTagsMap, err -} - -// MapToTags converts a map to a slice of tags. -func mapToTags(tagsMap map[string]string, tags []*ec2.Tag) []*ec2.Tag { - if tagsMap == nil { - return tags - } - - for key, value := range tagsMap { - keyPrefix := reservedTagKeyPrefix - if strings.Contains(key, keyPrefix) { - log.Warnf("Additional tags has %s prefix. Ignoring %s tag as it is reserved", keyPrefix, key) - continue - } - tags = append(tags, &ec2.Tag{ - Key: aws.String(key), - Value: aws.String(value), - }) - } - return tags -} - // containsPrivateIPAddressLimitExceededError returns whether exceeds ENI's IP address limit func containsPrivateIPAddressLimitExceededError(err error) bool { if aerr, ok := err.(awserr.Error); ok { @@ -1072,10 +1038,7 @@ func (cache *EC2InstanceMetadataCache) DescribeAllENIs() (DescribeAllENIsResult, } // Check IPv4 addresses logOutOfSyncState(eniID, eniMetadata.IPv4Addresses, ec2res.PrivateIpAddresses) - tags := getTags(ec2res, eniMetadata.ENIID) - if len(tags) > 0 { - tagMap[eniMetadata.ENIID] = tags - } + tagMap[eniMetadata.ENIID] = convertSDKTagsToTags(ec2res.TagSet) } return DescribeAllENIsResult{ ENIMetadata: verifiedENIs, @@ -1086,17 +1049,59 @@ func (cache *EC2InstanceMetadataCache) DescribeAllENIs() (DescribeAllENIsResult, }, nil } -// getTags collects tags from an EC2 DescribeNetworkInterfaces call -func getTags(ec2res *ec2.NetworkInterface, eniID string) map[string]string { - tags := make(map[string]string, len(ec2res.TagSet)) - for _, tag := range ec2res.TagSet { - if tag.Key == nil || tag.Value == nil { - log.Errorf("nil tag on ENI: %v", eniID) - continue +// convertTagsToSDKTags converts tags in stringMap format to AWS SDK format +func convertTagsToSDKTags(tagsMap map[string]string) []*ec2.Tag { + if len(tagsMap) == 0 { + return nil + } + + sdkTags := make([]*ec2.Tag, 0, len(tagsMap)) + for _, key := range sets.StringKeySet(tagsMap).List() { + sdkTags = append(sdkTags, &ec2.Tag{ + Key: aws.String(key), + Value: aws.String(tagsMap[key]), + }) + } + return sdkTags +} + +// convertSDKTagsToTags converts tags in AWS SDKs format to stringMap format +func convertSDKTagsToTags(sdkTags []*ec2.Tag) map[string]string { + if len(sdkTags) == 0 { + return nil + } + + tagsMap := make(map[string]string, len(sdkTags)) + for _, sdkTag := range sdkTags { + tagsMap[aws.StringValue(sdkTag.Key)] = aws.StringValue(sdkTag.Value) + } + return tagsMap +} + +// loadAdditionalENITags will load the additional ENI Tags from environment variables. +func loadAdditionalENITags() map[string]string { + additionalENITagsStr := os.Getenv(additionalEniTagsEnvVar) + if additionalENITagsStr == "" { + return nil + } + + // TODO: ideally we should fail in CNI init phase if the validation fails instead of warn. + // currently we only warn to be backwards-compatible and keep changes minimal in this version. + + var additionalENITags map[string]string + // If duplicate keys exist, the value of the key will be the value of latter key. + err := json.Unmarshal([]byte(additionalENITagsStr), &additionalENITags) + if err != nil { + log.Warnf("failed to parse additional ENI Tags from env %v due to %v", additionalEniTagsEnvVar, err) + return nil + } + for key := range additionalENITags { + if strings.Contains(key, reservedTagKeyPrefix) { + log.Warnf("ignoring tagKey %v from additional ENI Tags as it contains reserved prefix %v", key, reservedTagKeyPrefix) + delete(additionalENITags, key) } - tags[*tag.Key] = *tag.Value } - return tags + return additionalENITags } var eniErrorMessageRegex = regexp.MustCompile("'([a-zA-Z0-9-]+)'") @@ -1338,7 +1343,7 @@ func (cache *EC2InstanceMetadataCache) cleanUpLeakedENIsInternal(startupDelay ti time.Sleep(startupDelay) log.Debug("Checking for leaked AWS CNI ENIs.") - networkInterfaces, err := cache.getFilteredListOfNetworkInterfaces() + networkInterfaces, err := cache.getLeakedENIs() if err != nil { log.Warnf("Unable to get leaked ENIs: %v", err) } else { @@ -1388,26 +1393,34 @@ func (cache *EC2InstanceMetadataCache) tagENIcreateTS(eniID string, maxBackoffDe }) } -// getFilteredListOfNetworkInterfaces calls DescribeNetworkInterfaces to get all available ENIs that were allocated by +// getLeakedENIs calls DescribeNetworkInterfaces to get all available ENIs that were allocated by // the AWS CNI plugin, but were not deleted. -func (cache *EC2InstanceMetadataCache) getFilteredListOfNetworkInterfaces() ([]*ec2.NetworkInterface, error) { - // The tag key has to be "node.k8s.amazonaws.com/instance_id" - tagFilter := &ec2.Filter{ - Name: aws.String("tag-key"), - Values: []*string{ - aws.String(eniNodeTagKey), +func (cache *EC2InstanceMetadataCache) getLeakedENIs() ([]*ec2.NetworkInterface, error) { + leakedENIFilters := []*ec2.Filter{ + { + Name: aws.String("tag-key"), + Values: []*string{ + aws.String(eniNodeTagKey), + }, }, - } - // Only fetch "available" ENIs. - statusFilter := &ec2.Filter{ - Name: aws.String("status"), - Values: []*string{ - aws.String("available"), + { + Name: aws.String("status"), + Values: []*string{ + aws.String(ec2.NetworkInterfaceStatusAvailable), + }, }, } + if cache.clusterName != "" { + leakedENIFilters = append(leakedENIFilters, &ec2.Filter{ + Name: aws.String(fmt.Sprintf("tag:%s", eniClusterTagKey)), + Values: []*string{ + aws.String(cache.clusterName), + }, + }) + } input := &ec2.DescribeNetworkInterfacesInput{ - Filters: []*ec2.Filter{tagFilter, statusFilter}, + Filters: leakedENIFilters, MaxResults: aws.Int64(describeENIPageSize), } @@ -1418,7 +1431,7 @@ func (cache *EC2InstanceMetadataCache) getFilteredListOfNetworkInterfaces() ([]* return nil } // Check that it's not a newly created ENI - tags := getTags(networkInterface, aws.StringValue(networkInterface.NetworkInterfaceId)) + tags := convertSDKTagsToTags(networkInterface.TagSet) if value, ok := tags[eniCreatedAtTagKey]; ok { parsedTime, err := time.Parse(time.RFC3339, value) diff --git a/pkg/awsutils/awsutils_test.go b/pkg/awsutils/awsutils_test.go index 69b253d731..9ae41c97f8 100644 --- a/pkg/awsutils/awsutils_test.go +++ b/pkg/awsutils/awsutils_test.go @@ -19,7 +19,6 @@ import ( "fmt" "os" "reflect" - "sort" "strconv" "testing" "time" @@ -304,112 +303,6 @@ func TestDescribeAllENIs(t *testing.T) { } } -func TestTagEni(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) - defer cancel() - ctrl, mockEC2 := setup(t) - defer ctrl.Finish() - mockMetadata := testMetadata(nil) - - ins := &EC2InstanceMetadataCache{imds: TypedIMDS{mockMetadata}, ec2SVC: mockEC2} - - err := ins.initWithEC2Metadata(ctx) - assert.NoError(t, err) - mockEC2.EXPECT().CreateTagsWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("tagging failed")) - mockEC2.EXPECT().CreateTagsWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("tagging failed")) - mockEC2.EXPECT().CreateTagsWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("tagging failed")) - mockEC2.EXPECT().CreateTagsWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("tagging failed")) - mockEC2.EXPECT().CreateTagsWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) - - ins.tagENI(eniID, time.Millisecond) - assert.NoError(t, err) -} - -func TestClusterNameTag(t *testing.T) { - ctrl, mockEC2 := setup(t) - defer ctrl.Finish() - _ = os.Setenv(clusterNameEnvVar, "cni-test") - tagKey1 := eniClusterTagKey - tagValue1 := "cni-test" - additionalEniTags := ec2.Tag{ - Key: &tagKey1, - Value: &tagValue1, - } - tags := []*ec2.Tag{ - { - Key: aws.String(eniNodeTagKey), - Value: aws.String(instanceID), - }, - } - tags = append(tags, &additionalEniTags) - input := &ec2.CreateTagsInput{ - Resources: []*string{ - aws.String(eniID), - }, - Tags: tags, - } - - ins := &EC2InstanceMetadataCache{instanceID: instanceID, ec2SVC: mockEC2} - mockEC2.EXPECT().CreateTagsWithContext(gomock.Any(), input, gomock.Any()).Return(nil, nil) - ins.tagENI(eniID, time.Millisecond) - _ = os.Unsetenv(clusterNameEnvVar) -} - -func TestAdditionalTagsEni(t *testing.T) { - ctrl, mockEC2 := setup(t) - defer ctrl.Finish() - _ = os.Setenv(additionalEniTagsEnvVar, `{"testKey": "testing"}`) - tagKey1 := "testKey" - tagValue1 := "testing" - additionalEniTags := ec2.Tag{ - Key: &tagKey1, - Value: &tagValue1, - } - tags := []*ec2.Tag{ - { - Key: aws.String(eniNodeTagKey), - Value: aws.String(instanceID), - }, - } - tags = append(tags, &additionalEniTags) - input := &ec2.CreateTagsInput{ - Resources: []*string{ - aws.String(eniID), - }, - Tags: tags, - } - - ins := &EC2InstanceMetadataCache{instanceID: instanceID, ec2SVC: mockEC2} - mockEC2.EXPECT().CreateTagsWithContext(gomock.Any(), input, gomock.Any()).Return(nil, nil) - ins.tagENI(eniID, time.Millisecond) - os.Unsetenv(additionalEniTagsEnvVar) -} - -func TestMapToTags(t *testing.T) { - tagKey1 := "tagKey1" - tagKey2 := "tagKey2" - tagValue1 := "tagValue1" - tagValue2 := "tagValue2" - tagKey3 := "cluster.k8s.amazonaws.com/name" - tagValue3 := "clusterName" - tagsMap := map[string]string{ - tagKey1: tagValue1, - tagKey2: tagValue2, - tagKey3: tagValue3, - } - tags := make([]*ec2.Tag, 0) - tags = mapToTags(tagsMap, tags) - assert.Equal(t, 2, len(tags)) - sort.Slice(tags, func(i, j int) bool { - return aws.StringValue(tags[i].Key) < aws.StringValue(tags[j].Key) - }) - - assert.Equal(t, aws.StringValue(tags[0].Key), tagKey1) - assert.Equal(t, aws.StringValue(tags[0].Value), tagValue1) - assert.Equal(t, aws.StringValue(tags[1].Key), tagKey2) - assert.Equal(t, aws.StringValue(tags[1].Value), tagValue2) -} - func TestAllocENI(t *testing.T) { ctrl, mockEC2 := setup(t) defer ctrl.Finish() @@ -438,7 +331,6 @@ func TestAllocENI(t *testing.T) { attachResult := &ec2.AttachNetworkInterfaceOutput{ AttachmentId: &attachmentID} mockEC2.EXPECT().AttachNetworkInterfaceWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(attachResult, nil) - mockEC2.EXPECT().CreateTagsWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) mockEC2.EXPECT().ModifyNetworkInterfaceAttributeWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) ins := &EC2InstanceMetadataCache{ @@ -683,66 +575,6 @@ func TestAllocIPAddressesAlreadyFull(t *testing.T) { assert.NoError(t, err) } -func TestEC2InstanceMetadataCache_getFilteredListOfNetworkInterfaces_OneResult(t *testing.T) { - ctrl, mockEC2 := setup(t) - defer ctrl.Finish() - - attachmentID := eniAttachID - description := eniDescriptionPrefix + "test" - status := "available" - - tag := []*ec2.Tag{ - { - Key: aws.String(eniNodeTagKey), - Value: aws.String("test"), - }, - } - - timein := time.Now().Local().Add(time.Minute * time.Duration(-10)) - - tag = append(tag, &ec2.Tag{ - Key: aws.String(eniCreatedAtTagKey), - Value: aws.String(timein.Format(time.RFC3339)), - }) - attachment := &ec2.NetworkInterfaceAttachment{AttachmentId: &attachmentID} - cureniID := eniID - - interfaces := []*ec2.NetworkInterface{{Attachment: attachment, Status: &status, TagSet: tag, Description: &description, NetworkInterfaceId: &cureniID}} - setupDescribeNetworkInterfacesPagesWithContextMock(t, mockEC2, interfaces, nil, 1) - ins := &EC2InstanceMetadataCache{ec2SVC: mockEC2} - got, err := ins.getFilteredListOfNetworkInterfaces() - assert.NotNil(t, got) - assert.NoError(t, err) -} - -func TestEC2InstanceMetadataCache_getFilteredListOfNetworkInterfaces_NoResult(t *testing.T) { - ctrl, mockEC2 := setup(t) - defer ctrl.Finish() - - setupDescribeNetworkInterfacesPagesWithContextMock(t, mockEC2, []*ec2.NetworkInterface{}, nil, 1) - ins := &EC2InstanceMetadataCache{ec2SVC: mockEC2} - got, err := ins.getFilteredListOfNetworkInterfaces() - assert.Nil(t, got) - assert.NoError(t, err) -} - -func TestEC2InstanceMetadataCache_getFilteredListOfNetworkInterfaces_Error(t *testing.T) { - ctrl, mockEC2 := setup(t) - defer ctrl.Finish() - - interfaces := []*ec2.NetworkInterface{{ - TagSet: []*ec2.Tag{ - {Key: aws.String("foo"), Value: aws.String("foo-value")}, - }, - }} - setupDescribeNetworkInterfacesPagesWithContextMock(t, mockEC2, interfaces, errors.New("dummy error"), 1) - - ins := &EC2InstanceMetadataCache{ec2SVC: mockEC2} - got, err := ins.getFilteredListOfNetworkInterfaces() - assert.Nil(t, got) - assert.Error(t, err) -} - func Test_badENIID(t *testing.T) { tests := []struct { name string @@ -880,3 +712,825 @@ func setupDescribeNetworkInterfacesPagesWithContextMock( return err }) } + +func TestEC2InstanceMetadataCache_buildENITags(t *testing.T) { + type fields struct { + instanceID string + clusterName string + additionalENITags map[string]string + } + tests := []struct { + name string + fields fields + want map[string]string + }{ + { + name: "without clusterName or additionalENITags", + fields: fields{ + instanceID: "i-xxxxx", + }, + want: map[string]string{ + "node.k8s.amazonaws.com/instance_id": "i-xxxxx", + }, + }, + { + name: "with clusterName", + fields: fields{ + instanceID: "i-xxxxx", + clusterName: "awesome-cluster", + }, + want: map[string]string{ + "node.k8s.amazonaws.com/instance_id": "i-xxxxx", + "cluster.k8s.amazonaws.com/name": "awesome-cluster", + }, + }, + { + name: "with additional ENI tags", + fields: fields{ + instanceID: "i-xxxxx", + additionalENITags: map[string]string{ + "tagKey-1": "tagVal-1", + "tagKey-2": "tagVal-2", + }, + }, + want: map[string]string{ + "node.k8s.amazonaws.com/instance_id": "i-xxxxx", + "tagKey-1": "tagVal-1", + "tagKey-2": "tagVal-2", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cache := &EC2InstanceMetadataCache{ + instanceID: tt.fields.instanceID, + clusterName: tt.fields.clusterName, + additionalENITags: tt.fields.additionalENITags, + } + got := cache.buildENITags() + assert.Equal(t, tt.want, got) + }) + } +} + +func TestEC2InstanceMetadataCache_getLeakedENIs(t *testing.T) { + tenMinuteAgo := time.Now().Local().Add(time.Minute * time.Duration(-10)) + now := time.Now().Local() + type describeNetworkInterfacePagesCall struct { + input *ec2.DescribeNetworkInterfacesInput + outputPages []*ec2.DescribeNetworkInterfacesOutput + err error + } + type fields struct { + clusterName string + describeNetworkInterfacePagesCalls []describeNetworkInterfacePagesCall + } + tests := []struct { + name string + fields fields + want []*ec2.NetworkInterface + wantErr error + }{ + { + name: "without clusterName - no leaked ENIs", + fields: fields{ + clusterName: "", + describeNetworkInterfacePagesCalls: []describeNetworkInterfacePagesCall{ + { + input: &ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("tag-key"), + Values: []*string{aws.String("node.k8s.amazonaws.com/instance_id")}, + }, + { + Name: aws.String("status"), + Values: []*string{aws.String("available")}, + }, + }, + MaxResults: aws.Int64(1000), + }, + outputPages: []*ec2.DescribeNetworkInterfacesOutput{ + { + NetworkInterfaces: nil, + }, + }, + }, + }, + }, + want: nil, + }, + { + name: "without clusterName - one ENI leaked", + fields: fields{ + clusterName: "", + describeNetworkInterfacePagesCalls: []describeNetworkInterfacePagesCall{ + { + input: &ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("tag-key"), + Values: []*string{aws.String("node.k8s.amazonaws.com/instance_id")}, + }, + { + Name: aws.String("status"), + Values: []*string{aws.String("available")}, + }, + }, + MaxResults: aws.Int64(1000), + }, + outputPages: []*ec2.DescribeNetworkInterfacesOutput{ + { + NetworkInterfaces: []*ec2.NetworkInterface{ + { + NetworkInterfaceId: aws.String("eni-1"), + Description: aws.String("aws-K8S-i-xxxxx"), + Status: aws.String("available"), + TagSet: []*ec2.Tag{ + { + Key: aws.String("node.k8s.amazonaws.com/instance_id"), + Value: aws.String("i-xxxxx"), + }, + { + Key: aws.String("node.k8s.amazonaws.com/createdAt"), + Value: aws.String(tenMinuteAgo.Format(time.RFC3339)), + }, + }, + }, + }, + }, + }, + }, + }, + }, + want: []*ec2.NetworkInterface{ + { + NetworkInterfaceId: aws.String("eni-1"), + Description: aws.String("aws-K8S-i-xxxxx"), + Status: aws.String("available"), + TagSet: []*ec2.Tag{ + { + Key: aws.String("node.k8s.amazonaws.com/instance_id"), + Value: aws.String("i-xxxxx"), + }, + { + Key: aws.String("node.k8s.amazonaws.com/createdAt"), + Value: aws.String(tenMinuteAgo.Format(time.RFC3339)), + }, + }, + }, + }, + }, + { + name: "without clusterName - one ENI - description didn't match", + fields: fields{ + clusterName: "", + describeNetworkInterfacePagesCalls: []describeNetworkInterfacePagesCall{ + { + input: &ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("tag-key"), + Values: []*string{aws.String("node.k8s.amazonaws.com/instance_id")}, + }, + { + Name: aws.String("status"), + Values: []*string{aws.String("available")}, + }, + }, + MaxResults: aws.Int64(1000), + }, + outputPages: []*ec2.DescribeNetworkInterfacesOutput{ + { + NetworkInterfaces: []*ec2.NetworkInterface{ + { + NetworkInterfaceId: aws.String("eni-1"), + Description: aws.String("non-k8s-i-xxxxx"), + Status: aws.String("available"), + TagSet: []*ec2.Tag{ + { + Key: aws.String("node.k8s.amazonaws.com/instance_id"), + Value: aws.String("i-xxxxx"), + }, + { + Key: aws.String("node.k8s.amazonaws.com/createdAt"), + Value: aws.String(tenMinuteAgo.Format(time.RFC3339)), + }, + }, + }, + }, + }, + }, + }, + }, + }, + want: nil, + }, + { + name: "without clusterName - one ENI - creationTime within deletion coolDown", + fields: fields{ + clusterName: "", + describeNetworkInterfacePagesCalls: []describeNetworkInterfacePagesCall{ + { + input: &ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("tag-key"), + Values: []*string{aws.String("node.k8s.amazonaws.com/instance_id")}, + }, + { + Name: aws.String("status"), + Values: []*string{aws.String("available")}, + }, + }, + MaxResults: aws.Int64(1000), + }, + outputPages: []*ec2.DescribeNetworkInterfacesOutput{ + { + NetworkInterfaces: []*ec2.NetworkInterface{ + { + NetworkInterfaceId: aws.String("eni-1"), + Description: aws.String("aws-K8S-i-xxxxx"), + Status: aws.String("available"), + TagSet: []*ec2.Tag{ + { + Key: aws.String("node.k8s.amazonaws.com/instance_id"), + Value: aws.String("i-xxxxx"), + }, + { + Key: aws.String("node.k8s.amazonaws.com/createdAt"), + Value: aws.String(now.Format(time.RFC3339)), + }, + }, + }, + }, + }, + }, + }, + }, + }, + want: nil, + }, + { + name: "without clusterName - no leaked ENIs", + fields: fields{ + clusterName: "", + describeNetworkInterfacePagesCalls: []describeNetworkInterfacePagesCall{ + { + input: &ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("tag-key"), + Values: []*string{aws.String("node.k8s.amazonaws.com/instance_id")}, + }, + { + Name: aws.String("status"), + Values: []*string{aws.String("available")}, + }, + }, + MaxResults: aws.Int64(1000), + }, + outputPages: []*ec2.DescribeNetworkInterfacesOutput{ + { + NetworkInterfaces: nil, + }, + }, + }, + }, + }, + want: nil, + }, + { + name: "with clusterName - one ENI leaked", + fields: fields{ + clusterName: "awesome-cluster", + describeNetworkInterfacePagesCalls: []describeNetworkInterfacePagesCall{ + { + input: &ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("tag-key"), + Values: []*string{aws.String("node.k8s.amazonaws.com/instance_id")}, + }, + { + Name: aws.String("status"), + Values: []*string{aws.String("available")}, + }, + { + Name: aws.String("tag:cluster.k8s.amazonaws.com/name"), + Values: []*string{aws.String("awesome-cluster")}, + }, + }, + MaxResults: aws.Int64(1000), + }, + outputPages: []*ec2.DescribeNetworkInterfacesOutput{ + { + NetworkInterfaces: []*ec2.NetworkInterface{ + { + NetworkInterfaceId: aws.String("eni-1"), + Description: aws.String("aws-K8S-i-xxxxx"), + Status: aws.String("available"), + TagSet: []*ec2.Tag{ + { + Key: aws.String("node.k8s.amazonaws.com/instance_id"), + Value: aws.String("i-xxxxx"), + }, + { + Key: aws.String("node.k8s.amazonaws.com/createdAt"), + Value: aws.String(tenMinuteAgo.Format(time.RFC3339)), + }, + { + Key: aws.String("cluster.k8s.amazonaws.com/name"), + Value: aws.String("awesome-cluster"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + want: []*ec2.NetworkInterface{ + { + NetworkInterfaceId: aws.String("eni-1"), + Description: aws.String("aws-K8S-i-xxxxx"), + Status: aws.String("available"), + TagSet: []*ec2.Tag{ + { + Key: aws.String("node.k8s.amazonaws.com/instance_id"), + Value: aws.String("i-xxxxx"), + }, + { + Key: aws.String("node.k8s.amazonaws.com/createdAt"), + Value: aws.String(tenMinuteAgo.Format(time.RFC3339)), + }, + { + Key: aws.String("cluster.k8s.amazonaws.com/name"), + Value: aws.String("awesome-cluster"), + }, + }, + }, + }, + }, + { + name: "with clusterName - one ENI - description didn't match", + fields: fields{ + clusterName: "awesome-cluster", + describeNetworkInterfacePagesCalls: []describeNetworkInterfacePagesCall{ + { + input: &ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("tag-key"), + Values: []*string{aws.String("node.k8s.amazonaws.com/instance_id")}, + }, + { + Name: aws.String("status"), + Values: []*string{aws.String("available")}, + }, + { + Name: aws.String("tag:cluster.k8s.amazonaws.com/name"), + Values: []*string{aws.String("awesome-cluster")}, + }, + }, + MaxResults: aws.Int64(1000), + }, + outputPages: []*ec2.DescribeNetworkInterfacesOutput{ + { + NetworkInterfaces: []*ec2.NetworkInterface{ + { + NetworkInterfaceId: aws.String("eni-1"), + Description: aws.String("non-k8s-i-xxxxx"), + Status: aws.String("available"), + TagSet: []*ec2.Tag{ + { + Key: aws.String("node.k8s.amazonaws.com/instance_id"), + Value: aws.String("i-xxxxx"), + }, + { + Key: aws.String("node.k8s.amazonaws.com/createdAt"), + Value: aws.String(tenMinuteAgo.Format(time.RFC3339)), + }, + { + Key: aws.String("cluster.k8s.amazonaws.com/name"), + Value: aws.String("awesome-cluster"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + want: nil, + }, + { + name: "with clusterName - one ENI - creationTime within deletion coolDown", + fields: fields{ + clusterName: "awesome-cluster", + describeNetworkInterfacePagesCalls: []describeNetworkInterfacePagesCall{ + { + input: &ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("tag-key"), + Values: []*string{aws.String("node.k8s.amazonaws.com/instance_id")}, + }, + { + Name: aws.String("status"), + Values: []*string{aws.String("available")}, + }, + { + Name: aws.String("tag:cluster.k8s.amazonaws.com/name"), + Values: []*string{aws.String("awesome-cluster")}, + }, + }, + MaxResults: aws.Int64(1000), + }, + outputPages: []*ec2.DescribeNetworkInterfacesOutput{ + { + NetworkInterfaces: []*ec2.NetworkInterface{ + { + NetworkInterfaceId: aws.String("eni-1"), + Description: aws.String("aws-K8S-i-xxxxx"), + Status: aws.String("available"), + TagSet: []*ec2.Tag{ + { + Key: aws.String("node.k8s.amazonaws.com/instance_id"), + Value: aws.String("i-xxxxx"), + }, + { + Key: aws.String("node.k8s.amazonaws.com/createdAt"), + Value: aws.String(now.Format(time.RFC3339)), + }, + { + Key: aws.String("cluster.k8s.amazonaws.com/name"), + Value: aws.String("awesome-cluster"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl, mockEC2 := setup(t) + defer ctrl.Finish() + + for _, call := range tt.fields.describeNetworkInterfacePagesCalls { + mockEC2.EXPECT(). + DescribeNetworkInterfacesPagesWithContext(gomock.Any(), call.input, gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *ec2.DescribeNetworkInterfacesInput, + fn func(*ec2.DescribeNetworkInterfacesOutput, bool) bool) error { + if call.err != nil { + return call.err + } + for _, output := range call.outputPages { + fn(output, true) + } + return nil + }) + } + ins := &EC2InstanceMetadataCache{ec2SVC: mockEC2, clusterName: tt.fields.clusterName} + got, err := ins.getLeakedENIs() + if tt.wantErr != nil { + assert.EqualError(t, err, tt.wantErr.Error()) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.want, got) + } + }) + } +} + +func TestEC2InstanceMetadataCache_TagENI(t *testing.T) { + type createTagsCall struct { + input *ec2.CreateTagsInput + err error + } + type fields struct { + instanceID string + clusterName string + additionalENITags map[string]string + + createTagsCalls []createTagsCall + } + type args struct { + eniID string + currentTags map[string]string + } + tests := []struct { + name string + fields fields + args args + wantErr error + }{ + { + name: "eni currently have no tags", + fields: fields{ + instanceID: "i-xxxx", + clusterName: "awesome-cluster", + createTagsCalls: []createTagsCall{ + { + input: &ec2.CreateTagsInput{ + Resources: []*string{aws.String("eni-xxxx")}, + Tags: []*ec2.Tag{ + { + Key: aws.String("cluster.k8s.amazonaws.com/name"), + Value: aws.String("awesome-cluster"), + }, + { + Key: aws.String("node.k8s.amazonaws.com/instance_id"), + Value: aws.String("i-xxxx"), + }, + }, + }, + }, + }, + }, + args: args{ + eniID: "eni-xxxx", + currentTags: nil, + }, + wantErr: nil, + }, + { + name: "eni currently have all desired tags", + fields: fields{ + instanceID: "i-xxxx", + clusterName: "awesome-cluster", + createTagsCalls: nil, + }, + args: args{ + eniID: "eni-xxxx", + currentTags: map[string]string{ + "node.k8s.amazonaws.com/instance_id": "i-xxxx", + "cluster.k8s.amazonaws.com/name": "awesome-cluster", + }, + }, + wantErr: nil, + }, + { + name: "eni currently have partial tags", + fields: fields{ + instanceID: "i-xxxx", + clusterName: "awesome-cluster", + createTagsCalls: []createTagsCall{ + { + input: &ec2.CreateTagsInput{ + Resources: []*string{aws.String("eni-xxxx")}, + Tags: []*ec2.Tag{ + { + Key: aws.String("cluster.k8s.amazonaws.com/name"), + Value: aws.String("awesome-cluster"), + }, + }, + }, + }, + }, + }, + args: args{ + eniID: "eni-xxxx", + currentTags: map[string]string{ + "node.k8s.amazonaws.com/instance_id": "i-xxxx", + "anotherKey": "anotherDay", + }, + }, + wantErr: nil, + }, + { + name: "create tags fails", + fields: fields{ + instanceID: "i-xxxx", + clusterName: "awesome-cluster", + createTagsCalls: []createTagsCall{ + { + input: &ec2.CreateTagsInput{ + Resources: []*string{aws.String("eni-xxxx")}, + Tags: []*ec2.Tag{ + { + Key: aws.String("cluster.k8s.amazonaws.com/name"), + Value: aws.String("awesome-cluster"), + }, + { + Key: aws.String("node.k8s.amazonaws.com/instance_id"), + Value: aws.String("i-xxxx"), + }, + }, + }, + err: errors.New("permission denied"), + }, + }, + }, + args: args{ + eniID: "eni-xxxx", + currentTags: nil, + }, + wantErr: errors.New("permission denied"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl, mockEC2 := setup(t) + defer ctrl.Finish() + + for _, call := range tt.fields.createTagsCalls { + mockEC2.EXPECT().CreateTagsWithContext(gomock.Any(), call.input).Return(&ec2.CreateTagsOutput{}, call.err).AnyTimes() + } + + cache := &EC2InstanceMetadataCache{ + ec2SVC: mockEC2, + instanceID: tt.fields.instanceID, + clusterName: tt.fields.clusterName, + additionalENITags: tt.fields.additionalENITags, + } + err := cache.TagENI(tt.args.eniID, tt.args.currentTags) + if tt.wantErr != nil { + assert.EqualError(t, err, tt.wantErr.Error()) + } else { + assert.NoError(t, err) + } + }) + } +} + +func Test_convertTagsToSDKTags(t *testing.T) { + type args struct { + tags map[string]string + } + tests := []struct { + name string + args args + want []*ec2.Tag + }{ + { + name: "non-empty tags", + args: args{ + tags: map[string]string{ + "keyA": "valueA", + "keyB": "valueB", + }, + }, + want: []*ec2.Tag{ + { + Key: aws.String("keyA"), + Value: aws.String("valueA"), + }, + { + Key: aws.String("keyB"), + Value: aws.String("valueB"), + }, + }, + }, + { + name: "nil tags", + args: args{tags: nil}, + want: nil, + }, + { + name: "empty tags", + args: args{tags: map[string]string{}}, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := convertTagsToSDKTags(tt.args.tags) + assert.Equal(t, tt.want, got) + }) + } +} + +func Test_convertSDKTagsToTags(t *testing.T) { + type args struct { + sdkTags []*ec2.Tag + } + tests := []struct { + name string + args args + want map[string]string + }{ + { + name: "non-empty sdk tags", + args: args{ + sdkTags: []*ec2.Tag{ + { + Key: aws.String("keyA"), + Value: aws.String("valueA"), + }, + { + Key: aws.String("keyB"), + Value: aws.String("valueB"), + }, + }, + }, + want: map[string]string{ + "keyA": "valueA", + "keyB": "valueB", + }, + }, + { + name: "nil sdk tags", + args: args{ + sdkTags: nil, + }, + want: nil, + }, + { + name: "empty sdk tags", + args: args{ + sdkTags: []*ec2.Tag{}, + }, + want: nil, + }, + { + name: "nil sdk tag value", + args: args{ + sdkTags: []*ec2.Tag{ + { + Key: aws.String("keyA"), + Value: nil, + }, + }, + }, + want: map[string]string{ + "keyA": "", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := convertSDKTagsToTags(tt.args.sdkTags) + assert.Equal(t, tt.want, got) + }) + } +} + +func Test_loadAdditionalENITags(t *testing.T) { + tests := []struct { + name string + envVars map[string]string + want map[string]string + }{ + { + name: "no ADDITIONAL_ENI_TAGS env", + envVars: map[string]string{ + "ADDITIONAL_ENI_TAGS": "", + }, + want: nil, + }, + { + name: "ADDITIONAL_ENI_TAGS is valid format", + envVars: map[string]string{ + "ADDITIONAL_ENI_TAGS": "{\"tagKey1\": \"tagVal1\"}", + }, + want: map[string]string{ + "tagKey1": "tagVal1", + }, + }, + { + name: "ADDITIONAL_ENI_TAGS is invalid format", + envVars: map[string]string{ + "ADDITIONAL_ENI_TAGS": "xxxx", + }, + want: nil, + }, + { + name: "ADDITIONAL_ENI_TAGS is valid format but contains tags with restricted prefix", + envVars: map[string]string{ + "ADDITIONAL_ENI_TAGS": "{\"bla.k8s.amazonaws.com\": \"bla\"}", + }, + want: map[string]string{}, + }, + { + name: "ADDITIONAL_ENI_TAGS is valid format but contains valid tags and tags with restricted prefix", + envVars: map[string]string{ + "ADDITIONAL_ENI_TAGS": "{\"bla.k8s.amazonaws.com\": \"bla\", \"tagKey1\": \"tagVal1\"}", + }, + want: map[string]string{ + "tagKey1": "tagVal1", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for key, value := range tt.envVars { + if value != "" { + os.Setenv(key, value) + } else { + os.Unsetenv(key) + } + } + got := loadAdditionalENITags() + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/awsutils/mocks/awsutils_mocks.go b/pkg/awsutils/mocks/awsutils_mocks.go index 473a2881dc..40a779d366 100644 --- a/pkg/awsutils/mocks/awsutils_mocks.go +++ b/pkg/awsutils/mocks/awsutils_mocks.go @@ -321,6 +321,20 @@ func (mr *MockAPIsMockRecorder) SetUnmanagedENIs(arg0 interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetUnmanagedENIs", reflect.TypeOf((*MockAPIs)(nil).SetUnmanagedENIs), arg0) } +// TagENI mocks base method +func (m *MockAPIs) TagENI(arg0 string, arg1 map[string]string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TagENI", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// TagENI indicates an expected call of TagENI +func (mr *MockAPIsMockRecorder) TagENI(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TagENI", reflect.TypeOf((*MockAPIs)(nil).TagENI), arg0, arg1) +} + // WaitForENIAndIPsAttached mocks base method func (m *MockAPIs) WaitForENIAndIPsAttached(arg0 string, arg1 int) (awsutils.ENIMetadata, error) { m.ctrl.T.Helper() diff --git a/pkg/ipamd/ipamd.go b/pkg/ipamd/ipamd.go index 0b68236884..8ed814c15f 100644 --- a/pkg/ipamd/ipamd.go +++ b/pkg/ipamd/ipamd.go @@ -371,16 +371,24 @@ func (c *IPAMContext) nodeInit() error { 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 } + + isTrunkENI := eni.ENIID == metadataResult.TrunkENI + isEFAENI := metadataResult.EFAENIs[eni.ENIID] + if !isTrunkENI { + if err := c.awsClient.TagENI(eni.ENIID, metadataResult.TagMap[eni.ENIID]); err != nil { + return errors.Wrapf(err, "ipamd init: failed to tag managed ENI %v", eni.ENIID) + } + } + + // Retry ENI sync retry := 0 for { retry++ - - if err = c.setupENI(eni.ENIID, eni, eni.ENIID == metadataResult.TrunkENI, metadataResult.EFAENIs[eni.ENIID]); err == nil { + if err = c.setupENI(eni.ENIID, eni, isTrunkENI, isEFAENI); err == nil { log.Infof("ENI %s set up.", eni.ENIID) break } @@ -984,6 +992,8 @@ func (c *IPAMContext) nodeIPPoolReconcile(ctx context.Context, interval time.Dur break } } + + var eniTagMap map[string]awsutils.TagMap if needToUpdateTags { log.Debugf("A new ENI added but not by ipamd, updating tags by calling EC2") metadataResult, err := c.awsClient.DescribeAllENIs() @@ -1005,6 +1015,7 @@ func (c *IPAMContext) nodeIPPoolReconcile(ctx context.Context, interval time.Dur trunkENI = metadataResult.TrunkENI // Just copy values of the EFA set efaENIs = metadataResult.EFAENIs + eniTagMap = metadataResult.TagMap c.setUnmanagedENIs(metadataResult.TagMap) c.awsClient.SetCNIUnmanagedENIs(metadataResult.MultiCardENIIDs) attachedENIs = c.filterUnmanagedENIs(metadataResult.ENIMetadata) @@ -1023,9 +1034,19 @@ func (c *IPAMContext) nodeIPPoolReconcile(ctx context.Context, interval time.Dur continue } + isTrunkENI := attachedENI.ENIID == trunkENI + isEFAENI := efaENIs[attachedENI.ENIID] + if !isTrunkENI { + if err := c.awsClient.TagENI(attachedENI.ENIID, eniTagMap[attachedENI.ENIID]); err != nil { + log.Errorf("IP pool reconcile: failed to tag managed ENI %v: %v", attachedENI.ENIID, err) + ipamdErrInc("eniReconcileAdd") + continue + } + } + // Add new ENI log.Debugf("Reconcile and add a new ENI %s", attachedENI) - err = c.setupENI(attachedENI.ENIID, attachedENI, attachedENI.ENIID == trunkENI, efaENIs[attachedENI.ENIID]) + err = c.setupENI(attachedENI.ENIID, attachedENI, isTrunkENI, isEFAENI) if err != nil { log.Errorf("IP pool reconcile: Failed to set up ENI %s network: %v", attachedENI.ENIID, err) ipamdErrInc("eniReconcileAdd") diff --git a/pkg/ipamd/ipamd_test.go b/pkg/ipamd/ipamd_test.go index 6c12b71e8b..b4e971fb1e 100644 --- a/pkg/ipamd/ipamd_test.go +++ b/pkg/ipamd/ipamd_test.go @@ -125,6 +125,7 @@ func TestNodeInit(t *testing.T) { m.awsutils.EXPECT().GetIPv4sFromEC2(eni2.ENIID).AnyTimes().Return(eni2.IPv4Addresses, nil) m.awsutils.EXPECT().IsUnmanagedENI(eni1.ENIID).Return(false).AnyTimes() m.awsutils.EXPECT().IsUnmanagedENI(eni2.ENIID).Return(false).AnyTimes() + m.awsutils.EXPECT().TagENI(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() m.awsutils.EXPECT().IsCNIUnmanagedENI(eni1.ENIID).Return(false).AnyTimes() m.awsutils.EXPECT().IsCNIUnmanagedENI(eni2.ENIID).Return(false).AnyTimes() @@ -418,6 +419,7 @@ func TestNodeIPPoolReconcile(t *testing.T) { m.awsutils.EXPECT().GetPrimaryENI().AnyTimes().Return(primaryENIid) m.awsutils.EXPECT().IsUnmanagedENI(primaryENIid).AnyTimes().Return(false) m.awsutils.EXPECT().IsCNIUnmanagedENI(primaryENIid).AnyTimes().Return(false) + m.awsutils.EXPECT().TagENI(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() eniMetadataList := []awsutils.ENIMetadata{primaryENIMetadata} m.awsutils.EXPECT().GetAttachedENIs().Return(eniMetadataList, nil) resp := awsutils.DescribeAllENIsResult{ diff --git a/test/agent/cmd/networking/tester/network.go b/test/agent/cmd/networking/tester/network.go index f48df8c572..179f3e6450 100644 --- a/test/agent/cmd/networking/tester/network.go +++ b/test/agent/cmd/networking/tester/network.go @@ -172,7 +172,7 @@ func TestNetworkingSetupForRegularPod(podNetworkingValidationInput input.PodNetw } log.Printf("validated route table for secondary ENI %d has right routes", index) } - // TODO: validate iptables rules get setup correctly + // TODO: validate iptables rules get setup correctly return validationErrors }