Merge "Update the deploy drivers with network flipping logic"

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
+        multiple times for the same node on the same conductor.
-        called by multiple conductors in parallel. Therefore, it must not
-        require an exclusive lock.
 
         This method is called before `deploy`.
 

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()

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') %

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)
 

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,
+    @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.'
-                     pxe_prepare_ramdisk_mock):
+                '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)
 

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(
+            node_power_action_mock.assert_has_calls([
-                task, states.REBOOT)
+                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(
+            node_power_action_mock.assert_has_calls([
-                task, states.REBOOT)
+                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,
+            self, configure_tenant_net_mock, remove_provisioning_net_mock,
-            node_power_action_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(
+            node_power_action_mock.assert_has_calls([
-                task, states.REBOOT)
+                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(
+            node_power_action_mock.assert_has_calls([
-                task, states.REBOOT)
+                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(
+            node_power_action_mock.assert_has_calls([
-                task, states.REBOOT)
+                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 '

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):
+    def test_prepare_node_active(self, prepare_instance_mock,
-        with task_manager.acquire(self.context, self.node.uuid,
+                                 add_provisioning_net_mock):
-                                  shared=True) as task:
+        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,
+        with task_manager.acquire(self.context, self.node.uuid) as task:
-                                  shared=True) as task:
+            task.node.provision_state = states.DEPLOYING
-            task.node.provision_state = states.DEPLOYWAIT
 
             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')