From ba87d247d9a3b5b6347eb539e8fc22f2f700de07 Mon Sep 17 00:00:00 2001
From: huangtianhua <huangtianhua@huawei.com>
Date: Thu, 19 Nov 2015 11:21:13 +0800
Subject: [PATCH] Don't snapshot if server hasn't been created yet

If a server has not been created yet, do not to snapshot
when deleting if the deletion_policy is Snapshot, otherwise
an error raises: Instance None could not be found.

This patch fix another problem: get the status of server
resource by getting state[1] instead of state[0].

Change-Id: I44559df2f56d4f99e82208d776ab0609b6589e7c
Closes-Bug: #1517692
---
 .../engine/resources/openstack/nova/server.py | 32 +++++++++++--------
 heat/tests/openstack/nova/test_server.py      | 25 +++++++++++++++
 2 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/heat/engine/resources/openstack/nova/server.py b/heat/engine/resources/openstack/nova/server.py
index f9b301bf27..c11515ebaa 100644
--- a/heat/engine/resources/openstack/nova/server.py
+++ b/heat/engine/resources/openstack/nova/server.py
@@ -1268,18 +1268,7 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin,
             zaqar.queue(queue_id).delete()
         self.data_delete('metadata_queue_id')
 
-    def handle_snapshot_delete(self, state):
-        if state[0] != self.FAILED:
-            image_id = self.client().servers.create_image(
-                self.resource_id, self.physical_resource_name())
-            return progress.ServerDeleteProgress(
-                self.resource_id, image_id, False)
-        return self.handle_delete()
-
-    def handle_delete(self):
-        if self.resource_id is None:
-            return
-
+    def _delete(self):
         if self.user_data_software_config():
             self._delete_user()
             self._delete_temp_url()
@@ -1296,6 +1285,23 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin,
             return
         return progress.ServerDeleteProgress(self.resource_id)
 
+    def handle_snapshot_delete(self, state):
+        if self.resource_id is None:
+            return
+
+        if state[1] != self.FAILED:
+            image_id = self.client().servers.create_image(
+                self.resource_id, self.physical_resource_name())
+            return progress.ServerDeleteProgress(
+                self.resource_id, image_id, False)
+        return self._delete()
+
+    def handle_delete(self):
+        if self.resource_id is None:
+            return
+
+        return self._delete()
+
     def check_delete_complete(self, prg):
         if not prg:
             return True
@@ -1306,7 +1312,7 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin,
                 raise exception.Error(image.status)
             elif image.status == 'ACTIVE':
                 prg.image_complete = True
-                if not self.handle_delete():
+                if not self._delete():
                     return True
             return False
 
diff --git a/heat/tests/openstack/nova/test_server.py b/heat/tests/openstack/nova/test_server.py
index 9a9fc0af89..a9d35572b5 100644
--- a/heat/tests/openstack/nova/test_server.py
+++ b/heat/tests/openstack/nova/test_server.py
@@ -3829,6 +3829,31 @@ class ServersTest(common.HeatTestCase):
 
         delete_server.assert_not_called()
 
+    def test_handle_snapshot_delete(self):
+        t = template_format.parse(wp_template)
+        t['Resources']['WebServer']['DeletionPolicy'] = 'Snapshot'
+        tmpl = template.Template(t)
+        stack = parser.Stack(
+            utils.dummy_context(), 'snapshot_policy', tmpl)
+        stack.store()
+        rsrc = stack['WebServer']
+        mock_plugin = self.patchobject(nova.NovaClientPlugin, '_create')
+        mock_plugin.return_value = self.fc
+        delete_server = self.patchobject(self.fc.servers, 'delete')
+        delete_server.side_effect = nova_exceptions.NotFound(404)
+        create_image = self.patchobject(self.fc.servers, 'create_image')
+
+        # test resource_id is None
+        rsrc.handle_snapshot_delete((rsrc.CREATE, rsrc.FAILED))
+        delete_server.assert_not_called()
+        create_image.assert_not_called()
+
+        # test has resource_id but state is CREATE_FAILED
+        rsrc.resource_id = '4567'
+        rsrc.handle_snapshot_delete((rsrc.CREATE, rsrc.FAILED))
+        delete_server.assert_called_once_with('4567')
+        create_image.assert_not_called()
+
 
 class ServerInternalPortTest(common.HeatTestCase):
     def setUp(self):