From 4c94c6f9ba145257a31e5beec66f91936e43c31b Mon Sep 17 00:00:00 2001 From: Gaudenz Steinlin Date: Wed, 22 Nov 2017 10:30:31 +0100 Subject: [PATCH] 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 --- .../cloud/cloudscale/cloudscale_server.py | 51 +++++++++++-------- .../roles/cloudscale_server/defaults/main.yml | 4 +- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/lib/ansible/modules/cloud/cloudscale/cloudscale_server.py b/lib/ansible/modules/cloud/cloudscale/cloudscale_server.py index fdefba7fda..0b455afe1b 100644 --- a/lib/ansible/modules/cloud/cloudscale/cloudscale_server.py +++ b/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'),), diff --git a/test/legacy/roles/cloudscale_server/defaults/main.yml b/test/legacy/roles/cloudscale_server/defaults/main.yml index af078a79be..f318a20f64 100644 --- a/test/legacy/roles/cloudscale_server/defaults/main.yml +++ b/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 \ No newline at end of file + ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDSPmiqkvDH1/+MDAVDZT8381aYqp73Odz8cnD5hegNhqtXajqtiH0umVg7HybX3wt1HjcrwKJovZURcIbbcDvzdH2bnYbF93T4OLXA0bIfuIp6M86x1iutFtXdpN3TTicINrmSXEE2Ydm51iMu77B08ZERjVaToya2F7vC+egfoPvibf7OLxE336a5tPCywavvNihQjL8sjgpDT5AAScjb3YqK/6VLeQ18Ggt8/ufINsYkb+9/Ji/3OcGFeflnDXq80vPUyF3u4iIylob6RSZenC38cXmQB05tRNxS1B6BXCjMRdy0v4pa7oKM2GA4ADKpNrr0RI9ed+peRFwmsclH test@ansible