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] 75051dd5a2/glance/db/simple/api.py (L226)
[2] 75051dd5a2/glance/domain/__init__.py (L125)

Change-Id: I9ca6b78bec96b69e6946b65796f12314a1ec6ab4

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 }}"

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_disk=dict(type='int'),
-        min_ram=dict(type='int', default=0),
+        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),