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.

Sign up for GitHub

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

Jump to bottom

Call DisassociateTrunkInterface before deleting branch ENI #372

Merged
merged 2 commits into from
Feb 22, 2024
Merged

Call DisassociateTrunkInterface before deleting branch ENI #372

merged 2 commits into from
Feb 22, 2024

Conversation

sushrk
Copy link
Contributor

@sushrk sushrk commented Feb 21, 2024 •
edited
Loading

Issue #, if available:
N/A

Description of changes:
Call DisassociateTrunkInterface to remove association between branch and trunk ENI before deleting the branch ENI as per recommendation by EC2 team.

Tests done after adding DisassociateTrunkInterface permission in the control plane instance role:

  • Scale up and down pods using security group for pods and verify branch ENIs can be disassociated and deleted
  • Terminate nodes and verify branch ENIs are deleted- Sometimes the association is removed when trunk ENI is deleted with EC2 instance termination, in which case branch ENI is still deleted
  • Upgrade from old vpc-resource-controller which is missing association ID and verify branch ENIs can be deleted for backwards compatibility.

SGPP tests:

Ran 19 of 23 Specs in 929.417 seconds
FAIL! -- 18 Passed | 1 Failed | 0 Pending | 4 Skipped

Failed at [It] should have same number of CNINode no matter which mode, unrelated to the changes in this PR as I was testing finalizers on CNINode and updated(downgraded) controller on my cluster.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sushrk sushrk requested a review from a team as a code owner February 21, 2024 09:21
jdn5126
jdn5126 previously approved these changes Feb 21, 2024
View reviewed changes
Copy link
Contributor

@jdn5126 jdn5126 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM

@sushrk sushrk marked this pull request as draft February 21, 2024 23:07
@sushrk sushrk changed the base branch from master to eni-cleanup February 21, 2024 23:09
@sushrk sushrk dismissed jdn5126’s stale review February 21, 2024 23:09

The base branch was changed.

@sushrk sushrk marked this pull request as ready for review February 21, 2024 23:09
haouc
haouc reviewed Feb 21, 2024
View reviewed changes
pkg/aws/ec2/api/helper.go Outdated Show resolved Hide resolved
haouc
haouc approved these changes Feb 22, 2024
View reviewed changes
Copy link
Contributor

@haouc haouc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Thanks.

@sushrk sushrk merged commit a177073 into aws:eni-cleanup Feb 22, 2024
3 checks passed
@sushrk sushrk mentioned this pull request Mar 4, 2024
Merged
sushrk added a commit to sushrk/amazon-vpc-resource-controller-k8s that referenced this pull request Mar 8, 2024
@sushrk
Call DisassociateTrunkInterface before deleting branch ENI (aws#372)
d8dadd5 
* Call DisassociateTrunkInterface before deleting branch ENI
sushrk added a commit to sushrk/amazon-vpc-resource-controller-k8s that referenced this pull request Apr 29, 2024
@sushrk
Call DisassociateTrunkInterface before deleting branch ENI (aws#372)
a1f7bb7 
* Call DisassociateTrunkInterface before deleting branch ENI
sushrk added a commit to sushrk/amazon-vpc-resource-controller-k8s that referenced this pull request Apr 30, 2024
@sushrk
Call DisassociateTrunkInterface before deleting branch ENI (aws#372)
c2e47a2 
* Call DisassociateTrunkInterface before deleting branch ENI
sushrk added a commit that referenced this pull request May 1, 2024
@sushrk @dependabot
@xdu31 @haouc @orsenthil @GnatorX
Merge branch eni-cleanup into master
b5a054e 
* Call DisassociateTrunkInterface before deleting branch ENI (#372)

* Call DisassociateTrunkInterface before deleting branch ENI

* feat: Centralize leaked ENI cleanup (#374)

* feat: centralized eni cleanup

* Merge master into eni-cleanup (#385)

* fix: paginate DescribeNetworkInterfaces with deep filters (#375)

* fix: paginate DescribeNetworkInterfaces with deep filters

* update metrics and address review comments

* minor updates to address comments

* Bump github.com/aws/aws-sdk-go from 1.49.13 to 1.50.29 (#380)

Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.49.13 to 1.50.29.
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.49.13...v1.50.29)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump k8s.io/client-go from 0.29.1 to 0.29.2 (#377)

Bumps [k8s.io/client-go](https://github.com/kubernetes/client-go) from 0.29.1 to 0.29.2.
- [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md)
- [Commits](kubernetes/client-go@v0.29.1...v0.29.2)

---
updated-dependencies:
- dependency-name: k8s.io/client-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump github.com/prometheus/common from 0.46.0 to 0.49.0 (#378)

Bumps [github.com/prometheus/common](https://github.com/prometheus/common) from 0.46.0 to 0.49.0.
- [Release notes](https://github.com/prometheus/common/releases)
- [Commits](prometheus/common@v0.46.0...v0.49.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/common
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Repo controlled build go version (#381)

* update golang version (#383)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jason Du <[email protected]>

* fix:update cluster tag name in CNINode (#386)

* fix:add node OS label in CNINode, retry get CNINode with backoff

* update protobuf to 1.33.0 (#387)

* add CNINode integration tests (#391)

* use DescribeNetworkInterfaces with deep filters

* add integration test to validate ec2 permissions

* remove DisassociateAllBranchENIs as it is not useful (#400)

* remove DisassociateAllBranchENIs as it is not useful

* skip deletion success log for NotFound ENI

* fix govulncheck

* Merge master branch into eni-cleanup (#416)

* fix: paginate DescribeNetworkInterfaces with deep filters (#375)

* fix: paginate DescribeNetworkInterfaces with deep filters

* update metrics and address review comments

* minor updates to address comments

* Bump github.com/aws/aws-sdk-go from 1.49.13 to 1.50.29 (#380)

Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.49.13 to 1.50.29.
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.49.13...v1.50.29)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump k8s.io/client-go from 0.29.1 to 0.29.2 (#377)

Bumps [k8s.io/client-go](https://github.com/kubernetes/client-go) from 0.29.1 to 0.29.2.
- [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md)
- [Commits](kubernetes/client-go@v0.29.1...v0.29.2)

---
updated-dependencies:
- dependency-name: k8s.io/client-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump github.com/prometheus/common from 0.46.0 to 0.49.0 (#378)

Bumps [github.com/prometheus/common](https://github.com/prometheus/common) from 0.46.0 to 0.49.0.
- [Release notes](https://github.com/prometheus/common/releases)
- [Commits](prometheus/common@v0.46.0...v0.49.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/common
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Repo controlled build go version (#381)

* update golang version (#383)

* update protobuf to 1.33.0 (#387)

* pin envtest version due to an upstream bug (#390)

* Bump k8s.io/client-go from 0.29.2 to 0.29.3 (#392)

Bumps [k8s.io/client-go](https://github.com/kubernetes/client-go) from 0.29.2 to 0.29.3.
- [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md)
- [Commits](kubernetes/client-go@v0.29.2...v0.29.3)

---
updated-dependencies:
- dependency-name: k8s.io/client-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump github.com/aws/amazon-vpc-cni-k8s from 1.16.0 to 1.17.1 (#393)

Bumps [github.com/aws/amazon-vpc-cni-k8s](https://github.com/aws/amazon-vpc-cni-k8s) from 1.16.0 to 1.17.1.
- [Release notes](https://github.com/aws/amazon-vpc-cni-k8s/releases)
- [Changelog](https://github.com/aws/amazon-vpc-cni-k8s/blob/master/CHANGELOG.md)
- [Commits](aws/amazon-vpc-cni-k8s@v1.16.0...v1.17.1)

---
updated-dependencies:
- dependency-name: github.com/aws/amazon-vpc-cni-k8s
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump github.com/prometheus/common from 0.49.0 to 0.51.1 (#395)

Bumps [github.com/prometheus/common](https://github.com/prometheus/common) from 0.49.0 to 0.51.1.
- [Release notes](https://github.com/prometheus/common/releases)
- [Commits](prometheus/common@v0.49.0...v0.51.1)

---
updated-dependencies:
- dependency-name: github.com/prometheus/common
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump github.com/aws/aws-sdk-go from 1.50.29 to 1.51.12 (#397)

Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.50.29 to 1.51.12.
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.50.29...v1.51.12)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* add github action to run gosec static analysis (#398)

* add github action to run gosec static analysis

* install gosec

* update golang and dependency to fix CVE (#401)

* revert pagination and call DescribeNetworkInterfaces with vpcID or subnetID filter

* Revert "fix: paginate DescribeNetworkInterfaces with deep filters (#375)"

This reverts commit b5699de.

* call DescribeNetworkInterfaces with vpcID or subnetID filter

* update EC2 supported instance types (#402)

* remove global exclusion for G108,G114 and add nosec in code (#404)

* Update controller_auth_proxy_patch.yaml (#405)

Update the reference from gcr.io to registry.k8s.io

>  kube-rbac-proxy is moving to registry.k8s.io/kubebuilder/kube-rbac-proxy (from gcr.io/kubebuilder/kube-rbac-proxy) because GCR is being sunset. We need to update these references.

* Fix log which causes panic (#407)

* Fix log which causes panic

* Consistent key name

* consistent naming

* run go mod tidy

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jason Du <[email protected]>
Co-authored-by: Hao Zhou <[email protected]>
Co-authored-by: Senthil Kumaran <[email protected]>
Co-authored-by: Garvin Pang <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jason Du <[email protected]>
Co-authored-by: Hao Zhou <[email protected]>
Co-authored-by: Senthil Kumaran <[email protected]>
Co-authored-by: Garvin Pang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haouc haouc haouc approved these changes

@jdn5126 jdn5126 jdn5126 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sushrk @haouc @jdn5126