diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index a4a3b2b4cc..5181325648 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -338,6 +338,9 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): # NOTE(TheJulia): There is no reason to validate # image properties if we will not be writing an image # in a boot from volume case. As such, return to the caller. + LOG.debug('Skipping complete deployment interface validation ' + 'for node %s as it is set to boot from a remote ' + 'volume.', node.uuid) return params = {} diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 7d79403a49..81336871d9 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -1297,6 +1297,9 @@ def tear_down_storage_configuration(task): # this is dangerous if IPA is not handling the cleaning. for volume in task.volume_targets: volume.destroy() + LOG.info('Successfully deleted volume target %(target)s. ' + 'The node associated with the target was %(node)s.', + {'target': volume.uuid, 'node': task.node.uuid}) node = task.node driver_internal_info = node.driver_internal_info diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index c8a257693c..cba4a0bd1d 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -427,6 +427,9 @@ class ISCSIDeploy(AgentDeployMixin, base.DeployInterface): # Edit early if we are not writing a volume as the validate # tasks evaluate root device hints. if not task.driver.storage.should_write_image(task): + LOG.debug('Skipping complete deployment interface validation ' + 'for node %s as it is set to boot from a remote ' + 'volume.', node.uuid) return # TODO(rameshg87): iscsi_ilo driver uses this method. Remove @@ -521,8 +524,9 @@ class ISCSIDeploy(AgentDeployMixin, base.DeployInterface): manager_utils.node_power_action(task, states.POWER_OFF) # NOTE(vdrok): in case of rebuild, we have tenant network # already configured, unbind tenant ports if present - task.driver.network.unconfigure_tenant_networks(task) - task.driver.network.add_provisioning_network(task) + if task.driver.storage.should_write_image(task): + task.driver.network.unconfigure_tenant_networks(task) + task.driver.network.add_provisioning_network(task) task.driver.storage.attach_volumes(task) deploy_opts = deploy_utils.build_agent_options(node) diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 101f8f9d05..919f775504 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -29,6 +29,7 @@ from ironic.drivers.modules import agent_base_vendor from ironic.drivers.modules import agent_client 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.drivers.modules.storage import noop as noop_storage from ironic.drivers import utils as driver_utils @@ -300,8 +301,9 @@ class TestAgentDeploy(db_base.DbTestCase): @mock.patch.object(noop_storage.NoopStorage, 'detach_volumes', autospec=True) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'unconfigure_tenant_networks', 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(self, power_mock, unconfigure_tenant_nets_mock, @@ -328,10 +330,11 @@ class TestAgentDeploy(db_base.DbTestCase): @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('ironic.drivers.modules.network.flat.FlatNetwork.' - 'add_provisioning_network', autospec=True) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'unconfigure_tenant_networks', spec_set=True, 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( self, unconfigure_tenant_net_mock, add_provisioning_net_mock, build_instance_info_mock, build_options_mock, @@ -382,8 +385,8 @@ class TestAgentDeploy(db_base.DbTestCase): @mock.patch.object(noop_storage.NoopStorage, 'attach_volumes', autospec=True) @mock.patch.object(deploy_utils, 'populate_storage_driver_internal_info') - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'add_provisioning_network', autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'add_provisioning_network', + spec_set=True, autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_instance') @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk') @mock.patch.object(deploy_utils, 'build_agent_options') @@ -407,8 +410,46 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertTrue(storage_driver_info_mock.called) self.assertFalse(storage_attach_volumes_mock.called) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'add_provisioning_network', autospec=True) + @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', + 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(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(pxe.PXEBoot, 'prepare_instance', autospec=True) + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True) + @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) + @mock.patch.object(deploy_utils, 'build_instance_info_for_deploy', + autospec=True) + def test_prepare_storage_write_false( + self, build_instance_info_mock, build_options_mock, + pxe_prepare_ramdisk_mock, pxe_prepare_instance_mock, + remove_tenant_net_mock, add_provisioning_net_mock, + storage_driver_info_mock, storage_attach_volumes_mock, + should_write_image_mock): + should_write_image_mock.return_value = False + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + task.node.provision_state = states.DEPLOYING + + self.driver.prepare(task) + + self.assertFalse(build_instance_info_mock.called) + self.assertFalse(build_options_mock.called) + self.assertFalse(pxe_prepare_ramdisk_mock.called) + self.assertFalse(pxe_prepare_instance_mock.called) + self.assertFalse(add_provisioning_net_mock.called) + self.assertTrue(storage_driver_info_mock.called) + self.assertTrue(storage_attach_volumes_mock.called) + self.assertEqual(2, should_write_image_mock.call_count) + + @mock.patch.object(flat_network.FlatNetwork, 'add_provisioning_network', + spec_set=True, autospec=True) @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') @@ -426,8 +467,8 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertFalse(pxe_prepare_ramdisk_mock.called) self.assertFalse(add_provisioning_net_mock.called) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'add_provisioning_network', autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'add_provisioning_network', + spec_set=True, autospec=True) @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') diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index 9e9489c02e..ef4de9fb7f 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -36,6 +36,7 @@ from ironic.drivers.modules import agent_base_vendor from ironic.drivers.modules import agent_client from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import iscsi_deploy +from ironic.drivers.modules.network import flat as flat_network from ironic.drivers.modules import pxe from ironic.drivers.modules.storage import noop as noop_storage from ironic.drivers import utils as driver_utils @@ -593,8 +594,8 @@ class ISCSIDeployTestCase(db_base.DbTestCase): autospec=True) @mock.patch.object(deploy_utils, 'populate_storage_driver_internal_info', autospec=True) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'add_provisioning_network', spec_set=True, autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'add_provisioning_network', + spec_set=True, autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) def test_prepare_node_active(self, prepare_instance_mock, add_provisioning_net_mock, @@ -611,8 +612,8 @@ class ISCSIDeployTestCase(db_base.DbTestCase): storage_driver_info_mock.assert_called_once_with(task) self.assertFalse(storage_attach_volumes_mock.called) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'add_provisioning_network', spec_set=True, autospec=True) + @mock.patch.object(flat_network.FlatNetwork, 'add_provisioning_network', + spec_set=True, autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) def test_prepare_node_adopting(self, prepare_instance_mock, add_provisioning_net_mock): @@ -631,10 +632,11 @@ class ISCSIDeployTestCase(db_base.DbTestCase): autospec=True) @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'add_provisioning_network', spec_set=True, autospec=True) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'unconfigure_tenant_networks', spec_set=True, 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( self, unconfigure_tenant_net_mock, add_provisioning_net_mock, mock_prepare_ramdisk, mock_agent_options, @@ -654,6 +656,41 @@ class ISCSIDeployTestCase(db_base.DbTestCase): storage_attach_volumes_mock.assert_called_once_with( task.driver.storage, task) + @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', + 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_storage_should_write_false( + self, unconfigure_tenant_net_mock, add_provisioning_net_mock, + mock_prepare_ramdisk, mock_agent_options, + storage_driver_info_mock, storage_attach_volumes_mock, + storage_should_write_mock): + storage_should_write_mock.return_value = False + mock_agent_options.return_value = {'c': 'd'} + with task_manager.acquire(self.context, self.node.uuid) as task: + task.node.provision_state = states.DEPLOYING + + 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'}) + self.assertFalse(add_provisioning_net_mock.called) + self.assertFalse(unconfigure_tenant_net_mock.called) + storage_driver_info_mock.assert_called_once_with(task) + storage_attach_volumes_mock.assert_called_once_with( + task.driver.storage, task) + self.assertEqual(1, storage_should_write_mock.call_count) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(iscsi_deploy, 'check_image_size', autospec=True) @mock.patch.object(iscsi_deploy, 'cache_instance_image', autospec=True) @@ -670,10 +707,12 @@ class ISCSIDeployTestCase(db_base.DbTestCase): @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', autospec=True) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'configure_tenant_networks', spec_set=True, autospec=True) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'remove_provisioning_network', spec_set=True, 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(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(iscsi_deploy, 'check_image_size', autospec=True) @mock.patch.object(iscsi_deploy, 'cache_instance_image', autospec=True) @@ -699,8 +738,9 @@ class ISCSIDeployTestCase(db_base.DbTestCase): @mock.patch.object(noop_storage.NoopStorage, 'detach_volumes', autospec=True) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'unconfigure_tenant_networks', 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(self, node_power_action_mock, unconfigure_tenant_nets_mock, diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index 7c77e6328d..b7781bf6a7 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -1092,11 +1092,11 @@ class PXEBootTestCase(db_base.DbTestCase): self.assertFalse(switch_pxe_config_mock.called) self.assertFalse(set_boot_device_mock.called) - @mock.patch('os.path.isfile', return_value=False, autospec=True) + @mock.patch('os.path.isfile', lambda filename: False) @mock.patch.object(pxe_utils, 'create_pxe_config', autospec=True) - @mock.patch.object(deploy_utils, 'is_iscsi_boot', autospec=True) + @mock.patch.object(deploy_utils, 'is_iscsi_boot', lambda task: True) @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', - autospec=True) + lambda task: False) @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True) @mock.patch.object(deploy_utils, 'switch_pxe_config', autospec=True) @mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True) @@ -1105,13 +1105,10 @@ class PXEBootTestCase(db_base.DbTestCase): def test_prepare_instance_netboot_iscsi( self, get_image_info_mock, cache_mock, dhcp_factory_mock, switch_pxe_config_mock, - set_boot_device_mock, should_write_image_mock, - is_iscsi_boot_mock, create_pxe_config_mock, isfile_mock): + set_boot_device_mock, create_pxe_config_mock): http_url = 'http://192.1.2.3:1234' self.config(ipxe_enabled=True, group='pxe') self.config(http_url=http_url, group='deploy') - is_iscsi_boot_mock.return_value = True - should_write_image_mock.return_valule = False provider_mock = mock.MagicMock() dhcp_factory_mock.return_value = provider_mock vol_id = uuidutils.generate_uuid()