diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 742aa5a129..b59c82e14b 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -299,9 +299,7 @@ class DeployInterface(BaseInterface): this method should be implemented by the driver. If implemented, this method must be idempotent. It may be called - multiple times for the same node on the same conductor, and it may be - called by multiple conductors in parallel. Therefore, it must not - require an exclusive lock. + multiple times for the same node on the same conductor. This method is called before `deploy`. diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 79b19d5bd6..2cf56809f7 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -295,8 +295,16 @@ class AgentDeploy(base.DeployInterface): :param task: a TaskManager instance. :returns: status of the deploy. One of ironic.common.states. + :raises: NetworkError if the cleaning ports cannot be removed. + :raises: InvalidParameterValue when the wrong power state is specified + or the wrong driver info is specified for power management. + :raises: other exceptions by the node's power driver if something + wrong occurred during the power action. """ manager_utils.node_power_action(task, states.POWER_OFF) + + task.driver.network.unconfigure_tenant_networks(task) + return states.DELETED @task_manager.require_exclusive_lock @@ -304,11 +312,25 @@ class AgentDeploy(base.DeployInterface): """Prepare the deployment environment for this node. :param task: a TaskManager instance. + :raises: NetworkError: if the previous cleaning ports cannot be removed + or if new cleaning ports cannot be created. + :raises: InvalidParameterValue when the wrong power state is specified + or the wrong driver info is specified for power management. + :raises: other exceptions by the node's power driver if something + wrong occurred during the power action. + :raises: exception.ImageRefValidationFailed if image_source is not + Glance href and is not HTTP(S) URL. + :raises: any boot interface's prepare_ramdisk exceptions. """ # Nodes deployed by AgentDeploy always boot from disk now. So there # is nothing to be done in prepare() when it's called during # take over. node = task.node + if node.provision_state == states.DEPLOYING: + # Adding the node to provisioning network so that the dhcp + # options get added for the provisioning port. + manager_utils.node_power_action(task, states.POWER_OFF) + task.driver.network.add_provisioning_network(task) if node.provision_state != states.ACTIVE: node.instance_info = build_instance_info_for_deploy(task) node.save() diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py index caf66720e8..a5614d6095 100644 --- a/ironic/drivers/modules/agent_base_vendor.py +++ b/ironic/drivers/modules/agent_base_vendor.py @@ -768,6 +768,7 @@ class BaseAgentVendor(base.VendorInterface): {'node_uuid': node.uuid, 'timeout': (wait * (attempts - 1)) / 1000, 'error': e}) + manager_utils.node_power_action(task, states.POWER_OFF) else: # Flush the file system prior to hard rebooting the node result = self._client.sync(node) @@ -781,8 +782,12 @@ class BaseAgentVendor(base.VendorInterface): 'Failed to flush the file system prior to hard ' 'rebooting the node %(node)s. Error: %(error)s'), {'node': node.uuid, 'error': error}) + manager_utils.node_power_action(task, states.POWER_OFF) - manager_utils.node_power_action(task, states.REBOOT) + task.driver.network.remove_provisioning_network(task) + task.driver.network.configure_tenant_networks(task) + + manager_utils.node_power_action(task, states.POWER_ON) except Exception as e: msg = (_('Error rebooting node %(node)s after deploy. ' 'Error: %(error)s') % diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 2bd7deca27..957368eb32 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -456,10 +456,17 @@ class ISCSIDeploy(base.DeployInterface): :param task: a TaskManager instance containing the node to act on. :returns: deploy state DELETED. + :raises: NetworkError if the cleaning ports cannot be removed. + :raises: InvalidParameterValue when the wrong state is specified + or the wrong driver info is specified. + :raises: other exceptions by the node's power driver if something + wrong occurred during the power action. """ manager_utils.node_power_action(task, states.POWER_OFF) + task.driver.network.unconfigure_tenant_networks(task) return states.DELETED + @task_manager.require_exclusive_lock def prepare(self, task): """Prepare the deployment environment for this task's node. @@ -468,11 +475,24 @@ class ISCSIDeploy(base.DeployInterface): local cache. :param task: a TaskManager instance containing the node to act on. + :raises: NetworkError: if the previous cleaning ports cannot be removed + or if new cleaning ports cannot be created. + :raises: InvalidParameterValue when the wrong power state is specified + or the wrong driver info is specified for power management. + :raises: other exceptions by the node's power driver if something + wrong occurred during the power action. + :raises: any boot interface's prepare_ramdisk exceptions. """ node = task.node if node.provision_state == states.ACTIVE: task.driver.boot.prepare_instance(task) else: + if node.provision_state == states.DEPLOYING: + # Adding the node to provisioning network so that the dhcp + # options get added for the provisioning port. + manager_utils.node_power_action(task, states.POWER_OFF) + task.driver.network.add_provisioning_network(task) + deploy_opts = deploy_utils.build_agent_options(node) task.driver.boot.prepare_ramdisk(task, deploy_opts) diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 514d98f7bf..e2b647b89d 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -439,19 +439,25 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertEqual(driver_return, states.DEPLOYWAIT) power_mock.assert_called_once_with(task, states.REBOOT) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'unconfigure_tenant_networks', autospec=True) @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) - def test_tear_down(self, power_mock): + def test_tear_down(self, power_mock, unconfigure_tenant_nets_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: 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) @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk') @mock.patch.object(deploy_utils, 'build_agent_options') @mock.patch.object(agent, 'build_instance_info_for_deploy') - def test_prepare(self, build_instance_info_mock, build_options_mock, - pxe_prepare_ramdisk_mock): + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'add_provisioning_network', autospec=True) + def test_prepare(self, add_provisioning_net_mock, build_instance_info_mock, + build_options_mock, pxe_prepare_ramdisk_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: task.node.provision_state = states.DEPLOYING @@ -464,6 +470,7 @@ class TestAgentDeploy(db_base.DbTestCase): build_options_mock.assert_called_once_with(task.node) pxe_prepare_ramdisk_mock.assert_called_once_with( task, {'a': 'b'}) + add_provisioning_net_mock.assert_called_once_with(mock.ANY, task) self.node.refresh() self.assertEqual('bar', self.node.instance_info['foo']) @@ -489,12 +496,14 @@ class TestAgentDeploy(db_base.DbTestCase): self.node.refresh() self.assertEqual('bar', self.node.instance_info['foo']) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'add_provisioning_network', autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk') @mock.patch.object(deploy_utils, 'build_agent_options') @mock.patch.object(agent, 'build_instance_info_for_deploy') def test_prepare_active( self, build_instance_info_mock, build_options_mock, - pxe_prepare_ramdisk_mock): + pxe_prepare_ramdisk_mock, add_provisioning_net_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: task.node.provision_state = states.ACTIVE @@ -504,6 +513,7 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertFalse(build_instance_info_mock.called) self.assertFalse(build_options_mock.called) self.assertFalse(pxe_prepare_ramdisk_mock.called) + self.assertFalse(add_provisioning_net_mock.called) @mock.patch('ironic.common.dhcp_factory.DHCPFactory._set_dhcp_provider') @mock.patch('ironic.common.dhcp_factory.DHCPFactory.clean_dhcp') @@ -766,7 +776,7 @@ class TestAgentVendor(db_base.DbTestCase): power_off_mock.assert_called_once_with(task.node) get_power_state_mock.assert_called_once_with(task) node_power_action_mock.assert_called_once_with( - task, states.REBOOT) + task, states.POWER_ON) self.assertFalse(prepare_mock.called) self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) @@ -811,7 +821,7 @@ class TestAgentVendor(db_base.DbTestCase): power_off_mock.assert_called_once_with(task.node) get_power_state_mock.assert_called_once_with(task) node_power_action_mock.assert_called_once_with( - task, states.REBOOT) + task, states.POWER_ON) prepare_mock.assert_called_once_with(task.driver.boot, task) self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) @@ -860,7 +870,7 @@ class TestAgentVendor(db_base.DbTestCase): get_power_state_mock.assert_called_once_with(task) node_power_action_mock.assert_called_once_with( - task, states.REBOOT) + task, states.POWER_ON) self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) self.assertFalse(uuid_mock.called) @@ -945,7 +955,7 @@ class TestAgentVendor(db_base.DbTestCase): power_off_mock.assert_called_once_with(task.node) get_power_state_mock.assert_called_once_with(task) node_power_action_mock.assert_called_once_with( - task, states.REBOOT) + task, states.POWER_ON) self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) 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 9176aca744..08f91d4473 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py @@ -585,7 +585,7 @@ class TestBaseAgentVendor(db_base.DbTestCase): 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.REBOOT) + task, states.POWER_ON) self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) @@ -595,8 +595,13 @@ class TestBaseAgentVendor(db_base.DbTestCase): spec=types.FunctionType) @mock.patch.object(agent_client.AgentClient, 'power_off', spec=types.FunctionType) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'remove_provisioning_network', spec_set=True, autospec=True) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'configure_tenant_networks', spec_set=True, autospec=True) def test_reboot_and_finish_deploy_soft_poweroff_doesnt_complete( - self, power_off_mock, get_power_state_mock, + self, configure_tenant_net_mock, remove_provisioning_net_mock, + power_off_mock, get_power_state_mock, node_power_action_mock): self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE @@ -607,16 +612,25 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.passthru.reboot_and_finish_deploy(task) power_off_mock.assert_called_once_with(task.node) self.assertEqual(7, get_power_state_mock.call_count) - node_power_action_mock.assert_called_once_with( - task, states.REBOOT) + node_power_action_mock.assert_has_calls([ + mock.call(task, states.POWER_OFF), + mock.call(task, states.POWER_ON)]) + remove_provisioning_net_mock.assert_called_once_with(mock.ANY, + task) + configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(agent_client.AgentClient, 'power_off', spec=types.FunctionType) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'remove_provisioning_network', spec_set=True, autospec=True) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'configure_tenant_networks', spec_set=True, autospec=True) def test_reboot_and_finish_deploy_soft_poweroff_fails( - self, power_off_mock, node_power_action_mock): + self, configure_tenant_net_mock, remove_provisioning_net_mock, + power_off_mock, node_power_action_mock): power_off_mock.side_effect = RuntimeError("boom") self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE @@ -625,8 +639,12 @@ class TestBaseAgentVendor(db_base.DbTestCase): shared=True) as task: self.passthru.reboot_and_finish_deploy(task) power_off_mock.assert_called_once_with(task.node) - node_power_action_mock.assert_called_once_with( - task, states.REBOOT) + node_power_action_mock.assert_has_calls([ + mock.call(task, states.POWER_OFF), + mock.call(task, states.POWER_ON)]) + remove_provisioning_net_mock.assert_called_once_with(mock.ANY, + task) + configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) @@ -636,9 +654,13 @@ class TestBaseAgentVendor(db_base.DbTestCase): spec=types.FunctionType) @mock.patch.object(agent_client.AgentClient, 'power_off', spec=types.FunctionType) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'remove_provisioning_network', spec_set=True, autospec=True) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'configure_tenant_networks', spec_set=True, autospec=True) def test_reboot_and_finish_deploy_get_power_state_fails( - self, power_off_mock, get_power_state_mock, - node_power_action_mock): + self, configure_tenant_net_mock, remove_provisioning_net_mock, + power_off_mock, get_power_state_mock, node_power_action_mock): self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() @@ -648,8 +670,12 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.passthru.reboot_and_finish_deploy(task) power_off_mock.assert_called_once_with(task.node) self.assertEqual(7, get_power_state_mock.call_count) - node_power_action_mock.assert_called_once_with( - task, states.REBOOT) + node_power_action_mock.assert_has_calls([ + mock.call(task, states.POWER_OFF), + mock.call(task, states.POWER_ON)]) + remove_provisioning_net_mock.assert_called_once_with(mock.ANY, + task) + configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) @@ -675,7 +701,6 @@ class TestBaseAgentVendor(db_base.DbTestCase): power_off_mock.assert_called_once_with(task.node) self.assertEqual(7, get_power_state_mock.call_count) node_power_action_mock.assert_has_calls([ - mock.call(task, states.REBOOT), mock.call(task, states.POWER_OFF)]) self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) @@ -698,8 +723,10 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.passthru.reboot_and_finish_deploy(task) sync_mock.assert_called_once_with(task.node) - node_power_action_mock.assert_called_once_with( - task, states.REBOOT) + node_power_action_mock.assert_has_calls([ + mock.call(task, states.POWER_OFF), + mock.call(task, states.POWER_ON), + ]) self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) @@ -723,8 +750,10 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.passthru.reboot_and_finish_deploy(task) sync_mock.assert_called_once_with(task.node) - node_power_action_mock.assert_called_once_with( - task, states.REBOOT) + node_power_action_mock.assert_has_calls([ + mock.call(task, states.POWER_OFF), + mock.call(task, states.POWER_ON), + ]) self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) log_error = ('The version of the IPA ramdisk used in the ' diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index b8d3ca06f3..a4f6d9991b 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -527,31 +527,37 @@ class ISCSIDeployTestCase(db_base.DbTestCase): validate_capabilities_mock.assert_called_once_with(task.node) validate_mock.assert_called_once_with(task) + @mock.patch('ironic.drivers.modules.network.flat.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): - with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: + def test_prepare_node_active(self, prepare_instance_mock, + add_provisioning_net_mock): + with task_manager.acquire(self.context, self.node.uuid) as task: task.node.provision_state = states.ACTIVE task.driver.deploy.prepare(task) prepare_instance_mock.assert_called_once_with( task.driver.boot, task) + self.assertEqual(0, add_provisioning_net_mock.call_count) @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True) - def test_prepare_node_deploying(self, mock_prepare_ramdisk, + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'add_provisioning_network', spec_set=True, autospec=True) + def test_prepare_node_deploying(self, add_provisioning_net_mock, + mock_prepare_ramdisk, mock_agent_options): mock_agent_options.return_value = {'c': 'd'} - with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: - task.node.provision_state = states.DEPLOYWAIT + 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'}) + add_provisioning_net_mock.assert_called_once_with(mock.ANY, task) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(iscsi_deploy, 'check_image_size', autospec=True) @@ -567,14 +573,19 @@ class ISCSIDeployTestCase(db_base.DbTestCase): mock_check_image_size.assert_called_once_with(task) mock_node_power_action.assert_called_once_with(task, states.REBOOT) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'unconfigure_tenant_networks', autospec=True) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) - def test_tear_down(self, node_power_action_mock): + def test_tear_down(self, node_power_action_mock, + unconfigure_tenant_nets_mock): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: 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) @mock.patch('ironic.common.dhcp_factory.DHCPFactory._set_dhcp_provider') @mock.patch('ironic.common.dhcp_factory.DHCPFactory.clean_dhcp')