Skip to content

Commit

Permalink
Filter Managed ENI. (#2895)
Browse files Browse the repository at this point in the history
If the SG reconcile loop runs before the ENI/IP reconcile loop it will modify the security groups as the ENI/IP reconcile hasn't had a chance to check the tags on the ENI yet.

Without relying on cache, when the SG reconcile is run, it will not update the ENI with the node.k8s.amazonaws.com/no_manage: true tag
  • Loading branch information
orsenthil authored May 7, 2024
1 parent 92977ef commit 06828ce
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 16 deletions.
3 changes: 3 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,6 @@ replace golang.org/x/crypto => golang.org/x/crypto v0.17.0

// Cannot be removed until all dependencies use net library v0.23.0 or higher
replace golang.org/x/net => golang.org/x/net v0.23.0

// Version of go-cose v1.2.0 and v1.2.1 have been deprecated in favor v1.1.0
replace github.com/veraison/go-cose => github.com/veraison/go-cose v1.1.0
16 changes: 8 additions & 8 deletions pkg/awsutils/awsutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"sync"
"time"

"github.com/aws/amazon-vpc-cni-k8s/pkg/ipamd/datastore"

"github.com/aws/amazon-vpc-cni-k8s/pkg/awsutils/awssession"
"github.com/aws/amazon-vpc-cni-k8s/pkg/ec2wrapper"
"github.com/aws/amazon-vpc-cni-k8s/pkg/utils/eventrecorder"
Expand Down Expand Up @@ -168,7 +170,7 @@ type APIs interface {
IsPrimaryENI(eniID string) bool

//RefreshSGIDs
RefreshSGIDs(mac string) error
RefreshSGIDs(mac string, store *datastore.DataStore) error

//GetInstanceHypervisorFamily returns the hypervisor family for the instance
GetInstanceHypervisorFamily() string
Expand Down Expand Up @@ -474,7 +476,7 @@ func (cache *EC2InstanceMetadataCache) initWithEC2Metadata(ctx context.Context)
}

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

sgIDs, err := cache.imds.GetSecurityGroupIDs(ctx, mac)
Expand All @@ -501,14 +503,12 @@ func (cache *EC2InstanceMetadataCache) RefreshSGIDs(mac string) error {
cache.securityGroups.Set(sgIDs)

if !cache.useCustomNetworking && (addedSGsCount != 0 || deletedSGsCount != 0) {
allENIs, err := cache.GetAttachedENIs()
if err != nil {
return errors.Wrap(err, "DescribeAllENIs: failed to get local ENI metadata")
}
eniInfos := store.GetENIInfos()

var eniIDs []string
for _, eni := range allENIs {
eniIDs = append(eniIDs, eni.ENIID)

for eniID := range eniInfos.ENIs {
eniIDs = append(eniIDs, eniID)
}

newENIs := StringSet{}
Expand Down
10 changes: 6 additions & 4 deletions pkg/awsutils/mocks/awsutils_mocks.go

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

4 changes: 2 additions & 2 deletions pkg/ipamd/ipamd.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,14 +507,14 @@ func (c *IPAMContext) nodeInit() error {
// 1. after managed/unmanaged ENIs have been determined
// 2. before any new ENIs are attached
if c.enableIPv4 && !c.disableENIProvisioning {
if err := c.awsClient.RefreshSGIDs(primaryENIMac); err != nil {
if err := c.awsClient.RefreshSGIDs(primaryENIMac, c.dataStore); 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() {
c.awsClient.RefreshSGIDs(primaryENIMac)
c.awsClient.RefreshSGIDs(primaryENIMac, c.dataStore)
}, 30*time.Second)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/ipamd/ipamd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func TestNodeInit(t *testing.T) {
m.network.EXPECT().SetupHostNetwork(cidrs, "", &primaryIP, false, true, false).Return(nil)
m.network.EXPECT().CleanUpStaleAWSChains(true, false).Return(nil)
m.awsutils.EXPECT().GetPrimaryENI().AnyTimes().Return(primaryENIid)
m.awsutils.EXPECT().RefreshSGIDs(gomock.Any()).AnyTimes().Return(nil)
m.awsutils.EXPECT().RefreshSGIDs(gomock.Any(), gomock.Any()).AnyTimes().Return(nil)

eniMetadataSlice := []awsutils.ENIMetadata{eni1, eni2}
resp := awsutils.DescribeAllENIsResult{
Expand Down Expand Up @@ -243,7 +243,7 @@ func TestNodeInitwithPDenabledIPv4Mode(t *testing.T) {
m.network.EXPECT().SetupHostNetwork(cidrs, "", &primaryIP, false, true, false).Return(nil)
m.network.EXPECT().CleanUpStaleAWSChains(true, false).Return(nil)
m.awsutils.EXPECT().GetPrimaryENI().AnyTimes().Return(primaryENIid)
m.awsutils.EXPECT().RefreshSGIDs(gomock.Any()).AnyTimes().Return(nil)
m.awsutils.EXPECT().RefreshSGIDs(gomock.Any(), gomock.Any()).AnyTimes().Return(nil)

eniMetadataSlice := []awsutils.ENIMetadata{eni1, eni2}
resp := awsutils.DescribeAllENIsResult{
Expand Down

0 comments on commit 06828ce

Please sign in to comment.