azure: create bootimages using image gallery #6304

Prashanth684 · 2022-09-06T16:22:33Z

In order to support provisioning of arm64 bootimages, these need to be
created using the image gallery resource as they are not supported
through managed images. For this purpose, it would be good to unify the
implementation of x86 and arm64 to use image galleries as Microsoft have
said that it will be the supported way of creating bootimages going forward
for new instance types of new architectures.

This implementation would create one image gallery with two images: a
gen1 and a gen2 image which we would then pick appropriately based on
the VM generation.

xref: https://issues.redhat.com/browse/ARMOCP-16

Prashanth684 · 2022-09-06T16:24:54Z

/cc @patrickdillon @jstuever

Prashanth684 · 2022-09-06T21:19:10Z

/test e2e-azure

patrickdillon · 2022-09-06T22:19:25Z

/cc @r4f4

patrickdillon

In general this looks good. I can see that you're working through some errors in CI. The issue is not obvious, so feel free to provide an update or reach out if you want to discuss what's going on there.

I left a comment about setting the version. I think this is preferable, but I would not say i is a hard requirement. We can discuss this idea further. The first suggestion, in terms of piping the release through the codebase may be preferable to parsing the url, but may also be a bit more hairy to implement.

data/data/azure/variables-azure.tf

data/data/azure/vnet/main.tf

Prashanth684 · 2022-09-06T23:53:17Z

In general this looks good. I can see that you're working through some errors in CI. The issue is not obvious, so feel free to provide an update or reach out if you want to discuss what's going on there.

I left a comment about setting the version. I think this is preferable, but I would not say i is a hard requirement. We can discuss this idea further. The first suggestion, in terms of piping the release through the codebase may be preferable to parsing the url, but may also be a bit more hairy to implement.

yes the CI results concern me a bit..the current run seems to be progressing fine but the failure was essentially:

time="2022-09-06T18:12:04Z" level=error msg="Error: creating/updating Shared Image: (Image Name \"ci-op-2h3vjhpr-2dc90-7tk4z\" / Gallery Name \"gallery_ci_op_2h3vjhpr_2dc90_7tk4z\" / Resource Group \"ci-op-2h3vjhpr-2dc90-7tk4z-rg\"): compute.GalleryImagesClient#CreateOrUpdate: Failure sending request: StatusCode=404 -- Original Error: Code=\"ParentResourceNotFound\" Message=\"Can not perform requested operation on nested resource. Parent resource 'gallery_ci_op_2h3vjhpr_2dc90_7tk4z' not found.\""

so the flow here is image gallery -> image definition(inside the gallery) -> image version (inside the image definition).

the error above indicates that the image gallery could not be found, but in the previous few lines i see:

time="2022-09-06T18:08:55Z" level=debug msg="azurerm_shared_image_gallery.sig: Creation complete after 34s [id=/subscriptions/d38f1e38-4bed-438e-b227-833f997adf6a/resourceGroups/ci-op-2h3vjhpr-2dc90-7tk4z-rg/providers/Microsoft.Compute/galleries/gallery_ci_op_2h3vjhpr_2dc90_7tk4z]"
time="2022-09-06T18:08:55Z" level=debug msg="azurerm_shared_image.clustergen2: Creating..."
time="2022-09-06T18:08:55Z" level=debug msg="azurerm_shared_image.cluster: Creating..."

so from a timeline perspective it seems ok. I'm still wondering if we need these resources to each have a depends_on argument just to eliminate the possibility of a race?

patrickdillon · 2022-09-07T13:44:14Z

so the flow here is image gallery -> image definition(inside the gallery) -> image version (inside the image definition).

the error above indicates that the image gallery could not be found, but in the previous few lines i see:
time="2022-09-06T18:08:55Z" level=debug msg="azurerm_shared_image_gallery.sig: Creation complete after 34s [id=/subscriptions/d38f1e38-4bed-438e-b227-833f997adf6a/resourceGroups/ci-op-2h3vjhpr-2dc90-7tk4z-rg/providers/Microsoft.Compute/galleries/gallery_ci_op_2h3vjhpr_2dc90_7tk4z]"
time="2022-09-06T18:08:55Z" level=debug msg="azurerm_shared_image.clustergen2: Creating..."
time="2022-09-06T18:08:55Z" level=debug msg="azurerm_shared_image.cluster: Creating..."
so from a timeline perspective it seems ok. I'm still wondering if we need these resources to each have a depends_on argument just to eliminate the possibility of a race?

This error is an example of a common class of errors we see in the upstream terraform providers. The terraform providers should handle these dependencies, but frequently there are bugs. We've had two similar bugs reported in the past week.

Adding a depends_on may be a sufficient work around for this. It would also be good to open an issue in the upstream provider hashicorp/terraform-provider-azurerm. If the depends_on workaround relieves the symptoms, you could also open an installer bug for us to track the upstream work.

Digging into the upstream code: the creation of the image gallery is here:
https://github.com/hashicorp/terraform-provider-azurerm/blob/main/internal/services/compute/shared_image_gallery_resource.go#L66

and the image itself is here:
https://github.com/hashicorp/terraform-provider-azurerm/blob/main/internal/services/compute/shared_image_resource.go#L245

Prashanth684 · 2022-09-07T20:25:51Z

/retest-required

Prashanth684 · 2022-09-07T20:44:11Z

/test e2e-azure-shared-vpc

Prashanth684 · 2022-09-08T00:41:24Z

/test e2e-azure-shared-vpc
/test e2e-azurestack

Prashanth684 · 2022-09-09T14:21:21Z

/test e2e-azure-shared-vpc

pkg/asset/machines/azure/machines.go

Prashanth684 · 2022-09-13T03:24:17Z

/test e2e-azure-ovn-resourcegroup

data/data/azure/variables-azure.tf

pkg/asset/rhcos/release.go

pkg/asset/machines/azure/machines.go

pkg/tfvars/azure/azure.go

openshift-ci-robot · 2022-10-06T19:09:14Z

/retest-required

Remaining retests: 0 against base HEAD 3379601 and 2 for PR HEAD 7994d1c in total

openshift-ci-robot · 2022-10-06T20:42:09Z

/retest-required

Remaining retests: 0 against base HEAD 53727e6 and 1 for PR HEAD 7994d1c in total

patrickdillon · 2022-10-06T22:15:24Z

/override ci/prow/e2e-aws-ovn
ci/prow/e2e-gcp-ovn
ci/prow/e2e-vsphere-ovn

openshift-ci · 2022-10-06T22:15:52Z

@patrickdillon: Overrode contexts on behalf of patrickdillon: ci/prow/e2e-aws-ovn

Details

In response to this:

/override ci/prow/e2e-aws-ovn
ci/prow/e2e-gcp-ovn
ci/prow/e2e-vsphere-ovn

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 · 2022-10-07T03:50:54Z

/retest-required

Remaining retests: 0 against base HEAD c100594 and 0 for PR HEAD 7994d1c in total

r4f4 · 2022-10-07T07:20:59Z

/override ci/prow/e2e-vsphere-ovn

r4f4 · 2022-10-07T07:21:24Z

/override ci/prow/e2e-gcp-ovn

openshift-ci · 2022-10-07T07:21:28Z

@r4f4: Overrode contexts on behalf of r4f4: ci/prow/e2e-vsphere-ovn

Details

In response to this:

/override ci/prow/e2e-vsphere-ovn

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 · 2022-10-07T07:21:53Z

@r4f4: Overrode contexts on behalf of r4f4: ci/prow/e2e-gcp-ovn

Details

In response to this:

/override ci/prow/e2e-gcp-ovn

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.

r4f4 · 2022-10-07T07:27:46Z

/override ci/prow/e2e-azure-ovn

openshift-ci · 2022-10-07T07:28:11Z

@r4f4: Overrode contexts on behalf of r4f4: ci/prow/e2e-azure-ovn

Details

In response to this:

/override ci/prow/e2e-azure-ovn

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.

r4f4 · 2022-10-07T07:48:32Z

/override ci/prow/e2e-gcp-ovn

openshift-ci · 2022-10-07T07:48:59Z

@r4f4: Overrode contexts on behalf of r4f4: ci/prow/e2e-gcp-ovn

Details

In response to this:

/override ci/prow/e2e-gcp-ovn

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 · 2022-10-07T08:06:24Z

/hold

Revision 7994d1c was retested 3 times: holding

r4f4 · 2022-10-07T09:00:51Z

/skip

r4f4 · 2022-10-07T09:03:53Z

/hold cancel

r4f4 · 2022-10-07T09:04:13Z

/skip

r4f4 · 2022-10-07T09:11:06Z

/override ci/prow/e2e-aws-ovn ci/prow/okd-scos-images ci/prow/okd-scos-unit ci/prow/okd-scos-verify-codegen ci/prow/e2e-agent-compact

openshift-ci · 2022-10-07T09:11:25Z

@r4f4: Overrode contexts on behalf of r4f4: ci/prow/e2e-agent-compact, ci/prow/e2e-aws-ovn, ci/prow/okd-scos-images, ci/prow/okd-scos-unit, ci/prow/okd-scos-verify-codegen

Details

In response to this:

/override ci/prow/e2e-aws-ovn ci/prow/okd-scos-images ci/prow/okd-scos-unit ci/prow/okd-scos-verify-codegen ci/prow/e2e-agent-compact

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 · 2022-10-07T09:16:57Z

@Prashanth684: 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/e2e-ibmcloud	0b3e91e88a5b8b34c399f20dd21b51a9f776b06f	link	false	`/test e2e-ibmcloud`
ci/prow/e2e-vsphere	0b3e91e88a5b8b34c399f20dd21b51a9f776b06f	link	true	`/test e2e-vsphere`
ci/prow/e2e-azure-ovn-shared-vpc	`7994d1c`	link	false	`/test e2e-azure-ovn-shared-vpc`
ci/prow/e2e-openstack	`7994d1c`	link	false	`/test e2e-openstack`
ci/prow/e2e-azurestack	`7994d1c`	link	false	`/test e2e-azurestack`
ci/prow/okd-e2e-aws-upgrade	`7994d1c`	link	false	`/test okd-e2e-aws-upgrade`
ci/prow/e2e-libvirt	`7994d1c`	link	false	`/test e2e-libvirt`
ci/prow/okd-e2e-aws-ovn	`7994d1c`	link	false	`/test okd-e2e-aws-ovn`
ci/prow/e2e-agent-mce	`7994d1c`	link	false	`/test e2e-agent-mce`
ci/prow/e2e-metal-ipi	`7994d1c`	link	false	`/test e2e-metal-ipi`
ci/prow/e2e-ibmcloud-ovn	`7994d1c`	link	false	`/test e2e-ibmcloud-ovn`
ci/prow/e2e-azure-ovn-resourcegroup	`7994d1c`	link	false	`/test e2e-azure-ovn-resourcegroup`

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.

…resscontroller With the bootimage creation for Azure changing from managed images to image gallery with this installer PR: openshift/installer#6304, it is better to infer the image id path from existing machines as it has a version and gallery name which are not easy to derive.

…resscontroller (#33067) With the bootimage creation for Azure changing from managed images to image gallery with this installer PR: openshift/installer#6304, it is better to infer the image id path from existing machines as it has a version and gallery name which are not easy to derive.

openshift-ci bot requested review from jhixson74 and m1kola September 6, 2022 16:24

openshift-ci bot requested review from jstuever and patrickdillon September 6, 2022 16:24

Prashanth684 force-pushed the azure-imagegallery branch from fc69cc7 to 6960917 Compare September 6, 2022 16:41

openshift-ci bot requested a review from r4f4 September 6, 2022 22:19

patrickdillon reviewed Sep 6, 2022

View reviewed changes

data/data/azure/variables-azure.tf Outdated Show resolved Hide resolved

data/data/azure/vnet/main.tf Outdated Show resolved Hide resolved

Prashanth684 force-pushed the azure-imagegallery branch from 6960917 to d273965 Compare September 7, 2022 03:47

Prashanth684 mentioned this pull request Sep 7, 2022

404 Error when creating azurerm_shared_image: Can not perform requested operation on nested resource. Parent resource not found hashicorp/terraform-provider-azurerm#18287

Closed

1 task

Prashanth684 force-pushed the azure-imagegallery branch from d273965 to 0b3e91e Compare September 7, 2022 16:03

Prashanth684 force-pushed the azure-imagegallery branch from 33c21ae to f344b5b Compare September 12, 2022 18:53

Prashanth684 commented Sep 12, 2022

View reviewed changes

pkg/asset/machines/azure/machines.go Outdated Show resolved Hide resolved

Prashanth684 force-pushed the azure-imagegallery branch 2 times, most recently from f72486c to a333b21 Compare September 12, 2022 23:09

r4f4 suggested changes Sep 13, 2022

View reviewed changes

data/data/azure/variables-azure.tf Outdated Show resolved Hide resolved

pkg/asset/rhcos/release.go Outdated Show resolved Hide resolved

pkg/asset/machines/azure/machines.go Outdated Show resolved Hide resolved

Prashanth684 force-pushed the azure-imagegallery branch from a333b21 to 9e8fbf0 Compare September 13, 2022 23:12

patrickdillon reviewed Sep 14, 2022

View reviewed changes

pkg/tfvars/azure/azure.go Outdated Show resolved Hide resolved

Prashanth684 force-pushed the azure-imagegallery branch 3 times, most recently from 0a8560a to 553d13e Compare September 14, 2022 17:28

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 7, 2022

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 7, 2022

openshift-merge-robot merged commit 7b739cd into openshift:master Oct 7, 2022

Prashanth684 mentioned this pull request Oct 12, 2022

Infer imageID from existing machines for azure-ipi-ingress-custom-ingresscontroller openshift/release#33067

Merged

JoelSpeed mentioned this pull request Oct 13, 2022

Update default Azure image to use image galleries openshift/machine-api-operator#1076

Merged

dlom referenced this pull request in openshift/hive Dec 8, 2022

Revendor functional changes

2d59848

Prashanth684 mentioned this pull request Dec 13, 2022

OCPBUGS-4809: Azure: toggle image in machinesets #6694

Merged

azure: create bootimages using image gallery #6304

azure: create bootimages using image gallery #6304

Uh oh!

Conversation

Prashanth684 commented Sep 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Prashanth684 commented Sep 6, 2022

Uh oh!

Prashanth684 commented Sep 6, 2022

Uh oh!

patrickdillon commented Sep 6, 2022

Uh oh!

patrickdillon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Prashanth684 commented Sep 6, 2022

Uh oh!

patrickdillon commented Sep 7, 2022

Uh oh!

Prashanth684 commented Sep 7, 2022

Uh oh!

Prashanth684 commented Sep 7, 2022

Uh oh!

Prashanth684 commented Sep 8, 2022

Uh oh!

Prashanth684 commented Sep 9, 2022

Uh oh!

Uh oh!

Prashanth684 commented Sep 13, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openshift-ci-robot commented Oct 6, 2022

Uh oh!

openshift-ci-robot commented Oct 6, 2022

Uh oh!

patrickdillon commented Oct 6, 2022

Uh oh!

openshift-ci bot commented Oct 6, 2022

Uh oh!

openshift-ci-robot commented Oct 7, 2022

Uh oh!

r4f4 commented Oct 7, 2022

Uh oh!

r4f4 commented Oct 7, 2022

Uh oh!

openshift-ci bot commented Oct 7, 2022

Uh oh!

openshift-ci bot commented Oct 7, 2022

Uh oh!

r4f4 commented Oct 7, 2022

Uh oh!

openshift-ci bot commented Oct 7, 2022

Uh oh!

r4f4 commented Oct 7, 2022

Uh oh!

openshift-ci bot commented Oct 7, 2022

Uh oh!

openshift-ci-robot commented Oct 7, 2022

Uh oh!

r4f4 commented Oct 7, 2022

Uh oh!

r4f4 commented Oct 7, 2022

Uh oh!

r4f4 commented Oct 7, 2022

Uh oh!

r4f4 commented Oct 7, 2022

Uh oh!

openshift-ci bot commented Oct 7, 2022

Uh oh!

openshift-ci bot commented Oct 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Prashanth684 commented Sep 6, 2022 •

edited

Loading