cluster-api-provider-aws
cluster-api-provider-aws copied to clipboard
kubernetes-sigs
Deletion fails for aws cluster with vpc limit exceeded
State
READY
What is the purpose of the pull request
deletion fails for aws cluster with vpc limit exceeded
Implementation
Issue: In case cluster creation didn't go through due to the VPC limit being exceeded, the VPC ID will be nil and during deletion of the such cluster, the AWS client will give the error.
│ E0907 09:53:16.390472 1 network.go:89] controller/awscluster "msg"="non-fatal: VPC ID is missing, " "error"=null "cluster"="am-cluster-123-4" "name"="am-cluster-123-4" "namespace"="am-cluster-123-4" "reco │
│ nciler group"="infrastructure.cluster.x-k8s.io" "reconciler
Fix: Check the error code during VPC deletion and skip it if the error code is "MissingParameter"
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #
Special notes for your reviewer:
Checklist:
- [ ] squashed commits
- [ ] includes documentation
- [ ] adds unit tests
- [ ] adds or updates e2e tests
Hi @AmitSahastra. Thanks for your PR.
I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
/ok-to-test
Thank you for your contribution! Please include a unit test for your change. :) Thanks!
@Ankitasw: The following commands are available to trigger required jobs:
-
/test pull-cluster-api-provider-aws-build -
/test pull-cluster-api-provider-aws-test -
/test pull-cluster-api-provider-aws-verify
The following commands are available to trigger optional jobs:
-
/test pull-cluster-api-provider-aws-apidiff-main -
/test pull-cluster-api-provider-aws-e2e -
/test pull-cluster-api-provider-aws-e2e-blocking -
/test pull-cluster-api-provider-aws-e2e-clusterclass -
/test pull-cluster-api-provider-aws-e2e-conformance -
/test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts -
/test pull-cluster-api-provider-aws-e2e-eks
Use /test all to run the following jobs that were automatically triggered:
-
pull-cluster-api-provider-aws-apidiff-main -
pull-cluster-api-provider-aws-build -
pull-cluster-api-provider-aws-test -
pull-cluster-api-provider-aws-verify
In response to this:
/test ?
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@AmitSahastra - would you be able to add a test to cover this scenario?
@AmitSahastra would you be able to continue with the test scenario?
Hi, I am held up with a few things and unable to take time to finish UTs to cover test scenarios. I can close this PR for some time and reopen it with updated tests.
@richardcase I can raise a separate issue to write E2E test for this scenario, and merge this PR, if that's ok?
@Ankitasw - ok, lets go with creating an issue to do the e2e test as a follow-up and get this in
@AmitSahastra I think you might have to rebase the PR, I have created separate issue for E2E tests that we can address later, but this needs to fixed now.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: Ankitasw
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [Ankitasw]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment