From 53da712635a966e6ec6c4983149786b8ebf738e4 Mon Sep 17 00:00:00 2001
From: Denys Mishchenko <denis@mischenko.org.ua>
Date: Tue, 25 Oct 2022 09:55:44 +0200
Subject: [PATCH] Existing images update name, visibility etc

Dropped default values of min_disk and min_ram parameters because
it interferes with the update mechanism and glance uses those values
anyway [1], [2].

If the image is already present and visibility param is defined we
should check its visibility and correct it if needed. Added tests
to verify that both is_public and visibility can change the image.

If both name and id are specified for the image, we might want to
update the image name. This rely on fact that id pram is checked first.
Added rename tests to verify this.

For some reason if image object is used for the image update, 409
error produced and exception trown, but the change is in place. So
instead of image object, update query rely on the image.id

[1] https://github.com/openstack/glance/blob/75051dd5a2cdeef2919ba2062714bde7502416ba/glance/db/simple/api.py#L226
[2] https://github.com/openstack/glance/blob/75051dd5a2cdeef2919ba2062714bde7502416ba/glance/domain/__init__.py#L125

Change-Id: I9ca6b78bec96b69e6946b65796f12314a1ec6ab4
---
 ci/roles/image/tasks/main.yml | 66 +++++++++++++++++++++++++++++++++--
 plugins/modules/image.py      | 36 ++++++++++++++-----
 2 files changed, 91 insertions(+), 11 deletions(-)

diff --git a/ci/roles/image/tasks/main.yml b/ci/roles/image/tasks/main.yml
index acf93ab..58c5042 100644
--- a/ci/roles/image/tasks/main.yml
+++ b/ci/roles/image/tasks/main.yml
@@ -59,7 +59,6 @@
       - "image_info_result.images[0].name == image_name"
       - "image_info_result.images[0].tags | sort == image_tags | sort"
 
-
 - name: Create raw image again (defaults)
   openstack.cloud.image:
      cloud: "{{ cloud }}"
@@ -81,7 +80,7 @@
       - item in returned_image.image
   loop: "{{ expected_fields }}"
 
-- name: Update raw image (defaults)
+- name: Update is_protected on raw image (defaults)
   openstack.cloud.image:
      cloud: "{{ cloud }}"
      state: present
@@ -100,6 +99,69 @@
     that:
       - returned_image is changed
 
+- name: Update visibility on raw image (defaults)
+  openstack.cloud.image:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ image_name }}"
+     is_public: false
+  register: returned_image
+
+- name: Assert changed
+  assert:
+    that:
+      - returned_image.image.visibility == 'private'
+
+- name: Update again visibility on raw image (defaults)
+  openstack.cloud.image:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ image_name }}"
+     is_public: true
+  register: returned_image
+
+- name: Assert changed
+  assert:
+    that:
+      - returned_image is changed
+      - returned_image.image.visibility == 'public'
+
+- name: Define visibility on raw image (defaults)
+  openstack.cloud.image:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ image_name }}"
+     visibility: shared
+  register: returned_image
+
+- name: Assert changed
+  assert:
+    that:
+      - returned_image is changed
+      - returned_image.image.visibility == 'shared'
+
+- name: Rename raw image (defaults)
+  openstack.cloud.image:
+     cloud: "{{ cloud }}"
+     state: present
+     id: "{{ returned_image.id }}"
+     name: "{{ image_name }}-changed"
+  register: returned_image
+
+- name: Assert changed
+  assert:
+    that:
+      - returned_image is changed
+      - returned_image.image.name == "{{ image_name }}-changed"
+
+- name: Rename back raw image (defaults)
+  openstack.cloud.image:
+     cloud: "{{ cloud }}"
+     state: present
+     id: "{{ returned_image.id }}"
+     name: "{{ image_name }}"
+  register: returned_image
+
 - name: Delete raw image (defaults)
   openstack.cloud.image:
      cloud: "{{ cloud }}"
diff --git a/plugins/modules/image.py b/plugins/modules/image.py
index 7a2bbd6..8530b88 100644
--- a/plugins/modules/image.py
+++ b/plugins/modules/image.py
@@ -16,11 +16,13 @@ options:
    name:
      description:
         - The name of the image when uploading - or the name/ID of the image if deleting
+        - If provided with the id, it can be used to change the name of existing image
      required: true
      type: str
    id:
      description:
         - The ID of the image when uploading an image
+        - This image attribute cannot be changed.
      type: str
    checksum:
      description:
@@ -29,12 +31,14 @@ options:
    disk_format:
      description:
         - The format of the disk that is getting uploaded
+        - This image attribute cannot be changed.
      default: qcow2
      choices: ['ami', 'ari', 'aki', 'vhd', 'vmdk', 'raw', 'qcow2', 'vdi', 'iso', 'vhdx', 'ploop']
      type: str
    container_format:
      description:
         - The format of the container
+        - This image attribute cannot be changed.
      default: bare
      choices: ['ami', 'aki', 'ari', 'bare', 'ovf', 'ova', 'docker']
      type: str
@@ -73,6 +77,7 @@ options:
    filename:
      description:
         - The path to the file which has to be uploaded
+        - This image attribute cannot be changed.
      type: str
    ramdisk:
      description:
@@ -457,8 +462,8 @@ class ImageModule(OpenStackModule):
         container_format=dict(default='bare', choices=['ami', 'aki', 'ari', 'bare', 'ovf', 'ova', 'docker']),
         owner=dict(aliases=['project']),
         owner_domain=dict(aliases=['project_domain']),
-        min_disk=dict(type='int', default=0),
-        min_ram=dict(type='int', default=0),
+        min_disk=dict(type='int'),
+        min_ram=dict(type='int'),
         is_public=dict(type='bool', default=False),
         is_protected=dict(type='bool', aliases=['protected']),
         filename=dict(),
@@ -506,7 +511,11 @@ class ImageModule(OpenStackModule):
         return image
 
     def _build_update(self, image):
-        update_payload = dict(is_protected=self.params['is_protected'])
+        update_payload = {'visibility': self._resolve_visibility()}
+
+        for k in ('is_protected', 'min_disk', 'min_ram'):
+            update_payload[k] = self.params[k]
+
         for k in ('kernel', 'ramdisk'):
             if not self.params[k]:
                 continue
@@ -514,25 +523,31 @@ class ImageModule(OpenStackModule):
             k_image = self.conn.image.find_image(
                 name_or_id=self.params[k], ignore_missing=False)
             update_payload[k_id] = k_image.id
+
         update_payload = {k: v for k, v in update_payload.items()
                           if v is not None and image[k] != v}
+
         for p, v in self.params['properties'].items():
             if p not in image or image[p] != v:
                 update_payload[p] = v
+
         if (self.params['tags']
                 and set(image['tags']) != set(self.params['tags'])):
             update_payload['tags'] = self.params['tags']
+
+        # If both name and id are defined,then we might change the name
+        if self.params['id'] and \
+           self.params['name'] and \
+           self.params['name'] != image['name']:
+            update_payload['name'] = self.params['name']
+
         return update_payload
 
     def run(self):
         changed = False
-        image_filters = {}
         image_name_or_id = self.params['id'] or self.params['name']
         owner_name_or_id = self.params['owner']
         owner_domain_name_or_id = self.params['owner_domain']
-
-        if self.params['checksum']:
-            image_filters['checksum'] = image_filters
         owner_filters = {}
         if owner_domain_name_or_id:
             owner_domain = self.conn.identity.find_domain(
@@ -550,7 +565,10 @@ class ImageModule(OpenStackModule):
 
         image = None
         if image_name_or_id:
-            image = self.conn.image.find_image(image_name_or_id, **image_filters)
+            image = self.conn.get_image(
+                image_name_or_id,
+                filters={(k, self.params[k])
+                         for k in ['checksum'] if self.params[k] is not None})
 
         changed = False
         if self.params['state'] == 'present':
@@ -569,7 +587,7 @@ class ImageModule(OpenStackModule):
             update_payload = self._build_update(image)
 
             if update_payload:
-                self.conn.image.update_image(image, **update_payload)
+                self.conn.image.update_image(image.id, **update_payload)
                 changed = True
 
             self.exit_json(changed=changed, image=self._return_value(image.id),
-- 
GitLab