From a9216bb07f4ccd4dc202fd1f5c14b36a08ac6046 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 24 Jun 2016 17:24:58 +0200 Subject: [PATCH] Implement new heartbeat for AgentDeploy Refactor heartbeat and related calls to a new AgentDeployMixin used in AgentDeploy, ISCSIDeploy and BaseAgentVendor. Also removes excessive passing of kwargs to where they are not used. It has a potential of breaking out-of-tree drivers, but it also greatly simplifies tracking how values are passed during deployment. Also made several methods just functions. Change-Id: I56c728c13b06f1aea0baeec0dddc7ba160a7a211 Partial-Bug: #1570841 --- ironic/drivers/modules/agent.py | 308 ++++----- ironic/drivers/modules/agent_base_vendor.py | 596 ++++++++--------- ironic/drivers/modules/amt/vendor.py | 4 +- ironic/drivers/modules/ilo/vendor.py | 4 +- ironic/drivers/modules/iscsi_deploy.py | 55 +- ironic/drivers/modules/oneview/vendor.py | 3 +- .../unit/drivers/modules/amt/test_vendor.py | 10 +- .../unit/drivers/modules/ilo/test_vendor.py | 15 +- .../tests/unit/drivers/modules/test_agent.py | 6 + .../drivers/modules/test_agent_base_vendor.py | 598 +++++++++--------- .../unit/drivers/modules/test_iscsi_deploy.py | 6 + .../notes/agent-api-bf9f18d8d38075e4.yaml | 5 + 12 files changed, 825 insertions(+), 785 deletions(-) create mode 100644 releasenotes/notes/agent-api-bf9f18d8d38075e4.yaml diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 2cf56809f7..ae98653e61 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -224,7 +224,159 @@ def validate_image_proxies(node): raise exception.InvalidParameterValue(msg) -class AgentDeploy(base.DeployInterface): +class AgentDeployMixin(agent_base_vendor.AgentDeployMixin): + + def deploy_has_started(self, task): + commands = self._client.get_commands_status(task.node) + + for command in commands: + if command['command_name'] == 'prepare_image': + # deploy did start at some point + return True + return False + + def deploy_is_done(self, task): + commands = self._client.get_commands_status(task.node) + if not commands: + return False + + last_command = commands[-1] + + if last_command['command_name'] != 'prepare_image': + # catches race condition where prepare_image is still processing + # so deploy hasn't started yet + return False + + if last_command['command_status'] != 'RUNNING': + return True + + return False + + @task_manager.require_exclusive_lock + def continue_deploy(self, task): + task.process_event('resume') + node = task.node + image_source = node.instance_info.get('image_source') + LOG.debug('Continuing deploy for node %(node)s with image %(img)s', + {'node': node.uuid, 'img': image_source}) + + image_info = { + 'id': image_source.split('/')[-1], + 'urls': [node.instance_info['image_url']], + 'checksum': node.instance_info['image_checksum'], + # NOTE(comstud): Older versions of ironic do not set + # 'disk_format' nor 'container_format', so we use .get() + # to maintain backwards compatibility in case code was + # upgraded in the middle of a build request. + 'disk_format': node.instance_info.get('image_disk_format'), + 'container_format': node.instance_info.get( + 'image_container_format'), + 'stream_raw_images': CONF.agent.stream_raw_images, + } + + proxies = {} + for scheme in ('http', 'https'): + proxy_param = 'image_%s_proxy' % scheme + proxy = node.driver_info.get(proxy_param) + if proxy: + proxies[scheme] = proxy + if proxies: + image_info['proxies'] = proxies + no_proxy = node.driver_info.get('image_no_proxy') + if no_proxy is not None: + image_info['no_proxy'] = no_proxy + + iwdi = node.driver_internal_info.get('is_whole_disk_image') + if not iwdi: + for label in PARTITION_IMAGE_LABELS: + image_info[label] = node.instance_info.get(label) + boot_option = deploy_utils.get_boot_option(node) + boot_mode = deploy_utils.get_boot_mode_for_deploy(node) + if boot_mode: + image_info['deploy_boot_mode'] = boot_mode + else: + image_info['deploy_boot_mode'] = 'bios' + image_info['boot_option'] = boot_option + disk_label = deploy_utils.get_disk_label(node) + if disk_label is not None: + image_info['disk_label'] = disk_label + image_info['node_uuid'] = node.uuid + + # Tell the client to download and write the image with the given args + self._client.prepare_image(node, image_info) + + task.process_event('wait') + + def _get_uuid_from_result(self, task, type_uuid): + command = self._client.get_commands_status(task.node)[-1] + + if command['command_result'] is not None: + words = command['command_result']['result'].split() + for word in words: + if type_uuid in word: + result = word.split('=')[1] + if not result: + msg = (_('Command result did not return %(type_uuid)s ' + 'for node %(node)s. The version of the IPA ' + 'ramdisk used in the deployment might not ' + 'have support for provisioning of ' + 'partition images.') % + {'type_uuid': type_uuid, + 'node': task.node.uuid}) + LOG.error(msg) + deploy_utils.set_failed_state(task, msg) + return + return result + + def check_deploy_success(self, node): + # should only ever be called after we've validated that + # the prepare_image command is complete + command = self._client.get_commands_status(node)[-1] + if command['command_status'] == 'FAILED': + return command['command_error'] + + def reboot_to_instance(self, task): + task.process_event('resume') + node = task.node + iwdi = task.node.driver_internal_info.get('is_whole_disk_image') + error = self.check_deploy_success(node) + if error is not None: + # TODO(jimrollenhagen) power off if using neutron dhcp to + # align with pxe driver? + msg = (_('node %(node)s command status errored: %(error)s') % + {'node': node.uuid, 'error': error}) + LOG.error(msg) + deploy_utils.set_failed_state(task, msg) + return + if not iwdi: + root_uuid = self._get_uuid_from_result(task, 'root_uuid') + if deploy_utils.get_boot_mode_for_deploy(node) == 'uefi': + efi_sys_uuid = ( + self._get_uuid_from_result(task, + 'efi_system_partition_uuid')) + else: + efi_sys_uuid = None + task.node.driver_internal_info['root_uuid_or_disk_id'] = root_uuid + task.node.save() + self.prepare_instance_to_boot(task, root_uuid, efi_sys_uuid) + LOG.info(_LI('Image successfully written to node %s'), node.uuid) + LOG.debug('Rebooting node %s to instance', node.uuid) + if iwdi: + manager_utils.node_set_boot_device(task, 'disk', persistent=True) + + self.reboot_and_finish_deploy(task) + + # NOTE(TheJulia): If we deployed a whole disk image, we + # should expect a whole disk image and clean-up the tftp files + # on-disk incase the node is disregarding the boot preference. + # TODO(rameshg87): Not all in-tree drivers using reboot_to_instance + # have a boot interface. So include a check for now. Remove this + # check once all in-tree drivers have a boot interface. + if task.driver.boot and iwdi: + task.driver.boot.clean_up_ramdisk(task) + + +class AgentDeploy(AgentDeployMixin, base.DeployInterface): """Interface for deploy-related actions.""" def get_properties(self): @@ -422,156 +574,12 @@ class AgentDeploy(base.DeployInterface): task, manage_boot=CONF.agent.manage_agent_boot) -class AgentVendorInterface(agent_base_vendor.BaseAgentVendor): +class AgentVendorInterface(agent_base_vendor.BaseAgentVendor, + AgentDeployMixin): + """Implementation of agent vendor interface. - def deploy_has_started(self, task): - commands = self._client.get_commands_status(task.node) - - for command in commands: - if command['command_name'] == 'prepare_image': - # deploy did start at some point - return True - return False - - def deploy_is_done(self, task): - commands = self._client.get_commands_status(task.node) - if not commands: - return False - - last_command = commands[-1] - - if last_command['command_name'] != 'prepare_image': - # catches race condition where prepare_image is still processing - # so deploy hasn't started yet - return False - - if last_command['command_status'] != 'RUNNING': - return True - - return False - - @task_manager.require_exclusive_lock - def continue_deploy(self, task, **kwargs): - task.process_event('resume') - node = task.node - image_source = node.instance_info.get('image_source') - LOG.debug('Continuing deploy for node %(node)s with image %(img)s', - {'node': node.uuid, 'img': image_source}) - - image_info = { - 'id': image_source.split('/')[-1], - 'urls': [node.instance_info['image_url']], - 'checksum': node.instance_info['image_checksum'], - # NOTE(comstud): Older versions of ironic do not set - # 'disk_format' nor 'container_format', so we use .get() - # to maintain backwards compatibility in case code was - # upgraded in the middle of a build request. - 'disk_format': node.instance_info.get('image_disk_format'), - 'container_format': node.instance_info.get( - 'image_container_format'), - 'stream_raw_images': CONF.agent.stream_raw_images, - } - - proxies = {} - for scheme in ('http', 'https'): - proxy_param = 'image_%s_proxy' % scheme - proxy = node.driver_info.get(proxy_param) - if proxy: - proxies[scheme] = proxy - if proxies: - image_info['proxies'] = proxies - no_proxy = node.driver_info.get('image_no_proxy') - if no_proxy is not None: - image_info['no_proxy'] = no_proxy - - iwdi = node.driver_internal_info.get('is_whole_disk_image') - if not iwdi: - for label in PARTITION_IMAGE_LABELS: - image_info[label] = node.instance_info.get(label) - boot_option = deploy_utils.get_boot_option(node) - boot_mode = deploy_utils.get_boot_mode_for_deploy(node) - if boot_mode: - image_info['deploy_boot_mode'] = boot_mode - else: - image_info['deploy_boot_mode'] = 'bios' - image_info['boot_option'] = boot_option - disk_label = deploy_utils.get_disk_label(node) - if disk_label is not None: - image_info['disk_label'] = disk_label - image_info['node_uuid'] = node.uuid - - # Tell the client to download and write the image with the given args - self._client.prepare_image(node, image_info) - - task.process_event('wait') - - def _get_uuid_from_result(self, task, type_uuid): - command = self._client.get_commands_status(task.node)[-1] - - if command['command_result'] is not None: - words = command['command_result']['result'].split() - for word in words: - if type_uuid in word: - result = word.split('=')[1] - if not result: - msg = (_('Command result did not return %(type_uuid)s ' - 'for node %(node)s. The version of the IPA ' - 'ramdisk used in the deployment might not ' - 'have support for provisioning of ' - 'partition images.') % - {'type_uuid': type_uuid, - 'node': task.node.uuid}) - LOG.error(msg) - deploy_utils.set_failed_state(task, msg) - return - return result - - def check_deploy_success(self, node): - # should only ever be called after we've validated that - # the prepare_image command is complete - command = self._client.get_commands_status(node)[-1] - if command['command_status'] == 'FAILED': - return command['command_error'] - - def reboot_to_instance(self, task, **kwargs): - task.process_event('resume') - node = task.node - iwdi = task.node.driver_internal_info.get('is_whole_disk_image') - error = self.check_deploy_success(node) - if error is not None: - # TODO(jimrollenhagen) power off if using neutron dhcp to - # align with pxe driver? - msg = (_('node %(node)s command status errored: %(error)s') % - {'node': node.uuid, 'error': error}) - LOG.error(msg) - deploy_utils.set_failed_state(task, msg) - return - if not iwdi: - root_uuid = self._get_uuid_from_result(task, 'root_uuid') - if deploy_utils.get_boot_mode_for_deploy(node) == 'uefi': - efi_sys_uuid = ( - self._get_uuid_from_result(task, - 'efi_system_partition_uuid')) - else: - efi_sys_uuid = None - task.node.driver_internal_info['root_uuid_or_disk_id'] = root_uuid - task.node.save() - self.prepare_instance_to_boot(task, root_uuid, efi_sys_uuid) - LOG.info(_LI('Image successfully written to node %s'), node.uuid) - LOG.debug('Rebooting node %s to instance', node.uuid) - if iwdi: - manager_utils.node_set_boot_device(task, 'disk', persistent=True) - - self.reboot_and_finish_deploy(task) - - # NOTE(TheJulia): If we deployed a whole disk image, we - # should expect a whole disk image and clean-up the tftp files - # on-disk incase the node is disregarding the boot preference. - # TODO(rameshg87): Not all in-tree drivers using reboot_to_instance - # have a boot interface. So include a check for now. Remove this - # check once all in-tree drivers have a boot interface. - if task.driver.boot and iwdi: - task.driver.boot.clean_up_ramdisk(task) + Contains old lookup and heartbeat endpoints currently pending deprecation. + """ class AgentRAID(base.RAIDInterface): diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py index a5614d6095..4f4dc79cf7 100644 --- a/ironic/drivers/modules/agent_base_vendor.py +++ b/ironic/drivers/modules/agent_base_vendor.py @@ -142,87 +142,125 @@ def _get_post_clean_step_hook(node): pass -class BaseAgentVendor(base.VendorInterface): +def _cleaning_reboot(task): + """Reboots a node out of band after a clean step that requires it. + + If an agent clean step has 'reboot_requested': True, reboots the + node when the step is completed. Will put the node in CLEANFAIL + if the node cannot be rebooted. + + :param task: a TaskManager instance + """ + try: + manager_utils.node_power_action(task, states.REBOOT) + except Exception as e: + msg = (_('Reboot requested by clean step %(step)s failed for ' + 'node %(node)s: %(err)s') % + {'step': task.node.clean_step, + 'node': task.node.uuid, + 'err': e}) + LOG.error(msg) + # do not set cleaning_reboot if we didn't reboot + manager_utils.cleaning_error_handler(task, msg) + return + + # Signify that we've rebooted + driver_internal_info = task.node.driver_internal_info + driver_internal_info['cleaning_reboot'] = True + task.node.driver_internal_info = driver_internal_info + task.node.save() + + +def _notify_conductor_resume_clean(task): + LOG.debug('Sending RPC to conductor to resume cleaning for node %s', + task.node.uuid) + uuid = task.node.uuid + rpc = rpcapi.ConductorAPI() + topic = rpc.get_topic_for(task.node) + # Need to release the lock to let the conductor take it + task.release_resources() + rpc.continue_node_clean(task.context, uuid, topic=topic) + + +def _get_completed_cleaning_command(task, commands): + """Returns None or a completed cleaning command from the agent. + + :param task: a TaskManager instance to act on. + :param commands: a set of command results from the agent, typically + fetched with agent_client.get_commands_status(). + """ + if not commands: + return + + last_command = commands[-1] + + if last_command['command_name'] != 'execute_clean_step': + # catches race condition where execute_clean_step is still + # processing so the command hasn't started yet + LOG.debug('Expected agent last command to be "execute_clean_step" ' + 'for node %(node)s, instead got "%(command)s". Waiting ' + 'for next heartbeat.', + {'node': task.node.uuid, + 'command': last_command['command_name']}) + return + + last_result = last_command.get('command_result') or {} + last_step = last_result.get('clean_step') + if last_command['command_status'] == 'RUNNING': + LOG.debug('Clean step still running for node %(node)s: %(step)s', + {'step': last_step, 'node': task.node.uuid}) + return + elif (last_command['command_status'] == 'SUCCEEDED' and + last_step != task.node.clean_step): + # A previous clean_step was running, the new command has not yet + # started. + LOG.debug('Clean step not yet started for node %(node)s: %(step)s', + {'step': last_step, 'node': task.node.uuid}) + return + else: + return last_command + + +def log_and_raise_deployment_error(task, msg): + """Helper method to log the error and raise exception.""" + LOG.error(msg) + deploy_utils.set_failed_state(task, msg) + raise exception.InstanceDeployFailure(msg) + + +class AgentDeployMixin(object): + """Mixin with deploy methods.""" def __init__(self): - self.supported_payload_versions = ['2'] self._client = _get_client() - def continue_deploy(self, task, **kwargs): + def continue_deploy(self, task): """Continues the deployment of baremetal node. This method continues the deployment of the baremetal node after the ramdisk have been booted. :param task: a TaskManager instance - """ - pass def deploy_has_started(self, task): """Check if the deployment has started already. :returns: True if the deploy has started, False otherwise. """ - pass def deploy_is_done(self, task): """Check if the deployment is already completed. :returns: True if the deployment is completed. False otherwise - """ - pass - def reboot_to_instance(self, task, **kwargs): + def reboot_to_instance(self, task): """Method invoked after the deployment is completed. :param task: a TaskManager instance """ - pass - - def get_properties(self): - """Return the properties of the interface. - - :returns: dictionary of : entries. - """ - return VENDOR_PROPERTIES - - def validate(self, task, method, **kwargs): - """Validate the driver-specific Node deployment info. - - No validation necessary. - - :param task: a TaskManager instance - :param method: method to be validated - """ - pass - - def driver_validate(self, method, **kwargs): - """Validate the driver deployment info. - - :param method: method to be validated. - """ - version = kwargs.get('version') - - if not version: - raise exception.MissingParameterValue(_('Missing parameter ' - 'version')) - if version not in self.supported_payload_versions: - raise exception.InvalidParameterValue(_('Unknown lookup ' - 'payload version: %s') - % version) - - def notify_conductor_resume_clean(self, task): - LOG.debug('Sending RPC to conductor to resume cleaning for node %s', - task.node.uuid) - uuid = task.node.uuid - rpc = rpcapi.ConductorAPI() - topic = rpc.get_topic_for(task.node) - # Need to release the lock to let the conductor take it - task.release_resources() - rpc.continue_node_clean(task.context, uuid, topic=topic) def _refresh_clean_steps(self, task): """Refresh the node's cached clean steps from the booted agent. @@ -308,13 +346,13 @@ class BaseAgentVendor(base.VendorInterface): info = task.node.driver_internal_info info.pop('cleaning_reboot', None) task.node.driver_internal_info = info - self.notify_conductor_resume_clean(task) + _notify_conductor_resume_clean(task) return else: # Agent has no commands whatsoever return - command = self._get_completed_cleaning_command(task, agent_commands) + command = _get_completed_cleaning_command(task, agent_commands) LOG.debug('Cleaning command status for node %(node)s on step %(step)s:' ' %(command)s', {'node': node.uuid, 'step': node.clean_step, @@ -374,7 +412,7 @@ class BaseAgentVendor(base.VendorInterface): LOG.exception(msg) return manager_utils.cleaning_error_handler(task, msg) - self.notify_conductor_resume_clean(task) + _notify_conductor_resume_clean(task) elif command.get('command_status') == 'SUCCEEDED': clean_step_hook = _get_post_clean_step_hook(node) @@ -398,12 +436,12 @@ class BaseAgentVendor(base.VendorInterface): return manager_utils.cleaning_error_handler(task, msg) if task.node.clean_step.get('reboot_requested'): - self._cleaning_reboot(task) + _cleaning_reboot(task) return LOG.info(_LI('Agent on node %s returned cleaning command success, ' 'moving to next clean step'), node.uuid) - self.notify_conductor_resume_clean(task) + _notify_conductor_resume_clean(task) else: msg = (_('Agent returned unknown status for clean step %(step)s ' 'on node %(node)s : %(err)s.') % @@ -413,48 +451,16 @@ class BaseAgentVendor(base.VendorInterface): LOG.error(msg) return manager_utils.cleaning_error_handler(task, msg) - def _cleaning_reboot(self, task): - """Reboots a node out of band after a clean step that requires it. + def heartbeat(self, task, callback_url): + """Process a heartbeat. - If an agent clean step has 'reboot_requested': True, reboots the - node when the step is completed. Will put the node in CLEANFAIL - if the node cannot be rebooted. - - :param task: a TaskManager instance + :param task: task to work with. + :param callback_url: agent HTTP API URL. """ - try: - manager_utils.node_power_action(task, states.REBOOT) - except Exception as e: - msg = (_('Reboot requested by clean step %(step)s failed for ' - 'node %(node)s: %(err)s') % - {'step': task.node.clean_step, - 'node': task.node.uuid, - 'err': e}) - LOG.error(msg) - # do not set cleaning_reboot if we didn't reboot - manager_utils.cleaning_error_handler(task, msg) - return + # TODO(dtantsur): upgrade lock only if we actually take action other + # than updating the last timestamp. + task.upgrade_lock() - # Signify that we've rebooted - driver_internal_info = task.node.driver_internal_info - driver_internal_info['cleaning_reboot'] = True - task.node.driver_internal_info = driver_internal_info - task.node.save() - - @base.passthru(['POST']) - @task_manager.require_exclusive_lock - def heartbeat(self, task, **kwargs): - """Method for agent to periodically check in. - - The agent should be sending its agent_url (so Ironic can talk back) - as a kwarg. kwargs should have the following format:: - - { - 'agent_url': 'http://AGENT_HOST:AGENT_PORT' - } - - AGENT_PORT defaults to 9999. - """ node = task.node driver_internal_info = node.driver_internal_info LOG.debug( @@ -463,7 +469,7 @@ class BaseAgentVendor(base.VendorInterface): 'heartbeat': driver_internal_info.get('agent_last_heartbeat')}) driver_internal_info['agent_last_heartbeat'] = int(time.time()) try: - driver_internal_info['agent_url'] = kwargs['agent_url'] + driver_internal_info['agent_url'] = callback_url except KeyError: raise exception.MissingParameterValue(_('For heartbeat operation, ' '"agent_url" must be ' @@ -484,11 +490,11 @@ class BaseAgentVendor(base.VendorInterface): elif (node.provision_state == states.DEPLOYWAIT and not self.deploy_has_started(task)): msg = _('Node failed to get image for deploy.') - self.continue_deploy(task, **kwargs) + self.continue_deploy(task) elif (node.provision_state == states.DEPLOYWAIT and self.deploy_is_done(task)): msg = _('Node failed to move to active state.') - self.reboot_to_instance(task, **kwargs) + self.reboot_to_instance(task) elif (node.provision_state == states.DEPLOYWAIT and self.deploy_has_started(task)): node.touch_provisioning() @@ -504,10 +510,10 @@ class BaseAgentVendor(base.VendorInterface): self._refresh_clean_steps(task) # Then set/verify node clean steps and start cleaning manager_utils.set_node_cleaning_steps(task) - self.notify_conductor_resume_clean(task) + _notify_conductor_resume_clean(task) else: msg = _('Node failed to check cleaning progress.') - self.continue_cleaning(task, **kwargs) + self.continue_cleaning(task) except exception.NoFreeConductorWorker: # waiting for the next heartbeat, node.last_error and # logging message is filled already via conductor's hook @@ -523,6 +529,210 @@ class BaseAgentVendor(base.VendorInterface): elif node.provision_state in (states.DEPLOYING, states.DEPLOYWAIT): deploy_utils.set_failed_state(task, last_error) + def reboot_and_finish_deploy(self, task): + """Helper method to trigger reboot on the node and finish deploy. + + This method initiates a reboot on the node. On success, it + marks the deploy as complete. On failure, it logs the error + and marks deploy as failure. + + :param task: a TaskManager object containing the node + :raises: InstanceDeployFailure, if node reboot failed. + """ + wait = CONF.agent.post_deploy_get_power_state_retry_interval * 1000 + attempts = CONF.agent.post_deploy_get_power_state_retries + 1 + + @retrying.retry( + stop_max_attempt_number=attempts, + retry_on_result=lambda state: state != states.POWER_OFF, + wait_fixed=wait + ) + def _wait_until_powered_off(task): + return task.driver.power.get_power_state(task) + + node = task.node + # Whether ironic should power off the node via out-of-band or + # in-band methods + oob_power_off = strutils.bool_from_string( + node.driver_info.get('deploy_forces_oob_reboot', False)) + + try: + if not oob_power_off: + try: + self._client.power_off(node) + _wait_until_powered_off(task) + except Exception as e: + LOG.warning( + _LW('Failed to soft power off node %(node_uuid)s ' + 'in at least %(timeout)d seconds. ' + 'Error: %(error)s'), + {'node_uuid': node.uuid, + 'timeout': (wait * (attempts - 1)) / 1000, + 'error': e}) + manager_utils.node_power_action(task, states.POWER_OFF) + else: + # Flush the file system prior to hard rebooting the node + result = self._client.sync(node) + error = result.get('faultstring') + if error: + if 'Unknown command' in error: + error = _('The version of the IPA ramdisk used in ' + 'the deployment do not support the ' + 'command "sync"') + LOG.warning(_LW( + 'Failed to flush the file system prior to hard ' + 'rebooting the node %(node)s. Error: %(error)s'), + {'node': node.uuid, 'error': error}) + + manager_utils.node_power_action(task, states.POWER_OFF) + + task.driver.network.remove_provisioning_network(task) + task.driver.network.configure_tenant_networks(task) + + manager_utils.node_power_action(task, states.POWER_ON) + except Exception as e: + msg = (_('Error rebooting node %(node)s after deploy. ' + 'Error: %(error)s') % + {'node': node.uuid, 'error': e}) + log_and_raise_deployment_error(task, msg) + + task.process_event('done') + LOG.info(_LI('Deployment to node %s done'), task.node.uuid) + + def prepare_instance_to_boot(self, task, root_uuid, efi_sys_uuid): + """Prepares instance to boot. + + :param task: a TaskManager object containing the node + :param root_uuid: the UUID for root partition + :param efi_sys_uuid: the UUID for the efi partition + :raises: InvalidState if fails to prepare instance + """ + + node = task.node + if deploy_utils.get_boot_option(node) == "local": + # Install the boot loader + self.configure_local_boot( + task, root_uuid=root_uuid, + efi_system_part_uuid=efi_sys_uuid) + try: + task.driver.boot.prepare_instance(task) + except Exception as e: + LOG.error(_LE('Deploy failed for instance %(instance)s. ' + 'Error: %(error)s'), + {'instance': node.instance_uuid, 'error': e}) + msg = _('Failed to continue agent deployment.') + log_and_raise_deployment_error(task, msg) + + def configure_local_boot(self, task, root_uuid=None, + efi_system_part_uuid=None): + """Helper method to configure local boot on the node. + + This method triggers bootloader installation on the node. + On successful installation of bootloader, this method sets the + node to boot from disk. + + :param task: a TaskManager object containing the node + :param root_uuid: The UUID of the root partition. This is used + for identifying the partition which contains the image deployed + or None in case of whole disk images which we expect to already + have a bootloader installed. + :param efi_system_part_uuid: The UUID of the efi system partition. + This is used only in uefi boot mode. + :raises: InstanceDeployFailure if bootloader installation failed or + on encountering error while setting the boot device on the node. + """ + node = task.node + LOG.debug('Configuring local boot for node %s', node.uuid) + if not node.driver_internal_info.get( + 'is_whole_disk_image') and root_uuid: + LOG.debug('Installing the bootloader for node %(node)s on ' + 'partition %(part)s, EFI system partition %(efi)s', + {'node': node.uuid, 'part': root_uuid, + 'efi': efi_system_part_uuid}) + result = self._client.install_bootloader( + node, root_uuid=root_uuid, + efi_system_part_uuid=efi_system_part_uuid) + if result['command_status'] == 'FAILED': + msg = (_("Failed to install a bootloader when " + "deploying node %(node)s. Error: %(error)s") % + {'node': node.uuid, + 'error': result['command_error']}) + log_and_raise_deployment_error(task, msg) + + try: + deploy_utils.try_set_boot_device(task, boot_devices.DISK) + except Exception as e: + msg = (_("Failed to change the boot device to %(boot_dev)s " + "when deploying node %(node)s. Error: %(error)s") % + {'boot_dev': boot_devices.DISK, 'node': node.uuid, + 'error': e}) + log_and_raise_deployment_error(task, msg) + + LOG.info(_LI('Local boot successfully configured for node %s'), + node.uuid) + + +class BaseAgentVendor(AgentDeployMixin, base.VendorInterface): + + def __init__(self): + self.supported_payload_versions = ['2'] + super(BaseAgentVendor, self).__init__() + + def get_properties(self): + """Return the properties of the interface. + + :returns: dictionary of : entries. + """ + return VENDOR_PROPERTIES + + def validate(self, task, method, **kwargs): + """Validate the driver-specific Node deployment info. + + No validation necessary. + + :param task: a TaskManager instance + :param method: method to be validated + """ + pass + + def driver_validate(self, method, **kwargs): + """Validate the driver deployment info. + + :param method: method to be validated. + """ + version = kwargs.get('version') + + if not version: + raise exception.MissingParameterValue(_('Missing parameter ' + 'version')) + if version not in self.supported_payload_versions: + raise exception.InvalidParameterValue(_('Unknown lookup ' + 'payload version: %s') + % version) + + @base.passthru(['POST']) + @task_manager.require_exclusive_lock + def heartbeat(self, task, **kwargs): + """Method for agent to periodically check in. + + The agent should be sending its agent_url (so Ironic can talk back) + as a kwarg. kwargs should have the following format:: + + { + 'agent_url': 'http://AGENT_HOST:AGENT_PORT' + } + + AGENT_PORT defaults to 9999. + """ + try: + callback_url = kwargs['agent_url'] + except KeyError: + raise exception.MissingParameterValue(_('For heartbeat operation, ' + '"agent_url" must be ' + 'specified.')) + + super(BaseAgentVendor, self).heartbeat(task, callback_url) + @base.driver_passthru(['POST'], async=False) def lookup(self, context, **kwargs): """Find a matching node for the agent. @@ -593,43 +803,6 @@ class BaseAgentVendor(base.VendorInterface): 'node': ndict, } - def _get_completed_cleaning_command(self, task, commands): - """Returns None or a completed cleaning command from the agent. - - :param commands: a set of command results from the agent, typically - fetched with agent_client.get_commands_status() - """ - if not commands: - return - - last_command = commands[-1] - - if last_command['command_name'] != 'execute_clean_step': - # catches race condition where execute_clean_step is still - # processing so the command hasn't started yet - LOG.debug('Expected agent last command to be "execute_clean_step" ' - 'for node %(node)s, instead got "%(command)s". Waiting ' - 'for next heartbeat.', - {'node': task.node.uuid, - 'command': last_command['command_name']}) - return - - last_result = last_command.get('command_result') or {} - last_step = last_result.get('clean_step') - if last_command['command_status'] == 'RUNNING': - LOG.debug('Clean step still running for node %(node)s: %(step)s', - {'step': last_step, 'node': task.node.uuid}) - return - elif (last_command['command_status'] == 'SUCCEEDED' and - last_step != task.node.clean_step): - # A previous clean_step was running, the new command has not yet - # started. - LOG.debug('Clean step not yet started for node %(node)s: %(step)s', - {'step': last_step, 'node': task.node.uuid}) - return - else: - return last_command - def _get_interfaces(self, inventory): interfaces = [] try: @@ -721,150 +894,3 @@ class BaseAgentVendor(base.VendorInterface): # Only have one node_id left, return it. return node_ids.pop() - - def _log_and_raise_deployment_error(self, task, msg): - """Helper method to log the error and raise exception.""" - LOG.error(msg) - deploy_utils.set_failed_state(task, msg) - raise exception.InstanceDeployFailure(msg) - - def reboot_and_finish_deploy(self, task): - """Helper method to trigger reboot on the node and finish deploy. - - This method initiates a reboot on the node. On success, it - marks the deploy as complete. On failure, it logs the error - and marks deploy as failure. - - :param task: a TaskManager object containing the node - :raises: InstanceDeployFailure, if node reboot failed. - """ - wait = CONF.agent.post_deploy_get_power_state_retry_interval * 1000 - attempts = CONF.agent.post_deploy_get_power_state_retries + 1 - - @retrying.retry( - stop_max_attempt_number=attempts, - retry_on_result=lambda state: state != states.POWER_OFF, - wait_fixed=wait - ) - def _wait_until_powered_off(task): - return task.driver.power.get_power_state(task) - - node = task.node - # Whether ironic should power off the node via out-of-band or - # in-band methods - oob_power_off = strutils.bool_from_string( - node.driver_info.get('deploy_forces_oob_reboot', False)) - - try: - if not oob_power_off: - try: - self._client.power_off(node) - _wait_until_powered_off(task) - except Exception as e: - LOG.warning( - _LW('Failed to soft power off node %(node_uuid)s ' - 'in at least %(timeout)d seconds. ' - 'Error: %(error)s'), - {'node_uuid': node.uuid, - 'timeout': (wait * (attempts - 1)) / 1000, - 'error': e}) - manager_utils.node_power_action(task, states.POWER_OFF) - else: - # Flush the file system prior to hard rebooting the node - result = self._client.sync(node) - error = result.get('faultstring') - if error: - if 'Unknown command' in error: - error = _('The version of the IPA ramdisk used in ' - 'the deployment do not support the ' - 'command "sync"') - LOG.warning(_LW( - 'Failed to flush the file system prior to hard ' - 'rebooting the node %(node)s. Error: %(error)s'), - {'node': node.uuid, 'error': error}) - manager_utils.node_power_action(task, states.POWER_OFF) - - task.driver.network.remove_provisioning_network(task) - task.driver.network.configure_tenant_networks(task) - - manager_utils.node_power_action(task, states.POWER_ON) - except Exception as e: - msg = (_('Error rebooting node %(node)s after deploy. ' - 'Error: %(error)s') % - {'node': node.uuid, 'error': e}) - self._log_and_raise_deployment_error(task, msg) - - task.process_event('done') - LOG.info(_LI('Deployment to node %s done'), task.node.uuid) - - def prepare_instance_to_boot(self, task, root_uuid, efi_sys_uuid): - """Prepares instance to boot. - - :param task: a TaskManager object containing the node - :param root_uuid: the UUID for root partition - :param efi_sys_uuid: the UUID for the efi partition - :raises: InvalidState if fails to prepare instance - """ - - node = task.node - if deploy_utils.get_boot_option(node) == "local": - # Install the boot loader - self.configure_local_boot( - task, root_uuid=root_uuid, - efi_system_part_uuid=efi_sys_uuid) - try: - task.driver.boot.prepare_instance(task) - except Exception as e: - LOG.error(_LE('Deploy failed for instance %(instance)s. ' - 'Error: %(error)s'), - {'instance': node.instance_uuid, 'error': e}) - msg = _('Failed to continue agent deployment.') - self._log_and_raise_deployment_error(task, msg) - - def configure_local_boot(self, task, root_uuid=None, - efi_system_part_uuid=None): - """Helper method to configure local boot on the node. - - This method triggers bootloader installation on the node. - On successful installation of bootloader, this method sets the - node to boot from disk. - - :param task: a TaskManager object containing the node - :param root_uuid: The UUID of the root partition. This is used - for identifying the partition which contains the image deployed - or None in case of whole disk images which we expect to already - have a bootloader installed. - :param efi_system_part_uuid: The UUID of the efi system partition. - This is used only in uefi boot mode. - :raises: InstanceDeployFailure if bootloader installation failed or - on encountering error while setting the boot device on the node. - """ - node = task.node - LOG.debug('Configuring local boot for node %s', node.uuid) - if not node.driver_internal_info.get( - 'is_whole_disk_image') and root_uuid: - LOG.debug('Installing the bootloader for node %(node)s on ' - 'partition %(part)s, EFI system partition %(efi)s', - {'node': node.uuid, 'part': root_uuid, - 'efi': efi_system_part_uuid}) - result = self._client.install_bootloader( - node, root_uuid=root_uuid, - efi_system_part_uuid=efi_system_part_uuid) - if result['command_status'] == 'FAILED': - msg = (_("Failed to install a bootloader when " - "deploying node %(node)s. Error: %(error)s") % - {'node': node.uuid, - 'error': result['command_error']}) - self._log_and_raise_deployment_error(task, msg) - - try: - deploy_utils.try_set_boot_device(task, boot_devices.DISK) - except Exception as e: - msg = (_("Failed to change the boot device to %(boot_dev)s " - "when deploying node %(node)s. Error: %(error)s") % - {'boot_dev': boot_devices.DISK, 'node': node.uuid, - 'error': e}) - self._log_and_raise_deployment_error(task, msg) - - LOG.info(_LI('Local boot successfully configured for node %s'), - node.uuid) diff --git a/ironic/drivers/modules/amt/vendor.py b/ironic/drivers/modules/amt/vendor.py index 08124268c6..365f5e3636 100644 --- a/ironic/drivers/modules/amt/vendor.py +++ b/ironic/drivers/modules/amt/vendor.py @@ -23,8 +23,8 @@ from ironic.drivers.modules import iscsi_deploy class AMTPXEVendorPassthru(iscsi_deploy.VendorPassthru): @task_manager.require_exclusive_lock - def continue_deploy(self, task, **kwargs): + def continue_deploy(self, task): if deploy_utils.get_boot_option(task.node) == "netboot": task.driver.management.ensure_next_boot_device(task.node, boot_devices.PXE) - super(AMTPXEVendorPassthru, self).continue_deploy(task, **kwargs) + super(AMTPXEVendorPassthru, self).continue_deploy(task) diff --git a/ironic/drivers/modules/ilo/vendor.py b/ironic/drivers/modules/ilo/vendor.py index 525f73264d..55e7290adf 100644 --- a/ironic/drivers/modules/ilo/vendor.py +++ b/ironic/drivers/modules/ilo/vendor.py @@ -34,7 +34,7 @@ LOG = logging.getLogger(__name__) class IloVirtualMediaAgentVendorInterface(agent.AgentVendorInterface): """Interface for vendor passthru related actions.""" - def reboot_to_instance(self, task, **kwargs): + def reboot_to_instance(self, task): node = task.node LOG.debug('Preparing to reboot to instance for node %s', node.uuid) @@ -48,7 +48,7 @@ class IloVirtualMediaAgentVendorInterface(agent.AgentVendorInterface): ilo_common.update_secure_boot_mode(task, True) super(IloVirtualMediaAgentVendorInterface, - self).reboot_to_instance(task, **kwargs) + self).reboot_to_instance(task) class VendorPassthru(iscsi_deploy.VendorPassthru): diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 957368eb32..9f215dc194 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -403,7 +403,34 @@ def validate(task): deploy_utils.parse_instance_info(task.node) -class ISCSIDeploy(base.DeployInterface): +class AgentDeployMixin(agent_base_vendor.AgentDeployMixin): + + @task_manager.require_exclusive_lock + def continue_deploy(self, task): + """Method invoked when deployed using iSCSI. + + This method is invoked during a heartbeat from an agent when + the node is in wait-call-back state. This deploys the image on + the node and then configures the node to boot according to the + desired boot option (netboot or localboot). + + :param task: a TaskManager object containing the node. + :param kwargs: the kwargs passed from the heartbeat method. + :raises: InstanceDeployFailure, if it encounters some error during + the deploy. + """ + task.process_event('resume') + node = task.node + LOG.debug('Continuing the deployment on node %s', node.uuid) + + uuid_dict_returned = do_agent_iscsi_deploy(task, self._client) + root_uuid = uuid_dict_returned.get('root uuid') + efi_sys_uuid = uuid_dict_returned.get('efi system partition uuid') + self.prepare_instance_to_boot(task, root_uuid, efi_sys_uuid) + self.reboot_and_finish_deploy(task) + + +class ISCSIDeploy(AgentDeployMixin, base.DeployInterface): """iSCSI Deploy Interface for deploy-related actions.""" def get_properties(self): @@ -562,29 +589,5 @@ class ISCSIDeploy(base.DeployInterface): task, manage_boot=True) -class VendorPassthru(agent_base_vendor.BaseAgentVendor): +class VendorPassthru(AgentDeployMixin, agent_base_vendor.BaseAgentVendor): """Interface to mix IPMI and PXE vendor-specific interfaces.""" - - @task_manager.require_exclusive_lock - def continue_deploy(self, task, **kwargs): - """Method invoked when deployed using iSCSI. - - This method is invoked during a heartbeat from an agent when - the node is in wait-call-back state. This deploys the image on - the node and then configures the node to boot according to the - desired boot option (netboot or localboot). - - :param task: a TaskManager object containing the node. - :param kwargs: the kwargs passed from the heartbeat method. - :raises: InstanceDeployFailure, if it encounters some error during - the deploy. - """ - task.process_event('resume') - node = task.node - LOG.debug('Continuing the deployment on node %s', node.uuid) - - uuid_dict_returned = do_agent_iscsi_deploy(task, self._client) - root_uuid = uuid_dict_returned.get('root uuid') - efi_sys_uuid = uuid_dict_returned.get('efi system partition uuid') - self.prepare_instance_to_boot(task, root_uuid, efi_sys_uuid) - self.reboot_and_finish_deploy(task) diff --git a/ironic/drivers/modules/oneview/vendor.py b/ironic/drivers/modules/oneview/vendor.py index 98b0be218b..18001a6c65 100644 --- a/ironic/drivers/modules/oneview/vendor.py +++ b/ironic/drivers/modules/oneview/vendor.py @@ -23,6 +23,7 @@ from ironic.common.i18n import _LW from ironic.common import states from ironic.conductor import utils as manager_utils from ironic.drivers.modules import agent +from ironic.drivers.modules import agent_base_vendor from ironic.drivers.modules import deploy_utils LOG = log.getLogger(__name__) @@ -107,7 +108,7 @@ class AgentVendorInterface(agent.AgentVendorInterface): msg = (_('Error rebooting node %(node)s after deploy. ' 'Error: %(error)s') % {'node': node.uuid, 'error': e}) - self._log_and_raise_deployment_error(task, msg) + agent_base_vendor.log_and_raise_deployment_error(task, msg) task.process_event('done') LOG.info(_LI('Deployment to node %s done'), task.node.uuid) diff --git a/ironic/tests/unit/drivers/modules/amt/test_vendor.py b/ironic/tests/unit/drivers/modules/amt/test_vendor.py index d6f74ecbbb..f8dbb11885 100644 --- a/ironic/tests/unit/drivers/modules/amt/test_vendor.py +++ b/ironic/tests/unit/drivers/modules/amt/test_vendor.py @@ -59,7 +59,6 @@ class AMTPXEVendorPassthruTestCase(db_base.DbTestCase): def test_vendorpassthru_continue_deploy_netboot(self, mock_pxe_vendorpassthru, mock_ensure): - kwargs = {'address': '123456'} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.provision_state = states.DEPLOYWAIT @@ -67,11 +66,11 @@ class AMTPXEVendorPassthruTestCase(db_base.DbTestCase): task.node.instance_info['capabilities'] = { "boot_option": "netboot" } - task.driver.vendor.continue_deploy(task, **kwargs) + task.driver.vendor.continue_deploy(task) mock_ensure.assert_called_with( task.driver.management, task.node, boot_devices.PXE) mock_pxe_vendorpassthru.assert_called_once_with( - task.driver.vendor, task, **kwargs) + task.driver.vendor, task) @mock.patch.object(amt_mgmt.AMTManagement, 'ensure_next_boot_device', spec_set=True, autospec=True) @@ -80,13 +79,12 @@ class AMTPXEVendorPassthruTestCase(db_base.DbTestCase): def test_vendorpassthru_continue_deploy_localboot(self, mock_pxe_vendorpassthru, mock_ensure): - kwargs = {'address': '123456'} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.provision_state = states.DEPLOYWAIT task.node.target_provision_state = states.ACTIVE task.node.instance_info['capabilities'] = {"boot_option": "local"} - task.driver.vendor.continue_deploy(task, **kwargs) + task.driver.vendor.continue_deploy(task) self.assertFalse(mock_ensure.called) mock_pxe_vendorpassthru.assert_called_once_with( - task.driver.vendor, task, **kwargs) + task.driver.vendor, task) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_vendor.py b/ironic/tests/unit/drivers/modules/ilo/test_vendor.py index 7b0732a3f5..87e72cfb55 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_vendor.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_vendor.py @@ -117,16 +117,15 @@ class VendorPassthruTestCase(db_base.DbTestCase): func_update_boot_mode, func_update_secure_boot_mode, pxe_vendorpassthru_mock): - kwargs = {'address': '123456'} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.provision_state = states.DEPLOYWAIT task.node.target_provision_state = states.ACTIVE - task.driver.vendor.continue_deploy(task, **kwargs) + task.driver.vendor.continue_deploy(task) func_update_boot_mode.assert_called_once_with(task) func_update_secure_boot_mode.assert_called_once_with(task, True) pxe_vendorpassthru_mock.assert_called_once_with( - mock.ANY, task, **kwargs) + mock.ANY, task) class IloVirtualMediaAgentVendorInterfaceTestCase(db_base.DbTestCase): @@ -149,17 +148,16 @@ class IloVirtualMediaAgentVendorInterfaceTestCase(db_base.DbTestCase): func_update_boot_mode, check_deploy_success_mock, agent_reboot_to_instance_mock): - kwargs = {'address': '123456'} check_deploy_success_mock.return_value = None with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.driver.vendor.reboot_to_instance(task, **kwargs) + task.driver.vendor.reboot_to_instance(task) check_deploy_success_mock.assert_called_once_with( mock.ANY, task.node) func_update_boot_mode.assert_called_once_with(task) func_update_secure_boot_mode.assert_called_once_with(task, True) agent_reboot_to_instance_mock.assert_called_once_with( - mock.ANY, task, **kwargs) + mock.ANY, task) @mock.patch.object(agent.AgentVendorInterface, 'reboot_to_instance', spec_set=True, autospec=True) @@ -173,14 +171,13 @@ class IloVirtualMediaAgentVendorInterfaceTestCase(db_base.DbTestCase): func_update_boot_mode, check_deploy_success_mock, agent_reboot_to_instance_mock): - kwargs = {'address': '123456'} check_deploy_success_mock.return_value = "Error" with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.driver.vendor.reboot_to_instance(task, **kwargs) + task.driver.vendor.reboot_to_instance(task) check_deploy_success_mock.assert_called_once_with( mock.ANY, task.node) self.assertFalse(func_update_boot_mode.called) self.assertFalse(func_update_secure_boot_mode.called) agent_reboot_to_instance_mock.assert_called_once_with( - mock.ANY, task, **kwargs) + mock.ANY, task) diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index e2b647b89d..9595f98308 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -608,6 +608,12 @@ class TestAgentDeploy(db_base.DbTestCase): tear_down_cleaning_mock.assert_called_once_with( task, manage_boot=False) + def test_heartbeat(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + self.driver.heartbeat(task, 'url') + self.assertFalse(task.shared) + class TestAgentVendor(db_base.DbTestCase): diff --git a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py index 08f91d4473..389b9f0bc6 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py @@ -312,243 +312,6 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.assertRaises(exception.MissingParameterValue, self.passthru.heartbeat, task, **kwargs) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, 'deploy_has_started', - autospec=True) - @mock.patch.object(deploy_utils, 'set_failed_state', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, 'deploy_is_done', - autospec=True) - @mock.patch.object(agent_base_vendor.LOG, 'exception', autospec=True) - def test_heartbeat_deploy_done_fails(self, log_mock, done_mock, - failed_mock, deploy_started_mock): - deploy_started_mock.return_value = True - kwargs = { - 'agent_url': 'http://127.0.0.1:9999/bar' - } - done_mock.side_effect = Exception('LlamaException') - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - task.node.provision_state = states.DEPLOYWAIT - task.node.target_provision_state = states.ACTIVE - self.passthru.heartbeat(task, **kwargs) - failed_mock.assert_called_once_with(task, mock.ANY) - log_mock.assert_called_once_with( - 'Asynchronous exception for node ' - '1be26c0b-03f2-4d2e-ae87-c02d7f33c123: Failed checking if deploy ' - 'is done. Exception: LlamaException') - - @mock.patch.object(agent_base_vendor.BaseAgentVendor, 'deploy_has_started', - autospec=True) - @mock.patch.object(deploy_utils, 'set_failed_state', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, 'deploy_is_done', - autospec=True) - @mock.patch.object(agent_base_vendor.LOG, 'exception', autospec=True) - def test_heartbeat_deploy_done_raises_with_event(self, log_mock, done_mock, - failed_mock, - deploy_started_mock): - deploy_started_mock.return_value = True - kwargs = { - 'agent_url': 'http://127.0.0.1:9999/bar' - } - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - - def driver_failure(*args, **kwargs): - # simulate driver failure that both advances the FSM - # and raises an exception - task.node.provision_state = states.DEPLOYFAIL - raise Exception('LlamaException') - - task.node.provision_state = states.DEPLOYWAIT - task.node.target_provision_state = states.ACTIVE - done_mock.side_effect = driver_failure - self.passthru.heartbeat(task, **kwargs) - # task.node.provision_state being set to DEPLOYFAIL - # within the driver_failue, hearbeat should not call - # deploy_utils.set_failed_state anymore - self.assertFalse(failed_mock.called) - log_mock.assert_called_once_with( - 'Asynchronous exception for node ' - '1be26c0b-03f2-4d2e-ae87-c02d7f33c123: Failed checking if deploy ' - 'is done. Exception: LlamaException') - - @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - '_refresh_clean_steps', autospec=True) - @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'notify_conductor_resume_clean', autospec=True) - def test_heartbeat_resume_clean(self, mock_notify, mock_set_steps, - mock_refresh, mock_touch): - kwargs = { - 'agent_url': 'http://127.0.0.1:9999/bar' - } - self.node.clean_step = {} - self.node.provision_state = states.CLEANWAIT - self.node.save() - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - self.passthru.heartbeat(task, **kwargs) - - mock_touch.assert_called_once_with(mock.ANY) - mock_refresh.assert_called_once_with(mock.ANY, task) - mock_notify.assert_called_once_with(mock.ANY, task) - mock_set_steps.assert_called_once_with(task) - - @mock.patch.object(manager_utils, 'cleaning_error_handler') - @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - '_refresh_clean_steps', autospec=True) - @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'notify_conductor_resume_clean', autospec=True) - def test_heartbeat_resume_clean_fails(self, mock_notify, mock_set_steps, - mock_refresh, mock_touch, - mock_handler): - mocks = [mock_refresh, mock_set_steps, mock_notify] - kwargs = { - 'agent_url': 'http://127.0.0.1:9999/bar' - } - self.node.clean_step = {} - self.node.provision_state = states.CLEANWAIT - self.node.save() - for i in range(len(mocks)): - before_failed_mocks = mocks[:i] - failed_mock = mocks[i] - after_failed_mocks = mocks[i + 1:] - failed_mock.side_effect = Exception() - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - self.passthru.heartbeat(task, **kwargs) - - mock_touch.assert_called_once_with(mock.ANY) - mock_handler.assert_called_once_with(task, mock.ANY) - for called in before_failed_mocks + [failed_mock]: - self.assertTrue(called.called) - for not_called in after_failed_mocks: - self.assertFalse(not_called.called) - - # Reset mocks for the next interaction - for m in mocks + [mock_touch, mock_handler]: - m.reset_mock() - failed_mock.side_effect = None - - @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'continue_cleaning', autospec=True) - def test_heartbeat_continue_cleaning(self, mock_continue, mock_touch): - kwargs = { - 'agent_url': 'http://127.0.0.1:9999/bar' - } - self.node.clean_step = { - 'priority': 10, - 'interface': 'deploy', - 'step': 'foo', - 'reboot_requested': False - } - self.node.provision_state = states.CLEANWAIT - self.node.save() - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - self.passthru.heartbeat(task, **kwargs) - - mock_touch.assert_called_once_with(mock.ANY) - mock_continue.assert_called_once_with(mock.ANY, task, **kwargs) - - @mock.patch.object(manager_utils, 'cleaning_error_handler') - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'continue_cleaning', autospec=True) - def test_heartbeat_continue_cleaning_fails(self, mock_continue, - mock_handler): - kwargs = { - 'agent_url': 'http://127.0.0.1:9999/bar' - } - self.node.clean_step = { - 'priority': 10, - 'interface': 'deploy', - 'step': 'foo', - 'reboot_requested': False - } - - mock_continue.side_effect = Exception() - - self.node.provision_state = states.CLEANWAIT - self.node.save() - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - self.passthru.heartbeat(task, **kwargs) - - mock_continue.assert_called_once_with(mock.ANY, task, **kwargs) - mock_handler.assert_called_once_with(task, mock.ANY) - - @mock.patch.object(manager_utils, 'cleaning_error_handler') - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'continue_cleaning', autospec=True) - def test_heartbeat_continue_cleaning_no_worker(self, mock_continue, - mock_handler): - kwargs = { - 'agent_url': 'http://127.0.0.1:9999/bar' - } - self.node.clean_step = { - 'priority': 10, - 'interface': 'deploy', - 'step': 'foo', - 'reboot_requested': False - } - - mock_continue.side_effect = exception.NoFreeConductorWorker() - - self.node.provision_state = states.CLEANWAIT - self.node.save() - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - self.passthru.heartbeat(task, **kwargs) - - mock_continue.assert_called_once_with(mock.ANY, task, **kwargs) - self.assertFalse(mock_handler.called) - - @mock.patch.object(agent_base_vendor.BaseAgentVendor, 'continue_deploy', - autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, 'reboot_to_instance', - autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'notify_conductor_resume_clean', autospec=True) - def test_heartbeat_noops_maintenance_mode(self, ncrc_mock, rti_mock, - cd_mock): - """Ensures that heartbeat() no-ops for a maintenance node.""" - kwargs = { - 'agent_url': 'http://127.0.0.1:9999/bar' - } - self.node.maintenance = True - for state in (states.AVAILABLE, states.DEPLOYWAIT, states.DEPLOYING, - states.CLEANING): - self.node.provision_state = state - self.node.save() - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - self.passthru.heartbeat(task, **kwargs) - - self.assertEqual(0, ncrc_mock.call_count) - self.assertEqual(0, rti_mock.call_count) - self.assertEqual(0, cd_mock.call_count) - - @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, 'deploy_has_started', - autospec=True) - def test_heartbeat_touch_provisioning(self, mock_deploy_started, - mock_touch): - mock_deploy_started.return_value = True - kwargs = { - 'agent_url': 'http://127.0.0.1:9999/bar' - } - - self.node.provision_state = states.DEPLOYWAIT - self.node.save() - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - self.passthru.heartbeat(task, **kwargs) - - mock_touch.assert_called_once_with(mock.ANY) - def test_vendor_passthru_vendor_routes(self): expected = ['heartbeat'] with task_manager.acquire(self.context, self.node.uuid, @@ -565,6 +328,238 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.assertIsInstance(driver_routes, dict) self.assertEqual(expected, list(driver_routes)) + def test_get_properties(self): + expected = agent_base_vendor.VENDOR_PROPERTIES + self.assertEqual(expected, self.passthru.get_properties()) + + +class AgentDeployMixinBaseTest(db_base.DbTestCase): + + def setUp(self): + super(AgentDeployMixinBaseTest, self).setUp() + mgr_utils.mock_the_extension_manager(driver="fake_agent") + self.deploy = agent_base_vendor.AgentDeployMixin() + n = { + 'driver': 'fake_agent', + 'instance_info': INSTANCE_INFO, + 'driver_info': DRIVER_INFO, + 'driver_internal_info': DRIVER_INTERNAL_INFO, + } + self.node = object_utils.create_test_node(self.context, **n) + + +class TestHeartbeat(AgentDeployMixinBaseTest): + + @mock.patch.object(agent_base_vendor.AgentDeployMixin, + 'deploy_has_started', autospec=True) + @mock.patch.object(deploy_utils, 'set_failed_state', autospec=True) + @mock.patch.object(agent_base_vendor.AgentDeployMixin, 'deploy_is_done', + autospec=True) + @mock.patch.object(agent_base_vendor.LOG, 'exception', autospec=True) + def test_heartbeat_deploy_done_fails(self, log_mock, done_mock, + failed_mock, deploy_started_mock): + deploy_started_mock.return_value = True + done_mock.side_effect = Exception('LlamaException') + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + task.node.provision_state = states.DEPLOYWAIT + task.node.target_provision_state = states.ACTIVE + self.deploy.heartbeat(task, 'http://127.0.0.1:8080') + failed_mock.assert_called_once_with(task, mock.ANY) + log_mock.assert_called_once_with( + 'Asynchronous exception for node ' + '1be26c0b-03f2-4d2e-ae87-c02d7f33c123: Failed checking if deploy ' + 'is done. Exception: LlamaException') + + @mock.patch.object(agent_base_vendor.AgentDeployMixin, + 'deploy_has_started', autospec=True) + @mock.patch.object(deploy_utils, 'set_failed_state', autospec=True) + @mock.patch.object(agent_base_vendor.AgentDeployMixin, 'deploy_is_done', + autospec=True) + @mock.patch.object(agent_base_vendor.LOG, 'exception', autospec=True) + def test_heartbeat_deploy_done_raises_with_event(self, log_mock, done_mock, + failed_mock, + deploy_started_mock): + deploy_started_mock.return_value = True + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + + def driver_failure(*args, **kwargs): + # simulate driver failure that both advances the FSM + # and raises an exception + task.node.provision_state = states.DEPLOYFAIL + raise Exception('LlamaException') + + task.node.provision_state = states.DEPLOYWAIT + task.node.target_provision_state = states.ACTIVE + done_mock.side_effect = driver_failure + self.deploy.heartbeat(task, 'http://127.0.0.1:8080') + # task.node.provision_state being set to DEPLOYFAIL + # within the driver_failue, hearbeat should not call + # deploy_utils.set_failed_state anymore + self.assertFalse(failed_mock.called) + log_mock.assert_called_once_with( + 'Asynchronous exception for node ' + '1be26c0b-03f2-4d2e-ae87-c02d7f33c123: Failed checking if deploy ' + 'is done. Exception: LlamaException') + + @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True) + @mock.patch.object(agent_base_vendor.AgentDeployMixin, + '_refresh_clean_steps', autospec=True) + @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True) + @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + autospec=True) + def test_heartbeat_resume_clean(self, mock_notify, mock_set_steps, + mock_refresh, mock_touch): + self.node.clean_step = {} + self.node.provision_state = states.CLEANWAIT + self.node.save() + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.deploy.heartbeat(task, 'http://127.0.0.1:8080') + + mock_touch.assert_called_once_with(mock.ANY) + mock_refresh.assert_called_once_with(mock.ANY, task) + mock_notify.assert_called_once_with(task) + mock_set_steps.assert_called_once_with(task) + + @mock.patch.object(manager_utils, 'cleaning_error_handler') + @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True) + @mock.patch.object(agent_base_vendor.AgentDeployMixin, + '_refresh_clean_steps', autospec=True) + @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True) + @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + autospec=True) + def test_heartbeat_resume_clean_fails(self, mock_notify, mock_set_steps, + mock_refresh, mock_touch, + mock_handler): + mocks = [mock_refresh, mock_set_steps, mock_notify] + self.node.clean_step = {} + self.node.provision_state = states.CLEANWAIT + self.node.save() + for i in range(len(mocks)): + before_failed_mocks = mocks[:i] + failed_mock = mocks[i] + after_failed_mocks = mocks[i + 1:] + failed_mock.side_effect = Exception() + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.deploy.heartbeat(task, 'http://127.0.0.1:8080') + + mock_touch.assert_called_once_with(mock.ANY) + mock_handler.assert_called_once_with(task, mock.ANY) + for called in before_failed_mocks + [failed_mock]: + self.assertTrue(called.called) + for not_called in after_failed_mocks: + self.assertFalse(not_called.called) + + # Reset mocks for the next interaction + for m in mocks + [mock_touch, mock_handler]: + m.reset_mock() + failed_mock.side_effect = None + + @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True) + @mock.patch.object(agent_base_vendor.AgentDeployMixin, + 'continue_cleaning', autospec=True) + def test_heartbeat_continue_cleaning(self, mock_continue, mock_touch): + self.node.clean_step = { + 'priority': 10, + 'interface': 'deploy', + 'step': 'foo', + 'reboot_requested': False + } + self.node.provision_state = states.CLEANWAIT + self.node.save() + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.deploy.heartbeat(task, 'http://127.0.0.1:8080') + + mock_touch.assert_called_once_with(mock.ANY) + mock_continue.assert_called_once_with(mock.ANY, task) + + @mock.patch.object(manager_utils, 'cleaning_error_handler') + @mock.patch.object(agent_base_vendor.AgentDeployMixin, + 'continue_cleaning', autospec=True) + def test_heartbeat_continue_cleaning_fails(self, mock_continue, + mock_handler): + self.node.clean_step = { + 'priority': 10, + 'interface': 'deploy', + 'step': 'foo', + 'reboot_requested': False + } + + mock_continue.side_effect = Exception() + + self.node.provision_state = states.CLEANWAIT + self.node.save() + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.deploy.heartbeat(task, 'http://127.0.0.1:8080') + + mock_continue.assert_called_once_with(mock.ANY, task) + mock_handler.assert_called_once_with(task, mock.ANY) + + @mock.patch.object(manager_utils, 'cleaning_error_handler') + @mock.patch.object(agent_base_vendor.AgentDeployMixin, + 'continue_cleaning', autospec=True) + def test_heartbeat_continue_cleaning_no_worker(self, mock_continue, + mock_handler): + self.node.clean_step = { + 'priority': 10, + 'interface': 'deploy', + 'step': 'foo', + 'reboot_requested': False + } + + mock_continue.side_effect = exception.NoFreeConductorWorker() + + self.node.provision_state = states.CLEANWAIT + self.node.save() + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.deploy.heartbeat(task, 'http://127.0.0.1:8080') + + mock_continue.assert_called_once_with(mock.ANY, task) + self.assertFalse(mock_handler.called) + + @mock.patch.object(agent_base_vendor.AgentDeployMixin, 'continue_deploy', + autospec=True) + @mock.patch.object(agent_base_vendor.AgentDeployMixin, + 'reboot_to_instance', autospec=True) + @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + autospec=True) + def test_heartbeat_noops_maintenance_mode(self, ncrc_mock, rti_mock, + cd_mock): + """Ensures that heartbeat() no-ops for a maintenance node.""" + self.node.maintenance = True + for state in (states.AVAILABLE, states.DEPLOYWAIT, states.DEPLOYING, + states.CLEANING): + self.node.provision_state = state + self.node.save() + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + self.deploy.heartbeat(task, 'http://127.0.0.1:8080') + + self.assertEqual(0, ncrc_mock.call_count) + self.assertEqual(0, rti_mock.call_count) + self.assertEqual(0, cd_mock.call_count) + + @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True) + @mock.patch.object(agent_base_vendor.AgentDeployMixin, + 'deploy_has_started', autospec=True) + def test_heartbeat_touch_provisioning(self, mock_deploy_started, + mock_touch): + mock_deploy_started.return_value = True + + self.node.provision_state = states.DEPLOYWAIT + self.node.save() + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.deploy.heartbeat(task, 'http://127.0.0.1:8080') + + mock_touch.assert_called_once_with(mock.ANY) + @mock.patch.object(time, 'sleep', lambda seconds: None) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(fake.FakePower, 'get_power_state', @@ -581,7 +576,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): shared=True) as task: get_power_state_mock.side_effect = [states.POWER_ON, states.POWER_OFF] - self.passthru.reboot_and_finish_deploy(task) + self.deploy.reboot_and_finish_deploy(task) power_off_mock.assert_called_once_with(task.node) self.assertEqual(2, get_power_state_mock.call_count) node_power_action_mock.assert_called_once_with( @@ -609,7 +604,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: get_power_state_mock.return_value = states.POWER_ON - self.passthru.reboot_and_finish_deploy(task) + self.deploy.reboot_and_finish_deploy(task) power_off_mock.assert_called_once_with(task.node) self.assertEqual(7, get_power_state_mock.call_count) node_power_action_mock.assert_has_calls([ @@ -637,7 +632,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - self.passthru.reboot_and_finish_deploy(task) + self.deploy.reboot_and_finish_deploy(task) power_off_mock.assert_called_once_with(task.node) node_power_action_mock.assert_has_calls([ mock.call(task, states.POWER_OFF), @@ -667,7 +662,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: get_power_state_mock.side_effect = RuntimeError("boom") - self.passthru.reboot_and_finish_deploy(task) + self.deploy.reboot_and_finish_deploy(task) power_off_mock.assert_called_once_with(task.node) self.assertEqual(7, get_power_state_mock.call_count) node_power_action_mock.assert_has_calls([ @@ -696,7 +691,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): get_power_state_mock.return_value = states.POWER_ON node_power_action_mock.side_effect = RuntimeError("boom") self.assertRaises(exception.InstanceDeployFailure, - self.passthru.reboot_and_finish_deploy, + self.deploy.reboot_and_finish_deploy, task) power_off_mock.assert_called_once_with(task.node) self.assertEqual(7, get_power_state_mock.call_count) @@ -720,7 +715,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - self.passthru.reboot_and_finish_deploy(task) + self.deploy.reboot_and_finish_deploy(task) sync_mock.assert_called_once_with(task.node) node_power_action_mock.assert_has_calls([ @@ -747,7 +742,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: sync_mock.return_value = {'faultstring': 'Unknown command: blah'} - self.passthru.reboot_and_finish_deploy(task) + self.deploy.reboot_and_finish_deploy(task) sync_mock.assert_called_once_with(task.node) node_power_action_mock.assert_has_calls([ @@ -773,8 +768,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = False - self.passthru.configure_local_boot(task, - root_uuid='some-root-uuid') + self.deploy.configure_local_boot(task, root_uuid='some-root-uuid') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK) install_bootloader_mock.assert_called_once_with( @@ -791,7 +785,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = False - self.passthru.configure_local_boot( + self.deploy.configure_local_boot( task, root_uuid='some-root-uuid', efi_system_part_uuid='efi-system-part-uuid') try_set_boot_device_mock.assert_called_once_with( @@ -807,7 +801,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): self, install_bootloader_mock, try_set_boot_device_mock): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru.configure_local_boot(task) + self.deploy.configure_local_boot(task) self.assertFalse(install_bootloader_mock.called) try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK) @@ -820,7 +814,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = False - self.passthru.configure_local_boot(task) + self.deploy.configure_local_boot(task) self.assertFalse(install_bootloader_mock.called) try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK) @@ -838,7 +832,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = False self.assertRaises(exception.InstanceDeployFailure, - self.passthru.configure_local_boot, + self.deploy.configure_local_boot, task, root_uuid='some-root-uuid') install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', @@ -861,7 +855,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = False self.assertRaises(exception.InstanceDeployFailure, - self.passthru.configure_local_boot, + self.deploy.configure_local_boot, task, root_uuid='some-root-uuid') install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', @@ -874,7 +868,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): @mock.patch.object(deploy_utils, 'set_failed_state', autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) @mock.patch.object(deploy_utils, 'get_boot_option', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, + @mock.patch.object(agent_base_vendor.AgentDeployMixin, 'configure_local_boot', autospec=True) def test_prepare_instance_to_boot_netboot(self, configure_mock, boot_option_mock, @@ -889,8 +883,8 @@ class TestBaseAgentVendor(db_base.DbTestCase): efi_system_part_uuid = 'efi_sys_uuid' with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru.prepare_instance_to_boot(task, root_uuid, - efi_system_part_uuid) + self.deploy.prepare_instance_to_boot(task, root_uuid, + efi_system_part_uuid) self.assertFalse(configure_mock.called) boot_option_mock.assert_called_once_with(task.node) prepare_instance_mock.assert_called_once_with(task.driver.boot, @@ -900,7 +894,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): @mock.patch.object(deploy_utils, 'set_failed_state', autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) @mock.patch.object(deploy_utils, 'get_boot_option', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, + @mock.patch.object(agent_base_vendor.AgentDeployMixin, 'configure_local_boot', autospec=True) def test_prepare_instance_to_boot_localboot(self, configure_mock, boot_option_mock, @@ -915,9 +909,9 @@ class TestBaseAgentVendor(db_base.DbTestCase): efi_system_part_uuid = 'efi_sys_uuid' with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru.prepare_instance_to_boot(task, root_uuid, - efi_system_part_uuid) - configure_mock.assert_called_once_with(self.passthru, task, + self.deploy.prepare_instance_to_boot(task, root_uuid, + efi_system_part_uuid) + configure_mock.assert_called_once_with(self.deploy, task, root_uuid, efi_system_part_uuid) boot_option_mock.assert_called_once_with(task.node) @@ -928,7 +922,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): @mock.patch.object(deploy_utils, 'set_failed_state', autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) @mock.patch.object(deploy_utils, 'get_boot_option', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, + @mock.patch.object(agent_base_vendor.AgentDeployMixin, 'configure_local_boot', autospec=True) def test_prepare_instance_to_boot_configure_fails(self, configure_mock, boot_option_mock, @@ -949,17 +943,17 @@ class TestBaseAgentVendor(db_base.DbTestCase): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: self.assertRaises(exception.InstanceDeployFailure, - self.passthru.prepare_instance_to_boot, task, + self.deploy.prepare_instance_to_boot, task, root_uuid, efi_system_part_uuid) - configure_mock.assert_called_once_with(self.passthru, task, + configure_mock.assert_called_once_with(self.deploy, task, root_uuid, efi_system_part_uuid) boot_option_mock.assert_called_once_with(task.node) self.assertFalse(prepare_mock.called) self.assertFalse(failed_state_mock.called) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'notify_conductor_resume_clean', autospec=True) + @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) def test_continue_cleaning(self, status_mock, notify_mock): @@ -980,14 +974,14 @@ class TestBaseAgentVendor(db_base.DbTestCase): }] with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru.continue_cleaning(task) - notify_mock.assert_called_once_with(mock.ANY, task) + self.deploy.continue_cleaning(task) + notify_mock.assert_called_once_with(task) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) def test__cleaning_reboot(self, mock_reboot): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru._cleaning_reboot(task) + agent_base_vendor._cleaning_reboot(task) mock_reboot.assert_called_once_with(task, states.REBOOT) self.assertTrue(task.node.driver_internal_info['cleaning_reboot']) @@ -998,7 +992,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru._cleaning_reboot(task) + agent_base_vendor._cleaning_reboot(task) mock_reboot.assert_called_once_with(task, states.REBOOT) mock_handler.assert_called_once_with(task, mock.ANY) self.assertNotIn('cleaning_reboot', @@ -1025,11 +1019,11 @@ class TestBaseAgentVendor(db_base.DbTestCase): }] with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru.continue_cleaning(task) + self.deploy.continue_cleaning(task) reboot_mock.assert_called_once_with(task, states.REBOOT) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'notify_conductor_resume_clean', autospec=True) + @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) def test_continue_cleaning_after_reboot(self, status_mock, notify_mock): @@ -1048,15 +1042,15 @@ class TestBaseAgentVendor(db_base.DbTestCase): status_mock.return_value = [] with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru.continue_cleaning(task) - notify_mock.assert_called_once_with(mock.ANY, task) + self.deploy.continue_cleaning(task) + notify_mock.assert_called_once_with(task) self.assertNotIn('cleaning_reboot', task.node.driver_internal_info) @mock.patch.object(agent_base_vendor, '_get_post_clean_step_hook', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'notify_conductor_resume_clean', autospec=True) + @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) def test_continue_cleaning_with_hook( @@ -1076,14 +1070,14 @@ class TestBaseAgentVendor(db_base.DbTestCase): get_hook_mock.return_value = hook_mock with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - self.passthru.continue_cleaning(task) + self.deploy.continue_cleaning(task) get_hook_mock.assert_called_once_with(task.node) hook_mock.assert_called_once_with(task, command_status) - notify_mock.assert_called_once_with(mock.ANY, task) + notify_mock.assert_called_once_with(task) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'notify_conductor_resume_clean', autospec=True) + @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + autospec=True) @mock.patch.object(agent_base_vendor, '_get_post_clean_step_hook', autospec=True) @mock.patch.object(manager_utils, 'cleaning_error_handler', autospec=True) @@ -1108,15 +1102,15 @@ class TestBaseAgentVendor(db_base.DbTestCase): get_hook_mock.return_value = hook_mock with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - self.passthru.continue_cleaning(task) + self.deploy.continue_cleaning(task) get_hook_mock.assert_called_once_with(task.node) hook_mock.assert_called_once_with(task, command_status) error_handler_mock.assert_called_once_with(task, mock.ANY) self.assertFalse(notify_mock.called) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'notify_conductor_resume_clean', autospec=True) + @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) def test_continue_cleaning_old_command(self, status_mock, notify_mock): @@ -1141,11 +1135,11 @@ class TestBaseAgentVendor(db_base.DbTestCase): }] with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru.continue_cleaning(task) + self.deploy.continue_cleaning(task) self.assertFalse(notify_mock.called) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'notify_conductor_resume_clean', autospec=True) + @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) def test_continue_cleaning_running(self, status_mock, notify_mock): @@ -1157,7 +1151,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): }] with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru.continue_cleaning(task) + self.deploy.continue_cleaning(task) self.assertFalse(notify_mock.called) @mock.patch.object(manager_utils, 'cleaning_error_handler', autospec=True) @@ -1172,13 +1166,13 @@ class TestBaseAgentVendor(db_base.DbTestCase): }] with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru.continue_cleaning(task) + self.deploy.continue_cleaning(task) error_mock.assert_called_once_with(task, mock.ANY) @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'notify_conductor_resume_clean', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, + @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + autospec=True) + @mock.patch.object(agent_base_vendor.AgentDeployMixin, '_refresh_clean_steps', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) @@ -1195,8 +1189,8 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - self.passthru.continue_cleaning(task) - notify_mock.assert_called_once_with(mock.ANY, task) + self.deploy.continue_cleaning(task) + notify_mock.assert_called_once_with(task) refresh_steps_mock.assert_called_once_with(mock.ANY, task) if manual: self.assertFalse( @@ -1215,9 +1209,9 @@ class TestBaseAgentVendor(db_base.DbTestCase): @mock.patch.object(manager_utils, 'cleaning_error_handler', autospec=True) @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, - 'notify_conductor_resume_clean', autospec=True) - @mock.patch.object(agent_base_vendor.BaseAgentVendor, + @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + autospec=True) + @mock.patch.object(agent_base_vendor.AgentDeployMixin, '_refresh_clean_steps', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) @@ -1236,7 +1230,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - self.passthru.continue_cleaning(task) + self.deploy.continue_cleaning(task) status_mock.assert_called_once_with(mock.ANY, task.node) refresh_steps_mock.assert_called_once_with(mock.ANY, task) @@ -1256,7 +1250,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): }] with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: - self.passthru.continue_cleaning(task) + self.deploy.continue_cleaning(task) error_mock.assert_called_once_with(task, mock.ANY) def _test_clean_step_hook(self, hook_dict_mock): @@ -1339,7 +1333,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.assertIsNone(hook_returned) -class TestRefreshCleanSteps(TestBaseAgentVendor): +class TestRefreshCleanSteps(AgentDeployMixinBaseTest): def setUp(self): super(TestRefreshCleanSteps, self).setUp() @@ -1374,7 +1368,7 @@ class TestRefreshCleanSteps(TestBaseAgentVendor): with task_manager.acquire( self.context, self.node.uuid, shared=False) as task: - self.passthru._refresh_clean_steps(task) + self.deploy._refresh_clean_steps(task) client_mock.assert_called_once_with(mock.ANY, task.node, task.ports) @@ -1404,7 +1398,7 @@ class TestRefreshCleanSteps(TestBaseAgentVendor): self.context, self.node.uuid, shared=False) as task: self.assertRaisesRegex(exception.NodeCleaningFailure, 'invalid result', - self.passthru._refresh_clean_steps, + self.deploy._refresh_clean_steps, task) client_mock.assert_called_once_with(mock.ANY, task.node, task.ports) @@ -1421,11 +1415,7 @@ class TestRefreshCleanSteps(TestBaseAgentVendor): self.context, self.node.uuid, shared=False) as task: self.assertRaisesRegex(exception.NodeCleaningFailure, 'invalid clean step', - self.passthru._refresh_clean_steps, + self.deploy._refresh_clean_steps, task) client_mock.assert_called_once_with(mock.ANY, task.node, task.ports) - - def test_get_properties(self): - expected = agent_base_vendor.VENDOR_PROPERTIES - self.assertEqual(expected, self.passthru.get_properties()) diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index a4f6d9991b..558fc4a06b 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -649,6 +649,12 @@ class ISCSIDeployTestCase(db_base.DbTestCase): agent_execute_clean_step_mock.assert_called_once_with( task, {'some-step': 'step-info'}) + def test_heartbeat(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + self.driver.deploy.heartbeat(task, 'url') + self.assertFalse(task.shared) + class TestVendorPassthru(db_base.DbTestCase): diff --git a/releasenotes/notes/agent-api-bf9f18d8d38075e4.yaml b/releasenotes/notes/agent-api-bf9f18d8d38075e4.yaml new file mode 100644 index 0000000000..6cc426f406 --- /dev/null +++ b/releasenotes/notes/agent-api-bf9f18d8d38075e4.yaml @@ -0,0 +1,5 @@ +--- +other: + - The "continue_deploy" and "reboot_to_instance" methods in the + "BaseAgentVendor" class stopped accepting ** arguments. They were never + used anyway; the drivers should stop passing anything there.