cloudscale_server: add timeout param and increase default timeout (#33088)

* Improve error message in cloudscale_server module

Fix punctuation and add the full contents of "info" to the output in
case of failed API calls. This is useful in case of connection timeouts
and other error conditions where there is no response body available.

* Increase timeouts in cloudscale_server module

Increase the timeouts to not fail in case the API calls take a bit
longer than usual. The default timeout of fetch_url is 10s which is
quite short. Increase it to 30s. The timeout for waiting for a server
change is increased as well as it calls the API in a loop. Therefore
this value should be larger than the API timeout.

* Send API parameters as JSON in cloudscale_server module

Use JSON to send the POST data to the API instead of an urlencoded
string. Urlencoding is not really a good match for some Python
datatypes.

This fixes an issue when submitting a list of SSH keys which did not get
translated properly.

* Fix typo in cloudscale_server documentation

* cloudscale_sever: Replace timeout const by api_timeout param

Replace the static TIMEOUT_API constant by a user configurable
api_timeout parameter. Also eliminate the TIMEOUT_WAIT constant by
2*api_timeout. This means that the timeout to wait for server changes is
always double the timeout for API calls.

* Use Debian 9 image for cloudscale_server tests

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

@@ -21,7 +21,7 @@ description:
   - Create, start, stop and delete servers on the cloudscale.ch IaaS service.
   - All operations are performed using the cloudscale.ch public API v1.
   - "For details consult the full API documentation: U(https://www.cloudscale.ch/en/api/v1)."
-  - An valid API token is required for all operations. You can create as many tokens as you like using the cloudscale.ch control panel at
+  - A valid API token is required for all operations. You can create as many tokens as you like using the cloudscale.ch control panel at
     U(https://control.cloudscale.ch).
 notes:
   - Instead of the api_token parameter the CLOUDSCALE_API_TOKEN environment variable can be used.
@@ -86,6 +86,11 @@ options:
     description:
       - cloudscale.ch API token.
       - This can also be passed in the CLOUDSCALE_API_TOKEN environment variable.
+  api_timeout:
+    description:
+      - Timeout in seconds for calls to the cloudscale.ch API.
+    default: 30
+    version_added: "2.5"
 '''
 
 EXAMPLES = '''
@@ -203,12 +208,10 @@ from datetime import datetime, timedelta
 from time import sleep
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.six.moves.urllib.parse import urlencode
 from ansible.module_utils.urls import fetch_url
 
 
 API_URL = 'https://api.cloudscale.ch/v1/'
-TIMEOUT_WAIT = 30
 ALLOWED_STATES = ('running',
                   'stopped',
                   'absent',
@@ -245,26 +248,29 @@ class AnsibleCloudscaleServer(object):
                 self.info = self._transform_state(matching_server[0])
             elif len(matching_server) > 1:
                 self._module.fail_json(msg="More than one server with name '%s' exists. "
-                                       "Use the 'uuid' parameter to identify the server" % name)
+                                       "Use the 'uuid' parameter to identify the server." % name)
 
     def _get(self, api_call):
-        resp, info = fetch_url(self._module, API_URL + api_call, headers=self._auth_header)
+        resp, info = fetch_url(self._module, API_URL + api_call, headers=self._auth_header, timeout=self._module.params['api_timeout'])
 
         if info['status'] == 200:
             return json.loads(resp.read())
         else:
             self._module.fail_json(msg='Failure while calling the cloudscale.ch API with GET for '
-                                       '"%s": %s' % (api_call, info['body']))
+                                       '"%s".' % api_call, fetch_url_info=info)
 
     def _post(self, api_call, data=None):
+        headers = self._auth_header.copy()
         if data is not None:
-            data = urlencode(data)
+            data = self._module.jsonify(data)
+            headers['Content-type'] = 'application/json'
 
         resp, info = fetch_url(self._module,
                                API_URL + api_call,
-                               headers=self._auth_header,
+                               headers=headers,
                                method='POST',
-                               data=data)
+                               data=data,
+                               timeout=self._module.params['api_timeout'])
 
         if info['status'] == 201:
             return json.loads(resp.read())
@@ -272,19 +278,20 @@ class AnsibleCloudscaleServer(object):
             return None
         else:
             self._module.fail_json(msg='Failure while calling the cloudscale.ch API with POST for '
-                                       '"%s": %s' % (api_call, info['body']))
+                                       '"%s".' % api_call, fetch_url_info=info)
 
     def _delete(self, api_call):
         resp, info = fetch_url(self._module,
                                API_URL + api_call,
                                headers=self._auth_header,
-                               method='DELETE')
+                               method='DELETE',
+                               timeout=self._module.params['api_timeout'])
 
         if info['status'] == 204:
             return None
         else:
             self._module.fail_json(msg='Failure while calling the cloudscale.ch API with DELETE for '
-                                       '"%s": %s' % (api_call, info['body']))
+                                       '"%s".' % api_call, fetch_url_info=info)
 
     @staticmethod
     def _transform_state(server):
@@ -302,9 +309,11 @@ class AnsibleCloudscaleServer(object):
             return
 
         # Can't use _get here because we want to handle 404
+        url_path = 'servers/' + self.info['uuid']
         resp, info = fetch_url(self._module,
-                               API_URL + 'servers/' + self.info['uuid'],
-                               headers=self._auth_header)
+                               API_URL + url_path,
+                               headers=self._auth_header,
+                               timeout=self._module.params['api_timeout'])
         if info['status'] == 200:
             self.info = self._transform_state(json.loads(resp.read()))
         elif info['status'] == 404:
@@ -312,18 +321,19 @@ class AnsibleCloudscaleServer(object):
                          'name': self.info.get('name', None),
                          'state': 'absent'}
         else:
-            self._module.fail_json(msg='Failure while calling the cloudscale.ch API for '
-                                       'update_info: %s' % info['body'])
+            self._module.fail_json(msg='Failure while calling the cloudscale.ch API with GET for '
+                                       '"%s".' % url_path, fetch_url_info=info)
 
     def wait_for_state(self, states):
         start = datetime.now()
-        while datetime.now() - start < timedelta(seconds=TIMEOUT_WAIT):
+        timeout = self._module.params['api_timeout'] * 2
+        while datetime.now() - start < timedelta(seconds=timeout):
             self.update_info()
             if self.info['state'] in states:
                 return True
             sleep(1)
 
-        self._module.fail_json(msg='Timeout while waiting for a state change on server %s to states %s. Current state is %s'
+        self._module.fail_json(msg='Timeout while waiting for a state change on server %s to states %s. Current state is %s.'
                                % (self.info['name'], states, self.info['state']))
 
     def create_server(self):
@@ -336,14 +346,14 @@ class AnsibleCloudscaleServer(object):
                 missing_parameters.append(p)
 
         if len(missing_parameters) > 0:
-            self._module.fail_json(msg='Missing required parameter(s) to create a new server: %s' %
+            self._module.fail_json(msg='Missing required parameter(s) to create a new server: %s.' %
                                    ' '.join(missing_parameters))
 
         # Sanitize data dictionary
         for k, v in data.items():
 
             # Remove items not relevant to the create server call
-            if k in ('api_token', 'uuid', 'state'):
+            if k in ('api_token', 'api_timeout', 'uuid', 'state'):
                 del data[k]
                 continue
 
@@ -388,6 +398,7 @@ def main():
             anti_affinity_with=dict(),
             user_data=dict(),
             api_token=dict(no_log=True),
+            api_timeout=dict(default=30, type='int'),
         ),
         required_one_of=(('name', 'uuid'),),
         mutually_exclusive=(('name', 'uuid'),),

test/legacy/roles/cloudscale_server/defaults/main.yml

@@ -1,5 +1,5 @@
 ---
 cloudscale_test_flavor: flex-2
-cloudscale_test_image: debian-8
+cloudscale_test_image: debian-9
 cloudscale_test_ssh_key: |
-  ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDSPmiqkvDH1/+MDAVDZT8381aYqp73Odz8cnD5hegNhqtXajqtiH0umVg7HybX3wt1HjcrwKJovZURcIbbcDvzdH2bnYbF93T4OLXA0bIfuIp6M86x1iutFtXdpN3TTicINrmSXEE2Ydm51iMu77B08ZERjVaToya2F7vC+egfoPvibf7OLxE336a5tPCywavvNihQjL8sjgpDT5AAScjb3YqK/6VLeQ18Ggt8/ufINsYkb+9/Ji/3OcGFeflnDXq80vPUyF3u4iIylob6RSZenC38cXmQB05tRNxS1B6BXCjMRdy0v4pa7oKM2GA4ADKpNrr0RI9ed+peRFwmsclH test@ansible
+  ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDSPmiqkvDH1/+MDAVDZT8381aYqp73Odz8cnD5hegNhqtXajqtiH0umVg7HybX3wt1HjcrwKJovZURcIbbcDvzdH2bnYbF93T4OLXA0bIfuIp6M86x1iutFtXdpN3TTicINrmSXEE2Ydm51iMu77B08ZERjVaToya2F7vC+egfoPvibf7OLxE336a5tPCywavvNihQjL8sjgpDT5AAScjb3YqK/6VLeQ18Ggt8/ufINsYkb+9/Ji/3OcGFeflnDXq80vPUyF3u4iIylob6RSZenC38cXmQB05tRNxS1B6BXCjMRdy0v4pa7oKM2GA4ADKpNrr0RI9ed+peRFwmsclH test@ansible