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.

Sign up for GitHub

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

Jump to bottom

WIP: IPI installation to Azure Stack #4799

Closed
wants to merge 11 commits into from
Closed

WIP: IPI installation to Azure Stack #4799

wants to merge 11 commits into from

Conversation

staebler
Copy link
Contributor

@staebler staebler commented Mar 28, 2021 •
edited

  • Add AzureStack as a valid value for the azure cloudName field.
  • Add the platform.azure.armEndpoint field for specifying the ARM endpoint when installing on Azure Stack.
  • Add validation specific to Azure Stack.
  • Add the github.com/terraform-providers/terraform-provider-azurestack module. This is replaced with a fork that bumps the azurerm provider used and fixes a bug the an invalid ConflictsWith in the availability set resource.
  • Add terraform for provisioning a cluster using the azurestack provider.

https://issues.redhat.com/browse/CORS-1665

This builds on #4729

/cc @jhixson74 @patrickdillon

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 28, 2021
@openshift-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@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 ask for approval from staebler after the PR has been reviewed.

The full list of commands accepted by this bot can be found 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

patrickdillon
patrickdillon reviewed Mar 29, 2021
View reviewed changes
Copy link
Contributor

@patrickdillon patrickdillon left a comment

Choose a reason for hiding this comment

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

Rather than /data/data/azure/main-stack.tf and the exclusion logic couldn't we just write the tf configs in /data/data/azurestack? At a quick glance it doesn't look like azurestack is reusing any of the azure modules; but perhaps there is a benefit or need?

@staebler
Copy link
Contributor Author

Rather than /data/data/azure/main-stack.tf and the exclusion logic couldn't we just write the tf configs in /data/data/azurestack? At a quick glance it doesn't look like azurestack is reusing any of the azure modules; but perhaps there is a benefit or need?

That is how I originally wrote it. But I had used different variables names (like azurestack_region). I didn't like having to duplicate all the go code for building out the variables. And I didn't like the prospect of keeping those in sync. Now that I am using the same terraform variable names for both, I think that you are correct that it would be better to put the azure stack in /data/data/azurestack. I could symlink the terraform variables file from /data/data/azure to /data/data/azurestack.

@staebler
Copy link
Contributor Author

b833f34...503ea32

  • Move terraform files for Azure Stack to data/data/azurestack.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 5, 2021
@openshift-ci-robot
Copy link
Contributor

@staebler: PR needs rebase.

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.

@patrickdillon patrickdillon mentioned this pull request Apr 7, 2021
Closed
staebler and others added 9 commits June 17, 2021 15:03
@staebler
vendor: bump openshift/api
434c1a2 
Bump github.com/openshift/api to include new AzureStack
value for the AzureCloudEnvironment.
@staebler
types: add Azure Stack as an azure cloud
572f5f4 
Add AzureStack as a valid value for the azure cloudName
field. Add the `platform.azure.armEndpoint` field for
specifying the ARM endpoint when installing on Azure Stack.
Add validation specific to Azure Stack.
@staebler
terraform: add azurestack provider
3608e0a 
Include the azurestack provider in the plugins that
terraform knows.
@staebler
vendor: add terraform-provider-azurestack
43f86c7 
Add the github.com/terraform-providers/terraform-provider-azurestack
module. Replace it with a forked version that uses a new azurerm
provider and that fixes an invalid ConfictsWith declaraction in the
availability set resource.
@staebler
data: tf for azure stack
e2f5603 
Add the terraform files for provisioning a cluster in
Azure Stack.
@staebler
use azurestack as the terraform platform for azure stack
e8d26b9 
When provisioning a cluster onto Azure Stack, use the terraform
in data/data/azurestack instead of data/data/azure.
@patrickdillon @staebler
Update Azure SDK for hybrid compatibility with Azure Stack.
4b94226 
Utilize the 2018-03-01 profile, which according to
https://github.com/Azure/azure-sdk-for-go/blob/master/README.md
is the latest hybrid profile.

See https://docs.microsoft.com/en-us/azure-stack/user/azure-stack-version-profiles?view=azs-2102#azure-api-profiles-and-azure-stack-hub-compatibility
for more information on hybrid profiles.

Also incorporates changes from e978bd9 in
@staebler
vendor: bring in azure sdk for hybrid profiles
794f087
@staebler
temporary hack to work around a bug with the hard-coded RCHOS image t…
3cd6afe 
…hat we are using for ASH
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 18, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 18, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from staebler after the PR has been reviewed.

The full list of commands accepted by this bot can be found 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

@staebler
vendor: update azurestack terraform provider to include resource_arm_…
36a46f5 
…image
@staebler
azurestack: push up rhcos image
b52401b 
Use the azurestack_image resource to create an image to use
for the azurestack VMs.

This requires an existing blob with the OS image to be in the
environment. The OPENSHIFT_INSTALL_OS_IMAGE_OVERRIDE
environment variable must point to the existing blob.
@patrickdillon patrickdillon mentioned this pull request Jun 23, 2021
Merged
@ausil
Copy link

ausil commented Jul 14, 2021

what exactly does arm mean in this case? I am guessing not the arm architecture?

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 14, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 14, 2021

@staebler: PR needs rebase.

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.

@patrickdillon
Copy link
Contributor

what exactly does arm mean in this case? I am guessing not the arm architecture?

I believe it stands for azure resource manager

@ausil
Copy link

ausil commented Jul 14, 2021

what exactly does arm mean in this case? I am guessing not the arm architecture?

I believe it stands for azure resource manager

okay, thanks, I thought so, I only just learned it existed. This is likely to go cause confusion with the ARM architecture.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 1, 2021

@staebler: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-verify-codegen b52401b link /test okd-verify-codegen
ci/prow/okd-images b52401b link /test okd-images
ci/prow/e2e-aws-workers-rhel8 b52401b link /test e2e-aws-workers-rhel8
ci/prow/okd-unit b52401b link true /test okd-unit
ci/prow/openstack-manifests b52401b link true /test openstack-manifests
ci/prow/e2e-aws-upgrade b52401b link true /test e2e-aws-upgrade
ci/prow/e2e-aws b52401b link true /test e2e-aws

Full PR test history. Your PR dashboard.

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.

@staebler staebler closed this Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patrickdillon patrickdillon patrickdillon left review comments

@jhixson74 jhixson74 Awaiting requested review from jhixson74

Assignees
No one assigned
Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@staebler @openshift-ci-robot @ausil @patrickdillon