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 support #469

Merged
merged 9 commits into from
Jun 5, 2019
Merged

Azure support #469

merged 9 commits into from
Jun 5, 2019

Conversation

kron4eg
Copy link
Member

@kron4eg kron4eg commented May 30, 2019

Fixes #455

Azure support

@kubermatic-bot kubermatic-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 30, 2019
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kubermatic-bot kubermatic-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 30, 2019
Signed-off-by: Artiom Diomin <kron82@gmail.com>
* machinecontroller contains fix for azure credentials
* use short hostname on azure

Signed-off-by: Artiom Diomin <kron82@gmail.com>
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kubermatic-bot kubermatic-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 4, 2019
Signed-off-by: Artiom Diomin <kron82@gmail.com>
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg kron4eg changed the title WIP: Azure support Azure support Jun 4, 2019
@kubermatic-bot kubermatic-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 4, 2019
@kron4eg kron4eg requested a review from xmudrii June 4, 2019 16:03
@kron4eg
Copy link
Member Author

kron4eg commented Jun 5, 2019

/retest

Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

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

I left a question, but beside that I'd also like to see:

  • Quickstart for Azure
  • Environment variables documented in the environment_variables.md document
  • Documentation updated to mention that we support Azure
    • This could be a follow-up once we do the release, but we should ensure we don't forget it

I'm not going to block on this, so if you want, you can do it in a follow-up.

Otherwise LGTM 💯

# Provider specific settings

variable "location" {
description = ""
Copy link
Member

Choose a reason for hiding this comment

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

Are those descriptions missing intentionally? Should we add descriptions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Apparently just missed it 🤷‍♂️

Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Copy link
Member Author

kron4eg commented Jun 5, 2019

@xmudrii I've addressed (I think all) those comments, PTAL again.

Signed-off-by: Artiom Diomin <kron82@gmail.com>
Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

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

Left one more comment as per the quickstart guide.

@@ -60,6 +60,7 @@ func ValidateCloudProviderSpec(p kubeone.CloudProviderSpec, fldPath *field.Path)

switch p.Name {
case kubeone.CloudProviderNameAWS:
case kubeone.CloudProviderNameAzure:
Copy link
Member

Choose a reason for hiding this comment

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

Is cloudConfig required for Azure? If yes, should we enforce it here?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah... actually it's required for vSphere too and I don't remember that I've done it 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

done

* azure
* vsphere

Signed-off-by: Artiom Diomin <kron82@gmail.com>
Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 5, 2019
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 9495f7b0f48705d102385f1f56ef00aaaa723f7e

@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xmudrii

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

The pull request process is described here

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

@kubermatic-bot kubermatic-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 5, 2019
@xmudrii
Copy link
Member

xmudrii commented Jun 5, 2019

/retest

2 similar comments
@xmudrii
Copy link
Member

xmudrii commented Jun 5, 2019

/retest

@kron4eg
Copy link
Member Author

kron4eg commented Jun 5, 2019

/retest

@kubermatic-bot
Copy link
Contributor

@kron4eg: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubeone-e2e-digitalocean-conformance-1.13.5 77da30d link /test pull-kubeone-e2e-digitalocean-conformance-1.13.5

Full PR test history

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

@kron4eg
Copy link
Member Author

kron4eg commented Jun 5, 2019

/override pull-kubeone-e2e-digitalocean-conformance-1.13.5

DO API strikes again: 503 Service is currently unavailable.

@kubermatic-bot
Copy link
Contributor

@kron4eg: Overrode contexts on behalf of kron4eg: pull-kubeone-e2e-digitalocean-conformance-1.13.5

In response to this:

/override pull-kubeone-e2e-digitalocean-conformance-1.13.5

DO API strikes again: 503 Service is currently unavailable.

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/test-infra repository.

@kubermatic-bot kubermatic-bot merged commit 059ea2e into master Jun 5, 2019
@kubermatic-bot kubermatic-bot deleted the 455-azure branch June 5, 2019 14:31
Copy link

@craiglpeters craiglpeters left a comment

Choose a reason for hiding this comment

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

Couple of copy/paste errors in the Azure quick start

versions:
kubernetes: '1.14.2'
cloudProvider:
name: 'vsphere'

Choose a reason for hiding this comment

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

This should say 'azure'

```

This command will wait for all worker nodes to be gone. Once it's done you can
proceed and destroy the vSphere infrastructure using Terraform:

Choose a reason for hiding this comment

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

Another copy/paste error. Replace 'vSphere' with 'Azure'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for azure
4 participants