Refactored volume_snapshot{,_info} modules

Change-Id: I70fc744f786a9de654592c97188af48ddbe8751d

.zuul.yaml

@@ -112,6 +112,7 @@
         user_role
         volume
         volume_backup
+        volume_snapshot
       # failing tags
       # neutron_rbac
 

ci/roles/volume/tasks/main.yml

@@ -20,38 +20,12 @@
      volume: "{{ vol.volume.id }}"
      name: ansible_volume1
      description: Test volume
-  register: vol
 
-- name: Create volume snapshot
+- name: Delete volume
-  openstack.cloud.volume_snapshot:
-    cloud: "{{ cloud }}"
-    state: present
-    name: ansible_volume_snapshot
-    volume: ansible_volume
-  register: vol_snap
-
-- name: Get snapshot info
-  openstack.cloud.volume_snapshot_info:
-    cloud: "{{ cloud }}"
-    name: ansible_volume_snapshot
-  register: snap_info
-
-- name: Create volume from snapshot
   openstack.cloud.volume:
      cloud: "{{ cloud }}"
-     state: present
-     size: 1
-     snapshot: ansible_volume_snapshot
-     name: ansible_volume2
-     description: Test volume
-  register: vol
-
-- name: Delete volume snapshot
-  openstack.cloud.volume_snapshot:
-    cloud: "{{ cloud }}"
-    name: ansible_volume_snapshot
-    volume: ansible_volume
      state: absent
+     name: ansible_volume1
 
 - name: Delete volume
   openstack.cloud.volume:
@@ -59,15 +33,6 @@
      state: absent
      name: ansible_volume
 
-- name: Clean up
-  openstack.cloud.volume:
-     cloud: "{{ cloud }}"
-     state: absent
-     name: "{{ item }}"
-  loop:
-    - ansible_volume1
-    - ansible_volume2
-
 - name: Test images
   block:
     - name: Ensure clean environment

ci/roles/volume_snapshot/defaults/main.yml

@@ -0,0 +1,13 @@
+expected_fields:
+  - created_at
+  - description
+  - id
+  - is_forced
+  - metadata
+  - name
+  - progress
+  - project_id
+  - size
+  - status
+  - updated_at
+  - volume_id

ci/roles/volume_snapshot/tasks/main.yml

@@ -0,0 +1,96 @@
+---
+- name: Get existing snapshots
+  openstack.cloud.volume_snapshot_info:
+    cloud: "{{ cloud }}"
+  register: info
+
+- name: Assert volume_snapshot_info
+  assert:
+    that:
+      - info.volume_snapshots|length == 0
+
+- name: Get non-existing snapshot
+  openstack.cloud.volume_snapshot_info:
+    cloud: "{{ cloud }}"
+    name: non-existing-snapshot
+  register: info
+
+- name: Assert volume_snapshot_info
+  assert:
+    that:
+      - info.volume_snapshots|length == 0
+
+- name: Create volume
+  openstack.cloud.volume:
+     cloud: "{{ cloud }}"
+     state: present
+     size: 1
+     name: ansible_volume
+     description: Test volume
+  register: volume
+
+- name: Create volume snapshot
+  openstack.cloud.volume_snapshot:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_volume_snapshot
+    volume: ansible_volume
+  register: snapshot
+
+- name: Assert volume_snapshot
+  assert:
+    that:
+      - snapshot.volume_snapshot.name == "ansible_volume_snapshot"
+
+- name: Assert return values of volume_snapshot module
+  assert:
+    that:
+      # allow new fields to be introduced but prevent fields from being removed
+      - expected_fields|difference(snapshot.volume_snapshot.keys())|length == 0
+
+- name: Get snapshot info
+  openstack.cloud.volume_snapshot_info:
+    cloud: "{{ cloud }}"
+    name: ansible_volume_snapshot
+  register: info
+
+- name: Assert volume_snapshot_info
+  assert:
+    that:
+      - info.volume_snapshots|length == 1
+      - info.volume_snapshots[0].id == snapshot.volume_snapshot.id
+      - info.volume_snapshots[0].volume_id == volume.volume.id
+
+- name: Assert return values of volume_info module
+  assert:
+    that:
+      # allow new fields to be introduced but prevent fields from being removed
+      - expected_fields|difference(info.volume_snapshots[0].keys())|length == 0
+
+- name: Create volume from snapshot
+  openstack.cloud.volume:
+     cloud: "{{ cloud }}"
+     state: present
+     size: 1
+     snapshot: ansible_volume_snapshot
+     name: ansible_volume2
+     description: Test volume
+
+- name: Delete volume snapshot
+  openstack.cloud.volume_snapshot:
+    cloud: "{{ cloud }}"
+    name: ansible_volume_snapshot
+    volume: ansible_volume
+    state: absent
+
+- name: Delete volume
+  openstack.cloud.volume:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_volume2
+
+- name: Delete volume
+  openstack.cloud.volume:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_volume

ci/run-collection.yml

@@ -66,6 +66,7 @@
     - { role: user_role, tags: user_role }
     - { role: volume, tags: volume }
     - { role: volume_backup, tags: volume_backup }
+    - { role: volume_snapshot, tags: volume_snapshot }
     - role: loadbalancer
       tags: loadbalancer
     - { role: quota, tags: quota }

plugins/modules/volume_snapshot.py

@@ -4,7 +4,7 @@
 # Copyright (c) 2016, Mario Santos <mario.rf.santos@gmail.com>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-DOCUMENTATION = '''
+DOCUMENTATION = r'''
 ---
 module: volume_snapshot
 short_description: Create/Delete Cinder Volume Snapshots
@@ -12,75 +12,109 @@ author: OpenStack Ansible SIG
 description:
    - Create or Delete cinder block storage volume snapshots
 options:
-   display_name:
   description:
-        - Name of the snapshot
-     required: true
-     aliases: ['name']
-     type: str
-   display_description:
     description:
       - String describing the snapshot
-     aliases: ['description']
+    aliases: ['display_description']
-     type: str
-   volume:
-     description:
-       - The volume name or id to create/delete the snapshot
-     required: True
     type: str
   force:
     description:
-       - Allows or disallows snapshot of a volume to be created when the volume
+       - Allows or disallows snapshot of a volume to be created,
-         is attached to an instance.
+         when the volume is attached to an instance.
     type: bool
     default: 'no'
+  name:
+    description:
+      - Name of the snapshot
+    required: true
+    aliases: ['display_name']
+    type: str
   state:
     description:
-       - Should the resource be present or absent.
+      - Should the snapshot be C(present) or C(absent).
     choices: [present, absent]
     default: present
     type: str
+  volume:
+    description:
+      - Volume name or ID to create the snapshot from.
+      - Required when I(state) is C(present).
+    type: str
 requirements:
     - "python >= 3.6"
     - "openstacksdk"
 
+notes:
+    - Updating existing volume snapshots has not been implemented yet.
+
 extends_documentation_fragment:
 - openstack.cloud.openstack
 '''
 
-EXAMPLES = '''
+EXAMPLES = r'''
-# Creates a snapshot on volume 'test_volume'
-- name: create and delete snapshot
-  hosts: localhost
-  tasks:
 - name: create snapshot
   openstack.cloud.volume_snapshot:
     state: present
     cloud: mordred
-      availability_zone: az2
+    name: test_snapshot
-      display_name: test_snapshot
     volume: test_volume
 - name: delete snapshot
   openstack.cloud.volume_snapshot:
     state: absent
     cloud: mordred
-      availability_zone: az2
+    name: test_snapshot
-      display_name: test_snapshot
     volume: test_volume
 '''
 
-RETURN = '''
+RETURN = r'''
 snapshot:
-    description: The snapshot instance after the change
+    description: Same as C(volume_snapshot), kept for backward compatibility.
+    returned: On success when C(state=present)
+    type: dict
+volume_snapshot:
+    description: The snapshot instance
     returned: success
     type: dict
-    sample:
+    contains:
-      id: 837aca54-c0ee-47a2-bf9a-35e1b4fdac0c
+        created_at:
-      name: test_snapshot
+            description: Snapshot creation time.
-      volume_id: ec646a7c-6a35-4857-b38b-808105a24be6
+            type: str
-      size: 2
+        description:
-      status: available
+            description: Snapshot desciption.
-      display_name: test_snapshot
+            type: str
+        id:
+            description: Unique UUID.
+            type: str
+            sample: "39007a7e-ee4f-4d13-8283-b4da2e037c69"
+        is_forced:
+            description: Indicate whether to create snapshot,
+                         even if the volume is attached.
+            type: bool
+        metadata:
+            description: Snapshot metadata.
+            type: dict
+        name:
+            description: Snapshot Name.
+            type: str
+        progress:
+            description: The percentage of completeness the snapshot is
+                         currently at.
+            type: str
+        project_id:
+            description: The project ID this snapshot is associated with.
+            type: str
+        size:
+            description: The size of the volume, in GBs.
+            type: int
+        status:
+            description: Snapshot status.
+            type: str
+        updated_at:
+            description: Snapshot update time.
+            type: str
+        volume_id:
+            description: Volume ID.
+            type: str
 '''
 
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
@@ -88,74 +122,86 @@ from ansible_collections.openstack.cloud.plugins.module_utils.openstack import O
 
 class VolumeSnapshotModule(OpenStackModule):
     argument_spec = dict(
-        display_name=dict(required=True, aliases=['name']),
+        description=dict(aliases=['display_description']),
-        display_description=dict(aliases=['description']),
+        name=dict(required=True, aliases=['display_name']),
-        volume=dict(required=True),
         force=dict(default=False, type='bool'),
         state=dict(default='present', choices=['absent', 'present']),
+        volume=dict(),
     )
 
     module_kwargs = dict(
+        required_if=[
+            ('state', 'present', ['volume'])
+        ],
         supports_check_mode=True
     )
 
-    def _present_volume_snapshot(self):
-        volume = self.conn.get_volume(self.params['volume'])
-        snapshot = self.conn.get_volume_snapshot(
-            self.params['display_name'], filters={'volume_id': volume.id})
-        if not snapshot:
-            snapshot = self.conn.create_volume_snapshot(
-                volume.id,
-                force=self.params['force'],
-                wait=self.params['wait'],
-                timeout=self.params['timeout'],
-                name=self.params['display_name'],
-                description=self.params.get('display_description')
-            )
-            self.exit_json(changed=True, snapshot=snapshot)
-        else:
-            self.exit_json(changed=False, snapshot=snapshot)
-
-    def _absent_volume_snapshot(self):
-        volume = self.conn.get_volume(self.params['volume'])
-        snapshot = self.conn.get_volume_snapshot(
-            self.params['display_name'], filters={'volume_id': volume.id})
-        if not snapshot:
-            self.exit_json(changed=False)
-        else:
-            self.conn.delete_volume_snapshot(
-                name_or_id=snapshot.id,
-                wait=self.params['wait'],
-                timeout=self.params['timeout'],
-            )
-            self.exit_json(changed=True, snapshot_id=snapshot.id)
-
-    def _system_state_change(self):
-        volume = self.conn.get_volume(self.params['volume'])
-        snapshot = self.conn.get_volume_snapshot(
-            self.params['display_name'],
-            filters={'volume_id': volume.id})
-        state = self.params['state']
-
-        if state == 'present':
-            return snapshot is None
-        if state == 'absent':
-            return snapshot is not None
-
     def run(self):
+        name = self.params['name']
         state = self.params['state']
 
-        if self.conn.volume_exists(self.params['volume']):
+        snapshot = self.conn.block_storage.find_snapshot(name)
+
         if self.ansible.check_mode:
-                self.exit_json(changed=self._system_state_change())
+            self.exit_json(changed=self._will_change(state, snapshot))
-            if state == 'present':
+
-                self._present_volume_snapshot()
+        if state == 'present' and not snapshot:
-            if state == 'absent':
+            snapshot = self._create()
-                self._absent_volume_snapshot()
+            self.exit_json(changed=True,
+                           snapshot=snapshot.to_dict(computed=False),
+                           volume_snapshot=snapshot.to_dict(computed=False))
+
+        elif state == 'present' and snapshot:
+            # We do not support snapshot updates yet
+            # TODO: Implement module updates
+            self.exit_json(changed=False,
+                           snapshot=snapshot.to_dict(computed=False),
+                           volume_snapshot=snapshot.to_dict(computed=False))
+
+        elif state == 'absent' and snapshot:
+            self._delete(snapshot)
+            self.exit_json(changed=True)
+
+        else:  # state == 'absent' and not snapshot
+            self.exit_json(changed=False)
+
+    def _create(self):
+        args = dict()
+        for k in ['description', 'force', 'name']:
+            if self.params[k] is not None:
+                args[k] = self.params[k]
+
+        volume_name_or_id = self.params['volume']
+        volume = self.conn.block_storage.find_volume(volume_name_or_id,
+                                                     ignore_missing=False)
+        args['volume_id'] = volume.id
+
+        snapshot = self.conn.block_storage.create_snapshot(**args)
+
+        if self.params['wait']:
+            snapshot = self.conn.block_storage.wait_for_status(
+                snapshot, wait=self.params['timeout'])
+
+        return snapshot
+
+    def _delete(self, snapshot):
+        self.conn.block_storage.delete_snapshot(snapshot)
+        if self.params['wait']:
+            self.conn.block_storage.wait_for_delete(
+                snapshot, wait=self.params['timeout'])
+
+    def _will_change(self, state, snapshot):
+        if state == 'present' and not snapshot:
+            return True
+        elif state == 'present' and snapshot:
+            # We do not support snapshot updates yet
+            # TODO: Implement module updates
+            return False
+        elif state == 'absent' and snapshot:
+            return True
         else:
-            self.fail_json(
+            # state == 'absent' and not snapshot:
-                msg="No volume with name or id '{0}' was found.".format(
+            return False
-                    self.params['volume']))
 
 
 def main():

plugins/modules/volume_snapshot_info.py

@@ -5,7 +5,7 @@
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 
-DOCUMENTATION = '''
+DOCUMENTATION = r'''
 ---
 module: volume_snapshot_info
 short_description: Get volume snapshots
@@ -16,32 +16,35 @@ options:
   details:
     description: More detailed output
     type: bool
-    default: True
   name:
     description:
       - Name of the Snapshot.
     type: str
-  volume:
-    description:
-      - Name of the volume.
-    type: str
   status:
     description:
       - Specifies the snapshot status.
-    choices: [creating, available, error, deleting,
+    choices: ['available', 'backing-up', 'creating', 'deleted', 'deleting',
-              error_deleting, rollbacking, backing-up]
+              'error', 'error_deleting', 'restoring', 'unmanaging']
     type: str
-requirements: ["openstacksdk"]
+  volume:
+    description:
+      - Name or ID of the volume.
+    type: str
+
+requirements:
+  - "python >= 3.6"
+  - "openstacksdk"
+
 extends_documentation_fragment:
   - openstack.cloud.openstack
 '''
 
-RETURN = '''
+RETURN = r'''
 volume_snapshots:
     description: List of dictionaries describing volume snapshots.
     type: list
     elements: dict
-    returned: always.
+    returned: always
     contains:
         created_at:
             description: Snapshot creation time.
@@ -53,12 +56,26 @@ volume_snapshots:
             description: Unique UUID.
             type: str
             sample: "39007a7e-ee4f-4d13-8283-b4da2e037c69"
+        is_forced:
+            description: Indicate whether to create snapshot,
+                         even if the volume is attached.
+            type: bool
         metadata:
             description: Snapshot metadata.
             type: dict
         name:
             description: Snapshot Name.
             type: str
+        progress:
+            description: The percentage of completeness the snapshot is
+                         currently at.
+            type: str
+        project_id:
+            description: The project ID this snapshot is associated with.
+            type: str
+        size:
+            description: The size of the volume, in GBs.
+            type: int
         status:
             description: Snapshot status.
             type: str
@@ -68,62 +85,53 @@ volume_snapshots:
         volume_id:
             description: Volume ID.
             type: str
-
 '''
 
-EXAMPLES = '''
+EXAMPLES = r'''
-# Get snapshots.
+- name: List all snapshots
-- openstack.cloud.volume_snapshot_info:
+  openstack.cloud.volume_snapshot_info:
-  register: snapshots
 
-- openstack.cloud.volume_snapshotbackup_info:
+- name: Fetch data about a single snapshot
+  openstack.cloud.volume_snapshot_info:
     name: my_fake_snapshot
-  register: snapshot
 '''
 
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
 
 
 class VolumeSnapshotInfoModule(OpenStackModule):
-    module_min_sdk_version = '0.49.0'
-
     argument_spec = dict(
-        details=dict(default=True, type='bool'),
+        details=dict(type='bool'),
         name=dict(),
+        status=dict(choices=['available', 'backing-up', 'creating', 'deleted',
+                             'deleting', 'error', 'error_deleting',
+                             'restoring', 'unmanaging']),
         volume=dict(),
-        status=dict(choices=['creating', 'available', 'error',
-                             'deleting', 'error_deleting', 'rollbacking',
-                             'backing-up']),
     )
+
     module_kwargs = dict(
         supports_check_mode=True
     )
 
     def run(self):
+        kwargs = dict((k, self.params[k])
+                      for k in ['details', 'name', 'status']
+                      if self.params[k] is not None)
 
-        details_filter = self.params['details']
+        volume_name_or_id = self.params['volume']
-        name_filter = self.params['name']
+        volume = None
-        volume_filter = self.params['volume']
+        if volume_name_or_id:
-        status_filter = self.params['status']
+            volume = self.conn.block_storage.find_volume(volume_name_or_id)
+            if volume:
+                kwargs['volume_id'] = volume.id
 
-        data = []
+        if volume_name_or_id and not volume:
-        query = {}
+            snapshots = []
-        if name_filter:
+        else:
-            query['name'] = name_filter
+            snapshots = [b.to_dict(computed=False)
-        if volume_filter:
+                         for b in self.conn.block_storage.snapshots(**kwargs)]
-            query['volume_id'] = self.conn.block_storage.find_volume(volume_filter)
-        if status_filter:
-            query['status'] = status_filter.lower()
 
-        for raw in self.conn.block_storage.snapshots(details_filter, **query):
+        self.exit_json(changed=False, volume_snapshots=snapshots)
-            dt = raw.to_dict()
-            dt.pop('location')
-            data.append(dt)
-
-        self.exit_json(
-            changed=False,
-            volume_snapshots=data
-        )
 
 
 def main():