Add OpenstackModule to os_server_action

Redesign the module for more OOP
Add tests for server_action
Change-Id: I054de32ee3ff34988db53fc87b1cb63b8e551ae3

ci/roles/server/tasks/main.yml

@@ -156,3 +156,5 @@
      state: absent
      name: "{{ server_name }}"
      wait: true
+
+- include_tasks: server_actions.yml

ci/roles/server/tasks/server_actions.yml

@@ -0,0 +1,408 @@
+- name: Create server
+  openstack.cloud.server:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ server_name }}"
+     image: "{{ image }}"
+     flavor: "{{ flavor }}"
+     network: "{{ server_network }}"
+     auto_floating_ip: false
+     wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info1
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info1.openstack_servers.0.status == 'ACTIVE'
+
+- name: Stop server
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: stop
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info2
+
+- name: Ensure status for server is SHUTOFF
+  assert:
+    that:
+      - info2.openstack_servers.0.status == 'SHUTOFF'
+      - server is changed
+
+- name: Stop server again
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: stop
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info3
+
+- name: Ensure status for server is SHUTOFF
+  assert:
+    that:
+      - info3.openstack_servers.0.status == 'SHUTOFF'
+      - server is not changed
+
+- name: Start server
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: start
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info4
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info4.openstack_servers.0.status == 'ACTIVE'
+      - server is changed
+
+- name: Start server again
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: start
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info5
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info5.openstack_servers.0.status == 'ACTIVE'
+      - server is not changed
+
+- name: Pause server
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: pause
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info6
+
+- name: Ensure status for server is PAUSED
+  assert:
+    that:
+      - info6.openstack_servers.0.status == 'PAUSED'
+      - server is changed
+
+- name: Pause server again
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: pause
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info7
+
+- name: Ensure status for server is PAUSED
+  assert:
+    that:
+      - info7.openstack_servers.0.status == 'PAUSED'
+      - server is not changed
+
+- name: Unpause server
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: unpause
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info8
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info8.openstack_servers.0.status == 'ACTIVE'
+      - server is changed
+
+- name: Unpause server again
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: unpause
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info9
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info9.openstack_servers.0.status == 'ACTIVE'
+      - server is not changed
+
+- name: Lock server
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: lock
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info10
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info10.openstack_servers.0.status == 'ACTIVE'
+      # not in all versions 'locked' is supported
+      - info10.openstack_servers.0.locked in (None, True)
+      - server is changed
+
+- name: Lock server again
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: lock
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info11
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info11.openstack_servers.0.status == 'ACTIVE'
+      # not in all versions 'locked' is supported
+      - info11.openstack_servers.0.locked in (None, True)
+      - server is changed  # no support for lock idempotency
+
+- name: Unock server
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: unlock
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info12
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info12.openstack_servers.0.status == 'ACTIVE'
+      # not in all versions 'locked' is supported
+      - info12.openstack_servers.0.locked in (None, False)
+      - server is changed
+
+- name: Unlock server again
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: unlock
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info13
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info13.openstack_servers.0.status == 'ACTIVE'
+      - server is changed  # no support for unlock idempotency
+      # not in all versions 'locked' is supported
+      - info13.openstack_servers.0.locked in (None, False)
+
+- name: Suspend server
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: suspend
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info14
+
+- name: Ensure status for server is SUSPENDED
+  assert:
+    that:
+      - info14.openstack_servers.0.status == 'SUSPENDED'
+      - server is changed
+
+- name: Suspend server again
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: suspend
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info15
+
+- name: Ensure status for server is SUSPENDED
+  assert:
+    that:
+      - info15.openstack_servers.0.status == 'SUSPENDED'
+      - server is not changed
+
+- name: Resume server
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: resume
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info16
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info16.openstack_servers.0.status == 'ACTIVE'
+      - server is changed
+
+- name: Resume server again
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: resume
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info17
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info17.openstack_servers.0.status == 'ACTIVE'
+      - server is not changed
+
+- name: Rebuild server - error
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    action: rebuild
+    wait: true
+  register: server
+  ignore_errors: true
+
+- name: Ensure server rebuild failed
+  assert:
+    that:
+      - server is failed
+      - "'missing: image' in server.msg "
+
+- name: Rebuild server
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    image: "{{ image }}"
+    action: rebuild
+    wait: true
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info18
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info18.openstack_servers.0.status in ('ACTIVE', 'REBUILD')
+      - server is changed
+
+- name: Rebuild server with admin password
+  openstack.cloud.server_action:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+    image: "{{ image }}"
+    action: rebuild
+    wait: true
+    admin_password: random
+  register: server
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: "{{ server_name }}"
+  register: info19
+
+- name: Ensure status for server is ACTIVE
+  assert:
+    that:
+      - info19.openstack_servers.0.status in ('ACTIVE', 'REBUILD')
+      - server is changed

plugins/modules/server_action.py

@@ -10,38 +10,43 @@ module: server_action
 short_description: Perform actions on Compute Instances from OpenStack
 author: "Jesse Keating (@omgjlk)"
 description:
-   - Perform server actions on an existing compute instance from OpenStack.
+    - Perform server actions on an existing compute instance from OpenStack.
-     This module does not return any data other than changed true/false.
+        This module does not return any data other than changed true/false.
-     When I(action) is 'rebuild', then I(image) parameter is required.
+        When I(action) is 'rebuild', then I(image) parameter is required.
 options:
-   server:
+    server:
-     description:
+        description:
         - Name or ID of the instance
-     required: true
+        required: true
-     type: str
+        type: str
-   wait:
+    wait:
-     description:
+        description:
         - If the module should wait for the instance action to be performed.
-     type: bool
+        type: bool
-     default: 'yes'
+        default: 'yes'
-   timeout:
+    timeout:
-     description:
+        description:
         - The amount of time the module should wait for the instance to perform
-          the requested action.
+            the requested action.
-     default: 180
+        default: 180
-     type: int
+        type: int
-   action:
+    action:
-     description:
+        description:
-       - Perform the given action. The lock and unlock actions always return
+        - Perform the given action. The lock and unlock actions always return
-         changed as the servers API does not provide lock status.
+            changed as the servers API does not provide lock status.
-     choices: [stop, start, pause, unpause, lock, unlock, suspend, resume,
+        choices: [stop, start, pause, unpause, lock, unlock, suspend, resume,
-               rebuild]
+                rebuild]
-     type: str
+        type: str
-     required: true
+        required: true
-   image:
+    image:
-     description:
+        description:
-       - Image the server should be rebuilt with
+        - Image the server should be rebuilt with
-     type: str
+        type: str
+    admin_password:
+        description:
+        - Admin password for server to rebuild
+        type: str
+
 requirements:
     - "python >= 3.6"
     - "openstacksdk"
@@ -63,10 +68,9 @@ EXAMPLES = '''
       timeout: 200
 '''
 
-from ansible.module_utils.basic import AnsibleModule
+from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
-from ansible_collections.openstack.cloud.plugins.module_utils.openstack import (openstack_full_argument_spec,
+
-                                                                                openstack_module_kwargs,
+
-                                                                                openstack_cloud_from_module)
 _action_map = {'stop': 'SHUTOFF',
                'start': 'ACTIVE',
                'pause': 'PAUSED',
@@ -80,160 +84,98 @@ _action_map = {'stop': 'SHUTOFF',
 _admin_actions = ['pause', 'unpause', 'suspend', 'resume', 'lock', 'unlock']
 
 
-def _action_url(server_id):
+class ServerActionModule(OpenStackModule):
-    return '/servers/{server_id}/action'.format(server_id=server_id)
+    deprecated_names = ('os_server_action', 'openstack.cloud.os_server_action')
 
+    argument_spec = dict(
+        server=dict(required=True, type='str'),
+        action=dict(required=True, type='str',
+                    choices=['stop', 'start', 'pause', 'unpause',
+                             'lock', 'unlock', 'suspend', 'resume',
+                             'rebuild']),
+        image=dict(required=False, type='str'),
+        admin_password=dict(required=False, type='str'),
+    )
+    module_kwargs = dict(
+        required_if=[('action', 'rebuild', ['image'])],
+        supports_check_mode=True,
+    )
 
-def _wait(timeout, cloud, server, action, module, sdk):
+    def run(self):
-    """Wait for the server to reach the desired state for the given action."""
+        os_server = self._preliminary_checks()
+        self._execute_server_action(os_server)
+        # for some reason we don't wait for lock and unlock before exit
+        if self.params['action'] not in ('lock', 'unlock'):
+            if self.params['wait']:
+                self._wait(os_server)
+        self.exit_json(changed=True)
 
-    for count in sdk.utils.iterate_timeout(
+    def _preliminary_checks(self):
-            timeout,
+        # Using Munch object for getting information about a server
-            "Timeout waiting for server to complete %s" % action):
+        os_server = self.conn.get_server(self.params['server'])
+        if not os_server:
+            self.fail_json(msg='Could not find server %s' % self.params['server'])
+        # check mode
+        if self.ansible.check_mode:
+            self.exit_json(changed=self.__system_state_change(os_server))
+        # examine special cases
+        # lock, unlock and rebuild don't depend on state, just do it
+        if self.params['action'] not in ('lock', 'unlock', 'rebuild'):
+            if not self.__system_state_change(os_server):
+                self.exit_json(changed=False)
+        return os_server
+
+    def _execute_server_action(self, os_server):
+        if self.params['action'] == 'rebuild':
+            return self._rebuild_server(os_server)
+        action_name = self.params['action'] + "_server"
         try:
-            server = cloud.get_server(server.id)
+            func_name = getattr(self.conn.compute, action_name)
-        except Exception:
+        except AttributeError:
-            continue
+            self.fail_json(
+                msg="Method %s wasn't found in OpenstackSDK compute" % action_name)
+        func_name(os_server)
 
-        if server.status == _action_map[action]:
+    def _rebuild_server(self, os_server):
-            return
+        # rebuild should ensure images exists
+        try:
+            image = self.conn.get_image(self.params['image'])
+        except Exception as e:
+            self.fail_json(
+                msg="Can't find the image %s: %s" % (self.params['image'], e))
+        if not image:
+            self.fail_json(msg="Image %s was not found!" % self.params['image'])
+        # admin_password is required by SDK, but not required by Nova API
+        if self.params['admin_password']:
+            self.conn.compute.rebuild_server(
+                server=os_server,
+                name=os_server['name'],
+                image=image['id'],
+                admin_password=self.params['admin_password']
+            )
+        else:
+            self.conn.compute.post(
+                '/servers/{server_id}/action'.format(
+                    server_id=os_server['id']),
+                json={'rebuild': {'imageRef': image['id']}})
 
-        if server.status == 'ERROR':
+    def _wait(self, os_server):
-            module.fail_json(msg="Server reached ERROR state while attempting to %s" % action)
+        """Wait for the server to reach the desired state for the given action."""
+        # Using Server object for wait_for_server function
+        server = self.conn.compute.find_server(self.params['server'])
+        self.conn.compute.wait_for_server(
+            server,
+            status=_action_map[self.params['action']],
+            wait=self.params['timeout'])
 
-
+    def __system_state_change(self, os_server):
-def _system_state_change(action, status):
+        """Check if system state would change."""
-    """Check if system state would change."""
+        return os_server.status != _action_map[self.params['action']]
-    if status == _action_map[action]:
-        return False
-    return True
 
 
 def main():
-    argument_spec = openstack_full_argument_spec(
+    module = ServerActionModule()
-        server=dict(required=True),
+    module()
-        action=dict(required=True, choices=['stop', 'start', 'pause', 'unpause',
-                                            'lock', 'unlock', 'suspend', 'resume',
-                                            'rebuild']),
-        image=dict(required=False),
-    )
-
-    module_kwargs = openstack_module_kwargs()
-    module = AnsibleModule(argument_spec, supports_check_mode=True,
-                           required_if=[('action', 'rebuild', ['image'])],
-                           **module_kwargs)
-
-    action = module.params['action']
-    wait = module.params['wait']
-    timeout = module.params['timeout']
-    image = module.params['image']
-
-    sdk, cloud = openstack_cloud_from_module(module)
-    try:
-        server = cloud.get_server(module.params['server'])
-        if not server:
-            module.fail_json(msg='Could not find server %s' % server)
-        status = server.status
-
-        if module.check_mode:
-            module.exit_json(changed=_system_state_change(action, status))
-
-        if action == 'stop':
-            if not _system_state_change(action, status):
-                module.exit_json(changed=False)
-
-            cloud.compute.post(
-                _action_url(server.id),
-                json={'os-stop': None})
-            if wait:
-                _wait(timeout, cloud, server, action, module, sdk)
-            module.exit_json(changed=True)
-
-        if action == 'start':
-            if not _system_state_change(action, status):
-                module.exit_json(changed=False)
-
-            cloud.compute.post(
-                _action_url(server.id),
-                json={'os-start': None})
-            if wait:
-                _wait(timeout, cloud, server, action, module, sdk)
-            module.exit_json(changed=True)
-
-        if action == 'pause':
-            if not _system_state_change(action, status):
-                module.exit_json(changed=False)
-
-            cloud.compute.post(
-                _action_url(server.id),
-                json={'pause': None})
-            if wait:
-                _wait(timeout, cloud, server, action, module, sdk)
-            module.exit_json(changed=True)
-
-        elif action == 'unpause':
-            if not _system_state_change(action, status):
-                module.exit_json(changed=False)
-
-            cloud.compute.post(
-                _action_url(server.id),
-                json={'unpause': None})
-            if wait:
-                _wait(timeout, cloud, server, action, module, sdk)
-            module.exit_json(changed=True)
-
-        elif action == 'lock':
-            # lock doesn't set a state, just do it
-            cloud.compute.post(
-                _action_url(server.id),
-                json={'lock': None})
-            module.exit_json(changed=True)
-
-        elif action == 'unlock':
-            # unlock doesn't set a state, just do it
-            cloud.compute.post(
-                _action_url(server.id),
-                json={'unlock': None})
-            module.exit_json(changed=True)
-
-        elif action == 'suspend':
-            if not _system_state_change(action, status):
-                module.exit_json(changed=False)
-
-            cloud.compute.post(
-                _action_url(server.id),
-                json={'suspend': None})
-            if wait:
-                _wait(timeout, cloud, server, action, module, sdk)
-            module.exit_json(changed=True)
-
-        elif action == 'resume':
-            if not _system_state_change(action, status):
-                module.exit_json(changed=False)
-
-            cloud.compute.post(
-                _action_url(server.id),
-                json={'resume': None})
-            if wait:
-                _wait(timeout, cloud, server, action, module, sdk)
-            module.exit_json(changed=True)
-
-        elif action == 'rebuild':
-            image = cloud.get_image(image)
-
-            if image is None:
-                module.fail_json(msg="Image does not exist")
-
-            # rebuild doesn't set a state, just do it
-            cloud.compute.post(
-                _action_url(server.id),
-                json={'rebuild': {'imageRef': image.id}})
-            if wait:
-                _wait(timeout, cloud, server, action, module, sdk)
-            module.exit_json(changed=True)
-
-    except sdk.exceptions.OpenStackCloudException as e:
-        module.fail_json(msg=str(e), extra_data=e.extra_data)
 
 
 if __name__ == '__main__':