Skip to content

azure: create an image for booting machines for the cluster from vhd blob - #1976

Merged
openshift-merge-robot merged 4 commits into
openshift:masterfrom
abhinavdahiya:azure_image_from_vhd
Jul 24, 2019
Merged

azure: create an image for booting machines for the cluster from vhd blob#1976
openshift-merge-robot merged 4 commits into
openshift:masterfrom
abhinavdahiya:azure_image_from_vhd

Conversation

@abhinavdahiya

@abhinavdahiya abhinavdahiya commented Jul 11, 2019

Copy link
Copy Markdown
Contributor

previously the terraform expected an Azure Image [1] provided to the cluster, but that approach had some disadvantages:

  • the image needed to be created before creating the cluster
  • the image location was hardcoded to a specific resource-group

And since RHCOS isn't avaiable on the Public Registry of images, the new flow is using the vhd blob from Azure storage and creating an Azure Image from it for the cluster.

@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 Jul 11, 2019
@abhinavdahiya

abhinavdahiya commented Jul 11, 2019

Copy link
Copy Markdown
Contributor Author

/test e2e-azure

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 11, 2019
@abhinavdahiya
abhinavdahiya force-pushed the azure_image_from_vhd branch 2 times, most recently from 2ba4eca to 60ae2d4 Compare July 11, 2019 22:27
@abhinavdahiya abhinavdahiya changed the title [WIP] azure: create an image for booting machines for the cluster from vhd blob azure: create an image for booting machines for the cluster from vhd blob Jul 11, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 11, 2019
@abhinavdahiya

abhinavdahiya commented Jul 11, 2019

Copy link
Copy Markdown
Contributor Author

/cc @crawford @jhixson74 @imcleod

for approval if this is way forward for RHCOS image in Azure.

@abhinavdahiya
abhinavdahiya force-pushed the azure_image_from_vhd branch 2 times, most recently from 887f440 to 01ac37a Compare July 16, 2019 22:30
@abhinavdahiya

abhinavdahiya commented Jul 16, 2019

Copy link
Copy Markdown
Contributor Author

/test e2e-azure

1 similar comment
@yuqi-zhang

yuqi-zhang commented Jul 16, 2019

Copy link
Copy Markdown
Contributor

/test e2e-azure

@abhinavdahiya
abhinavdahiya force-pushed the azure_image_from_vhd branch from 01ac37a to dba39b1 Compare July 16, 2019 23:59
@abhinavdahiya

abhinavdahiya commented Jul 17, 2019

Copy link
Copy Markdown
Contributor Author

/test e2e-azure

@abhinavdahiya

abhinavdahiya commented Jul 17, 2019

Copy link
Copy Markdown
Contributor Author

/retest

@abhinavdahiya
abhinavdahiya force-pushed the azure_image_from_vhd branch from dba39b1 to 7472266 Compare July 17, 2019 14:16
@abhinavdahiya

abhinavdahiya commented Jul 17, 2019

Copy link
Copy Markdown
Contributor Author

/test e2e-azure

@yuqi-zhang yuqi-zhang mentioned this pull request Jul 17, 2019
Merged
@nstielau

nstielau commented Jul 17, 2019

Copy link
Copy Markdown

/test e2e-azure

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

crawford commented Jul 18, 2019

Copy link
Copy Markdown
Contributor

/approve

@abhinavdahiya
data/azure: use single storage account for cluster
bf8b396 
Currently we use a storage account for boot diagnostics for the masters, another one for storing the ignition config for bootstrap node.
In the future we might need to store the RHCOS vhd in a storage account, and there is no valid case for these many storage accounts created by installer. And each storage container can have its invidual ACL.

Using a single storage account is simpler.
@abhinavdahiya
data/azure: create an image for booting machines for the cluster from…
5416b42 
… vhd blob

previously the terraform expected an Azure Image [1] provided to the cluster, but that approach had some disadvantages:
* the image needed to be created before creating the cluster
* the image location was hardcoded to a specific resource-group

And since RHCOS isn't avaiable on the Public Registry of images, the new flow is using the vhd blob from Azure storage and creating an Azure Image from it for the cluster.

The public vhd is copied to cluster's resource group to make sure the image is not based directly on a public artifact.
@abhinavdahiya
pkg: update the machines and tfvars to correct image
da00241 
Currently the OS image source for Azure was hard coded to `/resourceGroups/rhcos_images/.../rhcos` in rhcos.Image asset which means the image needed to pre-exist in the same sunscription.

But now the OS image source for Azure will be a vhd blob in Azure storage account as specified by build pipeline and the machines will be using the image in the cluster's resource group.
@abhinavdahiya
abhinavdahiya force-pushed the azure_image_from_vhd branch from 7472266 to 8558590 Compare July 18, 2019 21:16
@abhinavdahiya
abhinavdahiya force-pushed the azure_image_from_vhd branch from 8558590 to 47edea9 Compare July 19, 2019 23:32
@abhinavdahiya

abhinavdahiya commented Jul 19, 2019

Copy link
Copy Markdown
Contributor Author

@abhinavdahiya could you update the URL to https://rhcospipelineimages2.blob.core.windows.net/imagebucket/rhcos-420devel.8.20190719.0.vhd
(devel pipeline output)

updated.

@abhinavdahiya

abhinavdahiya commented Jul 19, 2019

Copy link
Copy Markdown
Contributor Author

/test e2e-azure

3 similar comments
@abhinavdahiya

abhinavdahiya commented Jul 20, 2019

Copy link
Copy Markdown
Contributor Author

/test e2e-azure

@abhinavdahiya

abhinavdahiya commented Jul 22, 2019

Copy link
Copy Markdown
Contributor Author

/test e2e-azure

@abhinavdahiya

abhinavdahiya commented Jul 22, 2019

Copy link
Copy Markdown
Contributor Author

/test e2e-azure

@nstielau

nstielau commented Jul 23, 2019

Copy link
Copy Markdown

/retest e2e-azure

@nstielau

nstielau commented Jul 23, 2019

Copy link
Copy Markdown

It's concerning that this is this flakey. @abhinavdahiya do you think azure is more flakey than AWS?

@patrickdillon

patrickdillon commented Jul 23, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

@nstielau I think so, although aws may be having its own issues right now...
/this-is-fine
I will be working on https://jira.coreos.com/browse/CORS-1153 soon and hopefully this will shore things up azure ci wise.

@openshift-ci-robot

openshift-ci-robot commented Jul 23, 2019

Copy link
Copy Markdown
Contributor

@patrickdillon: dog image

Details

In response to this:

@nstielau I think so, although aws may be having it's own issues right now...
/this-is-fine
I will be working on https://jira.coreos.com/browse/CORS-1153 soon and hopefully this will shore things up azure ci wise.

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.

@abhinavdahiya

abhinavdahiya commented Jul 23, 2019

Copy link
Copy Markdown
Contributor Author

/test e2e-azure

1 similar comment
@abhinavdahiya

abhinavdahiya commented Jul 23, 2019

Copy link
Copy Markdown
Contributor Author

/test e2e-azure

jhixson74
jhixson74 approved these changes Jul 23, 2019
View reviewed changes

@jhixson74 jhixson74 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2019
@openshift-ci-robot

openshift-ci-robot commented Jul 23, 2019

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, crawford, jhixson74

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 [abhinavdahiya,crawford]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot

openshift-bot commented Jul 23, 2019

Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

4 similar comments
@openshift-bot

openshift-bot commented Jul 24, 2019

Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot

openshift-bot commented Jul 24, 2019

Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot

openshift-bot commented Jul 24, 2019

Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot

openshift-bot commented Jul 24, 2019

Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot
openshift-merge-robot merged commit 1ef9ea7 into openshift:master Jul 24, 2019
@openshift-ci-robot

openshift-ci-robot commented Jul 24, 2019

Copy link
Copy Markdown
Contributor

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

Test name Commit Details Rerun command
ci/prow/e2e-aws-scaleup-rhel7 47edea9 link /test e2e-aws-scaleup-rhel7
ci/prow/e2e-azure 47edea9 link /test e2e-azure

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.

@wking wking mentioned this pull request Jul 31, 2019
Merged
wking pushed a commit to wking/openshift-installer that referenced this pull request Oct 8, 2019
@jim-minter @wking
data/azure/main: Set metadata to URL, not the string "var.azure_image…
51452cb 
…_url"

Fixing a typo from 5416b42 (data/azure: create an image for booting
machines for the cluster from vhd blob, 2019-07-11, openshift#1976).
@wking wking mentioned this pull request Oct 8, 2019
Merged
jhixson74 pushed a commit to jhixson74/installer that referenced this pull request Dec 6, 2019
@jim-minter @jhixson74
data/azure/main: Set metadata to URL, not the string "var.azure_image…
945892f 
…_url"

Fixing a typo from 5416b42 (data/azure: create an image for booting
machines for the cluster from vhd blob, 2019-07-11, openshift#1976).
@cgwalters cgwalters mentioned this pull request Jan 25, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@patrickdillon patrickdillon Awaiting requested review from patrickdillon
@crawford crawford Awaiting requested review from crawford
@imcleod imcleod Awaiting requested review from imcleod
1 more reviewer
@jhixson74 jhixson74 jhixson74 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jhixson74 jhixson74

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@abhinavdahiya @yuqi-zhang @nstielau @crawford @patrickdillon @openshift-ci-robot @openshift-bot @jhixson74 @openshift-merge-robot