WIP: ⚠️ aws-sdk-go-v2 refactor starting with s3

[mjlshen]

What type of PR is this?
/kind feature
/kind cleanup

What this PR does / why we need it:
Just sharing how the aws-sdk-go-v2 refactor is going to allow for discussion when needed

Which issue(s) this PR fixes:
Part of #2225

Special notes for your reviewer:

Checklist:

• squashed commits
• includes documentation
• includes emojis
• adds unit tests
• adds or updates e2e tests

Release note:

Migrate the S3 service to aws-sdk-go-v2

[k8s-ci-robot]

[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 richardcase for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[mjlshen]

Worth calling out, it's now straightforward to unit test AWS clients without gomock https://docs.aws.amazon.com/sdk-for-go/v2/developer-guide/unit-testing.html, but I chose to keep doing so for now to limit the breadth of changes. Happy to start refactoring gomock away from the AWS services in a follow up PR if that sounds good.

[punkwalker]

@mjlshen Just checking if we have any timeline for getting this merged.

[mjlshen]

@punkwalker It's quite a big change. I think this MR can start to be reviewed as-is, however I have two more TODOs to re-write func CaptureRequestMetrics and add session logic to func NewS3Client for all of the code I modified to behave as similarly as possible to the original - for only the S3 AWS client. It's up to reviewers/maintainers if we would want to merge in just one client updated to aws-sdk-go-v2 or not.

I anticipate the following AWS clients can be refactored much more quickly though since this will lay the groundwork.

[punkwalker]

@punkwalker It's quite a big change. I think this MR can start to be reviewed as-is, however I have two more TODOs to re-write func CaptureRequestMetrics and add session logic to func NewS3Client for all of the code I modified to behave as similarly as possible to the original - for only the S3 AWS client. It's up to reviewers/maintainers if we would want to merge in just one client updated to aws-sdk-go-v2 or not.

I anticipate the following AWS clients can be refactored much more quickly though since this will lay the groundwork.

Agreed. I was planning to go for same route for EKSClient but I think we can let reviewers comment on this approach.

@richardcase any thoughts?

[phoban01]

@punkwalker It's quite a big change. I think this MR can start to be reviewed as-is, however I have two more TODOs to re-write func CaptureRequestMetrics and add session logic to func NewS3Client for all of the code I modified to behave as similarly as possible to the original - for only the S3 AWS client. It's up to reviewers/maintainers if we would want to merge in just one client updated to aws-sdk-go-v2 or not.

I anticipate the following AWS clients can be refactored much more quickly though since this will lay the groundwork.

@mjlshen Can we help to parallelize the work on additional clients?

[punkwalker]

@punkwalker It's quite a big change. I think this MR can start to be reviewed as-is, however I have two more TODOs to re-write func CaptureRequestMetrics and add session logic to func NewS3Client for all of the code I modified to behave as similarly as possible to the original - for only the S3 AWS client. It's up to reviewers/maintainers if we would want to merge in just one client updated to aws-sdk-go-v2 or not.
I anticipate the following AWS clients can be refactored much more quickly though since this will lay the groundwork.

@mjlshen Can we help to parallelize the work on additional clients?

I am working with @mjlshen and I have created a PR to his fork for speeding this up. I have already started with EKS client migration but the changes are on hold. Once S3Client is merged, we will take up EKS client.

[k8s-ci-robot]

@mjlshen: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-aws-test	293a401	link	true	/test pull-cluster-api-provider-aws-test
pull-cluster-api-provider-aws-verify	293a401	link	true	/test pull-cluster-api-provider-aws-verify

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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-sigs/prow repository. I understand the commands that are listed here.