Skip to content

CORS-4078: aws: standardize AWS client setup in the destroy code#10340

Open
tthvo wants to merge 3 commits intoopenshift:mainfrom
tthvo:CORS-4078
Open

CORS-4078: aws: standardize AWS client setup in the destroy code#10340
tthvo wants to merge 3 commits intoopenshift:mainfrom
tthvo:CORS-4078

Conversation

@tthvo
Copy link
Member

@tthvo tthvo commented Feb 26, 2026

This PR standardizes AWS client setup in the destroy code with the rest of the code base.

  • Endpoint resolvers: pkg/asset/installconfig/aws/endpoints.go
  • Clients: pkg/asset/installconfig/aws/clients.go

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

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
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Feb 26, 2026

@tthvo: This pull request references CORS-4078 which is a valid jira issue.

Details

In response to this:

This PR standardizes AWS client setup in the destroy code with the rest of the code base.

  • Endpoint resolvers: pkg/asset/installconfig/aws/endpoints.go
  • Clients: pkg/asset/installconfig/aws/clients.go

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

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.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 26, 2026
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 26, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign patrickdillon for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

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

@tthvo
Copy link
Member Author

tthvo commented Feb 26, 2026

/cc @barbacbd
/jira cc-qa
/label platform/aws

@openshift-ci openshift-ci bot requested a review from barbacbd February 26, 2026 21:47
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Feb 26, 2026

@tthvo: This pull request references CORS-4078 which is a valid jira issue.

Requesting review from QA contact:
/cc @yunjiang29

Details

In response to this:

/cc @barbacbd
/jira cc-qa
/label platform/aws

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.

@tthvo
Copy link
Member Author

tthvo commented Feb 26, 2026

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.

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.

@tthvo
Copy link
Member Author

tthvo commented Feb 27, 2026

/retest

@tthvo
Copy link
Member Author

tthvo commented Feb 27, 2026

/payload-job periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-ipi-localzone-fips-mini-perm-f14

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 27, 2026

@tthvo: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-ipi-localzone-fips-mini-perm-f14

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6c9e8c20-1374-11f1-94ce-1e1e5bae101d-0

@tthvo
Copy link
Member Author

tthvo commented Feb 27, 2026

/payload-job periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-usgov-ipi-private-ep-fips-f7

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 27, 2026

@tthvo: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-usgov-ipi-private-ep-fips-f7

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/72f83800-1374-11f1-8dc9-0817df020d6d-0

@tthvo
Copy link
Member Author

tthvo commented Feb 27, 2026

/payload-job periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-ipi-shared-vpc-phz-sts-fips-openldap-mini-perm-f7

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 27, 2026

@tthvo: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-openshift-tests-private-release-4.22-amd64-nightly-aws-ipi-shared-vpc-phz-sts-fips-openldap-mini-perm-f7

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/7b0671b0-1374-11f1-8dd6-d25fd0565607-0

@tthvo
Copy link
Member Author

tthvo commented Feb 27, 2026

Payload and presubmit testing showed the deprovisioning worked as expected :d

@tthvo
Copy link
Member Author

tthvo commented Feb 27, 2026

/verified by CI tests

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Feb 27, 2026
@openshift-ci-robot
Copy link
Contributor

@tthvo: This PR has been marked as verified by CI tests.

Details

In response to this:

/verified by CI tests

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.

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

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. platform/aws verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants