Move the 'instance_info' fields to GenericDriverFields

The instance_info parameters are common across all drivers and not
just for PXE drivers.

This patch corrects an issue in nova/virt/ironic/patcher.py by moving
the generation of the instance_info patch to the common base class.
It also adds notes stating the intent to remove the PXE-specific
patching in Kilo.

Closes-Bug: #1353631
Change-Id: I80d2208d41d21821f0da662637418ecd76b5db2b

2 files changed, 86 insertions, 48 deletions

diff --git a/ironic/nova/tests/virt/ironic/test_driver.py b/ironic/nova/tests/virt/ironic/test_driver.py
index 503231ab4..faa74058a 100644
--- a/ironic/nova/tests/virt/ironic/test_driver.py
+++ b/ironic/nova/tests/virt/ironic/test_driver.py
@@ -498,8 +498,16 @@ class IronicDriverTestCase(test.NoDBTestCase):
         node = ironic_utils.get_test_node(driver='fake')
         instance = fake_instance.fake_instance_obj(self.ctx,
                                                    node=node.uuid)
-        self.driver._add_driver_fields(node, instance, None, None)
-        expected_patch = [{'path': '/instance_uuid', 'op': 'add',
+        image_meta = ironic_utils.get_test_image_meta()
+        flavor = ironic_utils.get_test_flavor()
+        self.driver._add_driver_fields(node, instance, image_meta, flavor)
+        expected_patch = [{'path': '/instance_info/image_source', 'op': 'add',
+                           'value': image_meta['id']},
+                          {'path': '/instance_info/root_gb', 'op': 'add',
+                           'value': str(instance['root_gb'])},
+                          {'path': '/instance_info/swap_mb', 'op': 'add',
+                           'value': str(flavor['swap'])},
+                          {'path': '/instance_uuid', 'op': 'add',
                            'value': instance['uuid']}]
         mock_update.assert_called_once_with(node.uuid, expected_patch)
 
@@ -509,9 +517,11 @@ class IronicDriverTestCase(test.NoDBTestCase):
         node = ironic_utils.get_test_node(driver='fake')
         instance = fake_instance.fake_instance_obj(self.ctx,
                                                    node=node.uuid)
+        image_meta = ironic_utils.get_test_image_meta()
+        flavor = ironic_utils.get_test_flavor()
         self.assertRaises(exception.InstanceDeployFailure,
                           self.driver._add_driver_fields,
-                          node, instance, None, None)
+                          node, instance, image_meta, flavor)
 
     @mock.patch.object(flavor_obj.Flavor, 'get_by_id')
     @mock.patch.object(FAKE_CLIENT.node, 'update')
@@ -541,17 +551,18 @@ class IronicDriverTestCase(test.NoDBTestCase):
     @mock.patch.object(FAKE_CLIENT, 'node')
     @mock.patch.object(flavor_obj.Flavor, 'get_by_id')
     def test_spawn_node_driver_validation_fail(self, mock_flavor, mock_node):
+        mock_flavor.return_value = ironic_utils.get_test_flavor()
         node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
         node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid)
         instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
-        fake_flavor = {'ephemeral_gb': 0}
 
         mock_node.validate.return_value = ironic_utils.get_test_validation(
             power=False, deploy=False)
         mock_node.get.return_value = node
-        mock_flavor.return_value = fake_flavor
+        image_meta = ironic_utils.get_test_image_meta()
+
         self.assertRaises(exception.ValidationError, self.driver.spawn,
-                          self.ctx, instance, None, [], None)
+                          self.ctx, instance, image_meta, [], None)
         mock_node.get.assert_called_once_with(node_uuid)
         mock_node.validate.assert_called_once_with(node_uuid)
         mock_flavor.assert_called_with(mock.ANY, instance['instance_type_id'])
@@ -569,15 +580,15 @@ class IronicDriverTestCase(test.NoDBTestCase):
         instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
         mock_node.get.return_value = node
         mock_node.validate.return_value = ironic_utils.get_test_validation()
-        fake_flavor = {'ephemeral_gb': 0}
-        mock_flavor.return_value = fake_flavor
+        mock_flavor.return_value = ironic_utils.get_test_flavor()
+        image_meta = ironic_utils.get_test_image_meta()
 
         class TestException(Exception):
             pass
 
         mock_sf.side_effect = TestException()
         self.assertRaises(TestException, self.driver.spawn,
-                          self.ctx, instance, None, [], None)
+                          self.ctx, instance, image_meta, [], None)
 
         mock_node.get.assert_called_once_with(node_uuid)
         mock_node.validate.assert_called_once_with(node_uuid)
@@ -596,15 +607,15 @@ class IronicDriverTestCase(test.NoDBTestCase):
         node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
         node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid)
         instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
-        fake_flavor = {'ephemeral_gb': 0}
-        mock_flavor.return_value = fake_flavor
+        mock_flavor.return_value = ironic_utils.get_test_flavor()
+        image_meta = ironic_utils.get_test_image_meta()
 
         mock_node.get.return_value = node
         mock_node.validate.return_value = ironic_utils.get_test_validation()
 
         mock_node.set_provision_state.side_effect = exception.NovaException()
         self.assertRaises(exception.NovaException, self.driver.spawn,
-                          self.ctx, instance, None, [], None)
+                          self.ctx, instance, image_meta, [], None)
 
         mock_node.get.assert_called_once_with(node_uuid)
         mock_node.validate.assert_called_once_with(node_uuid)
@@ -623,15 +634,15 @@ class IronicDriverTestCase(test.NoDBTestCase):
         node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
         node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid)
         instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
-        fake_flavor = {'ephemeral_gb': 0}
-        mock_flavor.return_value = fake_flavor
+        mock_flavor.return_value = ironic_utils.get_test_flavor()
+        image_meta = ironic_utils.get_test_image_meta()
 
         mock_node.get.return_value = node
         mock_node.validate.return_value = ironic_utils.get_test_validation()
         mock_node.set_provision_state.side_effect = ironic_exception.BadRequest
         self.assertRaises(ironic_exception.BadRequest,
                           self.driver.spawn,
-                          self.ctx, instance, None, [], None)
+                          self.ctx, instance, image_meta, [], None)
 
         mock_node.get.assert_called_once_with(node_uuid)
         mock_node.validate.assert_called_once_with(node_uuid)
@@ -653,8 +664,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
         fake_net_info = utils.get_test_network_info()
         node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid)
         instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
-        fake_flavor = {'ephemeral_gb': 0}
-        mock_flavor.return_value = fake_flavor
+        mock_flavor.return_value = ironic_utils.get_test_flavor()
+        image_meta = ironic_utils.get_test_image_meta()
 
         mock_node.get.return_value = node
         mock_node.validate.return_value = ironic_utils.get_test_validation()
@@ -665,8 +676,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
         fake_looping_call.wait.side_effect = ironic_exception.BadRequest
         fake_net_info = utils.get_test_network_info()
         self.assertRaises(ironic_exception.BadRequest,
-                          self.driver.spawn,
-                          self.ctx, instance, None, [], None, fake_net_info)
+                          self.driver.spawn, self.ctx, instance,
+                          image_meta, [], None, fake_net_info)
         mock_destroy.assert_called_once_with(self.ctx, instance,
                                              fake_net_info)
 
@@ -681,15 +692,15 @@ class IronicDriverTestCase(test.NoDBTestCase):
                                                  mock_wait, mock_flavor,
                                                  mock_node, mock_save,
                                                  mock_looping):
+        mock_flavor.return_value = ironic_utils.get_test_flavor(ephemeral_gb=1)
         node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
         node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid)
         instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
-        fake_flavor = {'ephemeral_gb': 1}
-        mock_flavor.return_value = fake_flavor
         mock_node.get_by_instance_uuid.return_value = node
         mock_node.set_provision_state.return_value = mock.MagicMock()
+        image_meta = ironic_utils.get_test_image_meta()
 
-        self.driver.spawn(self.ctx, instance, None, [], None)
+        self.driver.spawn(self.ctx, instance, image_meta, [], None)
         mock_flavor.assert_called_once_with(self.ctx,
                                             instance['instance_type_id'])
         self.assertTrue(mock_save.called)
diff --git a/ironic/nova/virt/ironic/patcher.py b/ironic/nova/virt/ironic/patcher.py
index 4c8861f7e..20ad8dc6d 100644
--- a/ironic/nova/virt/ironic/patcher.py
+++ b/ironic/nova/virt/ironic/patcher.py
@@ -36,7 +36,7 @@ def create(node):
     :returns: GenericDriverFields or a subclass thereof, as appropriate
               for the supplied node.
     """
-    if 'pxe' in node.driver or 'agent' in node.driver:
+    if 'pxe' in node.driver:
         return PXEDriverFields(node)
     else:
         return GenericDriverFields(node)
@@ -49,9 +49,49 @@ class GenericDriverFields(object):
 
     def get_deploy_patch(self, instance, image_meta, flavor,
                          preserve_ephemeral=None):
-        return []
+        """Build a patch to add the required fields to deploy a node.
+
+        :param instance: the instance object.
+        :param image_meta: the metadata associated with the instance
+                           image.
+        :param flavor: the flavor object.
+        :param preserve_ephemeral: preserve_ephemeral status (bool) to be
+                                   specified during rebuild.
+        :returns: a json-patch with the fields that needs to be updated.
+
+        """
+        patch = []
+        patch.append({'path': '/instance_info/image_source', 'op': 'add',
+                      'value': image_meta['id']})
+        patch.append({'path': '/instance_info/root_gb', 'op': 'add',
+                      'value': str(instance['root_gb'])})
+        patch.append({'path': '/instance_info/swap_mb', 'op': 'add',
+                      'value': str(flavor['swap'])})
+
+        if instance.get('ephemeral_gb'):
+            patch.append({'path': '/instance_info/ephemeral_gb',
+                          'op': 'add',
+                          'value': str(instance['ephemeral_gb'])})
+            if CONF.default_ephemeral_format:
+                patch.append({'path': '/instance_info/ephemeral_format',
+                              'op': 'add',
+                              'value': CONF.default_ephemeral_format})
+
+        if preserve_ephemeral is not None:
+            patch.append({'path': '/instance_info/preserve_ephemeral',
+                          'op': 'add', 'value': str(preserve_ephemeral)})
+
+        return patch
 
     def get_cleanup_patch(self, instance, network_info, flavor):
+        """Build a patch to clean up the fields.
+
+        :param instance: the instance object.
+        :param network_info: the instance network information.
+        :param flavor: the flavor object.
+        :returns: a json-patch with the fields that needs to be updated.
+
+        """
         return []
 
 
@@ -91,32 +131,16 @@ class PXEDriverFields(GenericDriverFields):
         :returns: a json-patch with the fields that needs to be updated.
 
         """
-        patch = []
-        patch.append({'path': '/instance_info/image_source', 'op': 'add',
-                      'value': image_meta['id']})
-        patch.append({'path': '/instance_info/root_gb', 'op': 'add',
-                      'value': str(instance['root_gb'])})
-        patch.append({'path': '/instance_info/swap_mb', 'op': 'add',
-                      'value': str(flavor['swap'])})
+        patch = super(PXEDriverFields, self).get_deploy_patch(
+                    instance, image_meta, flavor, preserve_ephemeral)
 
-        # If flavor contains both ramdisk and kernel ids, use them
+        # TODO(lucasagomes): Remove it in Kilo. This is for backwards
+        # compatibility with Icehouse. If flavor contains both ramdisk
+        # and kernel ids, use them.
         for key, value in self._get_kernel_ramdisk_dict(flavor).items():
             patch.append({'path': '/driver_info/%s' % key,
                           'op': 'add', 'value': value})
 
-        if instance.get('ephemeral_gb'):
-            patch.append({'path': '/instance_info/ephemeral_gb',
-                          'op': 'add',
-                          'value': str(instance['ephemeral_gb'])})
-            if CONF.default_ephemeral_format:
-                patch.append({'path': '/instance_info/ephemeral_format',
-                              'op': 'add',
-                              'value': CONF.default_ephemeral_format})
-
-        if preserve_ephemeral is not None:
-            patch.append({'path': '/instance_info/preserve_ephemeral',
-                          'op': 'add', 'value': str(preserve_ephemeral)})
-
         return patch
 
     def get_cleanup_patch(self, instance, network_info, flavor):
@@ -133,9 +157,12 @@ class PXEDriverFields(GenericDriverFields):
         :returns: a json-patch with the fields that needs to be updated.
 
         """
-        patch = []
-        # If flavor contains a ramdisk and kernel id remove it from nodes
-        # as part of the tear down process
+        patch = super(PXEDriverFields, self).get_cleanup_patch(
+                    instance, network_info, flavor)
+
+        # TODO(lucasagomes): Remove it in Kilo. This is for backwards
+        # compatibility with Icehouse. If flavor contains a ramdisk and
+        # kernel id remove it from nodes as part of the tear down process
         for key in self._get_kernel_ramdisk_dict(flavor):
             if key in self.node.driver_info:
                 patch.append({'op': 'remove',