From 0dacd50c3ad7d1abe7dc11ebef99a723f24cd5c1 Mon Sep 17 00:00:00 2001 From: Bryan Strassner Date: Wed, 7 Mar 2018 16:46:56 -0600 Subject: [PATCH] [390136] Drydock client timeout options Adds options to drydock client to allow for override of both connect and read timeouts. Adds logging to the drydock client for get and post requests being made Bring yapf changes current. Change-Id: I5ff007315059e2087612e2209966815291433893 --- .gitignore | 1 + .../drivers/node/maasdriver/actions/node.py | 46 +++++---- .../drivers/node/maasdriver/driver.py | 8 +- .../node/maasdriver/models/interface.py | 4 +- .../drivers/node/maasdriver/models/machine.py | 8 +- .../drivers/node/maasdriver/models/tag.py | 4 +- .../node/maasdriver/models/volumegroup.py | 4 +- drydock_provisioner/drydock_client/session.py | 90 ++++++++++++++--- drydock_provisioner/error.py | 4 + drydock_provisioner/objects/node.py | 36 ++++--- .../orchestrator/orchestrator.py | 18 +++- drydock_provisioner/statemgmt/state.py | 13 ++- tests/conftest.py | 2 + .../postgres/test_action_prepare_nodes.py | 10 +- .../postgres/test_bootaction_signalling.py | 19 ++-- tests/integration/test_maasdriver_client.py | 4 +- tests/unit/test_api_validation.py | 3 +- tests/unit/test_apienforcer.py | 8 +- tests/unit/test_bootaction_scoping.py | 12 +-- tests/unit/test_cli_task.py | 7 +- tests/unit/test_drydock_client.py | 7 +- tests/unit/test_drydock_client_session.py | 99 +++++++++++++++++++ tests/unit/test_node_logicalnames.py | 13 ++- tests/unit/test_policy_engine.py | 7 +- ...st_validation_rule_storage_partitioning.py | 3 +- 25 files changed, 314 insertions(+), 116 deletions(-) create mode 100644 tests/unit/test_drydock_client_session.py diff --git a/.gitignore b/.gitignore index 5a97a3e3..fa4fb3f6 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,7 @@ nosetests.xml coverage.xml *.cover .hypothesis/ +.pytest_cache/ # Translations *.mo diff --git a/drydock_provisioner/drivers/node/maasdriver/actions/node.py b/drydock_provisioner/drivers/node/maasdriver/actions/node.py index d210c70d..4d7b5feb 100644 --- a/drydock_provisioner/drivers/node/maasdriver/actions/node.py +++ b/drydock_provisioner/drivers/node/maasdriver/actions/node.py @@ -306,7 +306,8 @@ class CreateNetworkTemplate(BaseMaasAction): vlan.mtu = l.mtu vlan.update() else: - self.logger.warning("Unable to find native VLAN on fabric %s." % link_fabric.resource_id) + self.logger.warning("Unable to find native VLAN on fabric %s." + % link_fabric.resource_id) # Now that we have the fabrics sorted out, check # that VLAN tags and subnet attributes are correct @@ -1003,7 +1004,8 @@ class ApplyNodeNetworking(BaseMaasAction): hw_iface_list = i.get_hw_slaves() hw_iface_logicalname_list = [] for hw_iface in hw_iface_list: - hw_iface_logicalname_list.append(n.get_logicalname(hw_iface)) + hw_iface_logicalname_list.append( + n.get_logicalname(hw_iface)) iface = machine.interfaces.create_bond( device_name=i.device_name, parent_names=hw_iface_logicalname_list, @@ -1184,8 +1186,8 @@ class ApplyNodeNetworking(BaseMaasAction): elif machine.status_name == 'Deployed': msg = ( "Located node %s in MaaS, status deployed. Skipping " - "and considering success. Destroy node first if redeploy needed." % - (n.name)) + "and considering success. Destroy node first if redeploy needed." + % (n.name)) self.logger.info(msg) self.task.add_status_msg( msg=msg, error=False, ctx=n.name, ctx_type='node') @@ -1283,8 +1285,8 @@ class ApplyNodePlatform(BaseMaasAction): if machine.status_name == 'Deployed': msg = ( "Located node %s in MaaS, status deployed. Skipping " - "and considering success. Destroy node first if redeploy needed." % - (n.name)) + "and considering success. Destroy node first if redeploy needed." + % (n.name)) self.logger.info(msg) self.task.add_status_msg( msg=msg, error=False, ctx=n.name, ctx_type='node') @@ -1453,8 +1455,8 @@ class ApplyNodeStorage(BaseMaasAction): if machine.status_name == 'Deployed': msg = ( "Located node %s in MaaS, status deployed. Skipping " - "and considering success. Destroy node first if redeploy needed." % - (n.name)) + "and considering success. Destroy node first if redeploy needed." + % (n.name)) self.logger.info(msg) self.task.add_status_msg( msg=msg, error=False, ctx=n.name, ctx_type='node') @@ -1481,7 +1483,8 @@ class ApplyNodeStorage(BaseMaasAction): storage_layout = dict() if isinstance(root_block, hostprofile.HostPartition): storage_layout['layout_type'] = 'flat' - storage_layout['root_device'] = n.get_logicalname(root_dev.name) + storage_layout['root_device'] = n.get_logicalname( + root_dev.name) storage_layout['root_size'] = root_block.size elif isinstance(root_block, hostprofile.HostVolume): storage_layout['layout_type'] = 'lvm' @@ -1493,7 +1496,8 @@ class ApplyNodeStorage(BaseMaasAction): msg=msg, error=True, ctx=n.name, ctx_type='node') self.task.failure(focus=n.get_id()) continue - storage_layout['root_device'] = n.get_logicalname(root_dev.physical_devices[0]) + storage_layout['root_device'] = n.get_logicalname( + root_dev.physical_devices[0]) storage_layout['root_lv_size'] = root_block.size storage_layout['root_lv_name'] = root_block.name storage_layout['root_vg_name'] = root_dev.name @@ -1511,16 +1515,20 @@ class ApplyNodeStorage(BaseMaasAction): for d in n.storage_devices: maas_dev = machine.block_devices.singleton({ - 'name': n.get_logicalname(d.name) + 'name': + n.get_logicalname(d.name) }) if maas_dev is None: - self.logger.warning("Dev %s (%s) not found on node %s" % - (d.name, n.get_logicalname(d.name), n.name)) + self.logger.warning( + "Dev %s (%s) not found on node %s" % + (d.name, n.get_logicalname(d.name), n.name)) continue if d.volume_group is not None: - self.logger.debug("Adding dev %s (%s) to volume group %s" % - (d.name, n.get_logicalname(d.name), d.volume_group)) + self.logger.debug( + "Adding dev %s (%s) to volume group %s" % + (d.name, n.get_logicalname(d.name), + d.volume_group)) if d.volume_group not in vg_devs: vg_devs[d.volume_group] = {'b': [], 'p': []} vg_devs[d.volume_group]['b'].append( @@ -1528,7 +1536,8 @@ class ApplyNodeStorage(BaseMaasAction): continue self.logger.debug("Partitioning dev %s (%s) on node %s" % - (d.name, n.get_logicalname(d.name), n.name)) + (d.name, n.get_logicalname(d.name), + n.name)) for p in d.partitions: if p.is_sys(): self.logger.debug( @@ -1542,9 +1551,8 @@ class ApplyNodeStorage(BaseMaasAction): self.maas_client, size=size, bootable=p.bootable) if p.part_uuid is not None: part.uuid = p.part_uuid - msg = "Creating partition %s on dev %s (%s)" % (p.name, - d.name, - n.get_logicalname(d.name)) + msg = "Creating partition %s on dev %s (%s)" % ( + p.name, d.name, n.get_logicalname(d.name)) self.logger.debug(msg) part = maas_dev.create_partition(part) self.task.add_status_msg( diff --git a/drydock_provisioner/drivers/node/maasdriver/driver.py b/drydock_provisioner/drivers/node/maasdriver/driver.py index 534ec8d3..91fcd76f 100644 --- a/drydock_provisioner/drivers/node/maasdriver/driver.py +++ b/drydock_provisioner/drivers/node/maasdriver/driver.py @@ -205,13 +205,9 @@ class MaasNodeDriver(NodeDriver): action.start() except Exception as e: msg = "Subtask for action %s raised unexpected exceptions" % task.action - self.logger.error( - msg, exc_info=e.exception()) + self.logger.error(msg, exc_info=e.exception()) task.add_status_msg( - msg, - error=True, - ctx=str(task.get_id()), - ctx_type='task') + msg, error=True, ctx=str(task.get_id()), ctx_type='task') task.failure() task.set_status(hd_fields.TaskStatus.Complete) diff --git a/drydock_provisioner/drivers/node/maasdriver/models/interface.py b/drydock_provisioner/drivers/node/maasdriver/models/interface.py index 33bd546b..6605ef5d 100644 --- a/drydock_provisioner/drivers/node/maasdriver/models/interface.py +++ b/drydock_provisioner/drivers/node/maasdriver/models/interface.py @@ -132,9 +132,7 @@ class Interface(model_base.ResourceBase): resp = self.api_client.post( url, op='unlink_subnet', - files={ - 'id': l.get('resource_id') - }) + files={'id': l.get('resource_id')}) if not resp.ok: raise errors.DriverError("Error unlinking subnet") diff --git a/drydock_provisioner/drivers/node/maasdriver/models/machine.py b/drydock_provisioner/drivers/node/maasdriver/models/machine.py index f71e3087..c833603d 100644 --- a/drydock_provisioner/drivers/node/maasdriver/models/machine.py +++ b/drydock_provisioner/drivers/node/maasdriver/models/machine.py @@ -285,9 +285,7 @@ class Machine(model_base.ResourceBase): url = self.interpolate_url() resp = self.api_client.post( - url, op='set_owner_data', files={ - key: value - }) + url, op='set_owner_data', files={key: value}) if resp.status_code != 200: self.logger.error( @@ -382,9 +380,7 @@ class Machines(model_base.ResourceCollectionBase): url = self.interpolate_url() resp = self.api_client.post( - url, op='allocate', files={ - 'system_id': node.resource_id - }) + url, op='allocate', files={'system_id': node.resource_id}) if not resp.ok: self.logger.error( diff --git a/drydock_provisioner/drivers/node/maasdriver/models/tag.py b/drydock_provisioner/drivers/node/maasdriver/models/tag.py index a240320d..c59e51c3 100644 --- a/drydock_provisioner/drivers/node/maasdriver/models/tag.py +++ b/drydock_provisioner/drivers/node/maasdriver/models/tag.py @@ -70,9 +70,7 @@ class Tag(model_base.ResourceBase): url = self.interpolate_url() resp = self.api_client.post( - url, op='update_nodes', files={ - 'add': system_id - }) + url, op='update_nodes', files={'add': system_id}) if not resp.ok: self.logger.error( diff --git a/drydock_provisioner/drivers/node/maasdriver/models/volumegroup.py b/drydock_provisioner/drivers/node/maasdriver/models/volumegroup.py index d16efb2d..aaa9ea11 100644 --- a/drydock_provisioner/drivers/node/maasdriver/models/volumegroup.py +++ b/drydock_provisioner/drivers/node/maasdriver/models/volumegroup.py @@ -103,9 +103,7 @@ class VolumeGroup(model_base.ResourceBase): url = self.interpolate_url() resp = self.api_client.post( - url, op='delete_logical_volume', files={ - 'id': target_lv - }) + url, op='delete_logical_volume', files={'id': target_lv}) if not resp.ok: raise Exception("MAAS error - %s - %s" % (resp.status_code, diff --git a/drydock_provisioner/drydock_client/session.py b/drydock_provisioner/drydock_client/session.py index 5ff89a32..9e1d89cc 100644 --- a/drydock_provisioner/drydock_client/session.py +++ b/drydock_provisioner/drydock_client/session.py @@ -27,10 +27,19 @@ class DrydockSession(object): :param function auth_gen: Callable that will generate a list of authentication header names and values (2 part tuple) :param string marker: (optional) external context marker + :param tuple timeout: (optional) a tuple of connect, read timeout values + to use as the default for invocations using this session. A single + value may also be supplied instead of a tuple to indicate only the + read timeout to use """ - def __init__(self, host, port=None, scheme='http', auth_gen=None, - marker=None): + def __init__(self, + host, + port=None, + scheme='http', + auth_gen=None, + marker=None, + timeout=None): self.logger = logging.getLogger(__name__) self.__session = requests.Session() self.auth_gen = auth_gen @@ -38,9 +47,7 @@ class DrydockSession(object): self.set_auth() self.marker = marker - self.__session.headers.update({ - 'X-Context-Marker': marker - }) + self.__session.headers.update({'X-Context-Marker': marker}) self.host = host self.scheme = scheme @@ -53,6 +60,8 @@ class DrydockSession(object): # assume default port for scheme self.base_url = "%s://%s/api/" % (self.scheme, self.host) + self.default_timeout = self._calc_timeout_tuple((20, 30), timeout) + def set_auth(self): """Set the session's auth header.""" if self.auth_gen: @@ -62,18 +71,23 @@ class DrydockSession(object): else: self.logger.debug("Cannot set auth header, no generator defined.") - def get(self, endpoint, query=None): + def get(self, endpoint, query=None, timeout=None): """ Send a GET request to Drydock. :param string endpoint: The URL string following the hostname and API prefix :param dict query: A dict of k, v pairs to add to the query string + :param timeout: A single or tuple value for connect, read timeout. + A single value indicates the read timeout only :return: A requests.Response object """ auth_refresh = False while True: + url = self.base_url + endpoint + self.logger.debug('GET ' + url) + self.logger.debug('Query Params: ' + str(query)) resp = self.__session.get( - self.base_url + endpoint, params=query, timeout=10) + url, params=query, timeout=self._timeout(timeout)) if resp.status_code == 401 and not auth_refresh: self.set_auth() @@ -83,7 +97,7 @@ class DrydockSession(object): return resp - def post(self, endpoint, query=None, body=None, data=None): + def post(self, endpoint, query=None, body=None, data=None, timeout=None): """ Send a POST request to Drydock. If both body and data are specified, body will will be used. @@ -92,20 +106,31 @@ class DrydockSession(object): :param dict query: A dict of k, v parameters to add to the query string :param string body: A string to use as the request body. Will be treated as raw :param data: Something json.dumps(s) can serialize. Result will be used as the request body + :param timeout: A single or tuple value for connect, read timeout. + A single value indicates the read timeout only :return: A requests.Response object """ auth_refresh = False + url = self.base_url + endpoint while True: - self.logger.debug("Sending POST with drydock_client session") + self.logger.debug('POST ' + url) + self.logger.debug('Query Params: ' + str(query)) if body is not None: - self.logger.debug("Sending POST with explicit body: \n%s" % body) + self.logger.debug( + "Sending POST with explicit body: \n%s" % body) resp = self.__session.post( - self.base_url + endpoint, params=query, data=body, timeout=10) + self.base_url + endpoint, + params=query, + data=body, + timeout=self._timeout(timeout)) else: - self.logger.debug("Sending POST with JSON body: \n%s" % str(data)) + self.logger.debug( + "Sending POST with JSON body: \n%s" % str(data)) resp = self.__session.post( - self.base_url + endpoint, params=query, json=data, timeout=10) - + self.base_url + endpoint, + params=query, + json=data, + timeout=self._timeout(timeout)) if resp.status_code == 401 and not auth_refresh: self.set_auth() auth_refresh = True @@ -114,6 +139,43 @@ class DrydockSession(object): return resp + def _timeout(self, timeout=None): + """Calculate the default timeouts for this session + + :param timeout: A single or tuple value for connect, read timeout. + A single value indicates the read timeout only + :return: the tuple of the default timeouts used for this session + """ + return self._calc_timeout_tuple(self.default_timeout, timeout) + + def _calc_timeout_tuple(self, def_timeout, timeout=None): + """Calculate the default timeouts for this session + + :param def_timeout: The default timeout tuple to be used if no specific + timeout value is supplied + :param timeout: A single or tuple value for connect, read timeout. + A single value indicates the read timeout only + :return: the tuple of the timeouts calculated + """ + connect_timeout, read_timeout = def_timeout + + try: + if isinstance(timeout, tuple): + if all(isinstance(v, int) + for v in timeout) and len(timeout) == 2: + connect_timeout, read_timeout = timeout + else: + raise ValueError("Tuple non-integer or wrong length") + elif isinstance(timeout, int): + read_timeout = timeout + elif timeout is not None: + raise ValueError("Non integer timeout value") + except ValueError: + self.logger.warn("Timeout value must be a tuple of integers or a " + "single integer. Proceeding with values of " + "(%s, %s)", connect_timeout, read_timeout) + return (connect_timeout, read_timeout) + class KeystoneClient(object): @staticmethod diff --git a/drydock_provisioner/error.py b/drydock_provisioner/error.py index 042a6d92..8f32cf3f 100644 --- a/drydock_provisioner/error.py +++ b/drydock_provisioner/error.py @@ -346,6 +346,7 @@ class InvalidFormat(ApiError): **Troubleshoot:** *Coming Soon* """ + def __init__(self, msg, code=400): super(InvalidFormat, self).__init__(msg, code=code) @@ -358,6 +359,7 @@ class ClientError(ApiError): **Troubleshoot:** *Coming Soon* """ + def __init__(self, msg, code=500): super().__init__(msg) @@ -370,6 +372,7 @@ class ClientUnauthorizedError(ClientError): **Troubleshoot:** *Try requesting a new token.* """ + def __init__(self, msg): super().__init__(msg, code=401) @@ -382,5 +385,6 @@ class ClientForbiddenError(ClientError): **Troubleshoot:** *Coming Soon* """ + def __init__(self, msg): super().__init__(msg, code=403) diff --git a/drydock_provisioner/objects/node.py b/drydock_provisioner/objects/node.py index f1c0b864..64c25991 100644 --- a/drydock_provisioner/objects/node.py +++ b/drydock_provisioner/objects/node.py @@ -129,16 +129,21 @@ class BaremetalNode(drydock_provisioner.objects.hostprofile.HostProfile): :param address: String value that is used to find the logicalname. :return: String value of the logicalname or the alias_name if logicalname is not found. """ - nodes = xml_root.findall(".//node[businfo='" + bus_type + "@" + address + "'].logicalname") + nodes = xml_root.findall( + ".//node[businfo='" + bus_type + "@" + address + "'].logicalname") if len(nodes) >= 1 and nodes[0].text: if (len(nodes) > 1): - self.logger.info("Multiple nodes found for businfo=%s@%s" % (bus_type, address)) + self.logger.info("Multiple nodes found for businfo=%s@%s" % + (bus_type, address)) for logicalname in reversed(nodes[0].text.split("/")): - self.logger.debug("Logicalname build dict: alias_name = %s, bus_type = %s, address = %s, " - "to logicalname = %s" % (alias_name, bus_type, address, logicalname)) + self.logger.debug( + "Logicalname build dict: alias_name = %s, bus_type = %s, address = %s, " + "to logicalname = %s" % (alias_name, bus_type, address, + logicalname)) return logicalname - self.logger.debug("Logicalname build dict: alias_name = %s, bus_type = %s, address = %s, not found" % - (alias_name, bus_type, address)) + self.logger.debug( + "Logicalname build dict: alias_name = %s, bus_type = %s, address = %s, not found" + % (alias_name, bus_type, address)) return alias_name def apply_logicalnames(self, site_design, state_manager): @@ -150,7 +155,8 @@ class BaremetalNode(drydock_provisioner.objects.hostprofile.HostProfile): """ logicalnames = {} - results = state_manager.get_build_data(node_name=self.get_name(), latest=True) + results = state_manager.get_build_data( + node_name=self.get_name(), latest=True) xml_data = None for result in results: if result.generator == "lshw": @@ -161,11 +167,13 @@ class BaremetalNode(drydock_provisioner.objects.hostprofile.HostProfile): xml_root = fromstring(xml_data) for hardware_profile in site_design.hardware_profiles: for device in hardware_profile.devices: - logicalname = self._apply_logicalname(xml_root, device.alias, device.bus_type, - device.address) + logicalname = self._apply_logicalname( + xml_root, device.alias, device.bus_type, + device.address) logicalnames[device.alias] = logicalname else: - self.logger.info("No Build Data found for node_name %s" % (self.get_name())) + self.logger.info("No Build Data found for node_name %s" % + (self.get_name())) self.logicalnames = logicalnames @@ -173,12 +181,16 @@ class BaremetalNode(drydock_provisioner.objects.hostprofile.HostProfile): """Gets the logicalname from self.logicalnames for an alias or returns the alias if not in the dictionary. """ if (self.logicalnames and self.logicalnames.get(alias)): - self.logger.debug("Logicalname input = %s with output %s." % (alias, self.logicalnames[alias])) + self.logger.debug("Logicalname input = %s with output %s." % + (alias, self.logicalnames[alias])) return self.logicalnames[alias] else: - self.logger.debug("Logicalname input = %s not in logicalnames dictionary." % alias) + self.logger.debug( + "Logicalname input = %s not in logicalnames dictionary." % + alias) return alias + @base.DrydockObjectRegistry.register class BaremetalNodeList(base.DrydockObjectListBase, base.DrydockObject): diff --git a/drydock_provisioner/orchestrator/orchestrator.py b/drydock_provisioner/orchestrator/orchestrator.py index 8534293f..08c1ef73 100644 --- a/drydock_provisioner/orchestrator/orchestrator.py +++ b/drydock_provisioner/orchestrator/orchestrator.py @@ -250,7 +250,8 @@ class Orchestrator(object): try: nodes = site_design.baremetal_nodes for n in nodes or []: - n.compile_applied_model(site_design, state_manager=self.state_manager) + n.compile_applied_model( + site_design, state_manager=self.state_manager) except AttributeError: self.logger.debug( "Model inheritance skipped, no node definitions in site design." @@ -556,11 +557,16 @@ class Orchestrator(object): else: init_status = hd_fields.ActionResult.Unreported self.logger.debug( - "Boot action %s has disabled signaling, marking unreported." % ba.name) + "Boot action %s has disabled signaling, marking unreported." + % ba.name) action_id = ulid2.generate_binary_ulid() self.state_manager.post_boot_action( - nodename, task.get_id(), identity_key, action_id, - ba.name, action_status=init_status) + nodename, + task.get_id(), + identity_key, + action_id, + ba.name, + action_status=init_status) return identity_key def render_route_domains(self, site_design): @@ -601,4 +607,6 @@ class Orchestrator(object): for cidr in rd_cidrs: if cidr != n.cidr: n.routes.append( - dict(subnet=cidr, gateway=gw, metric=metric)) + dict( + subnet=cidr, gateway=gw, + metric=metric)) diff --git a/drydock_provisioner/statemgmt/state.py b/drydock_provisioner/statemgmt/state.py index f92c2893..ec9ca0bf 100644 --- a/drydock_provisioner/statemgmt/state.py +++ b/drydock_provisioner/statemgmt/state.py @@ -111,7 +111,8 @@ class DrydockState(object): "parent_task_id = :parent_task_id AND " "status IN ('" + hd_fields.TaskStatus.Terminated + "','" + hd_fields.TaskStatus.Complete + "')") - return self._query_subtasks(task_id, query_text, "Error querying complete subtask: %s") + return self._query_subtasks(task_id, query_text, + "Error querying complete subtask: %s") def get_active_subtasks(self, task_id): """Query database for subtasks of the provided task that are active. @@ -126,7 +127,8 @@ class DrydockState(object): "parent_task_id = :parent_task_id AND " "status NOT IN ['" + hd_fields.TaskStatus.Terminated + "','" + hd_fields.TaskStatus.Complete + "']") - return self._query_subtasks(task_id, query_text, "Error querying active subtask: %s") + return self._query_subtasks(task_id, query_text, + "Error querying active subtask: %s") def get_all_subtasks(self, task_id): """Query database for all subtasks of the provided task. @@ -136,7 +138,8 @@ class DrydockState(object): query_text = sql.text( "SELECT * FROM tasks WHERE " # nosec no strings are user-sourced "parent_task_id = :parent_task_id") - return self._query_subtasks(task_id, query_text, "Error querying all subtask: %s") + return self._query_subtasks(task_id, query_text, + "Error querying all subtask: %s") def _query_subtasks(self, task_id, query_text, error): try: @@ -279,8 +282,8 @@ class DrydockState(object): """ try: conn = self.db_engine.connect() - query = self.tasks_tbl.insert().values( - **(task.to_db(include_id=True))) + query = self.tasks_tbl.insert().values(**( + task.to_db(include_id=True))) conn.execute(query) conn.close() return True diff --git a/tests/conftest.py b/tests/conftest.py index 4a9808e4..e4f9cda0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -130,6 +130,7 @@ def setup_logging(): ch.setFormatter(formatter) logger.addHandler(ch) + @pytest.fixture(scope='module') def mock_get_build_data(drydock_state): def side_effect(**kwargs): @@ -140,6 +141,7 @@ def mock_get_build_data(drydock_state): data_format="text/plain", data_element="") return [build_data] + drydock_state.real_get_build_data = drydock_state.get_build_data drydock_state.get_build_data = Mock(side_effect=side_effect) diff --git a/tests/integration/postgres/test_action_prepare_nodes.py b/tests/integration/postgres/test_action_prepare_nodes.py index 40077d96..c5e28579 100644 --- a/tests/integration/postgres/test_action_prepare_nodes.py +++ b/tests/integration/postgres/test_action_prepare_nodes.py @@ -21,11 +21,13 @@ from drydock_provisioner.orchestrator.actions.orchestrator import PrepareNodes class TestActionPrepareNodes(object): def test_preparenodes(self, mocker, input_files, deckhand_ingester, setup, drydock_state, mock_get_build_data): - mock_images = mocker.patch("drydock_provisioner.drivers.node.driver.NodeDriver" - ".get_available_images") + mock_images = mocker.patch( + "drydock_provisioner.drivers.node.driver.NodeDriver" + ".get_available_images") mock_images.return_value = ['xenial'] - mock_kernels = mocker.patch("drydock_provisioner.drivers.node.driver.NodeDriver" - ".get_available_kernels") + mock_kernels = mocker.patch( + "drydock_provisioner.drivers.node.driver.NodeDriver" + ".get_available_kernels") mock_kernels.return_value = ['ga-16.04', 'hwe-16.04'] input_file = input_files.join("deckhand_fullsite.yaml") diff --git a/tests/integration/postgres/test_bootaction_signalling.py b/tests/integration/postgres/test_bootaction_signalling.py index 5051c57d..cade22b3 100644 --- a/tests/integration/postgres/test_bootaction_signalling.py +++ b/tests/integration/postgres/test_bootaction_signalling.py @@ -18,7 +18,8 @@ from drydock_provisioner.objects import fields as hd_fields class TestBootActionSignal(object): def test_bootaction_signal_disable(self, deckhand_orchestrator, - drydock_state, input_files, mock_get_build_data): + drydock_state, input_files, + mock_get_build_data): """Test that disabled signaling omits a status entry in the DB.""" input_file = input_files.join("deckhand_fullsite.yaml") design_ref = "file://%s" % str(input_file) @@ -37,17 +38,17 @@ class TestBootActionSignal(object): assert len(bootactions) == 2 # one bootaction should expecting signaling - reported_bas = [x - for x - in bootactions.values() - if x.get('action_status') == hd_fields.ActionResult.Incomplete] + reported_bas = [ + x for x in bootactions.values() + if x.get('action_status') == hd_fields.ActionResult.Incomplete + ] assert len(reported_bas) == 1 # one bootaction should not expect signaling - unreported_bas = [x - for x - in bootactions.values() - if not x.get('action_status') == hd_fields.ActionResult.Unreported] + unreported_bas = [ + x for x in bootactions.values() + if not x.get('action_status') == hd_fields.ActionResult.Unreported + ] assert len(unreported_bas) == 1 design_status, design_data = deckhand_orchestrator.get_effective_site( diff --git a/tests/integration/test_maasdriver_client.py b/tests/integration/test_maasdriver_client.py index 672e3bb2..0436826b 100644 --- a/tests/integration/test_maasdriver_client.py +++ b/tests/integration/test_maasdriver_client.py @@ -23,9 +23,7 @@ class TestClass(object): client_config['api_key']) resp = maas_client.get( - 'account/', params={ - 'op': 'list_authorisation_tokens' - }) + 'account/', params={'op': 'list_authorisation_tokens'}) parsed = resp.json() diff --git a/tests/unit/test_api_validation.py b/tests/unit/test_api_validation.py index 974cc699..0c2ee624 100644 --- a/tests/unit/test_api_validation.py +++ b/tests/unit/test_api_validation.py @@ -82,7 +82,8 @@ class TestValidationApi(object): assert result.status == falcon.HTTP_400 - def test_invalid_post_resp(self, input_files, falcontest, drydock_state, mock_get_build_data): + def test_invalid_post_resp(self, input_files, falcontest, drydock_state, + mock_get_build_data): input_file = input_files.join("invalid_validation.yaml") design_ref = "file://%s" % str(input_file) diff --git a/tests/unit/test_apienforcer.py b/tests/unit/test_apienforcer.py index ee36d16e..a7dff692 100644 --- a/tests/unit/test_apienforcer.py +++ b/tests/unit/test_apienforcer.py @@ -48,10 +48,10 @@ class TestEnforcerDecorator(): self.target_function(req, resp) expected_calls = [ - mocker.call.authorize( - 'physical_provisioner:read_task', - {'project_id': project_id, - 'user_id': user_id}, ctx.to_policy_view()) + mocker.call.authorize('physical_provisioner:read_task', { + 'project_id': project_id, + 'user_id': user_id + }, ctx.to_policy_view()) ] policy_engine.enforcer.assert_has_calls(expected_calls) diff --git a/tests/unit/test_bootaction_scoping.py b/tests/unit/test_bootaction_scoping.py index 67e626e1..ca281dfc 100644 --- a/tests/unit/test_bootaction_scoping.py +++ b/tests/unit/test_bootaction_scoping.py @@ -16,9 +16,9 @@ import drydock_provisioner.objects as objects class TestClass(object): - def test_bootaction_scoping_blankfilter(self, input_files, - deckhand_orchestrator, drydock_state, - mock_get_build_data): + def test_bootaction_scoping_blankfilter( + self, input_files, deckhand_orchestrator, drydock_state, + mock_get_build_data): """Test a boot action with no node filter scopes correctly.""" input_file = input_files.join("deckhand_fullsite.yaml") @@ -36,9 +36,9 @@ class TestClass(object): assert 'compute01' in ba.target_nodes assert 'controller01' in ba.target_nodes - def test_bootaction_scoping_unionfilter(self, input_files, - deckhand_orchestrator, drydock_state, - mock_get_build_data): + def test_bootaction_scoping_unionfilter( + self, input_files, deckhand_orchestrator, drydock_state, + mock_get_build_data): """Test a boot action with a union node filter scopes correctly.""" input_file = input_files.join("deckhand_fullsite.yaml") diff --git a/tests/unit/test_cli_task.py b/tests/unit/test_cli_task.py index 3fa3694a..9e0f7564 100644 --- a/tests/unit/test_cli_task.py +++ b/tests/unit/test_cli_task.py @@ -1,4 +1,3 @@ - # Copyright 2017 AT&T Intellectual Property. All other rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -17,6 +16,7 @@ import drydock_provisioner.drydock_client.client as dc_client from drydock_provisioner.cli.task.actions import TaskCreate + def test_taskcli_blank_nodefilter(): """If no filter values are specified, node filter should be None.""" @@ -25,8 +25,7 @@ def test_taskcli_blank_nodefilter(): dd_ses = dc_session.DrydockSession(host) dd_client = dc_client.DrydockClient(dd_ses) - action = TaskCreate(dd_client, - "http://foo.bar", - action_name="deploy_nodes") + action = TaskCreate( + dd_client, "http://foo.bar", action_name="deploy_nodes") assert action.node_filter is None diff --git a/tests/unit/test_drydock_client.py b/tests/unit/test_drydock_client.py index 061f0f93..f4c529df 100644 --- a/tests/unit/test_drydock_client.py +++ b/tests/unit/test_drydock_client.py @@ -65,8 +65,10 @@ def test_session_get(): @responses.activate -@mock.patch.object(dc_session.KeystoneClient, 'get_token', - return_value='5f1e08b6-38ec-4a99-9d0f-00d29c4e325b') +@mock.patch.object( + dc_session.KeystoneClient, + 'get_token', + return_value='5f1e08b6-38ec-4a99-9d0f-00d29c4e325b') def test_session_get_returns_401(*args): responses.add( responses.GET, @@ -89,6 +91,7 @@ def test_session_get_returns_401(*args): assert req.headers.get('X-Context-Marker', None) == marker assert dc_session.KeystoneClient.get_token.call_count == 2 + @responses.activate def test_client_task_get(): task = { diff --git a/tests/unit/test_drydock_client_session.py b/tests/unit/test_drydock_client_session.py new file mode 100644 index 00000000..15170f95 --- /dev/null +++ b/tests/unit/test_drydock_client_session.py @@ -0,0 +1,99 @@ +# Copyright 2018 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import responses + +from drydock_provisioner.drydock_client.session import DrydockSession + + +class TestClientSession(object): + def test_create_session(self): + """Tests setting up an Drydock client session""" + sess = DrydockSession("testdrydock") + assert sess.default_timeout == (20, 30) + + sess = DrydockSession("testdrydock", timeout=60) + assert sess.default_timeout == (20, 60) + + sess = DrydockSession("testdrydock", timeout=(300, 300)) + assert sess.default_timeout == (300, 300) + + sess = DrydockSession("testdrydock", timeout=("cheese", "chicken")) + assert sess.default_timeout == (20, 30) + + sess = DrydockSession("testdrydock", timeout=(30, 60, 90)) + assert sess.default_timeout == (20, 30) + + sess = DrydockSession("testdrydock", timeout="cranium") + assert sess.default_timeout == (20, 30) + + get_responses_inp = { + 'method': 'GET', + 'url': 'http://testdrydock/api/bogus', + 'body': 'got it', + 'status': 200, + 'content_type': 'text/plain' + } + + @responses.activate + def test_get(self): + """Tests the get method""" + responses.add(**TestClientSession.get_responses_inp) + sess = DrydockSession("testdrydock") + result = sess.get('bogus') + assert result.status_code == 200 + return True + + @responses.activate + def test_get_with_timeout(self): + """Tests the get method""" + responses.add(**TestClientSession.get_responses_inp) + sess = DrydockSession("testdrydock") + result = sess.get('bogus', timeout=(60, 60)) + assert result.status_code == 200 + return True + + post_responses_inp = { + 'method': 'POST', + 'url': 'http://testdrydock/api/bogus', + 'body': 'got it', + 'status': 200, + 'content_type': 'text/plain' + } + + @responses.activate + def test_post(self): + """Tests the post method""" + responses.add(**TestClientSession.post_responses_inp) + sess = DrydockSession("testdrydock") + result = sess.post('bogus') + assert result.status_code == 200 + return True + + @responses.activate + def test_post_with_timeout(self): + """Tests the post method""" + responses.add(**TestClientSession.post_responses_inp) + sess = DrydockSession("testdrydock") + result = sess.post('bogus', timeout=(60, 60)) + assert result.status_code == 200 + return True + + def test_timeout(self): + """Tests the _timeout method""" + sess = DrydockSession("testdrydock") + resp = sess._timeout((60, 70)) + assert resp == (60, 70) + + resp = sess._timeout() + assert resp == (20, 30) diff --git a/tests/unit/test_node_logicalnames.py b/tests/unit/test_node_logicalnames.py index 1718d0b4..217d3f1d 100644 --- a/tests/unit/test_node_logicalnames.py +++ b/tests/unit/test_node_logicalnames.py @@ -30,6 +30,7 @@ class TestClass(object): def side_effect(**kwargs): return [] + drydock_state.get_build_data = Mock(side_effect=side_effect) nodes = design_data.baremetal_nodes @@ -37,8 +38,9 @@ class TestClass(object): n.apply_logicalnames(design_data, state_manager=drydock_state) assert n.logicalnames == {} - def test_apply_logicalnames_success(self, input_files, deckhand_orchestrator, - drydock_state, mock_get_build_data): + def test_apply_logicalnames_success(self, input_files, + deckhand_orchestrator, drydock_state, + mock_get_build_data): """Test node apply_logicalnames to get the proper dictionary""" input_file = input_files.join("deckhand_fullsite.yaml") @@ -134,6 +136,7 @@ class TestClass(object): data_format="text/plain", data_element=xml_example) return [build_data] + drydock_state.get_build_data = Mock(side_effect=side_effect) design_status, design_data = deckhand_orchestrator.get_effective_site( @@ -142,7 +145,11 @@ class TestClass(object): nodes = design_data.baremetal_nodes nodes[0].apply_logicalnames(design_data, state_manager=drydock_state) - expected = {'primary_boot': 'sda', 'prim_nic02': 'prim_nic02', 'prim_nic01': 'eno1'} + expected = { + 'primary_boot': 'sda', + 'prim_nic02': 'prim_nic02', + 'prim_nic01': 'eno1' + } # Tests the whole dictionary assert nodes[0].logicalnames == expected # Makes sure the path and / are both removed from primary_boot diff --git a/tests/unit/test_policy_engine.py b/tests/unit/test_policy_engine.py index 746174b5..96cdd336 100644 --- a/tests/unit/test_policy_engine.py +++ b/tests/unit/test_policy_engine.py @@ -58,9 +58,10 @@ class TestDefaultRules(): policy_engine.authorize(policy_action, ctx) expected_calls = [ - mocker.call.authorize( - policy_action, {'project_id': project_id, - 'user_id': user_id}, ctx.to_policy_view()) + mocker.call.authorize(policy_action, { + 'project_id': project_id, + 'user_id': user_id + }, ctx.to_policy_view()) ] policy_engine.enforcer.assert_has_calls(expected_calls) diff --git a/tests/unit/test_validation_rule_storage_partitioning.py b/tests/unit/test_validation_rule_storage_partitioning.py index 9bfa92fe..a90c91b1 100644 --- a/tests/unit/test_validation_rule_storage_partitioning.py +++ b/tests/unit/test_validation_rule_storage_partitioning.py @@ -56,7 +56,8 @@ class TestRationalNetworkTrunking(object): assert msg.get('error') is False def test_invalid_storage_partitioning(self, deckhand_ingester, - drydock_state, input_files, mock_get_build_data): + drydock_state, input_files, + mock_get_build_data): input_file = input_files.join("invalid_validation.yaml") design_ref = "file://%s" % str(input_file)