diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index 0a4759d292..f667e7bfba 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -659,7 +659,7 @@ def get_physnets_by_port_uuid(client, port_uuid): @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 + wait_fixed=CONF.agent.neutron_agent_status_retry_interval * 1000 ) def wait_for_host_agent(client, host_id, target_state='up'): """Wait for neutron agent to become target state @@ -670,6 +670,7 @@ def wait_for_host_agent(client, host_id, target_state='up'): down: wait for down status :returns: boolean indicates the agent state matches param value target_state_up. + :raises: exception.Invalid if 'target_state' is not valid. :raises: exception.NetworkError if host status didn't match the required status after max retry attempts. """ @@ -697,7 +698,7 @@ def wait_for_host_agent(client, host_id, target_state='up'): @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 + wait_fixed=CONF.agent.neutron_agent_status_retry_interval * 1000 ) def wait_for_port_status(client, port_id, status): """Wait for port status to be the desired status @@ -707,6 +708,7 @@ def wait_for_port_status(client, port_id, status): :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: InvalidParameterValue if the port does not exist. :raises: exception.NetworkError if port status didn't match the required status after max retry attempts. """ diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 0de36f8bc5..153ddfcd54 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -1009,6 +1009,8 @@ def power_on_node_if_needed(task): :param task: A TaskManager object :returns: the previous power state or None if no changes were made + :raises: exception.NetworkError if agent status didn't match the required + status after max retry attempts. """ if not task.driver.network.need_power_on(task): return @@ -1040,7 +1042,7 @@ def power_on_node_if_needed(task): 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 + """Change the node's power state if power_state_to_restore is not None :param task: A TaskManager object :param power_state_to_restore: power state diff --git a/ironic/conf/agent.py b/ironic/conf/agent.py index 58bec91f53..0e4be55674 100644 --- a/ironic/conf/agent.py +++ b/ironic/conf/agent.py @@ -119,9 +119,9 @@ opts = [ 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 ' + 'status before raising network error for a ' 'dead agent.')), - cfg.IntOpt('neutron_agent_wait_time_seconds', + cfg.IntOpt('neutron_agent_status_retry_interval', default=10, help=_('Wait time in seconds between attempts for validating ' 'Neutron agent status.')), diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index 1c8767e678..b4464edc19 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/ironic/tests/unit/common/test_neutron.py @@ -698,6 +698,24 @@ class TestNeutronNetworkActions(db_base.DbTestCase): neutron.wait_for_port_status(self.client_mock, 'port_id', 'ACTIVE') sleep_mock.assert_called_once() + @mock.patch.object(neutron, '_get_port_by_uuid') + @mock.patch.object(time, 'sleep') + def test_wait_for_port_status_active_max_retry(self, sleep_mock, + get_port_mock): + get_port_mock.return_value = {'status': 'DOWN'} + self.assertRaises(exception.NetworkError, + neutron.wait_for_port_status, + self.client_mock, 'port_id', 'ACTIVE') + + @mock.patch.object(neutron, '_get_port_by_uuid') + @mock.patch.object(time, 'sleep') + def test_wait_for_port_status_down_max_retry(self, sleep_mock, + get_port_mock): + get_port_mock.return_value = {'status': 'ACTIVE'} + self.assertRaises(exception.NetworkError, + neutron.wait_for_port_status, + self.client_mock, 'port_id', 'DOWN') + @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( diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index bfff40acca..8c8a69bb6b 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -19,6 +19,7 @@ from ironic.common import boot_devices from ironic.common import boot_modes from ironic.common import exception from ironic.common import network +from ironic.common import neutron from ironic.common import states from ironic.conductor import rpcapi from ironic.conductor import task_manager @@ -2071,6 +2072,37 @@ class MiscTestCase(db_base.DbTestCase): self.assertEqual(0, boot_device_mock.call_count) self.assertEqual(0, power_action_mock.call_count) + @mock.patch.object(neutron, 'get_client', autospec=True) + @mock.patch.object(neutron, 'wait_for_host_agent', autospec=True) + @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_with_smart_nic_port( + self, power_action_mock, boot_device_mock, + need_power_on_mock, get_power_state_mock, time_mock, + wait_agent_mock, get_client_mock): + llc = {'port_id': 'rep0-0', 'hostname': 'host1'} + port = obj_utils.get_test_port(self.context, node_id=self.node.id, + is_smartnic=True, + local_link_connection=llc) + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + task.ports = [port] + 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) + get_client_mock.assert_called_once_with(context=self.context) + wait_agent_mock.assert_called_once_with(mock.ANY, 'host1', + target_state='down') + @mock.patch.object(time, 'sleep', autospec=True) @mock.patch.object(conductor_utils, 'node_power_action', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 09ea886064..667e20e09d 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -2013,3 +2013,23 @@ class AgentRescueTestCase(db_base.DbTestCase): restore_power_state_mock.assert_has_calls( [mock.call(task, states.POWER_OFF), mock.call(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(fake.FakeBoot, 'clean_up_ramdisk', autospec=True) + def test_agent_rescue_clean_up_smartnic( + self, mock_clean_ramdisk, mock_remove_rescue_net, + power_on_node_if_needed_mock, restore_power_state_mock): + with task_manager.acquire(self.context, self.node.uuid) as task: + power_on_node_if_needed_mock.return_value = states.POWER_OFF + task.driver.rescue.clean_up(task) + self.assertNotIn('rescue_password', task.node.instance_info) + mock_clean_ramdisk.assert_called_once_with( + mock.ANY, task) + mock_remove_rescue_net.assert_called_once_with(mock.ANY, task) + restore_power_state_mock.assert_called_once_with( + task, states.POWER_OFF)