Don't always replace FAILED ServerGroup resources

ServerGroup resources work as scheduler hints for server
resources and replacement of it replaces the servers which
is undesirable in many circumstances.

- We don't allow update of ServerGroup resource properties
- If not created (CREATE_FAILED) without resource_id they will
  be replaced
- If does not exist in nova they would be created
- If exist in nova, FAILED resource status would be just
  changed to COMPLETE

Conflicts:
	heat/tests/openstack/nova/test_server_group.py

Fixed conflicts caused by 74164424016702bac6d52c6618360287ce7ea461 ,
which is present in stable/xena and later.

Task: 45748

Change-Id: I096dff2b541a5aa8afbbbcea5161e7ca1c244039
(cherry picked from commit a0e072b3204d736b9a4a5b2ceb2157f57405ac6f)
(cherry picked from commit f9f5d2cc368e5f850b962c8d89669a50544058d1)
(cherry picked from commit ead7cd61bd685ea46dd21c4ef3ea6b3fde273c10)

2 files changed, 45 insertions, 0 deletions

diff --git a/heat/engine/resources/openstack/nova/server_group.py b/heat/engine/resources/openstack/nova/server_group.py
index abaa8c6b7..13f3897e9 100644
--- a/heat/engine/resources/openstack/nova/server_group.py
+++ b/heat/engine/resources/openstack/nova/server_group.py
@@ -78,6 +78,16 @@ class ServerGroup(resource.Resource):
                                                    policies=policies)
         self.resource_id_set(server_group.id)
 
+    def needs_replace_failed(self):
+        if not self.resource_id:
+            return True
+
+        with self.client_plugin().ignore_not_found:
+            self._show_resource()
+            return False
+
+        return True
+
     def physical_resource_name(self):
         name = self.properties[self.NAME]
         if name:
diff --git a/heat/tests/openstack/nova/test_server_group.py b/heat/tests/openstack/nova/test_server_group.py
index 0a1e682e0..71db096ec 100644
--- a/heat/tests/openstack/nova/test_server_group.py
+++ b/heat/tests/openstack/nova/test_server_group.py
@@ -14,6 +14,9 @@
 import json
 from unittest import mock
 
+from novaclient import exceptions
+from oslo_utils import excutils
+
 from heat.common import template_format
 from heat.engine import scheduler
 from heat.tests import common
@@ -50,6 +53,20 @@ class NovaServerGroupTest(common.HeatTestCase):
         # create mock clients and objects
         nova = mock.MagicMock()
         self.sg.client = mock.MagicMock(return_value=nova)
+
+        class FakeNovaPlugin(object):
+
+            @excutils.exception_filter
+            def ignore_not_found(self, ex):
+                if not isinstance(ex, exceptions.NotFound):
+                    raise ex
+
+            def is_conflict(self, ex):
+                return False
+
+        self.patchobject(excutils.exception_filter, '__exit__')
+        self.patchobject(self.sg, 'client_plugin',
+                         return_value=FakeNovaPlugin())
         self.sg_mgr = nova.server_groups
 
     def _create_sg(self, name):
@@ -99,3 +116,21 @@ class NovaServerGroupTest(common.HeatTestCase):
         self.sg.client().server_groups = s_groups
         self.assertEqual({'server_gr': 'info'}, self.sg.FnGetAtt('show'))
         s_groups.get.assert_called_once_with('test')
+
+    def test_needs_replace_failed(self):
+        self._create_sg('test')
+        self.sg.state_set(self.sg.CREATE, self.sg.FAILED)
+        mock_show_resource = self.patchobject(self.sg, '_show_resource')
+        mock_show_resource.side_effect = [exceptions.NotFound(404), None]
+
+        self.sg.resource_id = None
+        self.assertTrue(self.sg.needs_replace_failed())
+        self.assertEqual(0, mock_show_resource.call_count)
+
+        self.sg.resource_id = 'sg_id'
+        self.assertTrue(self.sg.needs_replace_failed())
+        self.assertEqual(1, mock_show_resource.call_count)
+
+        mock_show_resource.return_value = None
+        self.assertFalse(self.sg.needs_replace_failed())
+        self.assertEqual(2, mock_show_resource.call_count)