Deletion fails for aws cluster with vpc limit exceeded

[AmitSahastra]

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

[k8s-ci-robot]

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.

[Skarlso]

/ok-to-test

Thank you for your contribution! Please include a unit test for your change. :) Thanks!

[Ankitasw]

/test ?

[k8s-ci-robot]

@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.

[Ankitasw]

/test pull-cluster-api-provider-aws-e2e

[richardcase]

@AmitSahastra - would you be able to add a test to cover this scenario?

[richardcase]

/milestone v2.0.0

[Ankitasw]

@AmitSahastra would you be able to continue with the test scenario?

[AmitSahastra]

@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.

[Ankitasw]

@richardcase I can raise a separate issue to write E2E test for this scenario, and merge this PR, if that's ok?

[richardcase]

@Ankitasw - ok, lets go with creating an issue to do the e2e test as a follow-up and get this in

[Ankitasw]

/lgtm
/approve

[Ankitasw]

@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.

[AmitSahastra]

/test pull-cluster-api-provider-aws-e2e

[AmitSahastra]

@Ankitasw have rebased it, pls do review it again.

[Ankitasw]

Also, please squash the commits.

[Ankitasw]

@AmitSahastra thanks will merge it once the prow failures resolve

[Ankitasw]

@AmitSahastra could you push a blank commit, such that jobs run again?

[Ankitasw]

/retest

[Ankitasw]

/lgtm
/approve

[k8s-ci-robot]

[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

Needs approval from an approver in each of these files:

• OWNERS [Ankitasw]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment