azurestackprivate Terraform Provider #5103

patrickdillon · 2021-07-22T18:15:39Z

This PR creates a private terraform provider for uploading RHCOS images to Azure Stack Hub environments as part of the install process. Based on the azurestack terraform provider and the azure-vhd-utils project, the majority of the original work here is in the /pkg/terraform/exec/plugins/azurestackprivate/resource_vhd_blob.go, particularly the resourceArmVHDBlobCreate function.

The main piece of feedback needed is regarding updating azure-vhd-utils to import openshift/azure-sdk-for-go to get around the antiquated version dependency. This is described in detail in the commit messages.

Status: This PR stands on its own and can be reviewed/merged but the overall image upload process still needs to be completed

private terraform provider to upload vhd
terraform configs to make use of the provider (testing has been completed)
tfvars work in the installer to cache rhcos image

openshift-ci · 2021-07-22T18:16:02Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

patrickdillon · 2021-07-22T18:30:54Z

I will address the linting/vetting errors.

The main piece of tech debt this would incur is that we would not be able to use our fork of azure-sdk-for-go without resolving these issues. We don't use our fork at the moment, so this is a hypothetical problem.

patrickdillon · 2021-07-22T20:01:32Z

Refactored a little which should hopefully make the go-vet test pass. Still need to fix go-lint.

Was able to reduce the amount of code we are copying directly into the provider.

patrickdillon · 2021-07-23T02:15:40Z

/retest

patrickdillon · 2021-07-23T21:02:42Z

Depends on #5084 for the TF config

Adds support for an azurestack Terraform provider in order to upload a local RHCOS VHD OS image to an Azure Stack storage blob. This provider makes use of azure-vhd-utils to speed up download, because an RHCOS VHD is a fixed 16GB size but most of that is zero-padding. The azure-vhd-utils determines the amount of data that needs to be uploaded and cuts out the empty ranges, which in effect results in a 2.5GB upload rather than 16GB. Unfortunately the azure-vhd-utils library is quite old and uses version 5 of the azure-sdk-for-go (currently we are on version 55). Version 5 of the sdk was created before semantic versioning was in place in the repo, therefore there is no graceful way to import version 5 of the SDK because other parts of the installer depend on later versions of the SDK. To work around this issue, I have incorporated the upload package from the azure-vhd-utils directly into the terraform provider and updated it to import the openshift fork of the azure-sdk-for go, pinned to version 5. This technical debt would need to be incurred until the azure-vhd-utils is updated to a newer version of the SDK, which may never happen. The config.go is modeled after the azurestack provider, so the majority of the original work is in the resource_vhd_blob.go and then some wiring in azurestackprivate.go and provider.go.

Updates the image asset to retrieve the correct image URL for Azure Stack Hub.

This is preliminary work to create two paths, one for downloading and uploading the RHCOS image to an ASH environment, the other for using an existing image in the ASH environment. This creates the variables to determine whether the image should be from a local path.

Creates Terraform configurations to upload an RHCOS image when a local path is specified or use a storage blub URI when the URI is in the same ASH environment.

azure-vhd-utils, used by the azurestackprivate terraform provider, uses an ancient version (v5) of the azure-sdk-for-go from before semantic versioning. Therefore there is no clean way to import v5 without breaking all other azure-sdk-for-go imports. To get around this limitation, we bring the relevant code from azure-vhd-utils into the private terraform provider and update the imports to point to the openshift fork pinned at version 5.

openshift-ci · 2021-07-25T08:36:52Z

@patrickdillon: 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.

patrickdillon · 2021-08-26T17:00:42Z

pkg/terraform/exec/plugins/azurestackprivate/resource_vhd_blob.go

+	blobServiceBaseURL := strings.TrimPrefix(armClient.environment.ResourceManagerEndpoint, "https://management.")
+	storageClient, err := storage.NewClient(storageAccountName, storageAccountKey, blobServiceBaseURL, apiProfileVersion, true)


perhaps there's a better way to do this

Can you elaborate on this comment?

Was wondering if there was a programmatic way to pull that baseURL directly with string manipulation.

Should also make sure that the blobServiceBaseURL will always equal the base url of the ARM endpoint.

Is it armClient.environment.StorageEndpointSuffix?

That seems correct.

patrickdillon · 2021-10-07T15:15:56Z

Using the env var OPENSHIFT_INSTALL_OS_IMAGE_OVERRIDE is not acceptable as part of a supported install plan. I will work on adding a field to the install config (only for ASH)

patrickdillon · 2021-10-07T15:17:54Z

/cc @staebler

Particularly need feedback around the kludge described in 50e5301 and 5f1fb89
I tried to lay out the tradeoffs here: #5103 (comment)

Let me know if it is not clear and I can try to explain better.

staebler

Patrick and I had a discussion about exploring the possibility of embedding a azure-vhd-utils binary that could be run as part of a local-exec resource rather than having to bring into and own the azure-hvd-utils code in the installer repo.

staebler · 2021-10-08T17:44:47Z

pkg/tfvars/azure/azure.go

+	storageBaseURL := strings.TrimPrefix(armEndpoint, "https://management.")
+
+	// If the URL for the image is in the ASH environment we don't want to upload a local VHD.
+	if strings.Contains(imageURL, storageBaseURL) {


I think we can get a little more precise with this by checking that storageBaseURL is the end of the host of imageURL.

openshift-ci · 2021-12-01T18:27:43Z

@patrickdillon: 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/tf-fmt	`5f1fb89`	link	`/test tf-fmt`
ci/prow/e2e-crc	`5f1fb89`	link	`/test e2e-crc`
ci/prow/e2e-openstack-kuryr	`5f1fb89`	link	`/test e2e-openstack-kuryr`
ci/prow/e2e-ovirt	`5f1fb89`	link	`/test e2e-ovirt`
ci/prow/e2e-libvirt	`5f1fb89`	link	`/test e2e-libvirt`
ci/prow/e2e-aws-workers-rhel7	`5f1fb89`	link	`/test e2e-aws-workers-rhel7`
ci/prow/e2e-aws-single-node	`5f1fb89`	link	`/test e2e-aws-single-node`
ci/prow/okd-verify-codegen	`5f1fb89`	link	`/test okd-verify-codegen`
ci/prow/okd-images	`5f1fb89`	link	`/test okd-images`
ci/prow/e2e-aws-workers-rhel8	`5f1fb89`	link	`/test e2e-aws-workers-rhel8`
ci/prow/okd-unit	`5f1fb89`	link	true	`/test okd-unit`
ci/prow/openstack-manifests	`5f1fb89`	link	true	`/test openstack-manifests`
ci/prow/e2e-aws-upgrade	`5f1fb89`	link	true	`/test e2e-aws-upgrade`
ci/prow/e2e-aws	`5f1fb89`	link	true	`/test e2e-aws`

Full PR test history. Your PR dashboard.

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.

patrickdillon · 2021-12-06T00:03:49Z

/close

We will add a field in the install config for an image URL but we expect users to upload first. Hopefully the burden will be mitigated in the future by making the images available in the Azure marketplace.

openshift-ci · 2021-12-06T00:04:17Z

@patrickdillon: Closed this PR.

Details

In response to this:

/close

We will add a field in the install config for an image URL but we expect users to upload first. Hopefully the burden will be mitigated in the future by making the images available in the Azure marketplace.

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 bot requested review from jhixson74 and rna-afk July 22, 2021 18:15

patrickdillon force-pushed the azurestack-upload branch from a1e53c7 to e6a1bce Compare July 22, 2021 19:59

patrickdillon force-pushed the azurestack-upload branch from e6a1bce to 40c7707 Compare July 22, 2021 20:12

patrickdillon force-pushed the azurestack-upload branch 3 times, most recently from f13f0c8 to 1a8320b Compare July 23, 2021 02:49

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

patrickdillon force-pushed the azurestack-upload branch from 8dc16ef to 99a262c Compare July 23, 2021 21:05

patrickdillon added 5 commits July 23, 2021 17:07

Azure Stack: resolve RHCOS image

a6ae135

Updates the image asset to retrieve the correct image URL for Azure Stack Hub.

Azure Stack: Terraform Configuration for RHCOS Upload

deee15e

Creates Terraform configurations to upload an RHCOS image when a local path is specified or use a storage blub URI when the URI is in the same ASH environment.

patrickdillon force-pushed the azurestack-upload branch from 99a262c to 5f1fb89 Compare July 23, 2021 21:08

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 23, 2021

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

patrickdillon commented Aug 26, 2021

View reviewed changes

openshift-ci bot requested a review from staebler October 7, 2021 15:17

staebler reviewed Oct 8, 2021

View reviewed changes

openshift-ci bot closed this Dec 6, 2021

		blobServiceBaseURL := strings.TrimPrefix(armClient.environment.ResourceManagerEndpoint, "https://management.")
		storageClient, err := storage.NewClient(storageAccountName, storageAccountKey, blobServiceBaseURL, apiProfileVersion, true)

azurestackprivate Terraform Provider #5103

azurestackprivate Terraform Provider #5103

Uh oh!

Conversation

patrickdillon commented Jul 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Jul 22, 2021

Uh oh!

patrickdillon commented Jul 22, 2021

Uh oh!

patrickdillon commented Jul 22, 2021

Uh oh!

patrickdillon commented Jul 23, 2021

Uh oh!

patrickdillon commented Jul 23, 2021

Uh oh!

openshift-ci bot commented Jul 25, 2021

Uh oh!

patrickdillon Aug 26, 2021

Choose a reason for hiding this comment

Uh oh!

staebler Oct 8, 2021

Choose a reason for hiding this comment

Uh oh!

patrickdillon Oct 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

staebler Oct 8, 2021

Choose a reason for hiding this comment

Uh oh!

patrickdillon Nov 30, 2021

Choose a reason for hiding this comment

Uh oh!

patrickdillon commented Oct 7, 2021

Uh oh!

patrickdillon commented Oct 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

staebler left a comment

Choose a reason for hiding this comment

Uh oh!

staebler Oct 8, 2021

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Dec 1, 2021

Uh oh!

patrickdillon commented Dec 6, 2021

Uh oh!

openshift-ci bot commented Dec 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

patrickdillon commented Jul 22, 2021 •

edited

Loading

patrickdillon Oct 8, 2021 •

edited

Loading

patrickdillon commented Oct 7, 2021 •

edited

Loading