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

feat: Centralize leaked ENI cleanup #418

Merged
merged 11 commits into from
May 1, 2024
Merged

feat: Centralize leaked ENI cleanup #418

merged 11 commits into from
May 1, 2024

Conversation

sushrk
Copy link
Contributor

@sushrk sushrk commented Apr 30, 2024

Issue #, if available:
N/A
Description of changes:
Merge feature branch eni-cleanup into master for release.
Centralize leaked ENI cleanup in the controller to delete leaked ENIs provisioned by the controller and VPC-CNI:

  • Added finalizer in CNINode: when kubernetes deletes an object that has finalizers specified for it, the Kubernetes API marks the object for deletion by populating .metadata.deletionTimestamp, and returns a 202 status code (HTTP "Accepted"). The target object remains in a terminating state while the control plane, or other components, take the actions defined by the finalizers. After these actions are complete, the controller removes the relevant finalizers from the target object. When the metadata.finalizers field is empty, Kubernetes considers the deletion complete and deletes the object.
  • Add a new field in Spec to add Tag: This is currently used to specify tags that need to added to network interfaces provisioned by the controller and VPC-CNI. VPC-CNI will read the CNINode to get the tags.

The CNINode reconciler will add the finalizer and tag if missing on existing CNINode objects and handles the finalizer routine to clean up leaked resources.

  • Re-create CNINode if deleted by user: The controller will re-create CNINode if deleted by user when node object is still present.
  • Added metrics to count number of available ENIs found and number of leaked ENIs that failed to be deleted

Tests:

Webhook
Ran 11 of 11 Specs in 107.573 seconds
SUCCESS! -- 11 Passed | 0 Failed | 0 Pending | 0 Skipped
PASS

Ginkgo ran 1 suite in 1m49.904100803s
Test Suite Passed

SGP
Ran 18 of 22 Specs in 1183.287 seconds
SUCCESS! -- 18 Passed | 0 Failed | 0 Pending | 4 Skipped
PASS

Ginkgo ran 1 suite in 19m46.06109991s
Test Suite Passed

CNINode 
Ran 6 of 6 Specs in 491.560 seconds
SUCCESS! -- 6 Passed | 0 Failed | 0 Pending | 0 Skipped
PASS

Ginkgo ran 1 suite in 8m14.030002297s
Test Suite Passed

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

sushrk and others added 11 commits April 30, 2024 19:37
@sushrk
Call DisassociateTrunkInterface before deleting branch ENI (aws#372)
c2e47a2 
* Call DisassociateTrunkInterface before deleting branch ENI
@sushrk
feat: Centralize leaked ENI cleanup (aws#374)
69def10 
* feat: centralized eni cleanup
@sushrk @dependabot @xdu31
Merge master into eni-cleanup (aws#385)
cf1c49b 
* fix: paginate DescribeNetworkInterfaces with deep filters (aws#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 (aws#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] <support@github.com>
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 (aws#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] <support@github.com>
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 (aws#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] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Repo controlled build go version (aws#381)

* update golang version (aws#383)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jason Du <jasonxdu@amazon.com>
@sushrk
fix:update cluster tag name in CNINode (aws#386)
35ed890
@sushrk
fix:add node OS label in CNINode, retry get CNINode with backoff
2602461
@haouc @sushrk
update protobuf to 1.33.0 (aws#387)
bc1df55
@sushrk
add CNINode integration tests (aws#391)
cdbdf4d
@sushrk
use DescribeNetworkInterfaces with deep filters
30fca5f
@sushrk
add integration test to validate ec2 permissions
2b1e29b
@sushrk
remove DisassociateAllBranchENIs as it is not useful (aws#400)
5b9de0f 
* remove DisassociateAllBranchENIs as it is not useful

* skip deletion success log for NotFound ENI

* fix govulncheck
@sushrk @dependabot
@xdu31 @haouc @orsenthil @GnatorX
Merge master branch into eni-cleanup (aws#416)
0514790 
* fix: paginate DescribeNetworkInterfaces with deep filters (aws#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 (aws#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] <support@github.com>
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 (aws#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] <support@github.com>
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 (aws#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] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Repo controlled build go version (aws#381)

* update golang version (aws#383)

* update protobuf to 1.33.0 (aws#387)

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

* Bump k8s.io/client-go from 0.29.2 to 0.29.3 (aws#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] <support@github.com>
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 (aws#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] <support@github.com>
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 (aws#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] <support@github.com>
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 (aws#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] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

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

* add github action to run gosec static analysis

* install gosec

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

* revert pagination and call DescribeNetworkInterfaces with vpcID or subnetID filter

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

This reverts commit b5699de.

* call DescribeNetworkInterfaces with vpcID or subnetID filter

* update EC2 supported instance types (aws#402)

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

* Update controller_auth_proxy_patch.yaml (aws#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 (aws#407)

* Fix log which causes panic

* Consistent key name

* consistent naming

* run go mod tidy

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jason Du <jasonxdu@amazon.com>
Co-authored-by: Hao Zhou <haouc@users.noreply.github.com>
Co-authored-by: Senthil Kumaran <senthilx@amazon.com>
Co-authored-by: Garvin Pang <garvinpang@protonmail.com>
@sushrk sushrk requested a review from a team as a code owner April 30, 2024 21:20
@haouc
Copy link
Contributor

haouc commented May 1, 2024

Merged #374 from dev branch.

haouc
haouc approved these changes May 1, 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

@sushrk sushrk merged commit b5a054e into aws:master May 1, 2024
4 checks passed
orsenthil
orsenthil reviewed May 1, 2024
View reviewed changes
Copy link
Contributor

@orsenthil orsenthil left a comment

Choose a reason for hiding this comment

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

LGTM.
It is the same change as this https://github.com/aws/amazon-vpc-resource-controller-k8s/pull/374/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orsenthil orsenthil orsenthil left review comments

@haouc haouc haouc approved these changes

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@sushrk @haouc @orsenthil