cloudscale_volume: refactor to implement tags support (#61531)

lib/ansible/modules/cloud/cloudscale/cloudscale_volume.py

@@ -1,7 +1,9 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
-# (c) 2018, Gaudenz Steinlin <gaudenz.steinlin@cloudscale.ch>
+# Copyright (c) 2018, Gaudenz Steinlin <gaudenz.steinlin@cloudscale.ch>
+# Copyright (c) 2019, René Moser <mail@renemoser.net>
+
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import absolute_import, division, print_function
@@ -16,43 +18,56 @@ ANSIBLE_METADATA = {'metadata_version': '1.1',
 DOCUMENTATION = '''
 ---
 module: cloudscale_volume
-short_description: Manages volumes on the cloudscale.ch IaaS service
+short_description: Manages volumes on the cloudscale.ch IaaS service.
 description:
-  - Create, attach/detach and delete volumes on the cloudscale.ch IaaS service.
+  - Create, attach/detach, update and delete volumes on the cloudscale.ch IaaS service.
 notes:
   - To create a new volume at least the I(name) and I(size_gb) options
     are required.
   - A volume can be created and attached to a server in the same task.
-version_added: 2.8
+version_added: '2.8'
-author: "Gaudenz Steinlin (@gaudenz)"
+author:
+  - Gaudenz Steinlin (@gaudenz)
+  - René Moser (@resmo)
 options:
   state:
     description:
       - State of the volume.
     default: present
     choices: [ present, absent ]
+    type: str
   name:
     description:
       - Name of the volume. Either name or UUID must be present to change an
         existing volume.
+    type: str
   uuid:
     description:
       - UUID of the volume. Either name or UUID must be present to change an
         existing volume.
+    type: str
   size_gb:
     description:
       - Size of the volume in GB.
+    type: int
   type:
     description:
       - Type of the volume. Cannot be changed after creating the volume.
-        Defaults to ssd on volume creation.
+        Defaults to C(ssd) on volume creation.
     choices: [ ssd, bulk ]
+    type: str
   server_uuids:
     description:
-      - UUIDs of the servers this volume is attached to. Set this to [] to
+      - UUIDs of the servers this volume is attached to. Set this to C([]) to
         detach the volume. Currently a volume can only be attached to a
         single server.
     aliases: [ server_uuid ]
+    type: list
+  tags:
+    description:
+      - Tags assosiated with the volume. Set this to C({}) to clear any tags.
+    type: dict
+    version_added: '2.9'
 extends_documentation_fragment: cloudscale
 '''
 
@@ -100,33 +115,32 @@ EXAMPLES = '''
 RETURN = '''
 href:
   description: The API URL to get details about this volume.
-  returned: success when state == present
+  returned: state == present
   type: str
   sample: https://api.cloudscale.ch/v1/volumes/2db69ba3-1864-4608-853a-0771b6885a3a
 uuid:
   description: The unique identifier for this volume.
-  returned: success when state == present
+  returned: state == present
   type: str
   sample: 2db69ba3-1864-4608-853a-0771b6885a3a
 name:
   description: The display name of the volume.
-  returned: success when state == present
+  returned: state == present
   type: str
   sample: my_ssd_volume
 size_gb:
   description: The size of the volume in GB.
-  returned: success when state == present
+  returned: state == present
   type: str
   sample: 50
 type:
-  description: "The type of the volume. There are currently two options:
+  description: The type of the volume.
-                ssd (default) or bulk."
+  returned: state == present
-  returned: success when state == present
   type: str
   sample: bulk
 server_uuids:
   description: The UUIDs of the servers this volume is attached to.
-  returned: success when state == present
+  returned: state == present
   type: list
   sample: ['47cec963-fcd2-482f-bdb6-24461b2d47b1']
 state:
@@ -134,6 +148,12 @@ state:
   returned: success
   type: str
   sample: present
+tags:
+  description: Tags assosiated with the volume.
+  returned: state == present
+  type: dict
+  sample: { 'project': 'my project' }
+  version_added: '2.9'
 '''
 
 from ansible.module_utils.basic import AnsibleModule
@@ -146,77 +166,94 @@ class AnsibleCloudscaleVolume(AnsibleCloudscaleBase):
 
     def __init__(self, module):
         super(AnsibleCloudscaleVolume, self).__init__(module)
-        params = self._module.params
+        self._info = {}
-        self.info = {
+
-            'name': params['name'],
+    def _init_container(self):
-            'uuid': params['uuid'],
+        return {
+            'uuid': self._module.params.get('uuid') or self._info.get('uuid'),
+            'name': self._module.params.get('name') or self._info.get('name'),
             'state': 'absent',
         }
-        self.changed = False
-        if params['uuid'] is not None:
-            vol = self._get('volumes/%s' % params['uuid'])
-            if vol is None and params['state'] == 'present':
-                self._module.fail_json(
-                    msg='Volume with UUID %s does not exist. Can\'t create a '
-                        'volume with a predefined UUID.' % params['uuid'],
-                )
-            elif vol is not None:
-                self.info = vol
-                self.info['state'] = 'present'
-        else:
-            resp = self._get('volumes')
-            volumes = [vol for vol in resp if vol['name'] == params['name']]
-            if len(volumes) == 1:
-                self.info = volumes[0]
-                self.info['state'] = 'present'
-            elif len(volumes) > 1:
-                self._module.fail_json(
-                    msg='More than 1 volume with name "%s" exists.'
-                        % params['name'],
-                )
-
-    def create(self):
-        params = self._module.params
-
-        # check for required parameters to create a volume
-        missing_parameters = []
-        for p in ('name', 'size_gb'):
-            if p not in params or not params[p]:
-                missing_parameters.append(p)
-
-        if len(missing_parameters) > 0:
-            self._module.fail_json(
-                msg='Missing required parameter(s) to create a volume: %s.'
-                    % ' '.join(missing_parameters),
-            )
 
+    def _create(self, volume):
+        # Fail when missing params for creation
+        self._module.fail_on_missing_params(['name', 'size_gb'])
+
+        # Fail if a user uses a UUID and state=present but the volume was not found.
+        if self._module.params.get('uuid'):
+            self._module.fail_json(msg="The volume with UUID '%s' was not found "
+                                   "and we would create a new one with different UUID, "
+                                   "this is probaly not want you have asked for." % self._module.params.get('uuid'))
+
+        self._result['changed'] = True
         data = {
-            'name': params['name'],
+            'name': self._module.params.get('name'),
-            'size_gb': params['size_gb'],
+            'type': self._module.params.get('type'),
-            'type': params['type'] or 'ssd',
+            'size_gb': self._module.params.get('size_gb') or 'ssd',
-            'server_uuids': params['server_uuids'] or [],
+            'server_uuids': self._module.params.get('server_uuids') or [],
+            'tags': self._module.params.get('tags'),
         }
+        if not self._module.check_mode:
+            volume = self._post('volumes', data)
+        return volume
+
+    def _update(self, volume):
+        update_params = (
+            'name',
+            'size_gb',
+            'server_uuids',
+            'tags',
+        )
+        updated = False
+        for param in update_params:
+            updated = self._param_updated(param, volume) or updated
 
-        self.info = self._post('volumes', data)
+        # Refresh if resource was updated in live mode
-        self.info['state'] = 'present'
+        if updated and not self._module.check_mode:
-        self.changed = True
+            volume = self.get_volume()
+        return volume
 
-    def delete(self):
+    def get_volume(self):
-        self._delete('volumes/%s' % self.info['uuid'])
+        self._info = self._init_container()
-        self.info = {
-            'name': self.info['name'],
-            'uuid': self.info['uuid'],
-            'state': 'absent',
-        }
-        self.changed = True
 
-    def update(self, param):
+        uuid = self._info.get('uuid')
-        self._patch(
+        if uuid is not None:
-            'volumes/%s' % self.info['uuid'],
+            volume = self._get('volumes/%s' % uuid)
-            {param: self._module.params[param]},
+            if volume:
-        )
+                self._info.update(volume)
-        self.info[param] = self._module.params[param]
+                self._info['state'] = 'present'
-        self.changed = True
+
+        else:
+            name = self._info.get('name')
+            matching_volumes = []
+            for volume in self._get('volumes'):
+                if volume['name'] == name:
+                    matching_volumes.append(volume)
+
+            if len(matching_volumes) > 1:
+                self._module.fail_json(msg="More than one volume with name exists: '%s'. "
+                                       "Use the 'uuid' parameter to identify the volume." % name)
+            elif len(matching_volumes) == 1:
+                self._info.update(matching_volumes[0])
+                self._info['state'] = 'present'
+        return self._info
+
+    def present(self):
+        volume = self.get_volume()
+        if volume.get('state') == 'absent':
+            volume = self._create(volume)
+        else:
+            volume = self._update(volume)
+        return volume
+
+    def absent(self):
+        volume = self.get_volume()
+        if volume.get('state') != 'absent':
+            self._result['changed'] = True
+            if not self._module.check_mode:
+                volume['state'] = "absent"
+                self._delete('volumes/%s' % volume['uuid'])
+        return volume
 
 
 def main():
@@ -228,52 +265,24 @@ def main():
         size_gb=dict(type='int'),
         type=dict(choices=('ssd', 'bulk')),
         server_uuids=dict(type='list', aliases=['server_uuid']),
+        tags=dict(type='dict'),
     ))
 
     module = AnsibleModule(
         argument_spec=argument_spec,
         required_one_of=(('name', 'uuid'),),
-        mutually_exclusive=(('name', 'uuid'),),
         supports_check_mode=True,
     )
 
-    volume = AnsibleCloudscaleVolume(module)
+    cloudscale_volume = AnsibleCloudscaleVolume(module)
-    if module.check_mode:
+
-        changed = False
+    if module.params['state'] == 'absent':
-        for param, conv in (('state', str),
+        server_group = cloudscale_volume.absent()
-                            ('server_uuids', set),
+    else:
-                            ('size_gb', int)):
+        server_group = cloudscale_volume.present()
-            if module.params[param] is None:
+
-                continue
+    result = cloudscale_volume.get_result(server_group)
-
+    module.exit_json(**result)
-            if conv(volume.info[param]) != conv(module.params[param]):
-                changed = True
-                break
-
-        module.exit_json(changed=changed,
-                         **volume.info)
-
-    if (volume.info['state'] == 'absent'
-       and module.params['state'] == 'present'):
-        volume.create()
-    elif (volume.info['state'] == 'present'
-          and module.params['state'] == 'absent'):
-        volume.delete()
-
-    if module.params['state'] == 'present':
-        if (module.params['type'] is not None
-           and volume.info['type'] != module.params['type']):
-            module.fail_json(
-                msg='Cannot change type of an existing volume.',
-            )
-
-        for param, conv in (('server_uuids', set), ('size_gb', int)):
-            if module.params[param] is None:
-                continue
-            if conv(volume.info[param]) != conv(module.params[param]):
-                volume.update(param)
-
-    module.exit_json(changed=volume.changed, **volume.info)
 
 
 if __name__ == '__main__':

test/integration/targets/cloudscale_volume/tasks/check-mode.yml

@@ -1,62 +0,0 @@
----
-- name: Create volume in check mode
-  cloudscale_volume:
-    name: '{{ cloudscale_resource_prefix }}-check-mode'
-    size_gb: 50
-  register: check_mode_vol
-  check_mode: True
-- name: Delete volume created in check mode
-  cloudscale_volume:
-    name: '{{ cloudscale_resource_prefix }}-check-mode'
-    state: 'absent'
-  register: check_mode_delete
-- name: 'VERIFY: Create volume in check mode'
-  assert:
-    that:
-      - check_mode_vol is successful
-      - check_mode_vol is changed
-      - check_mode_delete is successful
-      - check_mode_delete is not changed
-
-- name: Create volume
-  cloudscale_volume:
-    name: '{{ cloudscale_resource_prefix }}-vol'
-    size_gb: 50
-- name: Attach volume in check mode
-  cloudscale_volume:
-    name: '{{ cloudscale_resource_prefix }}-vol'
-    server_uuids:
-      - '{{ server.uuid }}'
-  check_mode: True
-  register: check_mode_attach
-- name: Detach volume
-  cloudscale_volume:
-    name: '{{ cloudscale_resource_prefix }}-vol'
-    server_uuids: []
-  register: check_mode_detach
-- name: 'VERIFY: Attach volume in check mode'
-  assert:
-    that:
-      - check_mode_attach is successful
-      - check_mode_attach is changed
-      - check_mode_detach is successful
-      - check_mode_detach is not changed
-
-- name: Resize volume in check mode
-  cloudscale_volume:
-    name: '{{ cloudscale_resource_prefix }}-vol'
-    size_gb: 100
-  register: check_mode_resize
-  check_mode: True
-- name: Get volume info
-  cloudscale_volume:
-    name: '{{ cloudscale_resource_prefix }}-vol'
-  register: check_mode_info
-- name: 'VERIFY: Resize volume in check mode'
-  assert:
-    that:
-      - check_mode_resize is successful
-      - check_mode_resize is changed
-      - check_mode_info is successful
-      - check_mode_info is not changed
-      - check_mode_info.size_gb == 50

test/integration/targets/cloudscale_volume/tasks/failures.yml

@@ -27,6 +27,7 @@
 - name: Fail volume creation with UUID
   cloudscale_volume:
     uuid: ea3b39a3-77a8-4d0b-881d-0bb00a1e7f48
+    name: '{{ cloudscale_resource_prefix }}-inexistent'
     size_gb: 10
   register: vol
   ignore_errors: True
@@ -34,20 +35,4 @@
   assert:
     that:
       - vol is failed
-
+      - vol.msg.startswith('The volume with UUID \'ea3b39a3-77a8-4d0b-881d-0bb00a1e7f48\' was not found')
-- name: Create volume
-  cloudscale_volume:
-    name: '{{ cloudscale_resource_prefix }}-name-UUID'
-    size_gb: 50
-  register: vol
-- name: Fail name and UUID
-  cloudscale_volume:
-    name: '{{ vol.name }}'
-    uuid: '{{ vol.uuid }}'
-    size_gb: 100
-  register: vol
-  ignore_errors: True
-- name: 'VERIFY: Fail name and UUID'
-  assert:
-    that:
-      - vol is failed

test/integration/targets/cloudscale_volume/tasks/tests.yml

@@ -1,8 +1,29 @@
 ---
+- name: Create volume in check mode
+  cloudscale_volume:
+    name: '{{ cloudscale_resource_prefix }}-vol'
+    size_gb: 50
+    tags:
+      project: ansible-test
+      stage: production
+      sla: 24-7
+  check_mode: yes
+  register: vol
+- name: 'VERIFY: Create volume in check mode'
+  assert:
+    that:
+      - vol is successful
+      - vol is changed
+      - vol.state == 'absent'
+
 - name: Create volume
   cloudscale_volume:
     name: '{{ cloudscale_resource_prefix }}-vol'
     size_gb: 50
+    tags:
+      project: ansible-test
+      stage: production
+      sla: 24-7
   register: vol
 - name: 'VERIFY: Create volume'
   assert:
@@ -11,17 +32,43 @@
       - vol is changed
       - vol.size_gb == 50
       - vol.name == '{{ cloudscale_resource_prefix }}-vol'
+      - vol.tags.project == 'ansible-test'
+      - vol.tags.stage == 'production'
+      - vol.tags.sla == '24-7'
 
 - name: Create volume indempotence
   cloudscale_volume:
     name: '{{ cloudscale_resource_prefix }}-vol'
     size_gb: 50
+    tags:
+      project: ansible-test
+      stage: production
+      sla: 24-7
   register: vol
 - name: 'VERIFY: Create volume indempotence'
   assert:
     that:
       - vol is successful
       - vol is not changed
+      - vol.size_gb == 50
+      - vol.name == '{{ cloudscale_resource_prefix }}-vol'
+      - vol.tags.project == 'ansible-test'
+      - vol.tags.stage == 'production'
+      - vol.tags.sla == '24-7'
+
+- name: Attach existing volume by name to server in check mode
+  cloudscale_volume:
+    name: '{{ cloudscale_resource_prefix }}-vol'
+    server_uuids:
+      - '{{ server.uuid }}'
+  check_mode: yes
+  register: vol
+- name: 'VERIFY: Attach existing volume by name to server in check mode'
+  assert:
+    that:
+      - vol is successful
+      - vol is changed
+      - server.uuid not in vol.server_uuids
 
 - name: Attach existing volume by name to server
   cloudscale_volume:
@@ -49,6 +96,19 @@
       - vol is not changed
       - server.uuid in vol.server_uuids
 
+- name: Resize attached volume by UUID in check mode
+  cloudscale_volume:
+    uuid: '{{ vol.uuid }}'
+    size_gb: 100
+  check_mode: yes
+  register: vol
+- name: 'VERIFY: Resize attached volume by UUID in check mode'
+  assert:
+    that:
+      - vol is successful
+      - vol is changed
+      - vol.size_gb == 50
+
 - name: Resize attached volume by UUID
   cloudscale_volume:
     uuid: '{{ vol.uuid }}'
@@ -73,6 +133,21 @@
       - vol is not changed
       - vol.size_gb == 100
 
+- name: Delete attached volume by UUID in check mode
+  cloudscale_volume:
+    uuid: '{{ vol.uuid }}'
+    state: 'absent'
+  check_mode: yes
+  register: deleted
+- name: 'VERIFY: Delete attached volume by UUID in check mode'
+  assert:
+    that:
+      - deleted is successful
+      - deleted is changed
+      - deleted.state == 'present'
+      - deleted.uuid == vol.uuid
+      - deleted.name == '{{ cloudscale_resource_prefix }}-vol'
+
 - name: Delete attached volume by UUID
   cloudscale_volume:
     uuid: '{{ vol.uuid }}'
@@ -84,6 +159,8 @@
       - deleted is successful
       - deleted is changed
       - deleted.state == 'absent'
+      - deleted.uuid == vol.uuid
+      - deleted.name == '{{ cloudscale_resource_prefix }}-vol'
 
 - name: Delete attached volume by UUID indempotence
   cloudscale_volume:
@@ -96,6 +173,8 @@
       - deleted is successful
       - deleted is not changed
       - deleted.state == 'absent'
+      - deleted.uuid == vol.uuid
+      - not deleted.name
 
 - name: Create bulk volume and attach
   cloudscale_volume:
@@ -137,6 +216,19 @@
       - bulk is changed
       - bulk.size_gb == 200
 
+- name: Delete volume by name in check mode
+  cloudscale_volume:
+    name: '{{ bulk.name }}'
+    state: 'absent'
+  check_mode: yes
+  register: bulk
+- name: 'VERIFY: Delete volume by name'
+  assert:
+    that:
+      - bulk is successful
+      - bulk is changed
+      - bulk.state == 'present'
+
 - name: Delete volume by name
   cloudscale_volume:
     name: '{{ bulk.name }}'
@@ -149,6 +241,16 @@
       - bulk is changed
       - bulk.state == 'absent'
 
-- import_tasks: failures.yml
+- name: Delete volume by name idempotence
+  cloudscale_volume:
+    name: '{{ bulk.name }}'
+    state: 'absent'
+  register: bulk
+- name: 'VERIFY: Delete volume by name idempotence'
+  assert:
+    that:
+      - bulk is successful
+      - bulk is not changed
+      - bulk.state == 'absent'
 
-- import_tasks: check-mode.yml
+- import_tasks: failures.yml