Refactor OS::Nova::Server delete handling

OS::Nova::Server used an internal coroutine to handle_delete instead of
implementing check_delete_complete. While this mostly worked, the coroutine
didn't account for errors in deletion from Nova nor did it use a timeout. This
would lead to infinite execution when servers didn't delete properly. This
patch refactors delete to use the standard handle->check lifecycle methodology
and accounts for error statuses during delete.

Change-Id: I6b13c1201e46c0e73aa11e78825045b345931fb7
Partial-Bug: #1329453

heat/engine/resources/server.py

@@ -966,9 +966,7 @@ class Server(stack_user.StackUser):
                                     limits['maxPersonalitySize'], msg)
 
     def handle_delete(self):
-        '''
-        Delete a server, blocking until it is disposed by OpenStack
-        '''
+
         if self.resource_id is None:
             return
 
@@ -978,12 +976,16 @@ class Server(stack_user.StackUser):
         try:
             server = self.nova().servers.get(self.resource_id)
         except nova_exceptions.NotFound:
-            pass
-        else:
-            delete = scheduler.TaskRunner(nova_utils.delete_server, server)
-            delete(wait_time=0.2)
+            return
+        deleter = scheduler.TaskRunner(nova_utils.delete_server, server)
+        deleter.start()
+        return deleter
 
-        self.resource_id_set(None)
+    def check_delete_complete(self, deleter):
+        if deleter is None or deleter.step():
+            self.resource_id_set(None)
+            return True
+        return False
 
     def handle_suspend(self):
         '''

heat/tests/test_server.py

@@ -970,6 +970,33 @@ class ServersTest(HeatTestCase):
         self.assertEqual((server.DELETE, server.COMPLETE), server.state)
         self.m.VerifyAll()
 
+    def test_server_delete_error(self):
+        return_server = self.fc.servers.list()[1]
+        server = self._create_test_server(return_server,
+                                          'create_delete')
+        server.resource_id = '1234'
+
+        # this makes sure the auto increment worked on server creation
+        self.assertTrue(server.id > 0)
+
+        server_get = self.fc.client.get_servers_1234()
+        self.m.StubOutWithMock(self.fc.client, 'get_servers_1234')
+
+        def make_error():
+            server_get[1]["server"]['status'] = "ERROR"
+
+        get = self.fc.client.get_servers_1234
+        get().AndReturn(server_get)
+        get().AndReturn(server_get)
+        get().WithSideEffects(make_error).AndReturn(server_get)
+        mox.Replay(get)
+
+        resf = self.assertRaises(exception.ResourceFailure,
+                                 scheduler.TaskRunner(server.delete))
+        self.assertIn("Server sample-server delete failed", str(resf))
+
+        self.m.VerifyAll()
+
     def test_server_update_metadata(self):
         return_server = self.fc.servers.list()[1]
         server = self._create_test_server(return_server,