Merge "Fix race condition for WaitCondition with several signals" into stable/mitaka

3 files changed, 143 insertions, 23 deletions

diff --git a/heat/engine/resources/openstack/heat/wait_condition_handle.py b/heat/engine/resources/openstack/heat/wait_condition_handle.py
index 7a93657a2..7cc70afc8 100644
--- a/heat/engine/resources/openstack/heat/wait_condition_handle.py
+++ b/heat/engine/resources/openstack/heat/wait_condition_handle.py
@@ -202,16 +202,18 @@ class HeatWaitConditionHandle(wc_base.BaseWaitConditionHandle):
         Optionally "id" may also be specified, but if missing the index
         of the signal received will be used.
         """
-        rsrc_metadata = self.metadata_get(refresh=True)
-        signal_num = len(rsrc_metadata) + 1
+        return super(HeatWaitConditionHandle, self).handle_signal(details)
+
+    def normalise_signal_data(self, signal_data, latest_metadata):
+        signal_num = len(latest_metadata) + 1
         reason = 'Signal %s received' % signal_num
         # Tolerate missing values, default to success
-        metadata = details or {}
+        metadata = signal_data.copy() if signal_data else {}
         metadata.setdefault(self.REASON, reason)
         metadata.setdefault(self.DATA, None)
         metadata.setdefault(self.UNIQUE_ID, signal_num)
         metadata.setdefault(self.STATUS, self.STATUS_SUCCESS)
-        return super(HeatWaitConditionHandle, self).handle_signal(metadata)
+        return metadata
 
 
 class UpdateWaitConditionHandle(aws_wch.WaitConditionHandle):
diff --git a/heat/engine/resources/wait_condition.py b/heat/engine/resources/wait_condition.py
index 6691e8a04..79b63b7be 100644
--- a/heat/engine/resources/wait_condition.py
+++ b/heat/engine/resources/wait_condition.py
@@ -52,26 +52,38 @@ class BaseWaitConditionHandle(signal_responder.SignalResponder):
         if sorted(tuple(six.iterkeys(metadata))) == sorted(self.METADATA_KEYS):
             return self._status_ok(metadata[self.STATUS])
 
-    def handle_signal(self, metadata=None):
-        signal_reason = None
-        if self._metadata_format_ok(metadata):
-            rsrc_metadata = self.metadata_get(refresh=True)
-            if metadata[self.UNIQUE_ID] in rsrc_metadata:
+    def normalise_signal_data(self, signal_data, latest_metadata):
+        return signal_data
+
+    def handle_signal(self, details=None):
+        write_attempts = []
+
+        def merge_signal_metadata(signal_data, latest_rsrc_metadata):
+            signal_data = self.normalise_signal_data(signal_data,
+                                                     latest_rsrc_metadata)
+
+            if not self._metadata_format_ok(signal_data):
+                LOG.error(_LE("Metadata failed validation for %s"), self.name)
+                raise ValueError(_("Metadata format invalid"))
+
+            new_entry = signal_data.copy()
+            unique_id = new_entry.pop(self.UNIQUE_ID)
+
+            new_rsrc_metadata = latest_rsrc_metadata.copy()
+            if unique_id in new_rsrc_metadata:
                 LOG.warning(_LW("Overwriting Metadata item for id %s!"),
-                            metadata[self.UNIQUE_ID])
-            safe_metadata = {}
-            for k in self.METADATA_KEYS:
-                if k == self.UNIQUE_ID:
-                    continue
-                safe_metadata[k] = metadata[k]
-            rsrc_metadata.update({metadata[self.UNIQUE_ID]: safe_metadata})
-            self.metadata_set(rsrc_metadata)
-            signal_reason = ('status:%s reason:%s' %
-                             (safe_metadata[self.STATUS],
-                              safe_metadata[self.REASON]))
-        else:
-            LOG.error(_LE("Metadata failed validation for %s"), self.name)
-            raise ValueError(_("Metadata format invalid"))
+                            unique_id)
+            new_rsrc_metadata.update({unique_id: new_entry})
+
+            write_attempts.append(signal_data)
+            return new_rsrc_metadata
+
+        self.metadata_set(details, merge_metadata=merge_signal_metadata)
+
+        data_written = write_attempts[-1]
+        signal_reason = ('status:%s reason:%s' %
+                         (data_written[self.STATUS],
+                          data_written[self.REASON]))
         return signal_reason
 
     def get_status(self):
diff --git a/heat_integrationtests/functional/test_os_wait_condition.py b/heat_integrationtests/functional/test_os_wait_condition.py
new file mode 100644
index 000000000..5eb329438
--- /dev/null
+++ b/heat_integrationtests/functional/test_os_wait_condition.py
@@ -0,0 +1,106 @@
+#    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.
+
+from heat_integrationtests.functional import functional_base
+
+
+class OSWaitCondition(functional_base.FunctionalTestsBase):
+
+    template = '''
+heat_template_version: 2013-05-23
+parameters:
+  flavor:
+    type: string
+  image:
+    type: string
+  network:
+    type: string
+  timeout:
+    type: number
+    default: 60
+resources:
+  instance1:
+    type: OS::Nova::Server
+    properties:
+      flavor: {get_param: flavor}
+      image: {get_param: image}
+      networks:
+      - network: {get_param: network}
+      user_data_format: RAW
+      user_data:
+        str_replace:
+          template: '#!/bin/sh
+
+            wc_notify --data-binary ''{"status": "SUCCESS"}''
+
+            # signals with reason
+
+            wc_notify --data-binary ''{"status": "SUCCESS", "reason":
+            "signal2"}''
+
+            # signals with data
+
+            wc_notify --data-binary ''{"status": "SUCCESS", "reason":
+            "signal3", "data": "data3"}''
+
+            wc_notify --data-binary ''{"status": "SUCCESS", "reason":
+            "signal4", "data": "data4"}''
+
+            # check signals with the same number
+
+            wc_notify --data-binary ''{"status": "SUCCESS", "id": "5"}''
+
+            wc_notify --data-binary ''{"status": "SUCCESS", "id": "5"}''
+
+            # loop for 25 signals without reasons and data
+
+            for i in `seq 1 25`; do wc_notify --data-binary ''{"status":
+            "SUCCESS"}'' & done
+
+            wait
+            '
+          params:
+            wc_notify:
+              get_attr: [wait_handle, curl_cli]
+
+  wait_condition:
+    type: OS::Heat::WaitCondition
+    depends_on: instance1
+    properties:
+      count: 30
+      handle: {get_resource: wait_handle}
+      timeout: {get_param: timeout}
+
+  wait_handle:
+    type: OS::Heat::WaitConditionHandle
+
+outputs:
+  curl_cli:
+    value:
+      get_attr: [wait_handle, curl_cli]
+  wc_data:
+    value:
+      get_attr: [wait_condition, data]
+'''
+
+    def setUp(self):
+        super(OSWaitCondition, self).setUp()
+        if not self.conf.minimal_image_ref:
+            raise self.skipException("No minimal image configured to test")
+        if not self.conf.minimal_instance_type:
+            raise self.skipException("No minimal flavor configured to test")
+
+    def test_create_stack_with_multi_signal_waitcondition(self):
+        params = {'flavor': self.conf.minimal_instance_type,
+                  'image': self.conf.minimal_image_ref,
+                  'network': self.conf.fixed_network_name}
+        self.stack_create(template=self.template, parameters=params)