Skip to content

Conversation

@cfBrianMiller
Copy link

Creating azure environment customization options with the openshift-installer. Added the ability to add the environment variable AZURE_ENVIRONMENT that when set will configure the correct endpoints for the installer to point to.

This is the baseline for future changes that will fully enable custom azure governments.

==Known Issues==
This is only targeted at the create install-config command to be used with the UPI azure mag documentation.

@openshift-ci-robot 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 30, 2020
@openshift-ci-robot
Copy link
Contributor

Hi @cfBrianMiller. 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
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign jhixson74
You can assign the PR to them by writing /assign @jhixson74 in a comment when ready.

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

Details 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

@openshift-ci-robot 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 Apr 30, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 30, 2020
@abhinavdahiya
Copy link
Contributor

Few general things,

we only consume credentials using env variables or user's host environment, because those are not technically configuration. and therefore the environment for Azure should be an install-config.yaml configuration.

secondly, since this PR is trying to target UPI only for this configuration, we should add validations to https://github.com/openshift/installer/blob/master/pkg/asset/installconfig/platformprovisioncheck.go such that any non-public environment is not allowed for creating clusters by the installer.

lastly, we do not prefer providing a shell script, rather a walk through of various steps in a document. like https://github.com/openshift/installer/blob/master/docs/user/azure/install_upi.md

  • each step should be defined in the document most importantly, but additionaly you can include a script to do that step too.
  • the document should never say run this script, rather include the script contents in the doc. this provides a much better ux for users
  • please try to document all the per-requisites.

@abhinavdahiya
Copy link
Contributor

Also if you are interested in driving the conversation of how to get the platform to support this natively,
here is an example of enhancement the is currently driving the aws custom endpoints work https://github.com/openshift/enhancements/blob/master/enhancements/installer/aws-custom-region-and-endpoints.md

@cfBrianMiller
Copy link
Author

cfBrianMiller commented May 1, 2020

@abhinavdahiya

I added it as an environment variable so the rest of the create install-config process would work properly without the need for a prompt. (Azure region and DNS zone selection) I will add the validation for the environment.

As you can tell by the provided shell script the tasks are non-trivial and would make the supporting document very long by including every step, not to mention the shell script makes it easy for users to consume. Is there a middle ground that can be reached here?

I like the enhancement feature, I'll create one a little bit later.

@abhinavdahiya
Copy link
Contributor

Also if you are interested in driving the conversation of how to get the platform to support this natively,
here is an example of enhancement the is currently driving the aws custom endpoints work https://github.com/openshift/enhancements/blob/master/enhancements/installer/aws-custom-region-and-endpoints.md

openshift/enhancements#321

I created an enhancement that should detail the long term plan for known azure cloud environments like MAG

PTAL @cfBrianMiller

@abhinavdahiya
Copy link
Contributor

/close

in favor of #3634

@openshift-ci-robot
Copy link
Contributor

@abhinavdahiya: Closed this PR.

Details

In response to this:

/close

in favor of #3634

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants