From a41c7a95920358af2fe23e4e102fc514a81d81c0 Mon Sep 17 00:00:00 2001 From: Hamdy Khader Date: Wed, 18 Jul 2018 16:21:43 +0300 Subject: [PATCH] Add support for Smart NICs Extend Ironic to enable use of Smart NICs to implement generic networking services for baremetal servers. Extending the ramdisk, direct, iscsi and ansible deployment Interfaces to support the Smart NIC use-cases. For Smart NIC use-case the baremetal node must be powered on and booted into bios then wait for agent that runs on the Smart NIC to be alive then do the network changes required. Task: #26932 Story: #2003346 Change-Id: I00d6f13dd991074e4f45ada4d7cf4ccc0edbc7e1 --- ironic/common/neutron.py | 124 ++++++++++ ironic/conductor/utils.py | 54 +++++ ironic/conf/agent.py | 15 ++ ironic/drivers/base.py | 8 + ironic/drivers/modules/agent.py | 21 +- ironic/drivers/modules/agent_base_vendor.py | 8 +- ironic/drivers/modules/ansible/deploy.py | 17 ++ ironic/drivers/modules/deploy_utils.py | 6 + ironic/drivers/modules/iscsi_deploy.py | 11 + ironic/drivers/modules/network/common.py | 15 ++ ironic/drivers/modules/network/neutron.py | 18 ++ ironic/drivers/modules/pxe.py | 3 + ironic/tests/unit/common/test_neutron.py | 118 +++++++++ ironic/tests/unit/conductor/test_utils.py | 82 +++++++ .../drivers/modules/ansible/test_deploy.py | 154 +++++++++++- .../drivers/modules/network/test_common.py | 20 ++ .../drivers/modules/network/test_neutron.py | 42 +++- .../tests/unit/drivers/modules/test_agent.py | 228 +++++++++++++++++- .../drivers/modules/test_agent_base_vendor.py | 102 +++++++- .../unit/drivers/modules/test_iscsi_deploy.py | 118 +++++++++ ironic/tests/unit/drivers/modules/test_pxe.py | 38 +++ ...upport-for-smart-nic-0fc5b10ba6772f7f.yaml | 10 + 22 files changed, 1184 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/add-support-for-smart-nic-0fc5b10ba6772f7f.yaml diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index 609d832dfe..f365fb1a8e 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -14,6 +14,7 @@ from neutronclient.common import exceptions as neutron_exceptions from neutronclient.v2_0 import client as clientv20 from oslo_log import log from oslo_utils import uuidutils +import retrying from ironic.common import context as ironic_context from ironic.common import exception @@ -21,6 +22,7 @@ from ironic.common.i18n import _ from ironic.common import keystone from ironic.common.pxe_utils import DHCP_CLIENT_ID from ironic.conf import CONF +from ironic import objects LOG = log.getLogger(__name__) @@ -31,6 +33,7 @@ DEFAULT_NEUTRON_URL = 'http://%s:9696' % CONF.my_ip _NEUTRON_SESSION = None VNIC_BAREMETAL = 'baremetal' +VNIC_SMARTNIC = 'smart-nic' PHYSNET_PARAM_NAME = 'provider:physical_network' """Name of the neutron network API physical network parameter.""" @@ -256,6 +259,17 @@ def add_ports_to_network(task, network_uuid, security_groups=None): binding_profile = {'local_link_information': [portmap[ironic_port.uuid]]} body['port']['binding:profile'] = binding_profile + link_info = binding_profile['local_link_information'][0] + is_smart_nic = is_smartnic_port(ironic_port) + if is_smart_nic: + LOG.debug('Setting hostname as host_id in case of Smart NIC, ' + 'port %(port_id)s, hostname %(hostname)s', + {'port_id': ironic_port.uuid, + 'hostname': link_info['hostname']}) + body['port']['binding:host_id'] = link_info['hostname'] + + # TODO(hamdyk): use portbindings.VNIC_SMARTNIC from neutron-lib + body['port']['binding:vnic_type'] = VNIC_SMARTNIC client_id = ironic_port.extra.get('client-id') if client_id: client_id_opt = {'opt_name': DHCP_CLIENT_ID, @@ -264,7 +278,11 @@ def add_ports_to_network(task, network_uuid, security_groups=None): extra_dhcp_opts.append(client_id_opt) body['port']['extra_dhcp_opts'] = extra_dhcp_opts try: + if is_smart_nic: + wait_for_host_agent(client, body['port']['binding:host_id']) port = client.create_port(body) + if is_smart_nic: + wait_for_port_status(client, port['port']['id'], 'ACTIVE') except neutron_exceptions.NeutronClientException as e: failures.append(ironic_port.uuid) LOG.warning("Could not create neutron port for node's " @@ -342,6 +360,8 @@ def remove_neutron_ports(task, params): '%(node_id)s.', {'vif_port_id': port['id'], 'node_id': node_uuid}) + if is_smartnic_port(port): + wait_for_host_agent(client, port['binding:host_id']) try: client.delete_port(port['id']) # NOTE(mgoddard): Ignore if the port was deleted by nova. @@ -488,6 +508,44 @@ def validate_port_info(node, port): return True +def validate_agent(client, **kwargs): + """Check that the given neutron agent is alive + + :param client: Neutron client + :param kwargs: Additional parameters to pass to the neutron client + list_agents method. + :returns: A boolean to describe the agent status, if more than one agent + returns by the client then return True if at least one of them is + alive. + :raises: NetworkError in case of failure contacting Neutron. + """ + try: + agents = client.list_agents(**kwargs)['agents'] + for agent in agents: + if agent['alive']: + return True + return False + except neutron_exceptions.NeutronClientException: + raise exception.NetworkError('Failed to contact Neutron server') + + +def is_smartnic_port(port_data): + """Check that the port is Smart NIC port + + :param port_data: an instance of ironic.objects.port.Port + or port data as dict. + :returns: A boolean to indicate port as Smart NIC port. + """ + if isinstance(port_data, objects.Port): + return port_data.supports_is_smartnic() and port_data.is_smartnic + + if isinstance(port_data, dict): + return port_data.get('is_smartnic', False) + + LOG.warning('Unknown port data type: %(type)s', {'type': type(port_data)}) + return False + + def _get_network_by_uuid_or_name(client, uuid_or_name, net_type=_('network'), **params): """Return a neutron network by UUID or name. @@ -586,6 +644,72 @@ def get_physnets_by_port_uuid(client, port_uuid): if segment[PHYSNET_PARAM_NAME]) +@retrying.retry( + stop_max_attempt_number=CONF.agent.neutron_agent_max_attempts, + retry_on_exception=lambda e: isinstance(e, exception.NetworkError), + wait_fixed=CONF.agent.neutron_agent_wait_time_seconds * 1000 +) +def wait_for_host_agent(client, host_id, target_state='up'): + """Wait for neutron agent to become target state + + :param client: A Neutron client object. + :param host_id: Agent host_id + :param target_state: up: wait for up status, + down: wait for down status + :returns: boolean indicates the agent state matches + param value target_state_up. + :raises: exception.NetworkError if host status didn't match the required + status after max retry attempts. + """ + if target_state not in ['up', 'down']: + raise exception.Invalid( + 'Invalid requested agent state to validate, accepted values: ' + 'up, down. Requested state: %(target_state)s' % { + 'target_state': target_state}) + + LOG.debug('Validating host %(host_id)s agent is %(status)s', + {'host_id': host_id, + 'status': target_state}) + is_alive = validate_agent(client, host=host_id) + LOG.debug('Agent on host %(host_id)s is %(status)s', + {'host_id': host_id, + 'status': 'up' if is_alive else 'down'}) + if ((target_state == 'up' and is_alive) or + (target_state == 'down' and not is_alive)): + return True + raise exception.NetworkError( + 'Agent on host %(host)s failed to reach state %(state)s' % { + 'host': host_id, 'state': target_state}) + + +@retrying.retry( + stop_max_attempt_number=CONF.agent.neutron_agent_max_attempts, + retry_on_exception=lambda e: isinstance(e, exception.NetworkError), + wait_fixed=CONF.agent.neutron_agent_wait_time_seconds * 1000 +) +def wait_for_port_status(client, port_id, status): + """Wait for port status to be the desired status + + :param client: A Neutron client object. + :param port_id: Neutron port_id + :param status: Port's target status, can be ACTIVE, DOWN ... etc. + :returns: boolean indicates that the port status matches the + required value passed by param status. + :raises: exception.NetworkError if port status didn't match + the required status after max retry attempts. + """ + LOG.debug('Validating Port %(port_id)s status is %(status)s', + {'port_id': port_id, 'status': status}) + port_info = _get_port_by_uuid(client, port_id) + LOG.debug('Port %(port_id)s status is: %(status)s', + {'port_id': port_id, 'status': port_info['status']}) + if port_info['status'] == status: + return True + raise exception.NetworkError( + 'Port %(port_id)s failed to reach status %(status)s' % { + 'port_id': port_id, 'status': status}) + + class NeutronNetworkInterfaceMixin(object): def get_cleaning_network_uuid(self, task): diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 833f93838b..0de36f8bc5 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -11,6 +11,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import time from oslo_config import cfg from oslo_log import log @@ -18,6 +19,7 @@ from oslo_service import loopingcall from oslo_utils import excutils import six +from ironic.common import boot_devices from ironic.common import exception from ironic.common import faults from ironic.common.i18n import _ @@ -1000,3 +1002,55 @@ def skip_automated_cleaning(node): :param node: the node to consider """ return not CONF.conductor.automated_clean and not node.automated_clean + + +def power_on_node_if_needed(task): + """Powers on node if it is powered off and has a Smart NIC port + + :param task: A TaskManager object + :returns: the previous power state or None if no changes were made + """ + if not task.driver.network.need_power_on(task): + return + + previous_power_state = task.driver.power.get_power_state(task) + if previous_power_state == states.POWER_OFF: + node_set_boot_device( + task, boot_devices.BIOS, persistent=False) + node_power_action(task, states.POWER_ON) + + # local import is necessary to avoid circular import + from ironic.common import neutron + + host_id = None + for port in task.ports: + if neutron.is_smartnic_port(port): + link_info = port.local_link_connection + host_id = link_info['hostname'] + break + + if host_id: + LOG.debug('Waiting for host %(host)s agent to be down', + {'host': host_id}) + + client = neutron.get_client(context=task.context) + neutron.wait_for_host_agent( + client, host_id, target_state='down') + return previous_power_state + + +def restore_power_state_if_needed(task, power_state_to_restore): + """Change the node's power state if power_state_to_restore is not empty + + :param task: A TaskManager object + :param power_state_to_restore: power state + """ + if power_state_to_restore: + + # Sleep is required here in order to give neutron agent + # a chance to apply the changes before powering off. + # Using twice the polling interval of the agent + # "CONF.AGENT.polling_interval" would give the agent + # enough time to apply network changes. + time.sleep(CONF.agent.neutron_agent_poll_interval * 2) + node_power_action(task, power_state_to_restore) diff --git a/ironic/conf/agent.py b/ironic/conf/agent.py index 7b2ea9ac57..58bec91f53 100644 --- a/ironic/conf/agent.py +++ b/ironic/conf/agent.py @@ -110,6 +110,21 @@ opts = [ help=_('This is the maximum number of attempts that will be ' 'done for IPA commands that fails due to network ' 'problems.')), + cfg.IntOpt('neutron_agent_poll_interval', + default=2, + help=_('The number of seconds Neutron agent will wait between ' + 'polling for device changes. This value should be ' + 'the same as CONF.AGENT.polling_interval in Neutron ' + 'configuration.')), + cfg.IntOpt('neutron_agent_max_attempts', + default=100, + help=_('Max number of attempts to validate a Neutron agent ' + 'status alive before raising network error for a ' + 'dead agent.')), + cfg.IntOpt('neutron_agent_wait_time_seconds', + default=10, + help=_('Wait time in seconds between attempts for validating ' + 'Neutron agent status.')), ] diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 963727c754..c71e678b4c 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -1334,6 +1334,14 @@ class NetworkInterface(BaseInterface): """ pass + def need_power_on(self, task): + """Check if ironic node must be powered on before applying network changes + + :param task: A TaskManager instance. + :returns: Boolean. + """ + return False + @six.add_metaclass(abc.ABCMeta) class StorageInterface(BaseInterface): diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index c9344ab153..851f6b652a 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -464,8 +464,12 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): # This is not being done now as it is expected to be # refactored in the near future. manager_utils.node_power_action(task, states.POWER_OFF) + power_state_to_restore = ( + manager_utils.power_on_node_if_needed(task)) task.driver.network.remove_provisioning_network(task) task.driver.network.configure_tenant_networks(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) task.driver.boot.prepare_instance(task) manager_utils.node_power_action(task, states.POWER_ON) LOG.info('Deployment to node %s done', task.node.uuid) @@ -489,11 +493,13 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): manager_utils.node_power_action(task, states.POWER_OFF) task.driver.storage.detach_volumes(task) deploy_utils.tear_down_storage_configuration(task) + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.unconfigure_tenant_networks(task) # NOTE(mgoddard): If the deployment was unsuccessful the node may have # ports on the provisioning network which were not deleted. task.driver.network.remove_provisioning_network(task) - + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) return states.DELETED @METRICS.timer('AgentDeploy.prepare') @@ -548,8 +554,12 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): 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 + 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) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) # Signal to storage driver to attach volumes task.driver.storage.attach_volumes(task) if not task.driver.storage.should_write_image(task): @@ -806,8 +816,11 @@ class AgentRescue(base.RescueInterface): task.node.save() task.driver.boot.clean_up_instance(task) + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.unconfigure_tenant_networks(task) task.driver.network.add_rescuing_network(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) if CONF.agent.manage_agent_boot: ramdisk_opts = deploy_utils.build_agent_options(task.node) # prepare_ramdisk will set the boot device @@ -842,7 +855,10 @@ class AgentRescue(base.RescueInterface): task.node.save() self.clean_up(task) + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.configure_tenant_networks(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) task.driver.boot.prepare_instance(task) manager_utils.node_power_action(task, states.POWER_ON) @@ -894,4 +910,7 @@ class AgentRescue(base.RescueInterface): manager_utils.remove_node_rescue_password(task.node, save=True) if CONF.agent.manage_agent_boot: task.driver.boot.clean_up_ramdisk(task) + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.remove_rescuing_network(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py index 48163f9ea9..eb386de4fc 100644 --- a/ironic/drivers/modules/agent_base_vendor.py +++ b/ironic/drivers/modules/agent_base_vendor.py @@ -373,7 +373,10 @@ class HeartbeatMixin(object): reason=fail_reason) task.process_event('resume') task.driver.rescue.clean_up(task) + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.configure_tenant_networks(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) task.process_event('done') @@ -642,9 +645,12 @@ class AgentDeployMixin(HeartbeatMixin): log_and_raise_deployment_error(task, msg, exc=e) try: + power_state_to_restore = ( + manager_utils.power_on_node_if_needed(task)) task.driver.network.remove_provisioning_network(task) task.driver.network.configure_tenant_networks(task) - + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) manager_utils.node_power_action(task, states.POWER_ON) except Exception as e: msg = (_('Error rebooting node %(node)s after deploy. ' diff --git a/ironic/drivers/modules/ansible/deploy.py b/ironic/drivers/modules/ansible/deploy.py index 273effb9ec..a5b28cd776 100644 --- a/ironic/drivers/modules/ansible/deploy.py +++ b/ironic/drivers/modules/ansible/deploy.py @@ -440,7 +440,10 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): def tear_down(self, task): """Tear down a previous deployment on the task's node.""" manager_utils.node_power_action(task, states.POWER_OFF) + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.unconfigure_tenant_networks(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) return states.DELETED @METRICS.timer('AnsibleDeploy.prepare') @@ -451,7 +454,11 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): if node.provision_state == states.DEPLOYING: # adding network-driver dependent provisioning ports manager_utils.node_power_action(task, states.POWER_OFF) + power_state_to_restore = ( + manager_utils.power_on_node_if_needed(task)) task.driver.network.add_provisioning_network(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) if node.provision_state not in [states.ACTIVE, states.ADOPTING]: node.instance_info = deploy_utils.build_instance_info_for_deploy( task) @@ -534,7 +541,10 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): if not node.driver_internal_info['clean_steps']: # no clean steps configured, nothing to do. return + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.add_cleaning_network(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) boot_opt = deploy_utils.build_agent_options(node) task.driver.boot.prepare_ramdisk(task, boot_opt) manager_utils.node_power_action(task, states.REBOOT) @@ -550,7 +560,10 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): """ manager_utils.node_power_action(task, states.POWER_OFF) task.driver.boot.clean_up_ramdisk(task) + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.remove_cleaning_network(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) @METRICS.timer('AnsibleDeploy.continue_deploy') def continue_deploy(self, task): @@ -622,8 +635,12 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): manager_utils.node_power_action(task, states.POWER_OFF) else: manager_utils.node_power_action(task, states.POWER_OFF) + power_state_to_restore = ( + manager_utils.power_on_node_if_needed(task)) task.driver.network.remove_provisioning_network(task) task.driver.network.configure_tenant_networks(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) manager_utils.node_power_action(task, states.POWER_ON) except Exception as e: msg = (_('Error rebooting node %(node)s after deploy. ' diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index ed73ed20b4..f8b81e9f77 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -899,7 +899,10 @@ def prepare_inband_cleaning(task, manage_boot=True): :raises: InvalidParameterValue if cleaning network UUID config option has an invalid value. """ + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.add_cleaning_network(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) # Append required config parameters to node's driver_internal_info # to pass to IPA. @@ -937,7 +940,10 @@ def tear_down_inband_cleaning(task, manage_boot=True): if manage_boot: task.driver.boot.clean_up_ramdisk(task) + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.remove_cleaning_network(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) def get_image_instance_info(node): diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index d81bc068ad..27a426d972 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -420,8 +420,12 @@ class ISCSIDeploy(AgentDeployMixin, base.DeployInterface): # This is not being done now as it is expected to be # refactored in the near future. manager_utils.node_power_action(task, states.POWER_OFF) + power_state_to_restore = ( + manager_utils.power_on_node_if_needed(task)) task.driver.network.remove_provisioning_network(task) task.driver.network.configure_tenant_networks(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) task.driver.boot.prepare_instance(task) manager_utils.node_power_action(task, states.POWER_ON) @@ -447,10 +451,13 @@ class ISCSIDeploy(AgentDeployMixin, base.DeployInterface): manager_utils.node_power_action(task, states.POWER_OFF) task.driver.storage.detach_volumes(task) deploy_utils.tear_down_storage_configuration(task) + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.unconfigure_tenant_networks(task) # NOTE(mgoddard): If the deployment was unsuccessful the node may have # ports on the provisioning network which were not deleted. task.driver.network.remove_provisioning_network(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) return states.DELETED @METRICS.timer('ISCSIDeploy.prepare') @@ -485,8 +492,12 @@ class ISCSIDeploy(AgentDeployMixin, base.DeployInterface): # NOTE(vdrok): in case of rebuild, we have tenant network # already configured, unbind tenant ports if present if task.driver.storage.should_write_image(task): + 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) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) task.driver.storage.attach_volumes(task) if not task.driver.storage.should_write_image(task): # We have nothing else to do as this is handled in the diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py index 485434c3dd..87859a1b5c 100644 --- a/ironic/drivers/modules/network/common.py +++ b/ironic/drivers/modules/network/common.py @@ -266,11 +266,26 @@ def plug_port_to_tenant_network(task, port_like_obj, client=None): if client_id_opt: body['port']['extra_dhcp_opts'] = [client_id_opt] + is_smart_nic = neutron.is_smartnic_port(port_like_obj) + if is_smart_nic: + link_info = local_link_info[0] + LOG.debug('Setting hostname as host_id in case of Smart NIC, ' + 'port %(port_id)s, hostname %(hostname)s', + {'port_id': vif_id, + 'hostname': link_info['hostname']}) + body['port']['binding:host_id'] = link_info['hostname'] + body['port']['binding:vnic_type'] = neutron.VNIC_SMARTNIC + if not client: client = neutron.get_client(context=task.context) + if is_smart_nic: + neutron.wait_for_host_agent(client, body['port']['binding:host_id']) + try: client.update_port(vif_id, body) + if is_smart_nic: + neutron.wait_for_port_status(client, vif_id, 'ACTIVE') except neutron_exceptions.ConnectionFailed as e: msg = (_('Could not add public network VIF %(vif)s ' 'to node %(node)s, possible network issue. %(exc)s') % diff --git a/ironic/drivers/modules/network/neutron.py b/ironic/drivers/modules/network/neutron.py index 4abac6d742..50507c9691 100644 --- a/ironic/drivers/modules/network/neutron.py +++ b/ironic/drivers/modules/network/neutron.py @@ -258,4 +258,22 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, or port_like_obj.extra.get('vif_port_id')) if not vif_port_id: continue + + is_smart_nic = neutron.is_smartnic_port(port_like_obj) + if is_smart_nic: + client = neutron.get_client(context=task.context) + link_info = port_like_obj.local_link_connection + neutron.wait_for_host_agent(client, link_info['hostname']) + neutron.unbind_neutron_port(vif_port_id, context=task.context) + + def need_power_on(self, task): + """Check if the node has any Smart NIC ports + + :param task: A TaskManager instance. + :return: A boolean to indicate Smart NIC port presence + """ + for port in task.ports: + if neutron.is_smartnic_port(port): + return True + return False diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index 4ee5c35b2d..2c24e7ba95 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -329,7 +329,10 @@ class PXERamdiskDeploy(agent.AgentDeploy): # IDEA(TheJulia): Maybe a "trusted environment" mode flag # that we otherwise fail validation on for drivers that # require explicit security postures. + power_state_to_restore = manager_utils.power_on_node_if_needed(task) task.driver.network.configure_tenant_networks(task) + manager_utils.restore_power_state_if_needed( + task, power_state_to_restore) # calling boot.prepare_instance will also set the node # to PXE boot, and update PXE templates accordingly diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index f4a35601d7..63c0646b7e 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/ironic/tests/unit/common/test_neutron.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +import time + from keystoneauth1 import loading as kaloading import mock from neutronclient.common import exceptions as neutron_client_exc @@ -185,6 +187,8 @@ class TestNeutronNetworkActions(db_base.DbTestCase): 'mac_address': '52:54:00:cf:2d:32'} self.network_uuid = uuidutils.generate_uuid() self.client_mock = mock.Mock() + self.client_mock.list_agents.return_value = { + 'agents': [{'alive': True}]} patcher = mock.patch('ironic.common.neutron.get_client', return_value=self.client_mock, autospec=True) patcher.start() @@ -582,6 +586,120 @@ class TestNeutronNetworkActions(db_base.DbTestCase): self.assertTrue(res) self.assertFalse(log_mock.warning.called) + def test_validate_agent_up(self): + self.client_mock.list_agents.return_value = { + 'agents': [{'alive': True}]} + self.assertTrue(neutron.validate_agent(self.client_mock)) + + def test_validate_agent_down(self): + self.client_mock.list_agents.return_value = { + 'agents': [{'alive': False}]} + self.assertFalse(neutron.validate_agent(self.client_mock)) + + def test_is_smartnic_port_true(self): + port = self.ports[0] + port.is_smartnic = True + self.assertTrue(neutron.is_smartnic_port(port)) + + def test_is_smartnic_port_false(self): + port = self.ports[0] + self.assertFalse(neutron.is_smartnic_port(port)) + + @mock.patch.object(neutron, 'validate_agent') + @mock.patch.object(time, 'sleep') + def test_wait_for_host_agent_up(self, sleep_mock, validate_agent_mock): + validate_agent_mock.return_value = True + neutron.wait_for_host_agent(self.client_mock, 'hostname') + sleep_mock.assert_not_called() + + @mock.patch.object(neutron, 'validate_agent') + @mock.patch.object(time, 'sleep') + def test_wait_for_host_agent_down(self, sleep_mock, validate_agent_mock): + validate_agent_mock.side_effect = [False, True] + neutron.wait_for_host_agent(self.client_mock, 'hostname') + sleep_mock.assert_called_once() + + @mock.patch.object(neutron, '_get_port_by_uuid') + @mock.patch.object(time, 'sleep') + def test_wait_for_port_status_up(self, sleep_mock, get_port_mock): + get_port_mock.return_value = {'status': 'ACTIVE'} + neutron.wait_for_port_status(self.client_mock, 'port_id', 'ACTIVE') + sleep_mock.assert_not_called() + + @mock.patch.object(neutron, '_get_port_by_uuid') + @mock.patch.object(time, 'sleep') + def test_wait_for_port_status_down(self, sleep_mock, get_port_mock): + get_port_mock.side_effect = [{'status': 'DOWN'}, {'status': 'ACTIVE'}] + neutron.wait_for_port_status(self.client_mock, 'port_id', 'ACTIVE') + sleep_mock.assert_called_once() + + @mock.patch.object(neutron, 'wait_for_host_agent', autospec=True) + @mock.patch.object(neutron, 'wait_for_port_status', autospec=True) + def test_add_smartnic_port_to_network( + self, wait_port_mock, wait_agent_mock): + # Ports will be created only if pxe_enabled is True + self.node.network_interface = 'neutron' + self.node.save() + object_utils.create_test_port( + self.context, node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address='52:54:00:cf:2d:22', + pxe_enabled=False + ) + port = self.ports[0] + + local_link_connection = port.local_link_connection + local_link_connection['hostname'] = 'hostname' + port.local_link_connection = local_link_connection + port.is_smartnic = True + port.save() + + expected_body = { + 'port': { + 'network_id': self.network_uuid, + 'admin_state_up': True, + 'binding:vnic_type': 'smart-nic', + 'device_owner': 'baremetal:none', + 'binding:host_id': port.local_link_connection['hostname'], + 'device_id': self.node.uuid, + 'mac_address': port.address, + 'binding:profile': { + 'local_link_information': [port.local_link_connection] + } + } + } + + # Ensure we can create ports + self.client_mock.create_port.return_value = { + 'port': self.neutron_port} + expected = {port.uuid: self.neutron_port['id']} + with task_manager.acquire(self.context, self.node.uuid) as task: + ports = neutron.add_ports_to_network(task, self.network_uuid) + self.assertEqual(expected, ports) + self.client_mock.create_port.assert_called_once_with( + expected_body) + wait_agent_mock.assert_called_once_with( + self.client_mock, 'hostname') + wait_port_mock.assert_called_once_with( + self.client_mock, self.neutron_port['id'], 'ACTIVE') + + @mock.patch.object(neutron, 'is_smartnic_port', autospec=True) + @mock.patch.object(neutron, 'wait_for_host_agent', autospec=True) + def test_remove_neutron_smartnic_ports( + self, wait_agent_mock, is_smartnic_mock): + with task_manager.acquire(self.context, self.node.uuid) as task: + is_smartnic_mock.return_value = True + self.neutron_port['binding:host_id'] = 'hostname' + self.client_mock.list_ports.return_value = { + 'ports': [self.neutron_port]} + neutron.remove_neutron_ports(task, {'param': 'value'}) + self.client_mock.list_ports.assert_called_once_with( + **{'param': 'value'}) + self.client_mock.delete_port.assert_called_once_with( + self.neutron_port['id']) + is_smartnic_mock.assert_called_once_with(self.neutron_port) + wait_agent_mock.assert_called_once_with(self.client_mock, 'hostname') + @mock.patch.object(neutron, 'get_client', autospec=True) class TestValidateNetwork(base.TestCase): diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index 8ae8777bc1..bfff40acca 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -9,11 +9,13 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import time import mock from oslo_config import cfg from oslo_utils import uuidutils +from ironic.common import boot_devices from ironic.common import boot_modes from ironic.common import exception from ironic.common import network @@ -2011,6 +2013,86 @@ class MiscTestCase(db_base.DbTestCase): mock_resume.assert_called_once_with( task, 'deploying', 'continue_node_deploy') + @mock.patch.object(time, 'sleep', autospec=True) + @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True) + @mock.patch.object(drivers_base.NetworkInterface, 'need_power_on') + @mock.patch.object(conductor_utils, 'node_set_boot_device', + autospec=True) + @mock.patch.object(conductor_utils, 'node_power_action', + autospec=True) + def test_power_on_node_if_needed_true( + self, power_action_mock, boot_device_mock, + need_power_on_mock, get_power_state_mock, time_mock): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + need_power_on_mock.return_value = True + get_power_state_mock.return_value = states.POWER_OFF + power_state = conductor_utils.power_on_node_if_needed(task) + self.assertEqual(power_state, states.POWER_OFF) + boot_device_mock.assert_called_once_with( + task, boot_devices.BIOS, persistent=False) + power_action_mock.assert_called_once_with(task, states.POWER_ON) + + @mock.patch.object(time, 'sleep', autospec=True) + @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True) + @mock.patch.object(drivers_base.NetworkInterface, 'need_power_on') + @mock.patch.object(conductor_utils, 'node_set_boot_device', + autospec=True) + @mock.patch.object(conductor_utils, 'node_power_action', + autospec=True) + def test_power_on_node_if_needed_false_power_on( + self, power_action_mock, boot_device_mock, + need_power_on_mock, get_power_state_mock, time_mock): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + need_power_on_mock.return_value = True + get_power_state_mock.return_value = states.POWER_ON + power_state = conductor_utils.power_on_node_if_needed(task) + self.assertIsNone(power_state) + self.assertEqual(0, boot_device_mock.call_count) + self.assertEqual(0, power_action_mock.call_count) + + @mock.patch.object(time, 'sleep', autospec=True) + @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True) + @mock.patch.object(drivers_base.NetworkInterface, 'need_power_on') + @mock.patch.object(conductor_utils, 'node_set_boot_device', + autospec=True) + @mock.patch.object(conductor_utils, 'node_power_action', + autospec=True) + def test_power_on_node_if_needed_false_no_need( + self, power_action_mock, boot_device_mock, + need_power_on_mock, get_power_state_mock, time_mock): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + need_power_on_mock.return_value = False + get_power_state_mock.return_value = states.POWER_OFF + power_state = conductor_utils.power_on_node_if_needed(task) + self.assertIsNone(power_state) + self.assertEqual(0, boot_device_mock.call_count) + self.assertEqual(0, power_action_mock.call_count) + + @mock.patch.object(time, 'sleep', autospec=True) + @mock.patch.object(conductor_utils, 'node_power_action', + autospec=True) + def test_restore_power_state_if_needed_true( + self, power_action_mock, time_mock): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + power_state = states.POWER_OFF + conductor_utils.restore_power_state_if_needed(task, power_state) + power_action_mock.assert_called_once_with(task, power_state) + + @mock.patch.object(time, 'sleep', autospec=True) + @mock.patch.object(conductor_utils, 'node_power_action', + autospec=True) + def test_restore_power_state_if_needed_false( + self, power_action_mock, time_mock): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + power_state = None + conductor_utils.restore_power_state_if_needed(task, power_state) + self.assertEqual(0, power_action_mock.call_count) + class ValidateInstanceInfoTraitsTestCase(tests_base.TestCase): diff --git a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py index 0e559708a2..69237b312a 100644 --- a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py +++ b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py @@ -23,6 +23,7 @@ from ironic.conductor import utils from ironic.drivers.modules.ansible import deploy as ansible_deploy from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import fake +from ironic.drivers.modules.network import flat as flat_network from ironic.drivers.modules import pxe from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as object_utils @@ -792,11 +793,13 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): run_playbook_mock.assert_called_once_with( task.node, 'test_pl', ironic_nodes, 'test_k') + @mock.patch.object(utils, 'power_on_node_if_needed', autospec=True) @mock.patch.object(fake.FakePower, 'get_power_state', return_value=states.POWER_OFF) @mock.patch.object(utils, 'node_power_action', autospec=True) - def test_reboot_and_finish_deploy_force_reboot(self, power_action_mock, - get_pow_state_mock): + def test_reboot_and_finish_deploy_force_reboot( + self, power_action_mock, get_pow_state_mock, + power_on_node_if_needed_mock): d_info = self.node.driver_info d_info['deploy_forces_oob_reboot'] = True self.node.driver_info = d_info @@ -806,6 +809,7 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): self.node.provision_state = states.DEPLOYING self.node.save() + power_on_node_if_needed_mock.return_value = None with task_manager.acquire(self.context, self.node.uuid) as task: with mock.patch.object(task.driver, 'network') as net_mock: self.driver.reboot_and_finish_deploy(task) @@ -819,11 +823,12 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): power_action_mock.call_args_list) get_pow_state_mock.assert_not_called() + @mock.patch.object(utils, 'power_on_node_if_needed', autospec=True) @mock.patch.object(ansible_deploy, '_run_playbook', autospec=True) @mock.patch.object(utils, 'node_power_action', autospec=True) - def test_reboot_and_finish_deploy_soft_poweroff_retry(self, - power_action_mock, - run_playbook_mock): + def test_reboot_and_finish_deploy_soft_poweroff_retry( + self, power_action_mock, run_playbook_mock, + power_on_node_if_needed_mock): self.config(group='ansible', post_deploy_get_power_state_retry_interval=0) self.config(group='ansible', @@ -834,6 +839,7 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): self.node.driver_internal_info = di_info self.node.save() + power_on_node_if_needed_mock.return_value = None with task_manager.acquire(self.context, self.node.uuid) as task: with mock.patch.object(task.driver, 'network') as net_mock: with mock.patch.object(task.driver.power, @@ -916,3 +922,141 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): task) task.driver.boot.clean_up_ramdisk.assert_called_once_with( task) + + @mock.patch.object(utils, 'restore_power_state_if_needed', autospec=True) + @mock.patch.object(utils, 'power_on_node_if_needed') + @mock.patch.object(utils, 'node_power_action', autospec=True) + def test_tear_down_with_smartnic_port( + self, power_mock, power_on_node_if_needed_mock, + restore_power_state_mock): + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + power_on_node_if_needed_mock.return_value = states.POWER_OFF + driver_return = self.driver.tear_down(task) + power_mock.assert_called_once_with(task, states.POWER_OFF) + self.assertEqual(driver_return, states.DELETED) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + + @mock.patch.object(flat_network.FlatNetwork, 'add_provisioning_network', + autospec=True) + @mock.patch.object(utils, 'restore_power_state_if_needed', autospec=True) + @mock.patch.object(utils, 'power_on_node_if_needed', autospec=True) + @mock.patch.object(utils, 'node_power_action', 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(pxe.PXEBoot, 'prepare_ramdisk') + def test_prepare_with_smartnic_port( + self, pxe_prepare_ramdisk_mock, + build_instance_info_mock, build_options_mock, + power_action_mock, power_on_node_if_needed_mock, + restore_power_state_mock, net_mock): + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + task.node.provision_state = states.DEPLOYING + build_instance_info_mock.return_value = {'test': 'test'} + build_options_mock.return_value = {'op1': 'test1'} + power_on_node_if_needed_mock.return_value = states.POWER_OFF + self.driver.prepare(task) + power_action_mock.assert_called_once_with( + task, states.POWER_OFF) + build_instance_info_mock.assert_called_once_with(task) + build_options_mock.assert_called_once_with(task.node) + pxe_prepare_ramdisk_mock.assert_called_once_with( + task, {'op1': 'test1'}) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + + self.node.refresh() + self.assertEqual('test', self.node.instance_info['test']) + + @mock.patch.object(utils, 'restore_power_state_if_needed', autospec=True) + @mock.patch.object(utils, 'power_on_node_if_needed', autospec=True) + @mock.patch.object(ansible_deploy, '_run_playbook', autospec=True) + @mock.patch.object(utils, 'set_node_cleaning_steps', autospec=True) + @mock.patch.object(utils, 'node_power_action', autospec=True) + @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk') + def test_prepare_cleaning_with_smartnic_port( + self, prepare_ramdisk_mock, build_options_mock, power_action_mock, + set_node_cleaning_steps, run_playbook_mock, + power_on_node_if_needed_mock, restore_power_state_mock): + step = {'priority': 10, 'interface': 'deploy', + 'step': 'erase_devices', 'tags': ['clean']} + driver_internal_info = dict(DRIVER_INTERNAL_INFO) + driver_internal_info['clean_steps'] = [step] + self.node.driver_internal_info = driver_internal_info + self.node.save() + + with task_manager.acquire(self.context, self.node.uuid) as task: + task.driver.network.add_cleaning_network = mock.Mock() + build_options_mock.return_value = {'op1': 'test1'} + power_on_node_if_needed_mock.return_value = states.POWER_OFF + state = self.driver.prepare_cleaning(task) + set_node_cleaning_steps.assert_called_once_with(task) + task.driver.network.add_cleaning_network.assert_called_once_with( + task) + build_options_mock.assert_called_once_with(task.node) + prepare_ramdisk_mock.assert_called_once_with( + task, {'op1': 'test1'}) + power_action_mock.assert_called_once_with(task, states.REBOOT) + self.assertFalse(run_playbook_mock.called) + self.assertEqual(states.CLEANWAIT, state) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + + @mock.patch.object(utils, 'restore_power_state_if_needed', autospec=True) + @mock.patch.object(utils, 'power_on_node_if_needed', autospec=True) + @mock.patch.object(utils, 'node_power_action', autospec=True) + @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk') + def test_tear_down_cleaning_with_smartnic_port( + self, clean_ramdisk_mock, power_action_mock, + power_on_node_if_needed_mock, restore_power_state_mock): + with task_manager.acquire(self.context, self.node.uuid) as task: + task.driver.network.remove_cleaning_network = mock.Mock() + power_on_node_if_needed_mock.return_value = states.POWER_OFF + self.driver.tear_down_cleaning(task) + power_action_mock.assert_called_once_with(task, states.POWER_OFF) + clean_ramdisk_mock.assert_called_once_with(task) + (task.driver.network.remove_cleaning_network + .assert_called_once_with(task)) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + + @mock.patch.object(flat_network.FlatNetwork, 'remove_provisioning_network', + autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'configure_tenant_networks', + autospec=True) + @mock.patch.object(utils, 'restore_power_state_if_needed', autospec=True) + @mock.patch.object(utils, 'power_on_node_if_needed', autospec=True) + @mock.patch.object(fake.FakePower, 'get_power_state', + return_value=states.POWER_OFF) + @mock.patch.object(utils, 'node_power_action', autospec=True) + def test_reboot_and_finish_deploy_with_smartnic_port( + self, power_action_mock, get_pow_state_mock, + power_on_node_if_needed_mock, restore_power_state_mock, + configure_tenant_networks_mock, remove_provisioning_network_mock): + d_info = self.node.driver_info + d_info['deploy_forces_oob_reboot'] = True + self.node.driver_info = d_info + self.node.save() + self.config(group='ansible', + post_deploy_get_power_state_retry_interval=0) + self.node.provision_state = states.DEPLOYING + self.node.save() + power_on_node_if_needed_mock.return_value = states.POWER_OFF + with task_manager.acquire(self.context, self.node.uuid) as task: + self.driver.reboot_and_finish_deploy(task) + expected_power_calls = [((task, states.POWER_OFF),), + ((task, states.POWER_ON),)] + self.assertEqual( + expected_power_calls, power_action_mock.call_args_list) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + get_pow_state_mock.assert_not_called() diff --git a/ironic/tests/unit/drivers/modules/network/test_common.py b/ironic/tests/unit/drivers/modules/network/test_common.py index ac9d4e4a4c..b8b4182300 100644 --- a/ironic/tests/unit/drivers/modules/network/test_common.py +++ b/ironic/tests/unit/drivers/modules/network/test_common.py @@ -430,6 +430,26 @@ class TestCommonFunctions(db_base.DbTestCase): common.plug_port_to_tenant_network, task, self.port) + @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True) + @mock.patch.object(neutron_common, 'wait_for_port_status', autospec=True) + @mock.patch.object(neutron_common, 'get_client', autospec=True) + def test_plug_port_to_tenant_network_smartnic_port( + self, mock_gc, wait_port_mock, wait_agent_mock): + nclient = mock.MagicMock() + mock_gc.return_value = nclient + local_link_connection = self.port.local_link_connection + local_link_connection['hostname'] = 'hostname' + self.port.local_link_connection = local_link_connection + self.port.internal_info = {common.TENANT_VIF_KEY: self.vif_id} + self.port.is_smartnic = True + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + common.plug_port_to_tenant_network(task, self.port) + wait_agent_mock.assert_called_once_with( + nclient, 'hostname') + wait_port_mock.assert_called_once_with( + nclient, self.vif_id, 'ACTIVE') + class TestVifPortIDMixin(db_base.DbTestCase): diff --git a/ironic/tests/unit/drivers/modules/network/test_neutron.py b/ironic/tests/unit/drivers/modules/network/test_neutron.py index 39c7fd9d80..28872a5d91 100644 --- a/ironic/tests/unit/drivers/modules/network/test_neutron.py +++ b/ironic/tests/unit/drivers/modules/network/test_neutron.py @@ -545,6 +545,24 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): mock_unbind_port.assert_called_once_with( self.port.extra['vif_port_id'], context=task.context) + @mock.patch.object(neutron_common, 'get_client') + @mock.patch.object(neutron_common, 'wait_for_host_agent') + @mock.patch.object(neutron_common, 'unbind_neutron_port') + def test_unconfigure_tenant_networks_smartnic( + self, mock_unbind_port, wait_agent_mock, client_mock): + nclient = mock.MagicMock() + client_mock.return_value = nclient + local_link_connection = self.port.local_link_connection + local_link_connection['hostname'] = 'hostname' + self.port.local_link_connection = local_link_connection + self.port.is_smartnic = True + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.unconfigure_tenant_networks(task) + mock_unbind_port.assert_called_once_with( + self.port.extra['vif_port_id'], context=task.context) + wait_agent_mock.assert_called_once_with(nclient, 'hostname') + def test_configure_tenant_networks_no_ports_for_node(self): n = utils.create_test_node(self.context, network_interface='neutron', uuid=uuidutils.generate_uuid()) @@ -571,10 +589,11 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.assertIn('No neutron ports or portgroups are associated with', log_mock.error.call_args[0][0]) + @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True) @mock.patch.object(neutron_common, 'get_client') @mock.patch.object(neutron, 'LOG') def test_configure_tenant_networks_multiple_ports_one_vif_id( - self, log_mock, client_mock): + self, log_mock, client_mock, wait_agent_mock): expected_body = { 'port': { 'binding:vnic_type': 'baremetal', @@ -592,8 +611,10 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): upd_mock.assert_called_once_with(self.port.extra['vif_port_id'], expected_body) + @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True) @mock.patch.object(neutron_common, 'get_client') - def test_configure_tenant_networks_update_fail(self, client_mock): + def test_configure_tenant_networks_update_fail(self, client_mock, + wait_agent_mock): client = client_mock.return_value client.update_port.side_effect = neutron_exceptions.ConnectionFailed( reason='meow') @@ -603,8 +624,10 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.interface.configure_tenant_networks, task) client_mock.assert_called_once_with(context=task.context) + @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True) @mock.patch.object(neutron_common, 'get_client') - def _test_configure_tenant_networks(self, client_mock, is_client_id=False, + def _test_configure_tenant_networks(self, client_mock, wait_agent_mock, + is_client_id=False, vif_int_info=False): upd_mock = mock.Mock() client_mock.return_value.update_port = upd_mock @@ -687,11 +710,12 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.node.save() self._test_configure_tenant_networks(is_client_id=True) + @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True) @mock.patch.object(neutron_common, 'get_client', autospec=True) @mock.patch.object(neutron_common, 'get_local_group_information', autospec=True) def test_configure_tenant_networks_with_portgroups( - self, glgi_mock, client_mock): + self, glgi_mock, client_mock, wait_agent_mock): pg = utils.create_test_portgroup( self.context, node_id=self.node.id, address='ff:54:00:cf:2d:32', extra={'vif_port_id': uuidutils.generate_uuid()}) @@ -745,3 +769,13 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): [mock.call(self.port.extra['vif_port_id'], call1_body), mock.call(pg.extra['vif_port_id'], call2_body)] ) + + def test_need_power_on_true(self): + self.port.is_smartnic = True + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + self.assertTrue(self.interface.need_power_on(task)) + + def test_need_power_on_false(self): + with task_manager.acquire(self.context, self.node.id) as task: + self.assertFalse(self.interface.need_power_on(task)) diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 5e893a98ca..09ea886064 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -1001,6 +1001,8 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertEqual(states.ACTIVE, task.node.target_provision_state) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) @mock.patch.object(deploy_utils, 'remove_http_instance_symlink', autospec=True) @mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True) @@ -1018,7 +1020,8 @@ class TestAgentDeploy(db_base.DbTestCase): def test_reboot_to_instance(self, check_deploy_mock, prepare_instance_mock, power_off_mock, get_power_state_mock, node_power_action_mock, - uuid_mock, log_mock, remove_symlink_mock): + uuid_mock, log_mock, remove_symlink_mock, + power_on_node_if_needed_mock): self.config(manage_agent_boot=True, group='agent') self.config(image_download_source='http', group='agent') check_deploy_mock.return_value = None @@ -1029,6 +1032,7 @@ class TestAgentDeploy(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: get_power_state_mock.return_value = states.POWER_OFF + power_on_node_if_needed_mock.return_value = None task.node.driver_internal_info['is_whole_disk_image'] = True task.driver.deploy.reboot_to_instance(task) check_deploy_mock.assert_called_once_with(mock.ANY, task.node) @@ -1048,6 +1052,8 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertEqual(states.NOSTATE, task.node.target_provision_state) self.assertTrue(remove_symlink_mock.called) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) @mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True) @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True) @mock.patch.object(agent.AgentDeployMixin, '_get_uuid_from_result', @@ -1061,13 +1067,10 @@ class TestAgentDeploy(db_base.DbTestCase): autospec=True) @mock.patch('ironic.drivers.modules.agent.AgentDeployMixin' '.check_deploy_success', autospec=True) - def test_reboot_to_instance_no_manage_agent_boot(self, check_deploy_mock, - prepare_instance_mock, - power_off_mock, - get_power_state_mock, - node_power_action_mock, - uuid_mock, bootdev_mock, - log_mock): + def test_reboot_to_instance_no_manage_agent_boot( + self, check_deploy_mock, prepare_instance_mock, power_off_mock, + get_power_state_mock, node_power_action_mock, uuid_mock, + bootdev_mock, log_mock, power_on_node_if_needed_mock): self.config(manage_agent_boot=False, group='agent') check_deploy_mock.return_value = None uuid_mock.return_value = None @@ -1076,6 +1079,7 @@ class TestAgentDeploy(db_base.DbTestCase): self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + power_on_node_if_needed_mock.return_value = None get_power_state_mock.return_value = states.POWER_OFF task.node.driver_internal_info['is_whole_disk_image'] = True task.driver.deploy.reboot_to_instance(task) @@ -1093,6 +1097,8 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) @mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True) @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', autospec=True) @@ -1113,7 +1119,8 @@ class TestAgentDeploy(db_base.DbTestCase): get_power_state_mock, node_power_action_mock, uuid_mock, boot_mode_mock, - log_mock): + log_mock, + power_on_node_if_needed_mock): check_deploy_mock.return_value = None uuid_mock.return_value = 'root_uuid' self.node.provision_state = states.DEPLOYWAIT @@ -1122,6 +1129,7 @@ class TestAgentDeploy(db_base.DbTestCase): boot_mode_mock.return_value = 'bios' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + power_on_node_if_needed_mock.return_value = None get_power_state_mock.return_value = states.POWER_OFF driver_internal_info = task.node.driver_internal_info driver_internal_info['is_whole_disk_image'] = False @@ -1146,6 +1154,8 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) @mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True) @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', autospec=True) @@ -1163,7 +1173,8 @@ class TestAgentDeploy(db_base.DbTestCase): def test_reboot_to_instance_partition_localboot_ppc64( self, check_deploy_mock, prepare_instance_mock, power_off_mock, get_power_state_mock, - node_power_action_mock, uuid_mock, boot_mode_mock, log_mock): + node_power_action_mock, uuid_mock, boot_mode_mock, log_mock, + power_on_node_if_needed_mock): check_deploy_mock.return_value = None uuid_mock.side_effect = ['root_uuid', 'prep_boot_part_uuid'] self.node.provision_state = states.DEPLOYWAIT @@ -1172,6 +1183,7 @@ class TestAgentDeploy(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + power_on_node_if_needed_mock.return_value = None get_power_state_mock.return_value = states.POWER_OFF driver_internal_info = task.node.driver_internal_info driver_internal_info['is_whole_disk_image'] = False @@ -1238,6 +1250,8 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) @mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True) @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', autospec=True) @@ -1258,7 +1272,8 @@ class TestAgentDeploy(db_base.DbTestCase): get_power_state_mock, node_power_action_mock, uuid_mock, boot_mode_mock, - log_mock): + log_mock, + power_on_node_if_needed_mock): check_deploy_mock.return_value = None uuid_mock.side_effect = ['root_uuid', 'efi_uuid'] self.node.provision_state = states.DEPLOYWAIT @@ -1267,6 +1282,7 @@ class TestAgentDeploy(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + power_on_node_if_needed_mock.return_value = None get_power_state_mock.return_value = states.POWER_OFF driver_internal_info = task.node.driver_internal_info driver_internal_info['is_whole_disk_image'] = False @@ -1367,6 +1383,125 @@ class TestAgentDeploy(db_base.DbTestCase): 'command_status': 'RUNNING'}] self.assertFalse(task.driver.deploy.deploy_is_done(task)) + @mock.patch.object(manager_utils, 'restore_power_state_if_needed', + autospec=True) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) + @mock.patch.object(noop_storage.NoopStorage, 'attach_volumes', + autospec=True) + @mock.patch.object(deploy_utils, 'populate_storage_driver_internal_info') + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk') + @mock.patch.object(deploy_utils, 'build_agent_options') + @mock.patch.object(deploy_utils, 'build_instance_info_for_deploy') + @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_with_smartnic_port( + 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, power_on_node_if_needed_mock, + restore_power_state_mock): + node = self.node + node.network_interface = 'flat' + node.save() + add_provisioning_net_mock.return_value = None + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + task.node.provision_state = states.DEPLOYING + build_instance_info_mock.return_value = {'foo': 'bar'} + build_options_mock.return_value = {'a': 'b'} + power_on_node_if_needed_mock.return_value = states.POWER_OFF + 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_called_once_with(task) + build_options_mock.assert_called_once_with(task.node) + pxe_prepare_ramdisk_mock.assert_called_once_with( + task, {'a': 'b'}) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + self.node.refresh() + self.assertEqual('bar', self.node.instance_info['foo']) + + @mock.patch.object(manager_utils, 'restore_power_state_if_needed', + autospec=True) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) + @mock.patch.object(noop_storage.NoopStorage, 'detach_volumes', + autospec=True) + @mock.patch.object(flat_network.FlatNetwork, + 'remove_provisioning_network', + spec_set=True, autospec=True) + @mock.patch.object(flat_network.FlatNetwork, + 'unconfigure_tenant_networks', + spec_set=True, autospec=True) + @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) + def test_tear_down_with_smartnic_port( + self, power_mock, unconfigure_tenant_nets_mock, + remove_provisioning_net_mock, storage_detach_volumes_mock, + power_on_node_if_needed_mock, restore_power_state_mock): + object_utils.create_test_volume_target( + self.context, node_id=self.node.id) + node = self.node + node.network_interface = 'flat' + node.save() + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + power_on_node_if_needed_mock.return_value = states.POWER_OFF + driver_return = self.driver.tear_down(task) + power_mock.assert_called_once_with(task, states.POWER_OFF) + self.assertEqual(driver_return, states.DELETED) + unconfigure_tenant_nets_mock.assert_called_once_with(mock.ANY, + task) + remove_provisioning_net_mock.assert_called_once_with(mock.ANY, + task) + storage_detach_volumes_mock.assert_called_once_with( + task.driver.storage, task) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + # Verify no volumes exist for new task instances. + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + self.assertEqual(0, len(task.volume_targets)) + + @mock.patch.object(manager_utils, 'restore_power_state_if_needed', + autospec=True) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) + @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) + @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', + autospec=True) + def test_deploy_storage_should_write_image_false_with_smartnic_port( + self, mock_write, mock_pxe_instance, + power_on_node_if_needed_mock, restore_power_state_mock): + 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: + power_on_node_if_needed_mock.return_value = states.POWER_OFF + driver_return = self.driver.deploy(task) + self.assertIsNone(driver_return) + self.assertTrue(mock_pxe_instance.called) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + class AgentRAIDTestCase(db_base.DbTestCase): @@ -1807,3 +1942,74 @@ class AgentRescueTestCase(db_base.DbTestCase): self.assertNotIn('rescue_password', task.node.instance_info) self.assertFalse(mock_clean_ramdisk.called) mock_remove_rescue_net.assert_called_once_with(mock.ANY, task) + + @mock.patch.object(manager_utils, 'restore_power_state_if_needed', + autospec=True) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'add_rescuing_network', + spec_set=True, autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'unconfigure_tenant_networks', + spec_set=True, autospec=True) + @mock.patch.object(fake.FakeBoot, 'prepare_ramdisk', autospec=True) + @mock.patch.object(fake.FakeBoot, 'clean_up_instance', autospec=True) + @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + def test_agent_rescue_with_smartnic_port( + self, mock_node_power_action, mock_build_agent_opts, + mock_clean_up_instance, mock_prepare_ramdisk, + mock_unconf_tenant_net, mock_add_rescue_net, + power_on_node_if_needed_mock, restore_power_state_mock): + self.config(manage_agent_boot=True, group='agent') + mock_build_agent_opts.return_value = {'ipa-api-url': 'fake-api'} + with task_manager.acquire(self.context, self.node.uuid) as task: + power_on_node_if_needed_mock.return_value = states.POWER_OFF + result = task.driver.rescue.rescue(task) + mock_node_power_action.assert_has_calls( + [mock.call(task, states.POWER_OFF), + mock.call(task, states.POWER_ON)]) + mock_clean_up_instance.assert_called_once_with(mock.ANY, task) + mock_unconf_tenant_net.assert_called_once_with(mock.ANY, task) + mock_add_rescue_net.assert_called_once_with(mock.ANY, task) + mock_build_agent_opts.assert_called_once_with(task.node) + mock_prepare_ramdisk.assert_called_once_with( + mock.ANY, task, {'ipa-api-url': 'fake-api'}) + self.assertEqual(states.RESCUEWAIT, result) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + + @mock.patch.object(manager_utils, 'restore_power_state_if_needed', + autospec=True) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'remove_rescuing_network', + spec_set=True, autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'configure_tenant_networks', + spec_set=True, autospec=True) + @mock.patch.object(fake.FakeBoot, 'prepare_instance', autospec=True) + @mock.patch.object(fake.FakeBoot, 'clean_up_ramdisk', autospec=True) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + def test_agent_unrescue_with_smartnic_port( + self, mock_node_power_action, mock_clean_ramdisk, + mock_prepare_instance, mock_conf_tenant_net, + mock_remove_rescue_net, power_on_node_if_needed_mock, + restore_power_state_mock): + self.config(manage_agent_boot=True, group='agent') + with task_manager.acquire(self.context, self.node.uuid) as task: + power_on_node_if_needed_mock.return_value = states.POWER_OFF + result = task.driver.rescue.unrescue(task) + mock_node_power_action.assert_has_calls( + [mock.call(task, states.POWER_OFF), + mock.call(task, states.POWER_ON)]) + mock_clean_ramdisk.assert_called_once_with( + mock.ANY, task) + mock_remove_rescue_net.assert_called_once_with(mock.ANY, task) + mock_conf_tenant_net.assert_called_once_with(mock.ANY, task) + mock_prepare_instance.assert_called_once_with(mock.ANY, task) + self.assertEqual(states.ACTIVE, result) + self.assertEqual(2, power_on_node_if_needed_mock.call_count) + self.assertEqual(2, power_on_node_if_needed_mock.call_count) + restore_power_state_mock.assert_has_calls( + [mock.call(task, states.POWER_OFF), + mock.call(task, states.POWER_OFF)]) 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 cc071bb4cd..41b3391d44 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py @@ -424,8 +424,38 @@ class AgentRescueTests(AgentDeployMixinBaseTest): self.deploy._finalize_rescue, task) mock_finalize_rescue.assert_called_once_with(task.node) + @mock.patch.object(manager_utils, 'restore_power_state_if_needed', + autospec=True) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) + @mock.patch.object(agent.AgentRescue, 'clean_up', + spec_set=True, autospec=True) + @mock.patch.object(agent_client.AgentClient, 'finalize_rescue', + spec=types.FunctionType) + def test__finalize_rescue_with_smartnic_port( + self, mock_finalize_rescue, mock_clean_up, + power_on_node_if_needed_mock, restore_power_state_mock): + node = self.node + node.provision_state = states.RESCUEWAIT + node.save() + mock_finalize_rescue.return_value = {'command_status': 'SUCCEEDED'} + with task_manager.acquire(self.context, self.node['uuid'], + shared=False) as task: + task.driver.network.configure_tenant_networks = mock.Mock() + task.process_event = mock.Mock() + power_on_node_if_needed_mock.return_value = states.POWER_OFF + self.deploy._finalize_rescue(task) + mock_finalize_rescue.assert_called_once_with(task.node) + task.process_event.assert_has_calls([mock.call('resume'), + mock.call('done')]) + mock_clean_up.assert_called_once_with(mock.ANY, task) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + class AgentDeployMixinTest(AgentDeployMixinBaseTest): + @mock.patch.object(manager_utils, 'power_on_node_if_needed') @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @@ -437,7 +467,8 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): spec=types.FunctionType) def test_reboot_and_finish_deploy( self, power_off_mock, get_power_state_mock, - node_power_action_mock, collect_mock, resume_mock): + node_power_action_mock, collect_mock, resume_mock, + power_on_node_if_needed_mock): cfg.CONF.set_override('deploy_logs_collect', 'always', 'agent') self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE @@ -448,6 +479,8 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): shared=True) as task: get_power_state_mock.side_effect = [states.POWER_ON, states.POWER_OFF] + + power_on_node_if_needed_mock.return_value = None 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) @@ -458,6 +491,8 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): collect_mock.assert_called_once_with(task.node) resume_mock.assert_called_once_with(task) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @@ -469,7 +504,8 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): spec=types.FunctionType) def test_reboot_and_finish_deploy_deprecated( self, power_off_mock, get_power_state_mock, - node_power_action_mock, collect_mock, resume_mock): + node_power_action_mock, collect_mock, resume_mock, + power_on_node_if_needed_mock): # TODO(rloo): no deploy steps; delete this when we remove support # for handling no deploy steps. cfg.CONF.set_override('deploy_logs_collect', 'always', 'agent') @@ -481,6 +517,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): shared=True) as task: get_power_state_mock.side_effect = [states.POWER_ON, states.POWER_OFF] + power_on_node_if_needed_mock.return_value = None 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) @@ -491,6 +528,8 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): collect_mock.assert_called_once_with(task.node) self.assertFalse(resume_mock.called) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(time, 'sleep', lambda seconds: None) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @@ -505,12 +544,14 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): def test_reboot_and_finish_deploy_soft_poweroff_doesnt_complete( self, configure_tenant_net_mock, remove_provisioning_net_mock, power_off_mock, get_power_state_mock, - node_power_action_mock, mock_collect): + node_power_action_mock, mock_collect, + power_on_node_if_needed_mock): self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: + power_on_node_if_needed_mock.return_value = None get_power_state_mock.return_value = states.POWER_ON self.deploy.reboot_and_finish_deploy(task) power_off_mock.assert_called_once_with(task.node) @@ -554,6 +595,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.assertEqual(states.NOSTATE, task.node.target_provision_state) self.assertFalse(mock_collect.called) + @mock.patch.object(manager_utils, 'power_on_node_if_needed') @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(time, 'sleep', lambda seconds: None) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @@ -568,13 +610,14 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): def test_reboot_and_finish_deploy_get_power_state_fails( self, configure_tenant_net_mock, remove_provisioning_net_mock, power_off_mock, get_power_state_mock, node_power_action_mock, - mock_collect): + mock_collect, power_on_node_if_needed_mock): self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: get_power_state_mock.side_effect = RuntimeError("boom") + power_on_node_if_needed_mock.return_value = None 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) @@ -588,6 +631,8 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.assertEqual(states.NOSTATE, task.node.target_provision_state) self.assertFalse(mock_collect.called) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(time, 'sleep', lambda seconds: None) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @@ -602,11 +647,12 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): def test_reboot_and_finish_deploy_configure_tenant_network_exception( self, configure_tenant_net_mock, remove_provisioning_net_mock, power_off_mock, get_power_state_mock, node_power_action_mock, - mock_collect): + mock_collect, power_on_node_if_needed_mock): self.node.network_interface = 'neutron' self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() + power_on_node_if_needed_mock.return_value = None with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: configure_tenant_net_mock.side_effect = exception.NetworkError( @@ -649,6 +695,8 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.assertEqual(states.ACTIVE, task.node.target_provision_state) mock_collect.assert_called_once_with(task.node) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(time, 'sleep', lambda seconds: None) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @@ -663,12 +711,14 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): def test_reboot_and_finish_deploy_power_on_fails( self, configure_tenant_net_mock, remove_provisioning_net_mock, power_off_mock, get_power_state_mock, - node_power_action_mock, mock_collect): + node_power_action_mock, mock_collect, + power_on_node_if_needed_mock): self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: + power_on_node_if_needed_mock.return_value = None get_power_state_mock.return_value = states.POWER_ON node_power_action_mock.side_effect = [None, RuntimeError("boom")] @@ -1398,6 +1448,46 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): hook_returned = agent_base_vendor._get_post_clean_step_hook(self.node) self.assertIsNone(hook_returned) + @mock.patch.object(manager_utils, 'restore_power_state_if_needed', + autospec=True) + @mock.patch.object(manager_utils, 'power_on_node_if_needed') + @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', + autospec=True) + @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) + @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', + spec=types.FunctionType) + @mock.patch.object(agent_client.AgentClient, 'power_off', + spec=types.FunctionType) + def test_reboot_and_finish_deploy_with_smartnic_port( + self, power_off_mock, get_power_state_mock, + node_power_action_mock, collect_mock, resume_mock, + power_on_node_if_needed_mock, restore_power_state_mock): + cfg.CONF.set_override('deploy_logs_collect', 'always', 'agent') + self.node.provision_state = states.DEPLOYING + self.node.target_provision_state = states.ACTIVE + self.node.deploy_step = { + 'step': 'deploy', 'priority': 50, 'interface': 'deploy'} + self.node.save() + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + get_power_state_mock.side_effect = [states.POWER_ON, + states.POWER_OFF] + power_on_node_if_needed_mock.return_value = states.POWER_OFF + 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( + task, states.POWER_ON) + self.assertEqual(states.DEPLOYING, task.node.provision_state) + self.assertEqual(states.ACTIVE, task.node.target_provision_state) + collect_mock.assert_called_once_with(task.node) + resume_mock.assert_called_once_with(task) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + class TestRefreshCleanSteps(AgentDeployMixinBaseTest): diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index e1712d40c8..d9d5826c2b 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -943,6 +943,124 @@ class ISCSIDeployTestCase(db_base.DbTestCase): set_boot_device_mock.assert_called_once_with( mock.ANY, task, device=boot_devices.DISK, persistent=True) + @mock.patch.object(manager_utils, 'restore_power_state_if_needed', + autospec=True) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', 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(deploy_utils, 'build_agent_options', autospec=True) + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', 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) + def test_prepare_node_deploying_with_smartnic_port( + self, unconfigure_tenant_net_mock, add_provisioning_net_mock, + mock_prepare_ramdisk, mock_agent_options, + storage_driver_info_mock, storage_attach_volumes_mock, + power_on_node_if_needed_mock, restore_power_state_mock): + mock_agent_options.return_value = {'c': 'd'} + with task_manager.acquire(self.context, self.node.uuid) as task: + task.node.provision_state = states.DEPLOYING + power_on_node_if_needed_mock.return_value = states.POWER_OFF + task.driver.deploy.prepare(task) + mock_agent_options.assert_called_once_with(task.node) + mock_prepare_ramdisk.assert_called_once_with( + task.driver.boot, task, {'c': 'd'}) + add_provisioning_net_mock.assert_called_once_with(mock.ANY, task) + unconfigure_tenant_net_mock.assert_called_once_with(mock.ANY, task) + storage_driver_info_mock.assert_called_once_with(task) + storage_attach_volumes_mock.assert_called_once_with( + task.driver.storage, task) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + + @mock.patch.object(manager_utils, 'restore_power_state_if_needed', + autospec=True) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', autospec=True) + @mock.patch.object(noop_storage.NoopStorage, 'detach_volumes', + autospec=True) + @mock.patch.object(flat_network.FlatNetwork, + 'remove_provisioning_network', + spec_set=True, autospec=True) + @mock.patch.object(flat_network.FlatNetwork, + 'unconfigure_tenant_networks', + spec_set=True, autospec=True) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + def test_tear_down_with_smartnic_port( + self, node_power_action_mock, unconfigure_tenant_nets_mock, + remove_provisioning_net_mock, storage_detach_volumes_mock, + power_on_node_if_needed_mock, restore_power_state_mock): + obj_utils.create_test_volume_target( + self.context, node_id=self.node.id) + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + power_on_node_if_needed_mock.return_value = states.POWER_OFF + state = task.driver.deploy.tear_down(task) + self.assertEqual(state, states.DELETED) + node_power_action_mock.assert_called_once_with( + task, states.POWER_OFF) + unconfigure_tenant_nets_mock.assert_called_once_with( + mock.ANY, task) + remove_provisioning_net_mock.assert_called_once_with( + mock.ANY, task) + storage_detach_volumes_mock.assert_called_once_with( + task.driver.storage, task) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + # Verify no volumes exist for new task instances. + with task_manager.acquire(self.context, + self.node.uuid, shared=False) as task: + self.assertEqual(0, len(task.volume_targets)) + + @mock.patch.object(manager_utils, 'restore_power_state_if_needed', + autospec=True) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', autospec=True) + @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', + autospec=True) + @mock.patch.object(flat_network.FlatNetwork, + 'configure_tenant_networks', + spec_set=True, autospec=True) + @mock.patch.object(flat_network.FlatNetwork, + 'remove_provisioning_network', + spec_set=True, autospec=True) + @mock.patch.object(pxe.PXEBoot, + 'prepare_instance', + spec_set=True, autospec=True) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + @mock.patch.object(iscsi_deploy, 'check_image_size', autospec=True) + @mock.patch.object(deploy_utils, 'cache_instance_image', autospec=True) + def test_deploy_storage_check_write_image_false_with_smartnic_port( + self, mock_cache_instance_image, mock_check_image_size, + mock_node_power_action, mock_prepare_instance, + mock_remove_network, mock_tenant_network, mock_write, + power_on_node_if_needed_mock, restore_power_state_mock): + 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: + power_on_node_if_needed_mock.return_value = states.POWER_OFF + ret = task.driver.deploy.deploy(task) + self.assertIsNone(ret) + self.assertFalse(mock_cache_instance_image.called) + self.assertFalse(mock_check_image_size.called) + mock_remove_network.assert_called_once_with(mock.ANY, task) + mock_tenant_network.assert_called_once_with(mock.ANY, task) + mock_prepare_instance.assert_called_once_with(mock.ANY, task) + self.assertEqual(2, mock_node_power_action.call_count) + self.assertEqual(states.DEPLOYING, task.node.provision_state) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + # Cleanup of iscsi_deploy with pxe boot interface class CleanUpFullFlowTestCase(db_base.DbTestCase): diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index 7121fc18ff..14c24368a8 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -1012,6 +1012,44 @@ class PXERamdiskDeployTestCase(db_base.DbTestCase): task.driver.deploy.validate(task) mock_validate.assert_called_once_with(mock.ANY, task) + @mock.patch.object(manager_utils, 'restore_power_state_if_needed', + autospec=True) + @mock.patch.object(manager_utils, 'power_on_node_if_needed', + autospec=True) + @mock.patch.object(pxe.LOG, 'warning', autospec=True) + @mock.patch.object(deploy_utils, 'switch_pxe_config', autospec=True) + @mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True) + @mock.patch.object(pxe_utils, 'cache_ramdisk_kernel', autospec=True) + @mock.patch.object(pxe_utils, 'get_instance_image_info', autospec=True) + def test_deploy_with_smartnic_port( + self, mock_image_info, mock_cache, + mock_dhcp_factory, mock_switch_config, mock_warning, + power_on_node_if_needed_mock, restore_power_state_mock): + image_info = {'kernel': ('', '/path/to/kernel'), + 'ramdisk': ('', '/path/to/ramdisk')} + mock_image_info.return_value = image_info + i_info = self.node.instance_info + i_info.update({'capabilities': {'boot_option': 'ramdisk'}}) + self.node.instance_info = i_info + self.node.save() + with task_manager.acquire(self.context, self.node.uuid) as task: + power_on_node_if_needed_mock.return_value = states.POWER_OFF + self.assertIsNone(task.driver.deploy.deploy(task)) + mock_image_info.assert_called_once_with(task) + mock_cache.assert_called_once_with( + task, image_info, ipxe_enabled=CONF.pxe.ipxe_enabled) + self.assertFalse(mock_warning.called) + power_on_node_if_needed_mock.assert_called_once_with(task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF) + i_info['configdrive'] = 'meow' + self.node.instance_info = i_info + self.node.save() + mock_warning.reset_mock() + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertIsNone(task.driver.deploy.deploy(task)) + self.assertTrue(mock_warning.called) + class PXEValidateRescueTestCase(db_base.DbTestCase): diff --git a/releasenotes/notes/add-support-for-smart-nic-0fc5b10ba6772f7f.yaml b/releasenotes/notes/add-support-for-smart-nic-0fc5b10ba6772f7f.yaml new file mode 100644 index 0000000000..0453d04578 --- /dev/null +++ b/releasenotes/notes/add-support-for-smart-nic-0fc5b10ba6772f7f.yaml @@ -0,0 +1,10 @@ +--- +prelude: > + Add support for Smart NICs in baremetal servers. +features: + - | + Enable use of Smart NICs by extending ironic to implement generic + networking services for baremetal servers. + + Extending the ramdisk, direct, iscsi and ansible deployment Interfaces + to support the Smart NIC use-cases.