Add prepare, clean_up, take_over methods to deploy

Add three new methods to the deploy interface,
which will be used by the ConductorManager to trigger
(re)building of the deploy environment and updating of
external mappings/dependencies when rebalancing the hash ring.

Also refactors the PXE driver to utilize these new methods.

blueprint instance-mapping-by-consistent-hash

Change-Id: I1bebb895cbe29a7059a9873ef90ee039790c1031

ironic/conductor/manager.py

@@ -254,6 +254,7 @@ class ConductorManager(service.PeriodicService):
                 node.save(context)
 
             try:
+                task.driver.deploy.prepare(task, node)
                 new_state = task.driver.deploy.deploy(task, node)
             except Exception as e:
                 with excutils.save_and_reraise_exception():
@@ -307,6 +308,7 @@ class ConductorManager(service.PeriodicService):
                 node.save(context)
 
             try:
+                task.driver.deploy.clean_up(task, node)
                 new_state = task.driver.deploy.tear_down(task, node)
             except Exception as e:
                 with excutils.save_and_reraise_exception():

ironic/drivers/base.py

@@ -91,8 +91,10 @@ class DeployInterface(object):
     def deploy(self, task, node):
         """Perform a deployment to a node.
 
-        Given a node with complete metadata, deploy the indicated image
-        to the node.
+        Perform the necessary work to deploy an image onto the specified node.
+        This method will be called after prepare(), which may have already
+        performed any preparatory steps, such as pre-caching some data for the
+        node.
 
         :param task: a TaskManager instance.
         :param node: the Node to act upon.
@@ -111,6 +113,65 @@ class DeployInterface(object):
         :returns: status of the deploy. One of ironic.common.states.
         """
 
+    @abc.abstractmethod
+    def prepare(self, task, node):
+        """Prepare the deployment environment for this node.
+
+        If preparation of the deployment environment ahead of time is possible,
+        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.
+
+        This method is called before `deploy`.
+
+        :param task: a TaskManager instance.
+        :param node: the Node for which to prepare a deployment environment
+                     on this Conductor.
+        """
+
+    @abc.abstractmethod
+    def clean_up(self, task, node):
+        """Clean up the deployment environment for this node.
+
+        If preparation of the deployment environment ahead of time is possible,
+        this method should be implemented by the driver. It should erase
+        anything cached by the `prepare` method.
+
+        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.
+
+        This method is called before `tear_down`.
+
+        :param task: a TaskManager instance.
+        :param node: the Node whose deployment environment should be cleaned up
+                     on this Conductor.
+        """
+
+    @abc.abstractmethod
+    def take_over(self, task, node):
+        """Take over management of this node from a dead conductor.
+
+        If conductors' hosts maintain a static relationship to nodes, this
+        method should be implemented by the driver to allow conductors to
+        perform the necessary work during the remapping of nodes to conductors
+        when a conductor joins or leaves the cluster.
+
+        For example, the PXE driver has an external dependency:
+            Neutron must forward DHCP BOOT requests to a conductor which has
+            prepared the tftpboot environment for the given node. When a
+            conductor goes offline, another conductor must change this setting
+            in Neutron as part of remapping that node's control to itself.
+            This is performed within the `takeover` method.
+
+        :param task: a TaskManager instance.
+        :param node: the Node which is now being managed by this Conductor.
+        """
+
 
 @six.add_metaclass(abc.ABCMeta)
 class PowerInterface(object):

ironic/drivers/modules/fake.py

@@ -56,6 +56,15 @@ class FakeDeploy(base.DeployInterface):
     def tear_down(self, task, node):
         pass
 
+    def prepare(self, task, node):
+        pass
+
+    def clean_up(self, task, node):
+        pass
+
+    def take_over(self, task, node):
+        pass
+
 
 class FakeVendor(base.VendorInterface):
     """Example implementation of a vendor passthru interface."""

ironic/drivers/modules/pxe.py

@@ -456,6 +456,12 @@ def _create_pxe_config(task, node, pxe_info):
         utils.create_link_without_raise(pxe_config_file_path, mac_path)
 
 
+def _update_neutron(task, node):
+    """Send the DHCP BOOT options to Neutron for this node."""
+    # FIXME: just a stub for the moment.
+    pass
+
+
 class PXEDeploy(base.DeployInterface):
     """PXE Deploy Interface: just a stub until the real driver is ported."""
 
@@ -482,15 +488,10 @@ class PXEDeploy(base.DeployInterface):
         :param node: the Node to act upon.
         :returns: deploy state DEPLOYING.
         """
-
-        pxe_info = _get_tftp_image_info(node)
-
-        _create_pxe_config(task, node, pxe_info)
-        _cache_images(node, pxe_info)
-        # TODO(yuriyz): more secure way needed for pass auth token to
-        # deploy ramdisk
+        # TODO(yuriyz): more secure way needed for pass auth token
+        #               to deploy ramdisk
         _create_token_file(task, node)
-
+        _update_neutron(task, node)
         manager_utils.node_power_action(task, node, states.REBOOT)
 
         return states.DEPLOYING
@@ -506,9 +507,21 @@ class PXEDeploy(base.DeployInterface):
         :param node: the Node to act upon.
         :returns: deploy state DELETED.
         """
-        #FIXME(ghe): Possible error to get image info if eliminated from glance
-        # Retrieve image info and store in db
-        # If we keep master images, no need to get the info, we may ignore this
+        manager_utils.node_power_action(task, node, states.POWER_OFF)
+
+        return states.DELETED
+
+    def prepare(self, task, node):
+        # TODO(deva): optimize this if rerun on existing files
+        pxe_info = _get_tftp_image_info(node)
+        _create_pxe_config(task, node, pxe_info)
+        _cache_images(node, pxe_info)
+
+    def clean_up(self, task, node):
+        # FIXME(ghe): Possible error to get image info if eliminated from
+        #             glance. Retrieve image info and store in db.
+        #             If we keep master images, no need to get the info,
+        #             and we may ignore this.
         pxe_info = _get_tftp_image_info(node)
         d_info = _parse_driver_info(node)
         for label in pxe_info:
@@ -527,10 +540,10 @@ class PXEDeploy(base.DeployInterface):
                 os.path.join(CONF.pxe.tftp_root, node['instance_uuid']))
 
         _destroy_images(d_info)
+        _destroy_token_file(node)
 
-        manager_utils.node_power_action(task, node, states.POWER_OFF)
-
-        return states.DELETED
+    def take_over(self, task, node):
+        _update_neutron(task, node)
 
 
 class PXERescue(base.RescueInterface):

ironic/tests/drivers/test_fake.py

@@ -60,7 +60,13 @@ class FakeDriverTestCase(base.TestCase):
 
     def test_deploy_interface(self):
         self.driver.deploy.validate(self.node)
+
+        self.driver.deploy.prepare(None, None)
         self.driver.deploy.deploy(None, None)
+
+        self.driver.deploy.take_over(None, None)
+
+        self.driver.deploy.clean_up(None, None)
         self.driver.deploy.tear_down(None, None)
 
     def test_vendor_interface(self):

ironic/tests/drivers/test_pxe.py

@@ -35,6 +35,7 @@ from ironic.common import images
 from ironic.common import states
 from ironic.common import utils
 from ironic.conductor import task_manager
+from ironic.conductor import utils as manager_utils
 from ironic.db import api as dbapi
 from ironic.drivers.modules import pxe
 from ironic.openstack.common import context
@@ -204,6 +205,10 @@ class PXEValidateParametersTestCase(base.TestCase):
         self.assertEqual(os.stat(os.path.join(master_path, 'instance_uuid')).
                              st_nlink, 2)
 
+    def test__update_neutron(self):
+        # FIXME: just a stub for the moment
+        pass
+
 
 class PXEPrivateMethodsTestCase(base.TestCase):
 
@@ -433,8 +438,9 @@ class PXEDriverTestCase(db_base.DbTestCase):
         super(PXEDriverTestCase, self).setUp()
         self.context = context.get_admin_context()
         self.context.auth_token = '4562138218392831'
-        temp_dir = tempfile.mkdtemp()
-        CONF.set_default('tftp_root', temp_dir, group='pxe')
+        self.temp_dir = tempfile.mkdtemp()
+        CONF.set_default('tftp_root', self.temp_dir, group='pxe')
+        CONF.set_default('images_path', self.temp_dir, group='pxe')
         mgr_utils.get_mocked_node_manager(driver='fake_pxe')
         driver_info = INFO_DICT
         driver_info['pxe_deploy_key'] = 'fake-56789'
@@ -509,20 +515,19 @@ class PXEDriverTestCase(db_base.DbTestCase):
                               address='123456', iqn='aaa-bbb',
                               key='fake-12345')
 
-    def test_start_deploy(self):
-        with mock.patch.object(pxe, '_create_pxe_config') \
-                as create_pxe_config_mock:
+    def test_prepare(self):
+        with mock.patch.object(pxe,
+                '_create_pxe_config') as create_pxe_config_mock:
             with mock.patch.object(pxe, '_cache_images') as cache_images_mock:
-                with mock.patch.object(pxe, '_get_tftp_image_info') \
-                        as get_tftp_image_info_mock:
+                with mock.patch.object(pxe,
+                        '_get_tftp_image_info') as get_tftp_image_info_mock:
                     get_tftp_image_info_mock.return_value = None
                     create_pxe_config_mock.return_value = None
                     cache_images_mock.return_value = None
 
                     with task_manager.acquire(self.context,
-                                    [self.node['uuid']], shared=False) as task:
-                        state = task.resources[0].driver.deploy.deploy(task,
-                                                                    self.node)
+                                    self.node['uuid'], shared=True) as task:
+                        task.driver.deploy.prepare(task, self.node)
                         get_tftp_image_info_mock.assert_called_once_with(
                                                                   self.node)
                         create_pxe_config_mock.assert_called_once_with(task,
@@ -530,11 +535,41 @@ class PXEDriverTestCase(db_base.DbTestCase):
                                                                     None)
                         cache_images_mock.assert_called_once_with(self.node,
                                                                   None)
-                        self.assertEqual(state, states.DEPLOYING)
 
-                        t_path = pxe._get_token_file_path(self.node['uuid'])
-                        token = open(t_path, 'r').read()
-                        self.assertEqual(self.context.auth_token, token)
+    def test_deploy(self):
+        with mock.patch.object(pxe, '_update_neutron') as update_neutron_mock:
+            with mock.patch.object(manager_utils,
+                    'node_power_action') as node_power_mock:
+                with task_manager.acquire(self.context,
+                        self.node['uuid'], shared=False) as task:
+                    state = task.driver.deploy.deploy(task, self.node)
+                    self.assertEqual(state, states.DEPLOYING)
+                    update_neutron_mock.assert_called_once_with(task,
+                                                                self.node)
+                    node_power_mock.assert_called_once_with(task, self.node,
+                                                            states.REBOOT)
+
+                    # ensure token file created
+                    t_path = pxe._get_token_file_path(self.node['uuid'])
+                    token = open(t_path, 'r').read()
+                    self.assertEqual(self.context.auth_token, token)
+
+    def test_tear_down(self):
+        with mock.patch.object(manager_utils,
+                'node_power_action') as node_power_mock:
+            with task_manager.acquire(self.context,
+                    self.node['uuid']) as task:
+                state = task.driver.deploy.tear_down(task, self.node)
+                self.assertEqual(state, states.DELETED)
+                node_power_mock.assert_called_once_with(task, self.node,
+                                                        states.POWER_OFF)
+
+    def test_take_over(self):
+        with mock.patch.object(pxe, '_update_neutron') as update_neutron_mock:
+            with task_manager.acquire(
+                    self.context, self.node['uuid'], shared=True) as task:
+                task.driver.deploy.take_over(task, self.node)
+                update_neutron_mock.assert_called_once_with(task, self.node)
 
     def test_continue_deploy_good(self):
         token_path = self._create_token_file()
@@ -580,11 +615,7 @@ class PXEDriverTestCase(db_base.DbTestCase):
                 # lock elevated w/o exception
                 _continue_deploy_mock.assert_called_once()
 
-    def tear_down_config(self, master=None):
-        temp_dir = tempfile.mkdtemp()
-        CONF.set_default('tftp_root', temp_dir, group='pxe')
-        CONF.set_default('images_path', temp_dir, group='pxe')
-
+    def clean_up_config(self, master=None):
         ports = []
         ports.append(
             self.dbapi.create_port(
@@ -594,7 +625,7 @@ class PXEDriverTestCase(db_base.DbTestCase):
                     uuid='bb43dc0b-03f2-4d2e-ae87-c02d7f33cc53',
                     node_id='123')))
 
-        d_kernel_path = os.path.join(temp_dir,
+        d_kernel_path = os.path.join(self.temp_dir,
                                      'instance_uuid_123/deploy_kernel')
         image_info = {'deploy_kernel': ['deploy_kernel_uuid', d_kernel_path]}
 
@@ -602,11 +633,11 @@ class PXEDriverTestCase(db_base.DbTestCase):
                 as get_tftp_image_info_mock:
             get_tftp_image_info_mock.return_value = image_info
 
-            pxecfg_dir = os.path.join(temp_dir, 'pxelinux.cfg')
+            pxecfg_dir = os.path.join(self.temp_dir, 'pxelinux.cfg')
             os.makedirs(pxecfg_dir)
 
-            instance_dir = os.path.join(temp_dir, 'instance_uuid_123')
-            image_dir = os.path.join(temp_dir, 'fake_instance_name')
+            instance_dir = os.path.join(self.temp_dir, 'instance_uuid_123')
+            image_dir = os.path.join(self.temp_dir, 'fake_instance_name')
             os.makedirs(instance_dir)
             os.makedirs(image_dir)
             config_path = os.path.join(instance_dir, 'config')
@@ -616,8 +647,9 @@ class PXEDriverTestCase(db_base.DbTestCase):
             open(config_path, 'w').close()
             os.link(config_path, pxe_mac_path)
             if master:
-                tftp_master_dir = os.path.join(temp_dir, 'tftp_master')
-                instance_master_dir = os.path.join(temp_dir, 'instance_master')
+                tftp_master_dir = os.path.join(self.temp_dir, 'tftp_master')
+                instance_master_dir = os.path.join(self.temp_dir,
+                                                   'instance_master')
                 CONF.set_default('tftp_master_path',
                                  tftp_master_dir,
                                  group='pxe')
@@ -635,9 +667,9 @@ class PXEDriverTestCase(db_base.DbTestCase):
                 os.link(master_deploy_kernel_path, deploy_kernel_path)
                 os.link(master_instance_path, image_path)
                 if master == 'in_use':
-                    deploy_kernel_link = os.path.join(temp_dir,
+                    deploy_kernel_link = os.path.join(self.temp_dir,
                                                       'deploy_kernel_link')
-                    image_link = os.path.join(temp_dir, 'image_link')
+                    image_link = os.path.join(self.temp_dir, 'image_link')
                     os.link(master_deploy_kernel_path, deploy_kernel_link)
                     os.link(master_instance_path, image_link)
 
@@ -648,36 +680,39 @@ class PXEDriverTestCase(db_base.DbTestCase):
                 open(image_path, 'w').close()
 
             with task_manager.acquire(self.context, [self.node['uuid']],
-                                      shared=False) as task:
-                task.resources[0].driver.deploy.tear_down(task, self.node)
+                                      shared=True) as task:
+                task.resources[0].driver.deploy.clean_up(task, self.node)
             get_tftp_image_info_mock.called_once_with(self.node)
             assert_false_path = [config_path, deploy_kernel_path, image_path,
                                  pxe_mac_path, image_dir, instance_dir]
             for path in assert_false_path:
                 self.assertFalse(os.path.exists(path))
 
-            return temp_dir
+    def test_clean_up_removes_token_file(self):
+        token_path = self._create_token_file()
+        self.clean_up_config(master=None)
+        self.assertFalse(os.path.exists(token_path))
 
-    def test_tear_down_no_master_images(self):
-        self.tear_down_config(master=None)
+    def test_clean_up_no_master_images(self):
+        self.clean_up_config(master=None)
 
-    def test_tear_down_master_images_not_in_use(self):
-        temp_dir = self.tear_down_config(master='not_in_use')
+    def test_clean_up_master_images_not_in_use(self):
+        self.clean_up_config(master='not_in_use')
 
-        master_d_kernel_path = os.path.join(temp_dir,
+        master_d_kernel_path = os.path.join(self.temp_dir,
                                             'tftp_master/deploy_kernel_uuid')
-        master_instance_path = os.path.join(temp_dir,
+        master_instance_path = os.path.join(self.temp_dir,
                                             'instance_master/image_uuid')
 
         self.assertFalse(os.path.exists(master_d_kernel_path))
         self.assertFalse(os.path.exists(master_instance_path))
 
-    def test_tear_down_master_images_in_use(self):
-        temp_dir = self.tear_down_config(master='in_use')
+    def test_clean_up_master_images_in_use(self):
+        self.clean_up_config(master='in_use')
 
-        master_d_kernel_path = os.path.join(temp_dir,
+        master_d_kernel_path = os.path.join(self.temp_dir,
                                             'tftp_master/deploy_kernel_uuid')
-        master_instance_path = os.path.join(temp_dir,
+        master_instance_path = os.path.join(self.temp_dir,
                                              'instance_master/image_uuid')
 
         self.assertTrue(os.path.exists(master_d_kernel_path))