CORS-4078: aws: standardize AWS client setup in the destroy code#10340
CORS-4078: aws: standardize AWS client setup in the destroy code#10340tthvo wants to merge 3 commits intoopenshift:mainfrom
Conversation
The region and partition IDs are defined in pkg/types/aws/regions.go. And any configurations to create an aws config is stored in pkg/asset/installconfig/aws/sessionv2.go
Endpoint resolvers: pkg/asset/installconfig/aws/endpoints.go Clients: pkg/asset/installconfig/aws/clients.go This way, the clients are created in a standard way with standardized endpoint resolvers. Also, this allows backwards compatibility with v1-styled custom service endpoint specifications.
The AWS API defines a list of API error in document [0]. This commit align the destroy code to handle the correct error codes when the resources are not found (i.e. already deleted). References [0] https://docs.aws.amazon.com/AWSEC2/latest/APIReference/errors-overview.html
|
@tthvo: This pull request references CORS-4078 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/cc @barbacbd |
|
@tthvo: This pull request references CORS-4078 which is a valid jira issue. Requesting review from QA contact: DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
Please see https://github.com/openshift/installer/blob/release-4.17/pkg/destroy/aws/ec2helpers.go for a version with the correct not-found error codes. |
|
/retest |
|
/payload-job periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-ipi-localzone-fips-mini-perm-f14 |
|
@tthvo: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6c9e8c20-1374-11f1-94ce-1e1e5bae101d-0 |
|
/payload-job periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-usgov-ipi-private-ep-fips-f7 |
|
@tthvo: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/72f83800-1374-11f1-8dc9-0817df020d6d-0 |
|
/payload-job periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-ipi-shared-vpc-phz-sts-fips-openldap-mini-perm-f7 |
|
@tthvo: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/7b0671b0-1374-11f1-8dd6-d25fd0565607-0 |
|
Payload and presubmit testing showed the deprovisioning worked as expected :d |
|
/verified by CI tests |
|
@tthvo: This PR has been marked as verified by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
This PR standardizes AWS client setup in the destroy code with the rest of the code base.
pkg/asset/installconfig/aws/endpoints.gopkg/asset/installconfig/aws/clients.goThis way, the client constructs are unified and easy to extend and maintain. Also, this allows backwards compatibility with
v1-styled custom service endpoint specifications.
Important
According to the AWS docs, there are a few not-found error codes that are incorrectly defined in the installer destroy code. The problem is a "regression" from migration to AWS SDK v2, which seems to be also present in v4.21- v4.18.
This PR will fix it, and we will need to backport the fix commit to older releases.