diff --git a/.go-version b/.go-version index 428abfd2..f124bfa1 100644 --- a/.go-version +++ b/.go-version @@ -1 +1 @@ -1.21.8 +1.21.9 diff --git a/go.mod b/go.mod index 1ec531e4..b643c41c 100644 --- a/go.mod +++ b/go.mod @@ -64,10 +64,10 @@ require ( github.com/spf13/pflag v1.0.5 // indirect go.uber.org/multierr v1.11.0 // indirect golang.org/x/exp v0.0.0-20230315142452-642cacee5cc0 - golang.org/x/net v0.21.0 // indirect - golang.org/x/oauth2 v0.17.0 // indirect - golang.org/x/sys v0.17.0 // indirect - golang.org/x/term v0.17.0 // indirect + golang.org/x/net v0.23.0 // indirect + golang.org/x/oauth2 v0.18.0 // indirect + golang.org/x/sys v0.18.0 // indirect + golang.org/x/term v0.18.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/tools v0.16.1 // indirect google.golang.org/appengine v1.6.7 // indirect diff --git a/go.sum b/go.sum index a4e345f2..bbc14d98 100644 --- a/go.sum +++ b/go.sum @@ -150,10 +150,10 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= -golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= -golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= -golang.org/x/oauth2 v0.17.0 h1:6m3ZPmLEFdVxKKWnKq4VqZ60gutO35zm+zrAHVmHyDQ= -golang.org/x/oauth2 v0.17.0/go.mod h1:OzPDGQiuQMguemayvdylqddI7qcD9lnSDb+1FiwQ5HA= +golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs= +golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= +golang.org/x/oauth2 v0.18.0 h1:09qnuIAgzdx1XplqJvW6CQqMCtGZykZWcXzPMPUusvI= +golang.org/x/oauth2 v0.18.0/go.mod h1:Wf7knwG0MPoWIMMBgFlEaSUDaKskp0dCfrlJRJXbBi8= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -165,11 +165,11 @@ golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= +golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.17.0 h1:mkTF7LCd6WGJNL3K1Ad7kwxNfYAW6a8a8QqtMblp/4U= -golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= +golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8= +golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go index 8120b4f9..59aae85c 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go @@ -76,18 +76,6 @@ func (mr *MockTrunkENIMockRecorder) DeleteCooledDownENIs() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteCooledDownENIs", reflect.TypeOf((*MockTrunkENI)(nil).DeleteCooledDownENIs)) } -// DisassociateAllBranchENIs mocks base method. -func (m *MockTrunkENI) DisassociateAllBranchENIs() { - m.ctrl.T.Helper() - m.ctrl.Call(m, "DisassociateAllBranchENIs") -} - -// DisassociateAllBranchENIs indicates an expected call of DisassociateAllBranchENIs. -func (mr *MockTrunkENIMockRecorder) DisassociateAllBranchENIs() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DisassociateAllBranchENIs", reflect.TypeOf((*MockTrunkENI)(nil).DisassociateAllBranchENIs)) -} - // InitTrunk mocks base method. func (m *MockTrunkENI) InitTrunk(arg0 ec2.EC2Instance, arg1 []v1.Pod) error { m.ctrl.T.Helper() diff --git a/pkg/aws/ec2/api/cleanup/eni_cleanup.go b/pkg/aws/ec2/api/cleanup/eni_cleanup.go index 723fc92e..edc58558 100644 --- a/pkg/aws/ec2/api/cleanup/eni_cleanup.go +++ b/pkg/aws/ec2/api/cleanup/eni_cleanup.go @@ -16,6 +16,7 @@ package cleanup import ( "context" "fmt" + "strings" "time" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api" @@ -23,6 +24,7 @@ import ( rcHealthz "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/healthz" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils" + ec2Errors "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/errors" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/go-logr/logr" @@ -150,11 +152,13 @@ func (e *ENICleaner) DeleteLeakedResources() error { NetworkInterfaceId: nwInterface.NetworkInterfaceId, }) if err != nil { - // append err and continue, we will retry deletion in the next period/reconcile - leakedENICount += 1 - errors = append(errors, fmt.Errorf("failed to delete leaked network interface %v:%v", *nwInterface.NetworkInterfaceId, err)) - e.Log.Error(err, "failed to delete the leaked network interface", - "id", *nwInterface.NetworkInterfaceId) + if !strings.Contains(err.Error(), ec2Errors.NotFoundInterfaceID) { // ignore InvalidNetworkInterfaceID.NotFound error + // append err and continue, we will retry deletion in the next period/reconcile + leakedENICount += 1 + errors = append(errors, fmt.Errorf("failed to delete leaked network interface %v:%v", *nwInterface.NetworkInterfaceId, err)) + e.Log.Error(err, "failed to delete the leaked network interface", + "id", *nwInterface.NetworkInterfaceId) + } continue } e.Log.Info("deleted leaked ENI successfully", "eni id", nwInterface.NetworkInterfaceId) diff --git a/pkg/node/manager/manager.go b/pkg/node/manager/manager.go index 5a0089f4..be48c959 100644 --- a/pkg/node/manager/manager.go +++ b/pkg/node/manager/manager.go @@ -453,7 +453,7 @@ func (m *manager) performAsyncOperation(job interface{}) (ctrl.Result, error) { log.V(1).Info("successfully performed node operation") return ctrl.Result{}, nil } - log.Error(err, "failed to performed node operation") + log.Error(err, "failed to perform node operation") return ctrl.Result{}, nil } diff --git a/pkg/provider/branch/provider.go b/pkg/provider/branch/provider.go index 6277f5a3..0fcbfb5b 100644 --- a/pkg/provider/branch/provider.go +++ b/pkg/provider/branch/provider.go @@ -234,12 +234,14 @@ func (b *branchENIProvider) ProcessAsyncJob(job interface{}) (ctrl.Result, error // DeleteNode deletes all the cached branch ENIs associated with the trunk and removes the trunk from the cache. func (b *branchENIProvider) DeleteNode(nodeName string) (ctrl.Result, error) { - trunkENI, isPresent := b.getTrunkFromCache(nodeName) + _, isPresent := b.getTrunkFromCache(nodeName) if !isPresent { return ctrl.Result{}, fmt.Errorf("failed to find node %s", nodeName) } - trunkENI.DisassociateAllBranchENIs() + // At this point, the finalizer routine should have deleted all available branch ENIs + // Any leaked ENIs will be deleted by the periodic cleanup routine if cluster is active + // remove trunk from cache and de-initializer the resource provider b.removeTrunkFromCache(nodeName) b.log.Info("de-initialized resource provider successfully", "node name", nodeName) diff --git a/pkg/provider/branch/trunk/trunk.go b/pkg/provider/branch/trunk/trunk.go index f8bc8baa..223160d3 100644 --- a/pkg/provider/branch/trunk/trunk.go +++ b/pkg/provider/branch/trunk/trunk.go @@ -33,8 +33,6 @@ import ( "github.com/go-logr/logr" "github.com/prometheus/client_golang/prometheus" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/metrics" ) @@ -95,8 +93,6 @@ type TrunkENI interface { Reconcile(pods []v1.Pod) bool // PushENIsToFrontOfDeleteQueue pushes the eni network interfaces to the front of the delete queue PushENIsToFrontOfDeleteQueue(*v1.Pod, []*ENIDetails) - // DisassociateAllBranchENIs removes association of all the branch ENI with the trunk - DisassociateAllBranchENIs() // Introspect returns the state of the Trunk ENI Introspect() IntrospectResponse } @@ -437,39 +433,6 @@ func (t *trunkENI) CreateAndAssociateBranchENIs(pod *v1.Pod, securityGroups []st return newENIs, nil } -// DisassociateAllBranchENIs removes association between all the branch ENIs with the trunk. The branch ENIs will be deleted by the node termination finalizer -// This is the last API call to the the Trunk ENI before it is removed from cache -func (t *trunkENI) DisassociateAllBranchENIs() { - // Disassociate all the branch used by the pod on this trunk ENI, it will be cleaned up by the finalizer routine - // Since after this call, the trunk will be removed from cache. No need to clean up its branch map - for _, podENIs := range t.uidToBranchENIMap { - for _, eni := range podENIs { - err := retry.OnError( - wait.Backoff{ - Duration: time.Millisecond * 100, - Factor: 3.0, - Jitter: 0.1, - Steps: 7, - Cap: time.Second * 10, - }, - func(err error) bool { - if strings.Contains(err.Error(), ec2Errors.NotFoundAssociationID) { - // association is already deleted, do not retry - return false - } - return true - }, func() error { - return t.ec2ApiHelper.DisassociateTrunkInterface(&eni.AssociationID) - }, - ) - if err != nil && !strings.Contains(err.Error(), ec2Errors.NotFoundAssociationID) { - // Just log, if the ENI disassociation fails, it will be force deleted - t.log.Error(err, "failed to disassociate eni", "eni id", eni.ID) - } - } - } -} - // DeleteBranchNetworkInterface deletes the branch network interface and returns an error in case of failure to delete func (t *trunkENI) PushBranchENIsToCoolDownQueue(UID string) { // Lock is required as Reconciler is also performing operation concurrently @@ -529,7 +492,7 @@ func (t *trunkENI) deleteENI(eniDetail *ENIDetails) (err error) { if err != nil { trunkENIOperationsErrCount.WithLabelValues("disassociate_trunk_error").Inc() if !strings.Contains(err.Error(), ec2Errors.NotFoundAssociationID) { - t.log.Error(err, "failed to disassciate branch ENI from trunk, will try to delete the branch ENI") + t.log.Error(err, "failed to disassociate branch ENI from trunk, will try to delete the branch ENI") // Not returning error here, fallback to force branch ENI deletion } else { t.log.Info("AssociationID not found when disassociating branch from trunk ENI, it is already disassociated so delete the branch ENI") diff --git a/pkg/provider/branch/trunk/trunk_test.go b/pkg/provider/branch/trunk/trunk_test.go index 4b79f641..05346311 100644 --- a/pkg/provider/branch/trunk/trunk_test.go +++ b/pkg/provider/branch/trunk/trunk_test.go @@ -29,7 +29,6 @@ import ( "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown" "github.com/aws/aws-sdk-go/aws" - awsEC2 "github.com/aws/aws-sdk-go/service/ec2" awsEc2 "github.com/aws/aws-sdk-go/service/ec2" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" @@ -148,8 +147,6 @@ var ( Ipv6Address: &BranchV6Ip2, } - branchENIs2 = []*ENIDetails{EniDetails2} - // Trunk Interface trunkId = "eni-00000000000000002" trunkInterface = &awsEc2.NetworkInterface{ @@ -159,12 +156,6 @@ var ( Status: aws.String(awsEc2.AttachmentStatusAttached), }, } - nodeNametag = []*awsEC2.Tag{ - { - Key: aws.String(config.NetworkInterfaceNodenameKey), - Value: aws.String(FakeInstance.Name()), - }, - } trunkIDTag = &awsEc2.Tag{ Key: aws.String(config.TrunkENIIDTag), @@ -201,17 +192,6 @@ var ( }, } - trunkAssociationsBranch1And2 = []*awsEc2.TrunkInterfaceAssociation{ - { - BranchInterfaceId: &EniDetails1.ID, - VlanId: aws.Int64(int64(EniDetails1.VlanID)), - }, - { - BranchInterfaceId: &EniDetails2.ID, - VlanId: aws.Int64(int64(EniDetails2.VlanID)), - }, - } - mockAssociationOutput1 = &awsEc2.AssociateTrunkInterfaceOutput{ InterfaceAssociation: &awsEc2.TrunkInterfaceAssociation{ AssociationId: &MockAssociationID1, @@ -262,7 +242,7 @@ func getMockTrunk() trunkENI { log: log, usedVlanIds: make([]bool, MaxAllocatableVlanIds), uidToBranchENIMap: map[string][]*ENIDetails{}, - nodeNameTag: []*awsEC2.Tag{ + nodeNameTag: []*awsEc2.Tag{ { Key: aws.String(config.NetworkInterfaceNodenameKey), Value: aws.String(FakeInstance.Name()), @@ -844,21 +824,6 @@ func TestTrunkENI_InitTrunk(t *testing.T) { } } -// TestTrunkENI_DisassociateAllBranchENIs tests all branch ENI are disassociated with the trunk -func TestTrunkENI_DisassociateAllBranchENIs(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - trunkENI, mockEC2APIHelper, _ := getMockHelperInstanceAndTrunkObject(ctrl) - trunkENI.uidToBranchENIMap[PodUID] = branchENIs1 - trunkENI.uidToBranchENIMap[PodUID2] = branchENIs2 - - mockEC2APIHelper.EXPECT().DisassociateTrunkInterface(&MockAssociationID1).Return(nil) - mockEC2APIHelper.EXPECT().DisassociateTrunkInterface(&MockAssociationID2).Return(nil) - - trunkENI.DisassociateAllBranchENIs() -} - // TestTrunkENI_CreateAndAssociateBranchENIs test branch is created and associated with the trunk and valid eni details // are returned func TestTrunkENI_CreateAndAssociateBranchENIs(t *testing.T) {