Merge "Update nova rebuild to account for new image"

ironic/nova/tests/virt/ironic/test_driver.py

@@ -28,6 +28,7 @@ from ironic.nova.virt.ironic import driver as ironic_driver
 from ironic.nova.virt.ironic import ironic_states
 
 from nova.compute import power_state as nova_states
+from nova.compute import task_states
 from nova import context as nova_context
 from nova import exception
 from nova.objects import flavor as flavor_obj
@@ -934,3 +935,96 @@ class IronicDriverTestCase(test.NoDBTestCase):
         fake_group = 'fake-security-group-members'
         self.driver.refresh_instance_security_rules(fake_group)
         mock_risr.assert_called_once_with(fake_group)
+
+    @mock.patch.object(ironic_driver.IronicDriver, '_wait_for_active')
+    @mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
+    @mock.patch.object(FAKE_CLIENT.node, 'set_provision_state')
+    @mock.patch.object(flavor_obj.Flavor, 'get_by_id')
+    @mock.patch.object(ironic_driver.IronicDriver, '_add_driver_fields')
+    @mock.patch.object(FAKE_CLIENT.node, 'get')
+    @mock.patch.object(instance_obj.Instance, 'save')
+    def _test_rebuild(self, mock_save, mock_get, mock_driver_fields,
+                      mock_fg_bid, mock_set_pstate, mock_looping,
+                      mock_wait_active, preserve=False):
+        node_uuid = uuidutils.generate_uuid()
+        instance_uuid = uuidutils.generate_uuid()
+        node = ironic_utils.get_test_node(uuid=node_uuid,
+                                          instance_uuid=instance_uuid,
+                                          instance_type_id=5)
+        mock_get.return_value = node
+
+        image_meta = ironic_utils.get_test_image_meta()
+        flavor_id = 5
+        flavor = {'id': flavor_id, 'name': 'baremetal'}
+        mock_fg_bid.return_value = flavor
+
+        instance = fake_instance.fake_instance_obj(self.ctx,
+                                                   uuid=instance_uuid,
+                                                   node=node_uuid,
+                                                   instance_type_id=flavor_id)
+
+
+        fake_looping_call = FakeLoopingCall()
+        mock_looping.return_value = fake_looping_call
+
+        self.driver.rebuild(
+            context=self.ctx, instance=instance, image_meta=image_meta,
+            injected_files=None, admin_password=None, bdms=None,
+            detach_block_devices=None, attach_block_devices=None,
+            preserve_ephemeral=preserve)
+
+        mock_save.assert_called_once_with(
+            expected_task_state=[task_states.REBUILDING])
+        mock_driver_fields.assert_called_once_with(node, instance, image_meta,
+                                                   flavor, preserve)
+        mock_set_pstate.assert_called_once_with(node_uuid,
+                                                ironic_states.REBUILD)
+        mock_looping.assert_called_once_with(mock_wait_active,
+                                             FAKE_CLIENT_WRAPPER,
+                                             instance)
+        fake_looping_call.start.assert_called_once_with(
+            interval=CONF.ironic.api_retry_interval)
+        fake_looping_call.wait.assert_called_once()
+
+    def test_rebuild_preserve_ephemeral(self):
+        self._test_rebuild(preserve=True)
+
+    def test_rebuild_no_preserve_ephemeral(self):
+        self._test_rebuild(preserve=False)
+
+    @mock.patch.object(FAKE_CLIENT.node, 'set_provision_state')
+    @mock.patch.object(flavor_obj.Flavor, 'get_by_id')
+    @mock.patch.object(ironic_driver.IronicDriver, '_add_driver_fields')
+    @mock.patch.object(FAKE_CLIENT.node, 'get')
+    @mock.patch.object(instance_obj.Instance, 'save')
+    def test_rebuild_failures(self, mock_save, mock_get, mock_driver_fields,
+                              mock_fg_bid, mock_set_pstate):
+        node_uuid = uuidutils.generate_uuid()
+        instance_uuid = uuidutils.generate_uuid()
+        node = ironic_utils.get_test_node(uuid=node_uuid,
+                                          instance_uuid=instance_uuid,
+                                          instance_type_id=5)
+        mock_get.return_value = node
+
+        image_meta = ironic_utils.get_test_image_meta()
+        flavor_id = 5
+        flavor = {'id': flavor_id, 'name': 'baremetal'}
+        mock_fg_bid.return_value = flavor
+
+        instance = fake_instance.fake_instance_obj(self.ctx,
+                                                   uuid=instance_uuid,
+                                                   node=node_uuid,
+                                                   instance_type_id=flavor_id)
+
+        exceptions = [
+            exception.NovaException(),
+            ironic_exception.BadRequest(),
+            ironic_exception.InternalServerError(),
+        ]
+        for e in exceptions:
+            mock_set_pstate.side_effect = e
+            self.assertRaises(exception.InstanceDeployFailure,
+                self.driver.rebuild,
+                context=self.ctx, instance=instance, image_meta=image_meta,
+                injected_files=None, admin_password=None, bdms=None,
+                detach_block_devices=None, attach_block_devices=None)

ironic/nova/tests/virt/ironic/test_patcher.py

@@ -70,6 +70,29 @@ class IronicDriverFieldsTestCase(test.NoDBTestCase):
         self.assertIn(expected1, patch)
         self.assertIn(expected2, patch)
 
+    def test_pxe_get_deploy_patch_preserve_ephemeral(self):
+        node = ironic_utils.get_test_node(driver='pxe_fake')
+        instance = fake_instance.fake_instance_obj(
+                        self.ctx, node=node.uuid, ephemeral_gb=10)
+        for preserve in [True, False]:
+            patch = patcher.create(node).get_deploy_patch(
+                    instance, self.image_meta, self.flavor,
+                    preserve_ephemeral=preserve)
+            expected =  {'path': '/instance_info/preserve_ephemeral',
+                         'value': str(preserve), 'op': 'add', }
+            self.assertIn(expected, patch)
+
+    def test_pxe_get_deploy_patch_no_preserve_ephemeral(self):
+        node = ironic_utils.get_test_node(driver='pxe_fake')
+        instance = fake_instance.fake_instance_obj(
+                        self.ctx, node=node.uuid, ephemeral_gb=10)
+        patch = patcher.create(node).get_deploy_patch(
+                    instance, self.image_meta, self.flavor)
+        for preserve in [True, False]:
+            unexpected =  {'path': '/instance_info/preserve_ephemeral',
+                           'value': str(preserve), 'op': 'add', }
+            self.assertNotIn(unexpected, patch)
+
     def test_pxe_get_deploy_patch_no_flavor_kernel_ramdisk_ids(self):
         self.flavor = ironic_utils.get_test_flavor(extra_specs={})
         node = ironic_utils.get_test_node(driver='pxe_fake')

ironic/nova/virt/ironic/driver.py

@@ -242,11 +242,13 @@ class IronicDriver(virt_driver.ComputeDriver):
     def _stop_firewall(self, instance, network_info):
         self.firewall_driver.unfilter_instance(instance, network_info)
 
-    def _add_driver_fields(self, node, instance, image_meta, flavor):
+    def _add_driver_fields(self, node, instance, image_meta, flavor,
+                           preserve_ephemeral=None):
         icli = client_wrapper.IronicClientWrapper()
         patch = patcher.create(node).get_deploy_patch(instance,
                                                       image_meta,
-                                                      flavor)
+                                                      flavor,
+                                                      preserve_ephemeral)
 
         # Associate the node with an instance
         patch.append({'path': '/instance_uuid', 'op': 'add',
@@ -868,21 +870,14 @@ class IronicDriver(virt_driver.ComputeDriver):
         instance.task_state = task_states.REBUILD_SPAWNING
         instance.save(expected_task_state=[task_states.REBUILDING])
 
-        node_uuid = instance.get('node')
-
+        node_uuid = instance['node']
         icli = client_wrapper.IronicClientWrapper()
+        node = icli.call("node.get", node_uuid)
+        flavor = flavor_obj.Flavor.get_by_id(context,
+                                             instance['instance_type_id'])
 
-        # Update instance_info for the ephemeral preservation value.
-        patch = []
-        patch.append({'path': '/instance_info/preserve_ephemeral',
-                      'op': 'add', 'value': str(preserve_ephemeral)})
-        try:
-            icli.call('node.update', node_uuid, patch)
-        except ironic_exception.BadRequest:
-            msg = (_("Failed to add deploy parameters on node %(node)s "
-                     "when rebuilding the instance %(instance)s")
-                   % {'node': node_uuid, 'instance': instance['uuid']})
-            raise exception.InstanceDeployFailure(msg)
+        self._add_driver_fields(node, instance, image_meta, flavor,
+                                preserve_ephemeral)
 
         # Trigger the node rebuild/redeploy.
         try:

ironic/nova/virt/ironic/patcher.py

@@ -47,7 +47,8 @@ class GenericDriverFields(object):
     def __init__(self, node):
         self.node = node
 
-    def get_deploy_patch(self, instance, image_meta, flavor):
+    def get_deploy_patch(self, instance, image_meta, flavor,
+                         preserve_ephemeral=None):
         return []
 
     def get_cleanup_patch(self, instance, network_info, flavor):
@@ -74,7 +75,8 @@ class PXEDriverFields(GenericDriverFields):
             deploy_ids['pxe_deploy_ramdisk'] = deploy_ramdisk
         return deploy_ids
 
-    def get_deploy_patch(self, instance, image_meta, flavor):
+    def get_deploy_patch(self, instance, image_meta, flavor,
+                         preserve_ephemeral=None):
         """Build a patch to add the required fields to deploy a node.
 
         Build a json-patch to add the required fields to deploy a node
@@ -82,8 +84,10 @@ class PXEDriverFields(GenericDriverFields):
 
         :param instance: the instance object.
         :param image_meta: the metadata associated with the instance
-                            image.
+                           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.
 
         """
@@ -108,6 +112,11 @@ class PXEDriverFields(GenericDriverFields):
                 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):