Allow creating gallery image versions from storage accounts #1466

jeremycline · 2024-02-26T20:10:04Z

SUMMARY

It's possible to upload a VHD into a container in a storage account and then use that to import an image version. This adds a third way to specify an os_disk in the galleryimageversion plugin to support that approach.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

galleryimageversion

jeremycline · 2024-02-26T20:11:11Z

plugins/modules/azure_rm_galleryimageversion.py

+      os_disk:
+        resource_group: myResourceGroup
+        storage_account: myStorageAccount
+        uri: "https://myStorageAccount.blob.core.windows.net/myContainer/myImage.vhd"


I realized after writing this that we could construct the URI by accepting two additional arguments, the container_name and blob_name, but I'm not sure if folks would prefer that two dropping the URI in there.

jeremycline · 2024-02-26T20:11:50Z

plugins/modules/azure_rm_galleryimageversion.py

@@ -561,7 +586,7 @@ def exec_module(self, **kwargs):
                            elif isinstance(kwargs[key]['os_disk']['source'], dict):
                                if kwargs[key]['os_disk']['source'].get('id') is not None:
                                    self.body['properties']['storageProfile']['osDiskImage']['source']['id'] = kwargs[key]['os_disk']['source'].get('id')
-                                if kwargs[key]['os_disk']['source'].get('resource_group') is not None and \
+                                elif kwargs[key]['os_disk']['source'].get('resource_group') is not None and \


I think this was a bug, I'm happy to pull it out into a separate commit/PR if folks would prefer that.

Fred-sun · 2024-02-28T06:41:00Z

plugins/modules/azure_rm_galleryimageversion.py

+                            - Reference to os disk snapshot.
+                            - Could be resource ID.
+                            - Could be a dictionary containing I(resource_group) and I(name).
+                            - Could be a dictionary containing I(resource_group), I(storage_account), and I(uri) if the snapshot is stored as a PageBlob in a storage account container.


line too long

Fred-sun · 2024-02-28T06:41:29Z

plugins/modules/azure_rm_galleryimageversion.py

@@ -561,7 +586,7 @@ def exec_module(self, **kwargs):
                            elif isinstance(kwargs[key]['os_disk']['source'], dict):
                                if kwargs[key]['os_disk']['source'].get('id') is not None:
                                    self.body['properties']['storageProfile']['osDiskImage']['source']['id'] = kwargs[key]['os_disk']['source'].get('id')
-                                if kwargs[key]['os_disk']['source'].get('resource_group') is not None and \
+                                elif kwargs[key]['os_disk']['source'].get('resource_group') is not None and \
                                   kwargs[key]['os_disk']['source'].get('name') is not None:


continuation line over-indented for visual indent

Fred-sun · 2024-02-28T06:41:40Z

plugins/modules/azure_rm_galleryimageversion.py

@@ -570,6 +595,18 @@ def exec_module(self, **kwargs):
                                                                                                                resource_group +
                                                                                                                '/providers/Microsoft.Compute/snapshots/' +
                                                                                                                kwargs[key]['os_disk']['source'].get('name'))
+                                elif kwargs[key]['os_disk']['source'].get('uri') is not None and \
+                                  kwargs[key]['os_disk']['source'].get('resource_group') is not None and \


continuation line over-indented for visual indent

Fred-sun · 2024-02-28T06:41:48Z

plugins/modules/azure_rm_galleryimageversion.py

@@ -570,6 +595,18 @@ def exec_module(self, **kwargs):
                                                                                                                resource_group +
                                                                                                                '/providers/Microsoft.Compute/snapshots/' +
                                                                                                                kwargs[key]['os_disk']['source'].get('name'))
+                                elif kwargs[key]['os_disk']['source'].get('uri') is not None and \
+                                  kwargs[key]['os_disk']['source'].get('resource_group') is not None and \
+                                  kwargs[key]['os_disk']['source'].get('storage_account') is not None:


continuation line over-indented for visual indent

Fred-sun · 2024-02-28T06:42:02Z

plugins/modules/azure_rm_galleryimageversion.py

+                                                                                                                self.subscription_id +
+                                                                                                                '/resourceGroups/' +
+                                                                                                                resource_group +
+                                                                                                                '/providers/Microsoft.Storage/storageAccounts/' +


line too long

jeremycline · 2024-02-28T14:38:12Z

Hi, thanks for the review!

I attempted to address the style comments. If it's still not looking right, is there a style guide for the project I should follow, or a linter? I did my best to follow the existing style of the file.

Fred-sun

Update test case! Thank you very much！

Fred-sun · 2024-02-29T02:50:10Z

plugins/modules/azure_rm_galleryimageversion.py

+        - name: East US
+          regional_replica_count: 1
+          storage_account_type: Standard_LRS
+    storage_profile:


Parameter definition sequence: storage_profile -> os_disk -> source -> uri

Fred-sun · 2024-02-29T02:50:11Z

tests/integration/targets/azure_rm_gallery/tasks/main.yml

+            os_disk_image:
+              disk_encryption_set_id: "{{ des_results.state.id }}"
+          storage_account_type: Standard_ZRS
+    storage_profile:


Parameter definition sequence: storage_profile -> os_disk -> source -> uri

383 storage_profile: 384 os_disk: 385 source: 386 resource_group: "{{ resource_group }}" 387 storage_account: '{{ vms_output.vms[0].storage_account_name }}' 388 uri: 'https://{{ vms_output.vms[0].storage_account_name }}.blob.core.windows.net/{{ vms_output.vms[0].stor age_container_name }}/{{ vms_output.vms[0].storage_blob_name }}'

@jeremycline The output variable on line 377 should be the one obtained by the VM on line 108. But lines 108 through 380 also register other output variables. So should pay attention to 120, 377, 378 this variable value acquisition! Thank you very much!

Ah I see, it's being shadowed. My Ansible is very rusty, sorry. I think I've fixed it.

jeremycline · 2024-03-19T13:35:03Z

I think this should be ready for a re-review (time permitting, of course). We're using this in some automation in Fedora so I'd love it to be in the next release if possible. Thanks!

jeremycline · 2024-06-03T20:10:18Z

Hi @Fred-sun,

Just wanted to check in if there's anything I can do to help move this along - we've been carrying this patch to upload Fedora images nightly for a couple months and it's been working fine, for what it's worth.

Fred-sun · 2024-06-04T02:32:44Z

@jeremycline Ok, I will finish the PR review today. Thank you!

Fred-sun · 2024-06-04T09:23:39Z

@jeremycline Is the location of your test case improperly arranged? It is between line 307 and 387. Should we create a new name of galleryimageversion?

Fred-sun · 2024-06-04T10:22:52Z

@jeremycline In addition, after the completion of the test, please delete the newly created galleryimage, otherwise there will be problems when deleting the gallery. Thank you!

It's possible to upload a VHD into a container in a storage account and then use that to import an image version. This adds a third way to specify an `os_disk` in the galleryimageversion plugin to support that approach.

There's no functional change.

Add the missing "source" intermediate key in the example and test case, and ensure the VM information isn't shadowed by later steps.

This also removes the image version at the end so the gallery can be deleted.

jeremycline · 2024-06-04T14:18:36Z

Thanks for the review!

I've given the image version a different name and ensured it's deleted with the other image version.

Fred-sun · 2024-06-05T00:19:30Z

@jeremycline Thanks for your update! I will recheck it and push for merged!

jeremycline commented Feb 26, 2024

View reviewed changes

Fred-sun reviewed Feb 28, 2024

View reviewed changes

Fred-sun reviewed Feb 29, 2024

View reviewed changes

Fred-sun added question Further information is requested medium_priority Medium priority work in In trying to solve, or in working with contributors labels Feb 29, 2024

Fred-sun mentioned this pull request Feb 29, 2024

Allow an OS disk blob URI as source in azure_rm_galleryimageversion #1437

Closed

jeremycline added 3 commits June 4, 2024 10:07

Allow creating gallery image versions from storage accounts

80281ce

It's possible to upload a VHD into a container in a storage account and then use that to import an image version. This adds a third way to specify an `os_disk` in the galleryimageversion plugin to support that approach.

Address PR comments for style

65a47c9

There's no functional change.

Fix example and tests based on review comments

a900040

Add the missing "source" intermediate key in the example and test case, and ensure the VM information isn't shadowed by later steps.

jeremycline force-pushed the image-version-from-vhd branch from 69afaab to 2644113 Compare June 4, 2024 14:15

galleryimageversion: rename image in test created from URI

c3be294

This also removes the image version at the end so the gallery can be deleted.

jeremycline force-pushed the image-version-from-vhd branch from 2644113 to c3be294 Compare June 4, 2024 14:17

Fred-sun added ready_for_review The PR has been modified and can be reviewed and merged and removed work in In trying to solve, or in working with contributors labels Jun 5, 2024

Fred-sun assigned xuzhang3 Jun 5, 2024

xuzhang3 approved these changes Jun 11, 2024

View reviewed changes

xuzhang3 merged commit 0acfac2 into ansible-collections:dev Jun 11, 2024

jeremycline deleted the image-version-from-vhd branch June 11, 2024 14:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow creating gallery image versions from storage accounts #1466

Allow creating gallery image versions from storage accounts #1466

jeremycline commented Feb 26, 2024

jeremycline Feb 26, 2024

jeremycline Feb 26, 2024

Fred-sun Feb 28, 2024

Fred-sun Feb 28, 2024

Fred-sun Feb 28, 2024

Fred-sun Feb 28, 2024

Fred-sun Feb 28, 2024

jeremycline commented Feb 28, 2024

Fred-sun left a comment

Fred-sun Feb 29, 2024

Fred-sun Feb 29, 2024

Fred-sun Feb 29, 2024

jeremycline Mar 1, 2024

jeremycline commented Mar 19, 2024

jeremycline commented Jun 3, 2024

Fred-sun commented Jun 4, 2024

Fred-sun commented Jun 4, 2024

Fred-sun commented Jun 4, 2024

jeremycline commented Jun 4, 2024

Fred-sun commented Jun 5, 2024

Allow creating gallery image versions from storage accounts #1466

Allow creating gallery image versions from storage accounts #1466

Conversation

jeremycline commented Feb 26, 2024

SUMMARY

ISSUE TYPE

COMPONENT NAME

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremycline commented Feb 28, 2024

Fred-sun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremycline commented Mar 19, 2024

jeremycline commented Jun 3, 2024

Fred-sun commented Jun 4, 2024

Fred-sun commented Jun 4, 2024

Fred-sun commented Jun 4, 2024

jeremycline commented Jun 4, 2024

Fred-sun commented Jun 5, 2024