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

Skip explicit LB deletion for Kubernetes >= v1.16 #290

Merged
merged 4 commits into from
Mar 5, 2021
Merged

Skip explicit LB deletion for Kubernetes >= v1.16 #290

merged 4 commits into from
Mar 5, 2021

Conversation

rfranzke
Copy link
Member

@rfranzke rfranzke commented Mar 5, 2021 •
edited
Loading

How to categorize this PR?

/area ops-productivity robustness scalability
/kind enhancement
/priority critical
/platform aws

What this PR does / why we need it:
We are now skipping the explicit LB deletion when the shoot is of at least Kubernetes v1.16

See also: gardener/gardener#129

Release note:

The load balancers and security groups are no longer explicitly deleted by the AWS provider extension when a shoot cluster of at least Kubernetes v1.16 is being deleted. Instead, it now relies on the service-controller in the `cloud-controller-manager` to properly clean up.

@rfranzke rfranzke requested review from a team as code owners March 5, 2021 07:34
@gardener-robot gardener-robot added area/ops-productivity Operator productivity related (how to improve operations) area/robustness Robustness, reliability, resilience related area/scalability Scalability related kind/enhancement Enhancement, improvement, extension platform/aws Amazon web services platform/infrastructure priority/normal labels Mar 5, 2021
@rfranzke
Copy link
Member Author

rfranzke commented Mar 5, 2021

/test

@gardener-robot gardener-robot added the needs/review Needs review label Mar 5, 2021
@testmachinery
Copy link

testmachinery bot commented Mar 5, 2021 •
edited
Loading

Testrun: e2e-dwfln
Workflow: e2e-dwfln-wf
Phase: Failed

+---------------------+---------------------+--------+----------+
|        NAME         |        STEP         | PHASE  | DURATION |
+---------------------+---------------------+--------+----------+
| infrastructure-test | infrastructure-test | Failed | 9m34s    |
+---------------------+---------------------+--------+----------+

@gardener-robot gardener-robot added the size/s Size of pull request is small (see gardener-robot robot/bots/size.py) label Mar 5, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 5, 2021
@gardener-robot gardener-robot added kind/test Test priority/critical Needs to be resolved soon, because it impacts users negatively and removed priority/normal labels Mar 5, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 5, 2021
@rfranzke
Copy link
Member Author

rfranzke commented Mar 5, 2021

/invite @MartinWeindel @ialidzhikov @vlerenc

@ialidzhikov
Copy link
Member

/assign

@rfranzke
Copy link
Member Author

rfranzke commented Mar 5, 2021

/test

@testmachinery
Copy link

testmachinery bot commented Mar 5, 2021 •
edited
Loading

Testrun: e2e-mjljw
Workflow: e2e-mjljw-wf
Phase: Failed

+---------------------+---------------------+--------+----------+
|        NAME         |        STEP         | PHASE  | DURATION |
+---------------------+---------------------+--------+----------+
| infrastructure-test | infrastructure-test | Failed | 3m9s     |
+---------------------+---------------------+--------+----------+

@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 5, 2021
@rfranzke
Copy link
Member Author

rfranzke commented Mar 5, 2021

/test

@testmachinery
Copy link

testmachinery bot commented Mar 5, 2021 •
edited
Loading

Testrun: e2e-4nxll
Workflow: e2e-4nxll-wf
Phase: Failed

+---------------------+---------------------+--------+----------+
|        NAME         |        STEP         | PHASE  | DURATION |
+---------------------+---------------------+--------+----------+
| infrastructure-test | infrastructure-test | Failed | 4m43s    |
+---------------------+---------------------+--------+----------+

@vlerenc
Copy link
Member

vlerenc commented Mar 5, 2021 •
edited
Loading

Thanks @rfranzke!

@rfranzke @ialidzhikov
Update test/integration/infrastructure/infrastructure_test.go
2476eea 
Co-authored-by: Ismail Alidzhikov <i.alidjikov@gmail.com>
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 5, 2021
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 5, 2021
@rfranzke
Copy link
Member Author

rfranzke commented Mar 5, 2021

/test
/squash

@gardener-robot gardener-robot added the merge/squash Should be merged via 'Squash and merge' label Mar 5, 2021
@rfranzke
Copy link
Member Author

rfranzke commented Mar 5, 2021

/test

@testmachinery
Copy link

testmachinery bot commented Mar 5, 2021 •
edited
Loading

Testrun: e2e-bwh9r
Workflow: e2e-bwh9r-wf
Phase: Failed

+---------------------+---------------------+--------+----------+
|        NAME         |        STEP         | PHASE  | DURATION |
+---------------------+---------------------+--------+----------+
| infrastructure-test | infrastructure-test | Failed | 2m38s    |
+---------------------+---------------------+--------+----------+

@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 5, 2021
@rfranzke
Copy link
Member Author

rfranzke commented Mar 5, 2021

/test

@testmachinery
Copy link

testmachinery bot commented Mar 5, 2021 •
edited
Loading

Testrun: e2e-gt2j6
Workflow: e2e-gt2j6-wf
Phase: Failed

+---------------------+---------------------+--------+----------+
|        NAME         |        STEP         | PHASE  | DURATION |
+---------------------+---------------------+--------+----------+
| infrastructure-test | infrastructure-test | Failed | 2m52s    |
+---------------------+---------------------+--------+----------+

@ialidzhikov
Copy link
Member

/test

@testmachinery
Copy link

testmachinery bot commented Mar 5, 2021 •
edited
Loading

Testrun: e2e-fwb25
Workflow: e2e-fwb25-wf
Phase: Succeeded

+---------------------+---------------------+-----------+----------+
|        NAME         |        STEP         |   PHASE   | DURATION |
+---------------------+---------------------+-----------+----------+
| infrastructure-test | infrastructure-test | Succeeded | 16m36s   |
+---------------------+---------------------+-----------+----------+

ialidzhikov
ialidzhikov approved these changes Mar 5, 2021
View reviewed changes
Copy link
Member

@ialidzhikov ialidzhikov left a comment

Choose a reason for hiding this comment

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

/lgtm
/squash

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Mar 5, 2021
@ialidzhikov ialidzhikov merged commit 2ad7820 into gardener:master Mar 5, 2021
rfranzke added a commit to rfranzke/gardener-extension-provider-aws that referenced this pull request Mar 5, 2021
@rfranzke
Skip explicit LB deletion for Kubernetes >= v1.16 (gardener#290)
f809ab5
@rfranzke rfranzke mentioned this pull request Mar 5, 2021
Merged
@rfranzke rfranzke deleted the enh/skip-explicit-lb-deletion branch March 5, 2021 11:44
rfranzke added a commit that referenced this pull request Mar 5, 2021
@rfranzke
Skip explicit LB deletion for Kubernetes >= v1.16 (#290) (#291)
3e6853d
rfranzke added a commit to rfranzke/gardener-extension-provider-aws that referenced this pull request Mar 17, 2021
@rfranzke
Revert "Skip explicit LB deletion for Kubernetes >= v1.16 (gardener#290…
fcdcfa1 
…)"

This reverts commit 2ad7820.
@rfranzke rfranzke mentioned this pull request Mar 17, 2021
Merged
ialidzhikov pushed a commit that referenced this pull request Mar 18, 2021
@rfranzke
Activate explicit LB/SG deletion again and improve code to reduce API…
26ce3c2 
… calls (#295)

* Revert "Skip explicit LB deletion for Kubernetes >= v1.16 (#290)"

This reverts commit 2ad7820.

* Improve explicit LB/SG deletion to reduce API calls

* Exit early when no load balancers found in VPC

* PR review suggestion from @ialidzhikov
@rfranzke rfranzke mentioned this pull request May 31, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ialidzhikov ialidzhikov ialidzhikov approved these changes

@MartinWeindel MartinWeindel Awaiting requested review from MartinWeindel

@vlerenc vlerenc Awaiting requested review from vlerenc

Assignees

@ialidzhikov ialidzhikov

Labels
area/ops-productivity Operator productivity related (how to improve operations) area/robustness Robustness, reliability, resilience related area/scalability Scalability related kind/enhancement Enhancement, improvement, extension kind/test Test merge/squash Should be merged via 'Squash and merge' needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) platform/aws Amazon web services platform/infrastructure priority/critical Needs to be resolved soon, because it impacts users negatively reviewed/lgtm Has approval for merging size/s Size of pull request is small (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@rfranzke @ialidzhikov @vlerenc @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot