Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update SDK to track2 in UpdateAPIIPEarly #3579

Merged
merged 4 commits into from
Jun 7, 2024
Merged

Conversation

bitoku
Copy link
Collaborator

@bitoku bitoku commented May 13, 2024

Which issue this PR addresses:

Fixes a part of https://issues.redhat.com/browse/ARO-4665 and https://issues.redhat.com/browse/ARO-7316

What this PR does / why we need it:

It contains three changes.

  1. add PublicIPAddresses track2 SDK.
  2. make updateAPIIPEarly use track2 SDK.
  3. add retryOption to track2 SDKs.

For retryOption see https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azcore@v1.11.1/policy#RetryOptions and https://learn.microsoft.com/en-us/azure/developer/go/azure-sdk-core-concepts#http-pipeline-flow

The reason for creating new methods instead of modifying the current ones is to make the impact of each change as small as possible.

Test plan for issue:

unittest for updateAPIIPEarly.
test cluster installation in local.
e2e

Is there any documentation that needs to be updated for this PR?

tech debt cleanup N/A

How do you know this will function as expected in production?

cluster installation test

@bitoku bitoku changed the title Update apiip early track2 change SDK of UpdateAPIIPEarly to track2 May 14, 2024
@github-actions github-actions bot added the needs-rebase branch needs a rebase label May 16, 2024
Copy link

Please rebase pull request.

@github-actions github-actions bot removed the needs-rebase branch needs a rebase label May 16, 2024
@bitoku bitoku marked this pull request as ready for review May 16, 2024 17:38
@bitoku bitoku changed the title change SDK of UpdateAPIIPEarly to track2 update SDK to track2 in UpdateAPIIPEarly May 16, 2024
@bitoku
Copy link
Collaborator Author

bitoku commented May 16, 2024

/azp run ci,e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Collaborator

@AldoFusterTurpin AldoFusterTurpin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this. No clear blockers, but I left some questions and some minor suggestions in case you can answer.

Thank you a lot.

pkg/util/azureclient/azuresdk/common/options.go Outdated Show resolved Hide resolved
pkg/util/azureclient/azuresdk/common/options.go Outdated Show resolved Hide resolved
pkg/util/azureclient/azuresdk/common/options.go Outdated Show resolved Hide resolved
pkg/util/azureclient/azuresdk/common/options.go Outdated Show resolved Hide resolved
pkg/cluster/ipaddresses_test.go Outdated Show resolved Hide resolved
pkg/cluster/ipaddresses_test.go Outdated Show resolved Hide resolved
pkg/cluster/cluster.go Show resolved Hide resolved
@bitoku
Copy link
Collaborator Author

bitoku commented Jun 4, 2024

/azp run ci,e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Collaborator

@AldoFusterTurpin AldoFusterTurpin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you @bitoku for the clarifications and applying the optional suggestions. Great work! 🚀

Copy link
Contributor

@tiguelu tiguelu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, great work!

@tiguelu tiguelu merged commit 9216fef into master Jun 7, 2024
20 checks passed
@tiguelu tiguelu deleted the updateAPIIPEarly-track2 branch June 7, 2024 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants