From 9a110e01bd8d1efe6249d3e7a68ae9e833312a34 Mon Sep 17 00:00:00 2001 From: Shivanand Tendulker Date: Fri, 2 Feb 2018 12:51:35 -0500 Subject: [PATCH] Add validate_rescue() method to network interface Adds validate_rescue() method to network interface to validate rescuing network. This method is called by rescue.validate(). Change-Id: Iccac602047eec10f03ef6eaf2dbe716efd6e7f9a Closes-Bug: #1747100 --- ironic/drivers/base.py | 10 ++++ ironic/drivers/modules/agent.py | 6 +-- ironic/drivers/modules/network/neutron.py | 10 ++++ .../drivers/modules/network/test_neutron.py | 21 ++++++++ .../tests/unit/drivers/modules/test_agent.py | 49 +++++++------------ .../add-validate-rescue-2202e8ce9a174ece.yaml | 8 +++ 6 files changed, 69 insertions(+), 35 deletions(-) create mode 100644 releasenotes/notes/add-validate-rescue-2202e8ce9a174ece.yaml diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index d6bfe37305..7412f88c2b 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -1107,6 +1107,16 @@ class NetworkInterface(BaseInterface): :raises: NetworkError """ + def validate_rescue(self, task): + """Validates the network interface for rescue operation. + + :param task: a TaskManager instance. + :raises: InvalidParameterValue, if the network interface configuration + is invalid. + :raises: MissingParameterValue, if some parameters are missing. + """ + pass + def add_rescuing_network(self, task): """Add the rescuing network to the node. diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 511747c98d..12b72f3817 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -32,7 +32,6 @@ from ironic.conf import CONF from ironic.drivers import base from ironic.drivers.modules import agent_base_vendor from ironic.drivers.modules import deploy_utils -from ironic.drivers.modules.network import neutron LOG = log.getLogger(__name__) @@ -778,9 +777,8 @@ class AgentRescue(base.RescueInterface): node = task.node missing_params = [] - # Validate rescuing network if node is using 'neutron' network - if isinstance(task.driver.network, neutron.NeutronNetwork): - task.driver.network.get_rescuing_network_uuid(task) + # Validate rescuing network + task.driver.network.validate_rescue(task) if CONF.agent.manage_agent_boot: # TODO(stendulker): boot.validate() performs validation of diff --git a/ironic/drivers/modules/network/neutron.py b/ironic/drivers/modules/network/neutron.py index 8ad562219f..7bef57ce02 100644 --- a/ironic/drivers/modules/network/neutron.py +++ b/ironic/drivers/modules/network/neutron.py @@ -139,6 +139,16 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, port.internal_info = internal_info port.save() + def validate_rescue(self, task): + """Validates the network interface for rescue operation. + + :param task: a TaskManager instance. + :raises: InvalidParameterValue, if the network interface configuration + is invalid. + :raises: MissingParameterValue, if some parameters are missing. + """ + self.get_rescuing_network_uuid(task) + def add_rescuing_network(self, task): """Create neutron ports for each port to boot the rescue ramdisk. diff --git a/ironic/tests/unit/drivers/modules/network/test_neutron.py b/ironic/tests/unit/drivers/modules/network/test_neutron.py index 892f715da2..7bff4892dd 100644 --- a/ironic/tests/unit/drivers/modules/network/test_neutron.py +++ b/ironic/tests/unit/drivers/modules/network/test_neutron.py @@ -339,6 +339,27 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.port.refresh() self.assertNotIn('cleaning_vif_port_id', self.port.internal_info) + @mock.patch.object(neutron_common, 'validate_network', + side_effect=lambda n, t, context=None: n) + def test_validate_rescue(self, validate_mock): + rescuing_network_uuid = '3aea0de6-4b92-44da-9aa0-52d134c83fdf' + driver_info = self.node.driver_info + driver_info['rescuing_network'] = rescuing_network_uuid + self.node.driver_info = driver_info + self.node.save() + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.validate_rescue(task) + validate_mock.assert_called_once_with( + rescuing_network_uuid, 'rescuing network', + context=task.context), + + def test_validate_rescue_exc(self): + self.config(rescuing_network="", group='neutron') + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaisesRegex(exception.MissingParameterValue, + 'rescuing network is not set', + self.interface.validate_rescue, task) + @mock.patch.object(neutron_common, 'validate_network', side_effect=lambda n, t, context=None: n) @mock.patch.object(neutron_common, 'rollback_ports') diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 29380805e5..3303e0b349 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -20,7 +20,6 @@ from oslo_config import cfg from ironic.common import dhcp_factory from ironic.common import exception from ironic.common import images -from ironic.common import neutron as neutron_common from ironic.common import raid from ironic.common import states from ironic.conductor import task_manager @@ -1358,30 +1357,18 @@ class AgentRescueTestCase(db_base.DbTestCase): mock_prepare_instance.assert_called_once_with(mock.ANY, task) self.assertEqual(states.ACTIVE, result) - @mock.patch.object(neutron_common, 'validate_network', autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue', + autospec=True) @mock.patch.object(fake.FakeBoot, 'validate', autospec=True) def test_agent_rescue_validate(self, mock_boot_validate, mock_validate_network): with task_manager.acquire(self.context, self.node.uuid) as task: task.driver.rescue.validate(task) - self.assertFalse(mock_validate_network.called) + mock_validate_network.assert_called_once_with(mock.ANY, task) mock_boot_validate.assert_called_once_with(mock.ANY, task) - @mock.patch('ironic.drivers.modules.network.neutron.NeutronNetwork.' - 'get_rescuing_network_uuid', autospec=True) - @mock.patch.object(fake.FakeBoot, 'validate', autospec=True) - def test_agent_rescue_validate_neutron_net(self, mock_boot_validate, - mock_rescuing_net): - self.config(enabled_network_interfaces=['neutron']) - self.node.network_interface = 'neutron' - self.node.save() - with task_manager.acquire(self.context, self.node.uuid) as task: - task.driver.rescue.validate(task) - mock_rescuing_net.assert_called_once_with(mock.ANY, task) - mock_boot_validate.assert_called_once_with(mock.ANY, task) - - @mock.patch('ironic.drivers.modules.network.neutron.NeutronNetwork.' - 'get_rescuing_network_uuid', autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue', + autospec=True) @mock.patch.object(fake.FakeBoot, 'validate', autospec=True) def test_agent_rescue_validate_no_manage_agent(self, mock_boot_validate, mock_rescuing_net): @@ -1395,11 +1382,11 @@ class AgentRescueTestCase(db_base.DbTestCase): self.node.save() with task_manager.acquire(self.context, self.node.uuid) as task: task.driver.rescue.validate(task) - self.assertFalse(mock_rescuing_net.called) + mock_rescuing_net.assert_called_once_with(mock.ANY, task) self.assertFalse(mock_boot_validate.called) - @mock.patch('ironic.drivers.modules.network.neutron.NeutronNetwork.' - 'get_rescuing_network_uuid', autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue', + autospec=True) @mock.patch.object(fake.FakeBoot, 'validate', autospec=True) def test_agent_rescue_validate_fails_no_rescue_ramdisk( self, mock_boot_validate, mock_rescuing_net): @@ -1411,11 +1398,11 @@ class AgentRescueTestCase(db_base.DbTestCase): self.assertRaisesRegex(exception.MissingParameterValue, 'Node.*missing.*rescue_ramdisk', task.driver.rescue.validate, task) - self.assertFalse(mock_rescuing_net.called) + mock_rescuing_net.assert_called_once_with(mock.ANY, task) mock_boot_validate.assert_called_once_with(mock.ANY, task) - @mock.patch('ironic.drivers.modules.network.neutron.NeutronNetwork.' - 'get_rescuing_network_uuid', autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue', + autospec=True) @mock.patch.object(fake.FakeBoot, 'validate', autospec=True) def test_agent_rescue_validate_fails_no_rescue_kernel( self, mock_boot_validate, mock_rescuing_net): @@ -1427,11 +1414,11 @@ class AgentRescueTestCase(db_base.DbTestCase): self.assertRaisesRegex(exception.MissingParameterValue, 'Node.*missing.*rescue_kernel', task.driver.rescue.validate, task) - self.assertFalse(mock_rescuing_net.called) + mock_rescuing_net.assert_called_once_with(mock.ANY, task) mock_boot_validate.assert_called_once_with(mock.ANY, task) - @mock.patch('ironic.drivers.modules.network.neutron.NeutronNetwork.' - 'get_rescuing_network_uuid', autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue', + autospec=True) @mock.patch.object(fake.FakeBoot, 'validate', autospec=True) def test_agent_rescue_validate_fails_no_rescue_password( self, mock_boot_validate, mock_rescuing_net): @@ -1443,11 +1430,11 @@ class AgentRescueTestCase(db_base.DbTestCase): self.assertRaisesRegex(exception.MissingParameterValue, 'Node.*missing.*rescue_password', task.driver.rescue.validate, task) - self.assertFalse(mock_rescuing_net.called) + mock_rescuing_net.assert_called_once_with(mock.ANY, task) mock_boot_validate.assert_called_once_with(mock.ANY, task) - @mock.patch('ironic.drivers.modules.network.neutron.NeutronNetwork.' - 'get_rescuing_network_uuid', autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue', + autospec=True) @mock.patch.object(fake.FakeBoot, 'validate', autospec=True) def test_agent_rescue_validate_fails_empty_rescue_password( self, mock_boot_validate, mock_rescuing_net): @@ -1459,7 +1446,7 @@ class AgentRescueTestCase(db_base.DbTestCase): self.assertRaisesRegex(exception.InvalidParameterValue, "'instance_info/rescue_password'.*empty", task.driver.rescue.validate, task) - self.assertFalse(mock_rescuing_net.called) + mock_rescuing_net.assert_called_once_with(mock.ANY, task) mock_boot_validate.assert_called_once_with(mock.ANY, task) @mock.patch.object(flat_network.FlatNetwork, 'remove_rescuing_network', diff --git a/releasenotes/notes/add-validate-rescue-2202e8ce9a174ece.yaml b/releasenotes/notes/add-validate-rescue-2202e8ce9a174ece.yaml new file mode 100644 index 0000000000..ab930cd302 --- /dev/null +++ b/releasenotes/notes/add-validate-rescue-2202e8ce9a174ece.yaml @@ -0,0 +1,8 @@ +--- +other: + - | + Adds new method ``validate_rescue()`` to network interface to validate + rescuing network. This method is called during validation of rescue + interface. This fixes the `validation of rescuing network based on + network type in rescue interface + `_.