Compute Add build_instance hook in compute manager

The build_instance hook provides a hook close to the actual spawn
of an instance. It distinguishes itself from the create_instance
hook because, in the case of a successful build, the instance will
exist on the hypervisor when the build_instance post hook is
called, instead of simply just being scheduled as is the case with
the create_instance post hook. This is useful as some details may
not be available until after scheduling.

Co-Authored-By: Corey Wright <corey.wright@rackspace.com>
Change-Id: Ic12abf6315bbc6b010b8222ef296ec65cb4b4f95

nova/compute/build_results.py

@@ -0,0 +1,26 @@
+# Copyright 2014 OpenStack Foundation
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""Possible results from instance build
+
+Results represent the ultimate result of an attempt to build an instance.
+
+Results describe whether an instance was actually built, failed to build, or
+was rescheduled.
+"""
+
+ACTIVE = 'active'  # Instance is running
+FAILED = 'failed'  # Instance failed to build and was not rescheduled
+RESCHEDULED = 'rescheduled'  # Instance failed to build, but was rescheduled

nova/compute/manager.py

@@ -51,6 +51,7 @@ from nova import block_device
 from nova.cells import rpcapi as cells_rpcapi
 from nova.cloudpipe import pipelib
 from nova import compute
+from nova.compute import build_results
 from nova.compute import power_state
 from nova.compute import resource_tracker
 from nova.compute import rpcapi as compute_rpcapi
@@ -2008,6 +2009,7 @@ class ComputeManager(manager.Manager):
                       requested_networks, security_groups,
                       block_device_mapping, node, limits)
 
+    @hooks.add_hook('build_instance')
     @wrap_exception()
     @reverts_task_state
     @wrap_instance_event
@@ -2027,10 +2029,10 @@ class ComputeManager(manager.Manager):
         except exception.InstanceNotFound:
             msg = 'Instance disappeared before build.'
             LOG.debug(msg, instance=instance)
-            return
+            return build_results.FAILED
         except exception.UnexpectedTaskStateError as e:
             LOG.debug(e.format_message(), instance=instance)
-            return
+            return build_results.FAILED
 
         # b64 decode the files to inject:
         decoded_files = self._decode_files(injected_files)
@@ -2048,6 +2050,7 @@ class ComputeManager(manager.Manager):
                     decoded_files, admin_password, requested_networks,
                     security_groups, block_device_mapping, node, limits,
                     filter_properties)
+            return build_results.ACTIVE
         except exception.RescheduledException as e:
             LOG.debug(e.format_message(), instance=instance)
             retry = filter_properties.get('retry', None)
@@ -2060,7 +2063,7 @@ class ComputeManager(manager.Manager):
                 compute_utils.add_instance_fault_from_exc(context,
                         instance, e, sys.exc_info())
                 self._set_instance_error_state(context, instance)
-                return
+                return build_results.FAILED
             retry['exc'] = traceback.format_exception(*sys.exc_info())
             # NOTE(comstud): Deallocate networks if the driver wants
             # us to do so.
@@ -2075,12 +2078,14 @@ class ComputeManager(manager.Manager):
                     image, filter_properties, admin_password,
                     injected_files, requested_networks, security_groups,
                     block_device_mapping)
+            return build_results.RESCHEDULED
         except (exception.InstanceNotFound,
                 exception.UnexpectedDeletingTaskStateError):
             msg = 'Instance disappeared during build.'
             LOG.debug(msg, instance=instance)
             self._cleanup_allocated_networks(context, instance,
                     requested_networks)
+            return build_results.FAILED
         except exception.BuildAbortException as e:
             LOG.exception(e.format_message(), instance=instance)
             self._cleanup_allocated_networks(context, instance,
@@ -2090,6 +2095,7 @@ class ComputeManager(manager.Manager):
             compute_utils.add_instance_fault_from_exc(context, instance,
                     e, sys.exc_info())
             self._set_instance_error_state(context, instance)
+            return build_results.FAILED
         except Exception as e:
             # Should not reach here.
             msg = _LE('Unexpected build failure, not rescheduling build.')
@@ -2101,6 +2107,7 @@ class ComputeManager(manager.Manager):
             compute_utils.add_instance_fault_from_exc(context, instance,
                     e, sys.exc_info())
             self._set_instance_error_state(context, instance)
+            return build_results.FAILED
 
     def _build_and_run_instance(self, context, instance, image, injected_files,
             admin_password, requested_networks, security_groups,

nova/tests/unit/compute/test_compute.py

@@ -11447,3 +11447,8 @@ class ComputeHooksTestCase(test.BaseHookTestCase):
     def test_create_instance_has_hook(self):
         create_func = compute_api.API.create
         self.assert_has_hook('create_instance', create_func)
+
+    def test_build_instance_has_hook(self):
+        build_instance_func = (compute_manager.ComputeManager.
+                               _do_build_and_run_instance)
+        self.assert_has_hook('build_instance', build_instance_func)

nova/tests/unit/compute/test_compute_mgr.py

@@ -24,6 +24,7 @@ from oslo import messaging
 from oslo.utils import importutils
 from oslo.utils import timeutils
 
+from nova.compute import build_results
 from nova.compute import manager
 from nova.compute import power_state
 from nova.compute import task_states
@@ -2192,8 +2193,19 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 exc_val=mox.IgnoreArg(), exc_tb=mox.IgnoreArg(),
                 want_result=False)
 
+    @staticmethod
+    def _assert_build_instance_hook_called(mock_hooks, result):
+        # NOTE(coreywright): we want to test the return value of
+        # _do_build_and_run_instance, but it doesn't bubble all the way up, so
+        # mock the hooking, which allows us to test that too, though a little
+        # too intimately
+        mock_hooks.setdefault().run_post.assert_called_once_with(
+            'build_instance', result, mock.ANY, mock.ANY, f=None)
+
+    @mock.patch('nova.hooks._HOOKS')
     @mock.patch('nova.utils.spawn_n')
-    def test_build_and_run_instance_called_with_proper_args(self, mock_spawn):
+    def test_build_and_run_instance_called_with_proper_args(self, mock_spawn,
+                                                            mock_hooks):
         mock_spawn.side_effect = lambda f, *a, **k: f(*a, **k)
         self.mox.StubOutWithMock(self.compute, '_build_and_run_instance')
         self._do_build_instance_update()
@@ -2214,6 +2226,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 security_groups=self.security_groups,
                 block_device_mapping=self.block_device_mapping, node=self.node,
                 limits=self.limits)
+        self._assert_build_instance_hook_called(mock_hooks,
+                                                build_results.ACTIVE)
 
     # This test when sending an icehouse compatible rpc call to juno compute
     # node, NetworkRequest object can load from three items tuple.
@@ -2242,8 +2256,9 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
         self.assertEqual('10.0.0.1', str(requested_network.address))
         self.assertEqual('fake_port_id', requested_network.port_id)
 
+    @mock.patch('nova.hooks._HOOKS')
     @mock.patch('nova.utils.spawn_n')
-    def test_build_abort_exception(self, mock_spawn):
+    def test_build_abort_exception(self, mock_spawn, mock_hooks):
         def fake_spawn(f, *args, **kwargs):
             # NOTE(danms): Simulate the detached nature of spawn so that
             # we confirm that the inner task has the fault logic
@@ -2288,9 +2303,12 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 security_groups=self.security_groups,
                 block_device_mapping=self.block_device_mapping, node=self.node,
                 limits=self.limits)
+        self._assert_build_instance_hook_called(mock_hooks,
+                                                build_results.FAILED)
 
+    @mock.patch('nova.hooks._HOOKS')
     @mock.patch('nova.utils.spawn_n')
-    def test_rescheduled_exception(self, mock_spawn):
+    def test_rescheduled_exception(self, mock_spawn, mock_hooks):
         mock_spawn.side_effect = lambda f, *a, **k: f(*a, **k)
         self.mox.StubOutWithMock(self.compute, '_build_and_run_instance')
         self.mox.StubOutWithMock(self.compute, '_set_instance_error_state')
@@ -2320,6 +2338,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 security_groups=self.security_groups,
                 block_device_mapping=self.block_device_mapping, node=self.node,
                 limits=self.limits)
+        self._assert_build_instance_hook_called(mock_hooks,
+                                                build_results.RESCHEDULED)
 
     def test_rescheduled_exception_with_non_ascii_exception(self):
         exc = exception.NovaException(u's\xe9quence')
@@ -2355,8 +2375,9 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 self.block_device_mapping, self.node,
                 self.limits, self.filter_properties)
 
+    @mock.patch('nova.hooks._HOOKS')
     @mock.patch('nova.utils.spawn_n')
-    def test_rescheduled_exception_without_retry(self, mock_spawn):
+    def test_rescheduled_exception_without_retry(self, mock_spawn, mock_hooks):
         mock_spawn.side_effect = lambda f, *a, **k: f(*a, **k)
         self.mox.StubOutWithMock(self.compute, '_build_and_run_instance')
         self.mox.StubOutWithMock(compute_utils, 'add_instance_fault_from_exc')
@@ -2389,9 +2410,13 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 security_groups=self.security_groups,
                 block_device_mapping=self.block_device_mapping, node=self.node,
                 limits=self.limits)
+        self._assert_build_instance_hook_called(mock_hooks,
+                                                build_results.FAILED)
 
+    @mock.patch('nova.hooks._HOOKS')
     @mock.patch('nova.utils.spawn_n')
-    def test_rescheduled_exception_do_not_deallocate_network(self, mock_spawn):
+    def test_rescheduled_exception_do_not_deallocate_network(self, mock_spawn,
+                                                             mock_hooks):
         mock_spawn.side_effect = lambda f, *a, **k: f(*a, **k)
         self.mox.StubOutWithMock(self.compute, '_build_and_run_instance')
         self.mox.StubOutWithMock(self.compute.driver,
@@ -2425,9 +2450,13 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 security_groups=self.security_groups,
                 block_device_mapping=self.block_device_mapping, node=self.node,
                 limits=self.limits)
+        self._assert_build_instance_hook_called(mock_hooks,
+                                                build_results.RESCHEDULED)
 
+    @mock.patch('nova.hooks._HOOKS')
     @mock.patch('nova.utils.spawn_n')
-    def test_rescheduled_exception_deallocate_network(self, mock_spawn):
+    def test_rescheduled_exception_deallocate_network(self, mock_spawn,
+                                                      mock_hooks):
         mock_spawn.side_effect = lambda f, *a, **k: f(*a, **k)
         self.mox.StubOutWithMock(self.compute, '_build_and_run_instance')
         self.mox.StubOutWithMock(self.compute.driver,
@@ -2463,6 +2492,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 security_groups=self.security_groups,
                 block_device_mapping=self.block_device_mapping, node=self.node,
                 limits=self.limits)
+        self._assert_build_instance_hook_called(mock_hooks,
+                                                build_results.RESCHEDULED)
 
     def _test_build_and_run_exceptions(self, exc, set_error=False,
             cleanup_volumes=False):
@@ -2492,7 +2523,13 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
         self._instance_action_events()
         self.mox.ReplayAll()
 
-        with mock.patch('nova.utils.spawn_n') as mock_spawn:
+        with contextlib.nested(
+            mock.patch('nova.utils.spawn_n'),
+            mock.patch('nova.hooks._HOOKS')
+        ) as (
+            mock_spawn,
+            mock_hooks
+        ):
             mock_spawn.side_effect = lambda f, *a, **k: f(*a, **k)
             self.compute.build_and_run_instance(self.context, self.instance,
                 self.image, request_spec={},
@@ -2503,6 +2540,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 security_groups=self.security_groups,
                 block_device_mapping=self.block_device_mapping, node=self.node,
                 limits=self.limits)
+            self._assert_build_instance_hook_called(mock_hooks,
+                                                    build_results.FAILED)
 
     def test_build_and_run_notfound_exception(self):
         self._test_build_and_run_exceptions(exception.InstanceNotFound(