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

Azure: Allow to manage machine with identity #395

Merged
merged 1 commit into from Jan 31, 2020

Conversation

dkistner
Copy link
Member

Allow to create and manage Azure virtual machines which have an user assigned managed identity assigned to.

What this PR does / why we need it:
Azure machines can now be deployed with an attached user assigned managed identity, which can be used to authenticate to other Azure services like ACR.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

Azure machines can now be deployed with an attached user assigned managed identity.

@dkistner dkistner requested review from ggaurav10 and a team as code owners January 23, 2020 13:30
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 23, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added 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 Jan 23, 2020
Copy link
Contributor

@ggaurav10 ggaurav10 left a comment

Choose a reason for hiding this comment

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

@dkistner thanks for the PR. I think rebasing it required too.

pkg/driver/driver_azure.go Outdated Show resolved Hide resolved
@prashanth26 prashanth26 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 24, 2020
@prashanth26 prashanth26 added this to the v0.26.0 milestone Jan 24, 2020
Allow to create and manage Azure virtual machines which have an user assigned managed identity assigned to.
@gardener-robot-ci-3 gardener-robot-ci-3 added 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 Jan 24, 2020
@dkistner
Copy link
Member Author

Thanks @ggaurav10. Should be done.

@prashanth26 prashanth26 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 25, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added 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 Jan 25, 2020
@prashanth26 prashanth26 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 27, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added 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 Jan 27, 2020
@@ -9834,7 +9798,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA
Properties: map[string]spec.Schema{
"replicas": {
SchemaProps: spec.SchemaProps{
Description: "Replicas is the number of desired replicas. This is a pointer to distinguish between explicit zero and unspecified. Defaults to 0. More info: https://kubernetes.io/docs/concepts/workloads/controllers/replicationcontroller#what-is-a-replicationcontroller",
Description: "Replicas is the number of desired replicas. This is a pointer to distinguish between explicit zero and unspecified. Defaults to 1. More info: https://kubernetes.io/docs/concepts/workloads/controllers/replicationcontroller#what-is-a-replicationcontroller",
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't change with this PR. Is it a rebasing issue?

Copy link
Member

Choose a reason for hiding this comment

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

This got in as hack/generatecode was not run previously. Though we need to check on the open-api generate-code path, as in reality it gets defaulted to 0, but not the concern of this PR.

Copy link
Member

@hardikdr hardikdr left a comment

Choose a reason for hiding this comment

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

@dkistner
Copy link
Member Author

Hmm I just called hack/generate-code and this change was produced by the script. I have build this based on the master. Is there something which needs to be fixed on the upstream master? If yes give me a hint when I need to rebase and re run the code generation.

@hardikdr
Copy link
Member

Sure @dkistner, We haven't yet been able to look into it. We shall soon fix it and let you know.

@hardikdr
Copy link
Member

One of the recent PR dint run ./hack/generatecode, which brought these changes into this PR.
We can merge this one.

@hardikdr hardikdr merged commit d5052df into gardener:master Jan 31, 2020
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

7 participants