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 github.com/Azure/azure-sdk-for-go #369

Merged
merged 1 commit into from Dec 12, 2019

Conversation

ialidzhikov
Copy link
Member

What this PR does / why we need it:
Recently we faced issues vendoring machine-controller-manager in other projects - for example gardener/gardener-extensions#463. gardener/gardener-extensions refers github.com/Azure/azure-sdk-for-go@v32.6.0 while machine-controller-manager - v26.1.0. This PR updates also machine-controller-manager to use v32.6.0.
Also github.com/Azure/go-autorest is split into go submodules and this PR switches to the new submodule dependencies - ref https://github.com/Azure/go-autorest/#using-with-go-modules.

Which issue(s) this PR fixes:
Ref gardener/gardener-extensions#463

Special notes for your reviewer:

Release note:

`github.com/Azure/azure-sdk-for-go` is updated to `v32.6.0`.

Signed-off-by: ialidzhikov <i.alidjikov@gmail.com>
@ialidzhikov ialidzhikov requested review from ggaurav10 and a team as code owners December 10, 2019 23:41
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Dec 10, 2019
@ialidzhikov
Copy link
Member Author

@gardener/mcm-maintainers , any feedback?

Copy link
Contributor

@prashanth26 prashanth26 left a comment

Choose a reason for hiding this comment

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

LGTM.

@hardikdr
Copy link
Member

Thanks @ialidzhikov for the PR. We were checking if the upgrade of the vendor for Azure is breaking any of the functions semantically.

As our integration test-pipeline is not enabled for Azure[explicitly disabled due to issues in past], I am wondering if you have tested your changes, if it works well?

  • At least for a happy path of creation/deletion/roll-outs of machines.

@ialidzhikov
Copy link
Member Author

Yes, I made a docker image and using imageVectorOverwrite in provider-azure I tested scale up, scale down, creation and hibernation.
Double verification is always welcome.

@hardikdr
Copy link
Member

I believe that should be sufficient then, thanks a lot. :)
Merging.

@hardikdr hardikdr merged commit b01307f into gardener:master Dec 12, 2019
@ialidzhikov ialidzhikov deleted the enh/update-azure-sdk branch December 12, 2019 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants