[WIP] feat: Basic implementation enabling azure platform #1460

serbrech · 2019-03-26T03:29:52Z

This PR is being split into smaller chunks and should not be merged.

Basic implementation of the platform specific code in the installer to enable Azure as a target platform.

It is bypassing the MachineSpecProvider until we have it ready.
There are a few shortcuts, but it allows to run the installer, generate all the assets and the tfvars needed for the terraform PR (#1454) to deploy a cluster on Azure.

The aim for this PR is to get a minimum working solution to be able to iterate in smaller subsequent PRs.

TODO :

Use https://github.com/openshift/cluster-api-provider-azure
Import azure terraform and required plugins
Remove hardcoded values

openshift-ci-robot · 2019-03-26T03:30:06Z

Hi @serbrech. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

openshift-ci-robot · 2019-03-26T03:30:08Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: serbrech
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: russellb

If they are not already assigned, you can assign the PR to them by writing /assign @russellb in a comment when ready.

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

The pull request process is described here

Details

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

serbrech · 2019-03-26T05:01:43Z

@enxebre @imcleod @juan-lee @awesomenix

enxebre · 2019-03-26T14:36:38Z

/ok-to-test

pkg/asset/installconfig/installconfig.go

pkg/asset/installconfig/platform.go

pkg/asset/manifests/dns.go

pkg/types/installconfig.go

pkg/types/azure/platform.go

pkg/types/azure/validation/platform_test.go

enxebre · 2019-03-27T09:46:13Z

vendor/sigs.k8s.io/cluster-api-provider-azure/LICENSE

do we need this yet? Is go I can't find the Gopkg.* changes

it was initially faked by adding a dummy implementation in the vendor.
it's dirty, but we can soon remove it.

pkg/types/azure/validation/platform.go

abhinavdahiya · 2019-03-29T23:29:34Z

pkg/asset/installconfig/platform.go

Why do you need this change?

allows to get the platform name without depending on more assets : https://github.com/openshift/installer/pull/1460/files?file-filters%5B%5D=.go#diff-4d231774d44727d779db320e7b6c189eR39

allows to get the platform name without depending on more assets

😕 do you mind expanding on that?
in the current usage, why not call platform.Name() directly compared to CurrentPlatformName ?

that gives returns the name of the asset. not the name of the platform.

https://github.com/openshift/installer/blob/0be524e5a2d67459bc0ee6e2614b6b257c213de0/pkg/asset/installconfig/platform.go#L81

pkg/types/azure/dns.go

pkg/dns/dns.go

abhinavdahiya · 2019-03-29T23:42:50Z

pkg/asset/cluster/azure/azure.go

TODO: this might not be required.

azure doesn't support / in tags, and I don't know what these tags are used for in openshift.
to get the terraform deployment to work, I changed the / with a ..
if the `kubernetes/io/cluster' tags are not needed for openshift, I'm happy to remove them

Adding the `[[override]]` and running `dep ensure` did not remove the package It was necessary to remove the dependency from `Gopkg.lock` manually before running `dep ensure`, and adding a required statement to the root Gopkg.toml

redirecting to private branch for the moment

…v1alpha1

openshift-ci-robot · 2019-04-15T02:29:22Z

@serbrech: PR needs rebase.

Details

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.

openshift-ci-robot · 2019-05-15T19:44:47Z

@serbrech: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/gofmt	`a14f996`	link	`/test gofmt`
ci/prow/images	`a14f996`	link	`/test images`
ci/prow/e2e-aws-upgrade	`a14f996`	link	`/test e2e-aws-upgrade`
ci/prow/verify-vendor	`a14f996`	link	`/test verify-vendor`
ci/prow/e2e-aws	`a14f996`	link	`/test e2e-aws`

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.

Details

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.

abhinavdahiya · 2019-05-20T18:48:44Z

this was replaced with multiple PRs
like #1721 #1454 etc.

/close

openshift-ci-robot · 2019-05-20T18:48:46Z

@abhinavdahiya: Closed this PR.

Details

In response to this:

this was replaced with multiple PRs
like #1721 #1454 etc.

/close

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.

openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 26, 2019

openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 26, 2019

openshift-ci-robot requested review from hardys and staebler March 26, 2019 03:30

openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 26, 2019

abhinavdahiya reviewed Mar 26, 2019

View reviewed changes

pkg/asset/installconfig/installconfig.go Show resolved Hide resolved

abhinavdahiya reviewed Mar 26, 2019

View reviewed changes

pkg/asset/installconfig/platform.go Show resolved Hide resolved

abhinavdahiya reviewed Mar 26, 2019

View reviewed changes

pkg/asset/manifests/dns.go Show resolved Hide resolved

serbrech commented Mar 27, 2019

View reviewed changes

pkg/types/installconfig.go Outdated Show resolved Hide resolved

enxebre reviewed Mar 27, 2019

View reviewed changes

pkg/types/azure/platform.go Outdated Show resolved Hide resolved

enxebre reviewed Mar 27, 2019

View reviewed changes

pkg/types/azure/validation/platform_test.go Outdated Show resolved Hide resolved

enxebre reviewed Mar 27, 2019

View reviewed changes

pkg/types/azure/validation/platform.go Outdated Show resolved Hide resolved

serbrech force-pushed the azure-platform branch from 66eec8f to f798cc1 Compare March 28, 2019 18:54

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 28, 2019

serbrech force-pushed the azure-platform branch from f798cc1 to 4d5b8d6 Compare March 28, 2019 19:02

openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 28, 2019

abhinavdahiya reviewed Mar 29, 2019

View reviewed changes

pkg/types/azure/dns.go Outdated Show resolved Hide resolved

abhinavdahiya reviewed Mar 29, 2019

View reviewed changes

pkg/dns/dns.go Outdated Show resolved Hide resolved

abhinavdahiya reviewed Mar 29, 2019

View reviewed changes

pkg/dns/dns.go Outdated Show resolved Hide resolved

abhinavdahiya reviewed Mar 29, 2019

View reviewed changes

serbrech force-pushed the azure-platform branch from c677cf3 to 6902954 Compare March 30, 2019 03:25

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2019

serbrech added 20 commits April 8, 2019 10:01

change azure tfvar variable name to match terraform

d6064bc

naming for dnsConfig implementation + trying to fix dep

063868e

moved azure types into asset and installconfig

03fb2ca

fix dns tests

3fdc27f

fix: addressed multiple golang.org/x/net/trace package

483843b

Adding the `[[override]]` and running `dep ensure` did not remove the package It was necessary to remove the dependency from `Gopkg.lock` manually before running `dep ensure`, and adding a required statement to the root Gopkg.toml

fix installconfig_test initialization

18c1682

feat: fetch available regions from provided subscription

719c428

move mock configuration hook into dns factory package

9f1cffd

feat: querying for public dns zone

2617d80

fix: go lint

03f2719

fix: adding missing instanceClass to azure machinePool

df70afb

adding cluster-api-provider-azure dependency

979ca53

redirecting to private branch for the moment

support for azure in cloud-creds-secret templates

aad503f

add a systemd unit to set hostname azure

b567e91

fix: AzureMachineProviderSpec configuration to match vendored capz

a999777

removing azure region restriction

f08051d

dep ensure to pull cluster-api-provider-azure/pkg/apis/azureprovider/…

8ea07fc

…v1alpha1

add azure provider credentials needed for cluster-api

b888463

fix: gofmt

8f6c38e

remove unused Identifier from azure.Metadata

a14f996

serbrech force-pushed the azure-platform branch from 54bb867 to a14f996 Compare April 8, 2019 17:02

serbrech mentioned this pull request Apr 8, 2019

pkg/asset: add azure installconfig, machines and manifests #1555

Merged

serbrech changed the title ~~feat: Basic implementation enabling azure platform~~ [WIP] feat: Basic implementation enabling azure platform Apr 8, 2019

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 8, 2019

openshift-ci-robot closed this May 20, 2019

[WIP] feat: Basic implementation enabling azure platform #1460

[WIP] feat: Basic implementation enabling azure platform #1460

Uh oh!

Conversation

serbrech commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Mar 26, 2019

Uh oh!

openshift-ci-robot commented Mar 26, 2019

Uh oh!

serbrech commented Mar 26, 2019

Uh oh!

enxebre commented Mar 26, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Apr 15, 2019

Uh oh!

openshift-ci-robot commented May 15, 2019

Uh oh!

abhinavdahiya commented May 20, 2019

Uh oh!

openshift-ci-robot commented May 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

serbrech commented Mar 26, 2019 •

edited

Loading