diff --git a/doc/source/contributor/deploy-steps.rst b/doc/source/contributor/deploy-steps.rst index 21022b2b11..ae79e56f18 100644 --- a/doc/source/contributor/deploy-steps.rst +++ b/doc/source/contributor/deploy-steps.rst @@ -8,8 +8,9 @@ deploy step in the ``AgentDeploy`` class. .. code-block:: python - class AgentDeploy(AgentDeployMixin, base.DeployInterface): - ... + from ironic.drivers.modules import agent + + class AgentDeploy(agent.AgentDeploy): @base.deploy_step(priority=200, argsinfo={ 'test_arg': { @@ -22,6 +23,27 @@ deploy step in the ``AgentDeploy`` class. def do_nothing(self, task, **kwargs): return None +If you want to completely replace the deployment procedure, but still have the +agent up and running, inherit ``CustomAgentDeploy``: + +.. code-block:: python + + from ironic.drivers.modules import agent + + class AgentDeploy(agent.CustomAgentDeploy): + + def validate(self, task): + super().validate(task) + # ... custom validation + + @base.deploy_step(priority=80) + def my_write_image(self, task, **kwargs): + pass # ... custom image writing + + @base.deploy_step(priority=70) + def my_configure_bootloader(self, task, **kwargs): + pass # ... custom bootloader configuration + After deployment of the baremetal node, check the updated deploy steps:: baremetal node show $node_ident -f json -c driver_internal_info diff --git a/ironic/drivers/generic.py b/ironic/drivers/generic.py index e8b359f18e..fddd2242c1 100644 --- a/ironic/drivers/generic.py +++ b/ironic/drivers/generic.py @@ -51,7 +51,7 @@ class GenericHardware(hardware_type.AbstractHardwareType): """List of supported deploy interfaces.""" return [agent.AgentDeploy, iscsi_deploy.ISCSIDeploy, ansible_deploy.AnsibleDeploy, pxe.PXERamdiskDeploy, - pxe.PXEAnacondaDeploy] + pxe.PXEAnacondaDeploy, agent.CustomAgentDeploy] @property def supported_inspect_interfaces(self): diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index bcef5495e1..1dc3881e24 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -200,10 +200,310 @@ def validate_http_provisioning_configuration(node): deploy_utils.check_for_missing_params(params, error_msg) -class AgentDeployMixin(agent_base.AgentDeployMixin): +class CustomAgentDeploy(agent_base.AgentDeployMixin, agent_base.AgentBaseMixin, + base.DeployInterface): + """A deploy interface that relies on a custom agent to deploy. + + Only provides the basic deploy steps to start the ramdisk, tear down + the ramdisk and prepare the instance boot. + """ has_decomposed_deploy_steps = True + def get_properties(self): + """Return the properties of the interface. + + :returns: dictionary of : entries. + """ + return COMMON_PROPERTIES + + def should_manage_boot(self, task): + """Whether agent boot is managed by ironic.""" + return CONF.agent.manage_agent_boot + + @METRICS.timer('CustomAgentDeploy.validate') + def validate(self, task): + """Validate the driver-specific Node deployment info. + + This method validates whether the properties of the supplied node + contain the required information for this driver to deploy images to + the node. + + :param task: a TaskManager instance + :raises: MissingParameterValue, if any of the required parameters are + missing. + :raises: InvalidParameterValue, if any of the parameters have invalid + value. + """ + if CONF.agent.manage_agent_boot: + task.driver.boot.validate(task) + + deploy_utils.validate_capabilities(task.node) + # Validate the root device hints + deploy_utils.get_root_device_for_deploy(task.node) + + @METRICS.timer('CustomAgentDeploy.deploy') + @base.deploy_step(priority=100) + @task_manager.require_exclusive_lock + def deploy(self, task): + """Perform a deployment to a node. + + Perform the necessary work to deploy an image onto the specified node. + This method will be called after prepare(), which may have already + performed any preparatory steps, such as pre-caching some data for the + node. + + :param task: a TaskManager instance. + :returns: status of the deploy. One of ironic.common.states. + """ + if manager_utils.is_fast_track(task): + # NOTE(mgoddard): For fast track we can skip this step and proceed + # immediately to the next deploy step. + LOG.debug('Performing a fast track deployment for %(node)s.', + {'node': task.node.uuid}) + # NOTE(dtantsur): while the node is up and heartbeating, we don't + # necessary have the deploy steps cached. Force a refresh here. + self.refresh_steps(task, 'deploy') + deployments.validate_deploy_steps(task) + elif task.driver.storage.should_write_image(task): + # Check if the driver has already performed a reboot in a previous + # deploy step. + if not task.node.driver_internal_info.get('deployment_reboot'): + manager_utils.node_power_action(task, states.REBOOT) + info = task.node.driver_internal_info + info.pop('deployment_reboot', None) + task.node.driver_internal_info = info + task.node.save() + return states.DEPLOYWAIT + + @METRICS.timer('CustomAgentDeployMixin.prepare_instance_boot') + @base.deploy_step(priority=60) + @task_manager.require_exclusive_lock + def prepare_instance_boot(self, task): + """Prepare instance for booting. + + The base version only calls prepare_instance on the boot interface. + """ + try: + task.driver.boot.prepare_instance(task) + except Exception as e: + LOG.error('Preparing instance for booting failed for node ' + '%(node)s. %(cls)s: %(error)s', + {'node': task.node.uuid, + 'cls': e.__class__.__name__, 'error': e}) + msg = _('Failed to prepare instance for booting') + agent_base.log_and_raise_deployment_error(task, msg, exc=e) + + def _update_instance_info(self, task): + """Update instance information with extra data for deploy. + + Called from `prepare` to populate fields that can be deduced from + the already provided information. + + Does nothing in the base class. + """ + + @METRICS.timer('CustomAgentDeploy.prepare') + @task_manager.require_exclusive_lock + def prepare(self, task): + """Prepare the deployment environment for this node. + + :param task: a TaskManager instance. + :raises: NetworkError: if the previous cleaning ports cannot be removed + or if new cleaning ports cannot be created. + :raises: InvalidParameterValue when the wrong power state is specified + or the wrong driver info is specified for power management. + :raises: StorageError If the storage driver is unable to attach the + configured volumes. + :raises: other exceptions by the node's power driver if something + wrong occurred during the power action. + :raises: exception.ImageRefValidationFailed if image_source is not + Glance href and is not HTTP(S) URL. + :raises: exception.InvalidParameterValue if network validation fails. + :raises: any boot interface's prepare_ramdisk exceptions. + """ + + node = task.node + deploy_utils.populate_storage_driver_internal_info(task) + if node.provision_state == states.DEPLOYING: + # Validate network interface to ensure that it supports boot + # options configured on the node. + try: + task.driver.network.validate(task) + except exception.InvalidParameterValue: + # For 'neutron' network interface validation will fail + # if node is using 'netboot' boot option while provisioning + # a whole disk image. Updating 'boot_option' in node's + # 'instance_info' to 'local for backward compatibility. + # TODO(stendulker): Fail here once the default boot + # option is local. + # NOTE(TheJulia): Fixing the default boot mode only + # masks the failure as the lack of a user definition + # can be perceived as both an invalid configuration and + # reliance upon the default configuration. The reality + # being that in most scenarios, users do not want network + # booting, so the changed default should be valid. + with excutils.save_and_reraise_exception(reraise=False) as ctx: + instance_info = node.instance_info + capabilities = utils.parse_instance_info_capabilities(node) + if 'boot_option' not in capabilities: + capabilities['boot_option'] = 'local' + instance_info['capabilities'] = capabilities + node.instance_info = instance_info + node.save() + # Re-validate the network interface + task.driver.network.validate(task) + else: + ctx.reraise = True + # Determine if this is a fast track sequence + fast_track_deploy = manager_utils.is_fast_track(task) + if fast_track_deploy: + # The agent has already recently checked in and we are + # configured to take that as an indicator that we can + # skip ahead. + LOG.debug('The agent for node %(node)s has recently checked ' + 'in, and the node power will remain unmodified.', + {'node': task.node.uuid}) + else: + # Powering off node to setup networking for port and + # ensure that the state is reset if it is inadvertently + # on for any unknown reason. + manager_utils.node_power_action(task, states.POWER_OFF) + if task.driver.storage.should_write_image(task): + # NOTE(vdrok): in case of rebuild, we have tenant network + # already configured, unbind tenant ports if present + if not fast_track_deploy: + power_state_to_restore = ( + manager_utils.power_on_node_if_needed(task)) + + task.driver.network.unconfigure_tenant_networks(task) + task.driver.network.add_provisioning_network(task) + if not fast_track_deploy: + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) + else: + # Fast track sequence in progress + self._update_instance_info(task) + # Signal to storage driver to attach volumes + task.driver.storage.attach_volumes(task) + if (not task.driver.storage.should_write_image(task) + or fast_track_deploy): + # We have nothing else to do as this is handled in the + # backend storage system, and we can return to the caller + # as we do not need to boot the agent to deploy. + # Alternatively, we could be in a fast track deployment + # and again, we should have nothing to do here. + return + if node.provision_state in (states.ACTIVE, states.UNRESCUING): + # Call is due to conductor takeover + task.driver.boot.prepare_instance(task) + elif node.provision_state != states.ADOPTING: + if node.provision_state not in (states.RESCUING, states.RESCUEWAIT, + states.RESCUE, states.RESCUEFAIL): + self._update_instance_info(task) + if CONF.agent.manage_agent_boot: + deploy_opts = deploy_utils.build_agent_options(node) + task.driver.boot.prepare_ramdisk(task, deploy_opts) + + @METRICS.timer('CustomAgentDeploy.clean_up') + @task_manager.require_exclusive_lock + def clean_up(self, task): + """Clean up the deployment environment for this node. + + If preparation of the deployment environment ahead of time is possible, + this method should be implemented by the driver. It should erase + anything cached by the `prepare` method. + + If implemented, this method must be idempotent. It may be called + multiple times for the same node on the same conductor, and it may be + called by multiple conductors in parallel. Therefore, it must not + require an exclusive lock. + + This method is called before `tear_down`. + + :param task: a TaskManager instance. + """ + super().clean_up(task) + deploy_utils.destroy_http_instance_images(task.node) + + +class AgentDeploy(CustomAgentDeploy): + """Interface for deploy-related actions.""" + + def _update_instance_info(self, task): + """Update instance information with extra data for deploy.""" + task.node.instance_info = ( + deploy_utils.build_instance_info_for_deploy(task)) + task.node.save() + + @METRICS.timer('AgentDeploy.validate') + def validate(self, task): + """Validate the driver-specific Node deployment info. + + This method validates whether the properties of the supplied node + contain the required information for this driver to deploy images to + the node. + + :param task: a TaskManager instance + :raises: MissingParameterValue, if any of the required parameters are + missing. + :raises: InvalidParameterValue, if any of the parameters have invalid + value. + """ + super().validate(task) + + node = task.node + + if not task.driver.storage.should_write_image(task): + # NOTE(TheJulia): There is no reason to validate + # image properties if we will not be writing an image + # in a boot from volume case. As such, return to the caller. + LOG.debug('Skipping complete deployment interface validation ' + 'for node %s as it is set to boot from a remote ' + 'volume.', node.uuid) + return + params = {} + image_source = node.instance_info.get('image_source') + image_checksum = node.instance_info.get('image_checksum') + image_disk_format = node.instance_info.get('image_disk_format') + os_hash_algo = node.instance_info.get('image_os_hash_algo') + os_hash_value = node.instance_info.get('image_os_hash_value') + + params['instance_info.image_source'] = image_source + error_msg = _('Node %s failed to validate deploy image info. Some ' + 'parameters were missing') % node.uuid + + deploy_utils.check_for_missing_params(params, error_msg) + + # NOTE(dtantsur): glance images contain a checksum; for file images we + # will recalculate the checksum anyway. + if (not service_utils.is_glance_image(image_source) + and not image_source.startswith('file://')): + + def _raise_missing_checksum_exception(node): + raise exception.MissingParameterValue(_( + 'image_source\'s "image_checksum", or ' + '"image_os_hash_algo" and "image_os_hash_value" ' + 'must be provided in instance_info for ' + 'node %s') % node.uuid) + + if os_hash_value and not os_hash_algo: + # We are missing a piece of information, + # so we still need to raise an error. + _raise_missing_checksum_exception(node) + elif not os_hash_value and os_hash_algo: + # We have the hash setting, but not the hash. + _raise_missing_checksum_exception(node) + elif not os_hash_value and not image_checksum: + # We are lacking the original image_checksum, + # so we raise the error. + _raise_missing_checksum_exception(node) + + validate_http_provisioning_configuration(node) + + check_image_size(task, image_source, image_disk_format) + validate_image_proxies(node) + @METRICS.timer('AgentDeployMixin.write_image') @base.deploy_step(priority=80) @task_manager.require_exclusive_lock @@ -408,259 +708,6 @@ class AgentDeployMixin(agent_base.AgentDeployMixin): deploy_utils.remove_http_instance_symlink(task.node.uuid) -class AgentDeploy(AgentDeployMixin, agent_base.AgentBaseMixin, - base.DeployInterface): - """Interface for deploy-related actions.""" - - def get_properties(self): - """Return the properties of the interface. - - :returns: dictionary of : entries. - """ - return COMMON_PROPERTIES - - def should_manage_boot(self, task): - """Whether agent boot is managed by ironic.""" - return CONF.agent.manage_agent_boot - - @METRICS.timer('AgentDeploy.validate') - def validate(self, task): - """Validate the driver-specific Node deployment info. - - This method validates whether the properties of the supplied node - contain the required information for this driver to deploy images to - the node. - - :param task: a TaskManager instance - :raises: MissingParameterValue, if any of the required parameters are - missing. - :raises: InvalidParameterValue, if any of the parameters have invalid - value. - """ - if CONF.agent.manage_agent_boot: - task.driver.boot.validate(task) - - node = task.node - - # Validate node capabilities - deploy_utils.validate_capabilities(node) - - if not task.driver.storage.should_write_image(task): - # NOTE(TheJulia): There is no reason to validate - # image properties if we will not be writing an image - # in a boot from volume case. As such, return to the caller. - LOG.debug('Skipping complete deployment interface validation ' - 'for node %s as it is set to boot from a remote ' - 'volume.', node.uuid) - return - - params = {} - image_source = node.instance_info.get('image_source') - image_checksum = node.instance_info.get('image_checksum') - image_disk_format = node.instance_info.get('image_disk_format') - os_hash_algo = node.instance_info.get('image_os_hash_algo') - os_hash_value = node.instance_info.get('image_os_hash_value') - - params['instance_info.image_source'] = image_source - error_msg = _('Node %s failed to validate deploy image info. Some ' - 'parameters were missing') % node.uuid - - deploy_utils.check_for_missing_params(params, error_msg) - - # NOTE(dtantsur): glance images contain a checksum; for file images we - # will recalculate the checksum anyway. - if (not service_utils.is_glance_image(image_source) - and not image_source.startswith('file://')): - - def _raise_missing_checksum_exception(node): - raise exception.MissingParameterValue(_( - 'image_source\'s "image_checksum", or ' - '"image_os_hash_algo" and "image_os_hash_value" ' - 'must be provided in instance_info for ' - 'node %s') % node.uuid) - - if os_hash_value and not os_hash_algo: - # We are missing a piece of information, - # so we still need to raise an error. - _raise_missing_checksum_exception(node) - elif not os_hash_value and os_hash_algo: - # We have the hash setting, but not the hash. - _raise_missing_checksum_exception(node) - elif not os_hash_value and not image_checksum: - # We are lacking the original image_checksum, - # so we raise the error. - _raise_missing_checksum_exception(node) - - validate_http_provisioning_configuration(node) - - check_image_size(task, image_source, image_disk_format) - # Validate the root device hints - deploy_utils.get_root_device_for_deploy(node) - validate_image_proxies(node) - - @METRICS.timer('AgentDeploy.deploy') - @base.deploy_step(priority=100) - @task_manager.require_exclusive_lock - def deploy(self, task): - """Perform a deployment to a node. - - Perform the necessary work to deploy an image onto the specified node. - This method will be called after prepare(), which may have already - performed any preparatory steps, such as pre-caching some data for the - node. - - :param task: a TaskManager instance. - :returns: status of the deploy. One of ironic.common.states. - """ - if manager_utils.is_fast_track(task): - # NOTE(mgoddard): For fast track we can skip this step and proceed - # immediately to the next deploy step. - LOG.debug('Performing a fast track deployment for %(node)s.', - {'node': task.node.uuid}) - # NOTE(dtantsur): while the node is up and heartbeating, we don't - # necessary have the deploy steps cached. Force a refresh here. - self.refresh_steps(task, 'deploy') - deployments.validate_deploy_steps(task) - elif task.driver.storage.should_write_image(task): - # Check if the driver has already performed a reboot in a previous - # deploy step. - if not task.node.driver_internal_info.get('deployment_reboot'): - manager_utils.node_power_action(task, states.REBOOT) - info = task.node.driver_internal_info - info.pop('deployment_reboot', None) - task.node.driver_internal_info = info - task.node.save() - return states.DEPLOYWAIT - - @METRICS.timer('AgentDeploy.prepare') - @task_manager.require_exclusive_lock - def prepare(self, task): - """Prepare the deployment environment for this node. - - :param task: a TaskManager instance. - :raises: NetworkError: if the previous cleaning ports cannot be removed - or if new cleaning ports cannot be created. - :raises: InvalidParameterValue when the wrong power state is specified - or the wrong driver info is specified for power management. - :raises: StorageError If the storage driver is unable to attach the - configured volumes. - :raises: other exceptions by the node's power driver if something - wrong occurred during the power action. - :raises: exception.ImageRefValidationFailed if image_source is not - Glance href and is not HTTP(S) URL. - :raises: exception.InvalidParameterValue if network validation fails. - :raises: any boot interface's prepare_ramdisk exceptions. - """ - - def _update_instance_info(): - node.instance_info = ( - deploy_utils.build_instance_info_for_deploy(task)) - node.save() - - node = task.node - deploy_utils.populate_storage_driver_internal_info(task) - if node.provision_state == states.DEPLOYING: - # Validate network interface to ensure that it supports boot - # options configured on the node. - try: - task.driver.network.validate(task) - except exception.InvalidParameterValue: - # For 'neutron' network interface validation will fail - # if node is using 'netboot' boot option while provisioning - # a whole disk image. Updating 'boot_option' in node's - # 'instance_info' to 'local for backward compatibility. - # TODO(stendulker): Fail here once the default boot - # option is local. - # NOTE(TheJulia): Fixing the default boot mode only - # masks the failure as the lack of a user definition - # can be perceived as both an invalid configuration and - # reliance upon the default configuration. The reality - # being that in most scenarios, users do not want network - # booting, so the changed default should be valid. - with excutils.save_and_reraise_exception(reraise=False) as ctx: - instance_info = node.instance_info - capabilities = utils.parse_instance_info_capabilities(node) - if 'boot_option' not in capabilities: - capabilities['boot_option'] = 'local' - instance_info['capabilities'] = capabilities - node.instance_info = instance_info - node.save() - # Re-validate the network interface - task.driver.network.validate(task) - else: - ctx.reraise = True - # Determine if this is a fast track sequence - fast_track_deploy = manager_utils.is_fast_track(task) - if fast_track_deploy: - # The agent has already recently checked in and we are - # configured to take that as an indicator that we can - # skip ahead. - LOG.debug('The agent for node %(node)s has recently checked ' - 'in, and the node power will remain unmodified.', - {'node': task.node.uuid}) - else: - # Powering off node to setup networking for port and - # ensure that the state is reset if it is inadvertently - # on for any unknown reason. - manager_utils.node_power_action(task, states.POWER_OFF) - if task.driver.storage.should_write_image(task): - # NOTE(vdrok): in case of rebuild, we have tenant network - # already configured, unbind tenant ports if present - if not fast_track_deploy: - power_state_to_restore = ( - manager_utils.power_on_node_if_needed(task)) - - task.driver.network.unconfigure_tenant_networks(task) - task.driver.network.add_provisioning_network(task) - if not fast_track_deploy: - manager_utils.restore_power_state_if_needed( - task, power_state_to_restore) - else: - # Fast track sequence in progress - _update_instance_info() - # Signal to storage driver to attach volumes - task.driver.storage.attach_volumes(task) - if (not task.driver.storage.should_write_image(task) - or fast_track_deploy): - # We have nothing else to do as this is handled in the - # backend storage system, and we can return to the caller - # as we do not need to boot the agent to deploy. - # Alternatively, we could be in a fast track deployment - # and again, we should have nothing to do here. - return - if node.provision_state in (states.ACTIVE, states.UNRESCUING): - # Call is due to conductor takeover - task.driver.boot.prepare_instance(task) - elif node.provision_state != states.ADOPTING: - if node.provision_state not in (states.RESCUING, states.RESCUEWAIT, - states.RESCUE, states.RESCUEFAIL): - _update_instance_info() - if CONF.agent.manage_agent_boot: - deploy_opts = deploy_utils.build_agent_options(node) - task.driver.boot.prepare_ramdisk(task, deploy_opts) - - @METRICS.timer('AgentDeploy.clean_up') - @task_manager.require_exclusive_lock - def clean_up(self, task): - """Clean up the deployment environment for this node. - - If preparation of the deployment environment ahead of time is possible, - this method should be implemented by the driver. It should erase - anything cached by the `prepare` method. - - If implemented, this method must be idempotent. It may be called - multiple times for the same node on the same conductor, and it may be - called by multiple conductors in parallel. Therefore, it must not - require an exclusive lock. - - This method is called before `tear_down`. - - :param task: a TaskManager instance. - """ - super(AgentDeploy, self).clean_up(task) - deploy_utils.destroy_http_instance_images(task.node) - - class AgentRAID(base.RAIDInterface): """Implementation of RAIDInterface which uses agent ramdisk.""" diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 77a5f30b45..0494c30d03 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -241,7 +241,231 @@ class TestAgentMethods(db_base.DbTestCase): self.node) -class TestAgentDeploy(db_base.DbTestCase): +class CommonTestsMixin: + "Tests for methods shared between CustomAgentDeploy and AgentDeploy.""" + + @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) + @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) + def test_deploy(self, power_mock, mock_pxe_instance): + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + driver_return = self.driver.deploy(task) + self.assertEqual(driver_return, states.DEPLOYWAIT) + power_mock.assert_called_once_with(task, states.REBOOT) + self.assertFalse(mock_pxe_instance.called) + + @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) + @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) + def test_deploy_with_deployment_reboot(self, power_mock, + mock_pxe_instance): + driver_internal_info = self.node.driver_internal_info + driver_internal_info['deployment_reboot'] = True + self.node.driver_internal_info = driver_internal_info + self.node.save() + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + driver_return = self.driver.deploy(task) + self.assertEqual(driver_return, states.DEPLOYWAIT) + self.assertFalse(power_mock.called) + self.assertFalse(mock_pxe_instance.called) + self.assertNotIn( + 'deployment_reboot', task.node.driver_internal_info) + + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', + autospec=True) + def test_deploy_storage_should_write_image_false( + self, mock_write, mock_power): + mock_write.return_value = False + self.node.provision_state = states.DEPLOYING + self.node.deploy_step = { + 'step': 'deploy', 'priority': 50, 'interface': 'deploy'} + self.node.save() + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + driver_return = self.driver.deploy(task) + self.assertIsNone(driver_return) + self.assertFalse(mock_power.called) + + @mock.patch.object(agent.CustomAgentDeploy, 'refresh_steps', + autospec=True) + @mock.patch.object(agent_client.AgentClient, 'prepare_image', + autospec=True) + @mock.patch('ironic.conductor.utils.is_fast_track', autospec=True) + @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) + @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) + def test_deploy_fast_track(self, power_mock, mock_pxe_instance, + mock_is_fast_track, prepare_image_mock, + refresh_mock): + mock_is_fast_track.return_value = True + self.node.target_provision_state = states.ACTIVE + self.node.provision_state = states.DEPLOYING + self.node.save() + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + result = self.driver.deploy(task) + self.assertIsNone(result) + self.assertFalse(power_mock.called) + self.assertFalse(mock_pxe_instance.called) + self.assertFalse(prepare_image_mock.called) + self.assertEqual(states.DEPLOYING, task.node.provision_state) + self.assertEqual(states.ACTIVE, + task.node.target_provision_state) + refresh_mock.assert_called_once_with(self.driver, task, 'deploy') + + @mock.patch.object(deploy_utils, 'destroy_http_instance_images', + autospec=True) + @mock.patch('ironic.common.dhcp_factory.DHCPFactory', autospec=True) + @mock.patch.object(pxe.PXEBoot, 'clean_up_instance', autospec=True) + @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) + def test_clean_up(self, pxe_clean_up_ramdisk_mock, + pxe_clean_up_instance_mock, dhcp_factor_mock, + destroy_images_mock): + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + self.driver.clean_up(task) + pxe_clean_up_ramdisk_mock.assert_called_once_with( + task.driver.boot, task) + pxe_clean_up_instance_mock.assert_called_once_with( + task.driver.boot, task) + dhcp_factor_mock.assert_called_once_with() + destroy_images_mock.assert_called_once_with(task.node) + + +class TestCustomAgentDeploy(CommonTestsMixin, db_base.DbTestCase): + def setUp(self): + super(TestCustomAgentDeploy, self).setUp() + self.config(enabled_deploy_interfaces=['direct', 'custom-agent']) + self.driver = agent.CustomAgentDeploy() + # NOTE(TheJulia): We explicitly set the noop storage interface as the + # default below for deployment tests in order to raise any change + # in the default which could be a breaking behavior change + # as the storage interface is explicitly an "opt-in" interface. + n = { + 'boot_interface': 'pxe', + 'deploy_interface': 'custom-agent', + 'instance_info': {}, + 'driver_info': DRIVER_INFO, + 'driver_internal_info': DRIVER_INTERNAL_INFO, + 'storage_interface': 'noop', + 'network_interface': 'noop' + } + self.node = object_utils.create_test_node(self.context, **n) + self.ports = [ + object_utils.create_test_port(self.context, node_id=self.node.id)] + dhcp_factory.DHCPFactory._dhcp_provider = None + + def test_get_properties(self): + expected = agent.COMMON_PROPERTIES + self.assertEqual(expected, self.driver.get_properties()) + + @mock.patch.object(agent, 'validate_http_provisioning_configuration', + autospec=True) + @mock.patch.object(deploy_utils, 'validate_capabilities', + spec_set=True, autospec=True) + @mock.patch.object(images, 'image_show', autospec=True) + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate(self, pxe_boot_validate_mock, show_mock, + validate_capability_mock, validate_http_mock): + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + self.driver.validate(task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) + validate_capability_mock.assert_called_once_with(task.node) + # No images required for custom-agent + show_mock.assert_not_called() + validate_http_mock.assert_not_called() + + @mock.patch.object(noop_storage.NoopStorage, 'attach_volumes', + autospec=True) + @mock.patch.object(deploy_utils, 'populate_storage_driver_internal_info', + autospec=True) + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True) + @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) + @mock.patch.object(deploy_utils, 'build_instance_info_for_deploy', + autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'add_provisioning_network', + spec_set=True, autospec=True) + @mock.patch.object(flat_network.FlatNetwork, + 'unconfigure_tenant_networks', + spec_set=True, autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'validate', + spec_set=True, autospec=True) + def test_prepare( + self, validate_net_mock, + unconfigure_tenant_net_mock, add_provisioning_net_mock, + build_instance_info_mock, build_options_mock, + pxe_prepare_ramdisk_mock, storage_driver_info_mock, + storage_attach_volumes_mock): + node = self.node + node.network_interface = 'flat' + node.save() + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + task.node.provision_state = states.DEPLOYING + build_options_mock.return_value = {'a': 'b'} + self.driver.prepare(task) + storage_driver_info_mock.assert_called_once_with(task) + validate_net_mock.assert_called_once_with(mock.ANY, task) + add_provisioning_net_mock.assert_called_once_with(mock.ANY, task) + unconfigure_tenant_net_mock.assert_called_once_with(mock.ANY, task) + storage_attach_volumes_mock.assert_called_once_with( + task.driver.storage, task) + build_instance_info_mock.assert_not_called() + build_options_mock.assert_called_once_with(task.node) + pxe_prepare_ramdisk_mock.assert_called_once_with( + task.driver.boot, task, {'a': 'b'}) + + @mock.patch('ironic.conductor.utils.is_fast_track', autospec=True) + @mock.patch.object(noop_storage.NoopStorage, 'attach_volumes', + autospec=True) + @mock.patch.object(deploy_utils, 'populate_storage_driver_internal_info', + autospec=True) + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True) + @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) + @mock.patch.object(deploy_utils, 'build_instance_info_for_deploy', + autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'add_provisioning_network', + spec_set=True, autospec=True) + @mock.patch.object(flat_network.FlatNetwork, + 'unconfigure_tenant_networks', + spec_set=True, autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'validate', + spec_set=True, autospec=True) + def test_prepare_fast_track( + self, validate_net_mock, + unconfigure_tenant_net_mock, add_provisioning_net_mock, + build_instance_info_mock, build_options_mock, + pxe_prepare_ramdisk_mock, storage_driver_info_mock, + storage_attach_volumes_mock, is_fast_track_mock): + # TODO(TheJulia): We should revisit this test. Smartnic + # support didn't wire in tightly on testing for power in + # these tests, and largely fast_track impacts power operations. + node = self.node + node.network_interface = 'flat' + node.save() + is_fast_track_mock.return_value = True + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + task.node.provision_state = states.DEPLOYING + build_options_mock.return_value = {'a': 'b'} + self.driver.prepare(task) + storage_driver_info_mock.assert_called_once_with(task) + validate_net_mock.assert_called_once_with(mock.ANY, task) + add_provisioning_net_mock.assert_called_once_with(mock.ANY, task) + unconfigure_tenant_net_mock.assert_called_once_with(mock.ANY, task) + self.assertTrue(storage_attach_volumes_mock.called) + self.assertFalse(build_instance_info_mock.called) + # TODO(TheJulia): We should likely consider executing the + # next two methods at some point in order to facilitate + # continuity. While not explicitly required for this feature + # to work, reboots as part of deployment would need the ramdisk + # present and ready. + self.assertFalse(build_options_mock.called) + + +class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): def setUp(self): super(TestAgentDeploy, self).setUp() self.driver = agent.AgentDeploy() @@ -436,8 +660,8 @@ class TestAgentDeploy(db_base.DbTestCase): task.driver.deploy.validate, task) pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) - show_mock.assert_called_once_with(self.context, 'fake-image') - validate_http_mock.assert_called_once_with(task.node) + show_mock.assert_not_called() + validate_http_mock.assert_not_called() @mock.patch.object(agent, 'validate_http_provisioning_configuration', autospec=True) @@ -453,8 +677,8 @@ class TestAgentDeploy(db_base.DbTestCase): task.driver.deploy.validate, task) pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) - show_mock.assert_called_once_with(self.context, 'fake-image') - validate_http_mock.assert_called_once_with(task.node) + show_mock.assert_not_called() + validate_http_mock.assert_not_called() @mock.patch.object(agent, 'validate_http_provisioning_configuration', autospec=True) @@ -494,74 +718,6 @@ class TestAgentDeploy(db_base.DbTestCase): mock_capabilities.assert_called_once_with(task.node) self.assertFalse(mock_params.called) - @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) - @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) - def test_deploy(self, power_mock, mock_pxe_instance): - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - driver_return = self.driver.deploy(task) - self.assertEqual(driver_return, states.DEPLOYWAIT) - power_mock.assert_called_once_with(task, states.REBOOT) - self.assertFalse(mock_pxe_instance.called) - - @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) - @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) - def test_deploy_with_deployment_reboot(self, power_mock, - mock_pxe_instance): - driver_internal_info = self.node.driver_internal_info - driver_internal_info['deployment_reboot'] = True - self.node.driver_internal_info = driver_internal_info - self.node.save() - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - driver_return = self.driver.deploy(task) - self.assertEqual(driver_return, states.DEPLOYWAIT) - self.assertFalse(power_mock.called) - self.assertFalse(mock_pxe_instance.called) - self.assertNotIn( - 'deployment_reboot', task.node.driver_internal_info) - - @mock.patch.object(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', - autospec=True) - def test_deploy_storage_should_write_image_false( - self, mock_write, mock_power): - mock_write.return_value = False - self.node.provision_state = states.DEPLOYING - self.node.deploy_step = { - 'step': 'deploy', 'priority': 50, 'interface': 'deploy'} - self.node.save() - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - driver_return = self.driver.deploy(task) - self.assertIsNone(driver_return) - self.assertFalse(mock_power.called) - - @mock.patch.object(agent.AgentDeploy, 'refresh_steps', autospec=True) - @mock.patch.object(agent_client.AgentClient, 'prepare_image', - autospec=True) - @mock.patch('ironic.conductor.utils.is_fast_track', autospec=True) - @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) - @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) - def test_deploy_fast_track(self, power_mock, mock_pxe_instance, - mock_is_fast_track, prepare_image_mock, - refresh_mock): - mock_is_fast_track.return_value = True - self.node.target_provision_state = states.ACTIVE - self.node.provision_state = states.DEPLOYING - self.node.save() - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - result = self.driver.deploy(task) - self.assertIsNone(result) - self.assertFalse(power_mock.called) - self.assertFalse(mock_pxe_instance.called) - self.assertFalse(prepare_image_mock.called) - self.assertEqual(states.DEPLOYING, task.node.provision_state) - self.assertEqual(states.ACTIVE, - task.node.target_provision_state) - refresh_mock.assert_called_once_with(self.driver, task, 'deploy') - @mock.patch.object(noop_storage.NoopStorage, 'detach_volumes', autospec=True) @mock.patch.object(flat_network.FlatNetwork, @@ -1115,43 +1271,6 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertFalse(build_options_mock.called) self.assertFalse(pxe_prepare_ramdisk_mock.called) - @mock.patch.object(deploy_utils, 'destroy_http_instance_images', - autospec=True) - @mock.patch('ironic.common.dhcp_factory.DHCPFactory', autospec=True) - @mock.patch.object(pxe.PXEBoot, 'clean_up_instance', autospec=True) - @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) - def test_clean_up(self, pxe_clean_up_ramdisk_mock, - pxe_clean_up_instance_mock, dhcp_factor_mock, - destroy_images_mock): - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - self.driver.clean_up(task) - pxe_clean_up_ramdisk_mock.assert_called_once_with( - task.driver.boot, task) - pxe_clean_up_instance_mock.assert_called_once_with( - task.driver.boot, task) - dhcp_factor_mock.assert_called_once_with() - destroy_images_mock.assert_called_once_with(task.node) - - @mock.patch.object(deploy_utils, 'destroy_http_instance_images', - autospec=True) - @mock.patch('ironic.common.dhcp_factory.DHCPFactory', autospec=True) - @mock.patch.object(pxe.PXEBoot, 'clean_up_instance', autospec=True) - @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) - def test_clean_up_manage_agent_boot_false(self, pxe_clean_up_ramdisk_mock, - pxe_clean_up_instance_mock, - dhcp_factor_mock, - destroy_images_mock): - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - self.config(group='agent', manage_agent_boot=False) - self.driver.clean_up(task) - self.assertFalse(pxe_clean_up_ramdisk_mock.called) - pxe_clean_up_instance_mock.assert_called_once_with( - task.driver.boot, task) - dhcp_factor_mock.assert_called_once_with() - destroy_images_mock.assert_called_once_with(task.node) - @mock.patch.object(agent_base, 'get_steps', autospec=True) def test_get_clean_steps(self, mock_get_steps): # Test getting clean steps @@ -1378,7 +1497,7 @@ class TestAgentDeploy(db_base.DbTestCase): @mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_partition_uuids', autospec=True) - @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', + @mock.patch.object(agent.AgentDeploy, 'prepare_instance_to_boot', autospec=True) def test_prepare_instance_boot(self, prepare_instance_mock, uuid_mock, log_mock, remove_symlink_mock): @@ -1406,7 +1525,7 @@ class TestAgentDeploy(db_base.DbTestCase): @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_partition_uuids', autospec=True) - @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', + @mock.patch.object(agent.AgentDeploy, 'prepare_instance_to_boot', autospec=True) def test_prepare_instance_boot_no_manage_agent_boot( self, prepare_instance_mock, uuid_mock, @@ -1434,7 +1553,7 @@ class TestAgentDeploy(db_base.DbTestCase): autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_partition_uuids', autospec=True) - @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', + @mock.patch.object(agent.AgentDeploy, 'prepare_instance_to_boot', autospec=True) def test_prepare_instance_boot_partition_image(self, prepare_instance_mock, uuid_mock, boot_mode_mock, @@ -1470,11 +1589,11 @@ class TestAgentDeploy(db_base.DbTestCase): @mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True) @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', autospec=True) - @mock.patch.object(agent.AgentDeployMixin, '_get_uuid_from_result', + @mock.patch.object(agent.AgentDeploy, '_get_uuid_from_result', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_partition_uuids', autospec=True) - @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', + @mock.patch.object(agent.AgentDeploy, 'prepare_instance_to_boot', autospec=True) def test_prepare_instance_boot_partition_image_compat( self, prepare_instance_mock, uuid_mock, @@ -1512,7 +1631,7 @@ class TestAgentDeploy(db_base.DbTestCase): autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_partition_uuids', autospec=True) - @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', + @mock.patch.object(agent.AgentDeploy, 'prepare_instance_to_boot', autospec=True) def test_prepare_instance_boot_partition_localboot_ppc64( self, prepare_instance_mock, @@ -1556,7 +1675,7 @@ class TestAgentDeploy(db_base.DbTestCase): autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_partition_uuids', autospec=True) - @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', + @mock.patch.object(agent.AgentDeploy, 'prepare_instance_to_boot', autospec=True) def test_prepare_instance_boot_localboot(self, prepare_instance_mock, uuid_mock, boot_mode_mock, diff --git a/ironic/tests/unit/drivers/test_generic.py b/ironic/tests/unit/drivers/test_generic.py index f2bc2623fc..cc0ecaa472 100644 --- a/ironic/tests/unit/drivers/test_generic.py +++ b/ironic/tests/unit/drivers/test_generic.py @@ -50,17 +50,18 @@ class ManualManagementHardwareTestCase(db_base.DbTestCase): def test_supported_interfaces(self): self.config(enabled_inspect_interfaces=['inspector', 'no-inspect'], + enabled_deploy_interfaces=['direct', 'custom-agent'], enabled_raid_interfaces=['agent']) node = obj_utils.create_test_node(self.context, driver='manual-management', management_interface='fake', - deploy_interface='direct', + deploy_interface='custom-agent', raid_interface='agent') with task_manager.acquire(self.context, node.id) as task: self.assertIsInstance(task.driver.management, fake.FakeManagement) self.assertIsInstance(task.driver.power, fake.FakePower) self.assertIsInstance(task.driver.boot, pxe.PXEBoot) - self.assertIsInstance(task.driver.deploy, agent.AgentDeploy) + self.assertIsInstance(task.driver.deploy, agent.CustomAgentDeploy) self.assertIsInstance(task.driver.inspect, inspector.Inspector) self.assertIsInstance(task.driver.raid, agent.AgentRAID) diff --git a/releasenotes/notes/custom-agent-deploy-88989512c29a14c1.yaml b/releasenotes/notes/custom-agent-deploy-88989512c29a14c1.yaml new file mode 100644 index 0000000000..0444132518 --- /dev/null +++ b/releasenotes/notes/custom-agent-deploy-88989512c29a14c1.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + Adds a new deploy interface ``custom-agent`` that can be used when all + necessary deploy steps to provision an image are provided in the agent + ramdisk. The default ``write_image`` deploy step is not present. +other: + - | + A new class ``ironic.drivers.modules.agent.CustomAgentDeploy`` can be used + as a base class for deploy interfaces based on ironic-python-agent. diff --git a/setup.cfg b/setup.cfg index 9d99f6dfff..a6719d46a8 100644 --- a/setup.cfg +++ b/setup.cfg @@ -87,6 +87,7 @@ ironic.hardware.interfaces.console = ironic.hardware.interfaces.deploy = anaconda = ironic.drivers.modules.pxe:PXEAnacondaDeploy ansible = ironic.drivers.modules.ansible.deploy:AnsibleDeploy + custom-agent = ironic.drivers.modules.agent:CustomAgentDeploy direct = ironic.drivers.modules.agent:AgentDeploy fake = ironic.drivers.modules.fake:FakeDeploy iscsi = ironic.drivers.modules.iscsi_deploy:ISCSIDeploy