Retry on conflict updating labels/annotations

We retry up to 10 times, with a delay of 500ms between attempts.

This is necessary because OpenShift itself may be updating the Build
object as well.

Add debug logging of the object before modification, for clarification
of why the object is changing.

Signed-off-by: Tim Waugh <twaugh@redhat.com>

osbs/core.py

@@ -6,6 +6,7 @@
 of the BSD license. See the LICENSE file for details.
 """
 from __future__ import print_function, unicode_literals, absolute_import
+from functools import wraps
 import json
 import os
 import numbers
@@ -53,6 +54,27 @@ def check_response(response):
         raise OsbsResponseException(message=content, status_code=response.status_code)
 
 
+def retry_on_conflict(func, sleep_seconds=0.5, max_attempts=10):
+    @wraps(func)
+    def retry(*args, **kwargs):
+        last_exception = None
+        for attempt in range(max_attempts):
+            if attempt != 0:
+                time.sleep(sleep_seconds)
+
+            try:
+                return func(*args, **kwargs)
+            except OsbsResponseException as ex:
+                if ex.status_code != httplib.CONFLICT:
+                    raise
+
+                last_exception = ex
+
+        raise last_exception or RuntimeError("operation not attempted")
+
+    return retry
+
+
 # TODO: error handling: create function which handles errors in response object
 class Openshift(object):
     def __init__(self, openshift_api_url, openshift_api_version, openshift_oauth_url,
@@ -641,6 +663,7 @@ def _update_metadata_things(metadata, things, values):
     def _replace_metadata_things(metadata, things, values):
         metadata[things] = values
 
+    @retry_on_conflict
     def adjust_attributes_on_object(self, collection, name, things, values, how):
         """
         adjust labels or annotations on object
@@ -657,7 +680,9 @@ def adjust_attributes_on_object(self, collection, name, things, values, how):
         :return:
         """
         url = self._build_url("%s/%s" % (collection, name))
-        build_json = self._get(url).json()
+        response = self._get(url)
+        logger.debug("before modification: %s", response.content)
+        build_json = response.json()
         how(build_json['metadata'], things, values)
         response = self._put(url, data=json.dumps(build_json), use_json=True)
         check_response(response)

tests/test_core.py

@@ -229,6 +229,58 @@ def test_get_multiple_build_config_by_labels(self, openshift):
             openshift.get_build_config_by_labels(label_selectors)
         assert str(exc.value).startswith('More than one build config found')
 
+    @pytest.mark.parametrize(('status_codes', 'should_raise'), [
+        ([httplib.OK], False),
+        ([httplib.CONFLICT, httplib.CONFLICT, httplib.OK], False),
+        ([httplib.CONFLICT, httplib.OK], False),
+        ([httplib.CONFLICT, httplib.CONFLICT, httplib.UNAUTHORIZED], True),
+        ([httplib.UNAUTHORIZED], True),
+        ([httplib.CONFLICT for _ in range(10)], True),
+    ])
+    @pytest.mark.parametrize('update_or_set', ['update', 'set'])
+    @pytest.mark.parametrize('attr_type', ['labels', 'annotations'])
+    @pytest.mark.parametrize('object_type', ['build', 'build_config'])
+    def test_retry_update_attributes(self, openshift,
+                                     status_codes, should_raise,
+                                     update_or_set,
+                                     attr_type,
+                                     object_type):
+        try:
+            fn = getattr(openshift,
+                         "{update}_{attr}_on_{object}"
+                         .format(update=update_or_set,
+                                 attr=attr_type,
+                                 object=object_type))
+        except AttributeError:
+            return  # not every combination is implemented
+
+        get_expectation = (flexmock(openshift)
+                           .should_receive('_get')
+                           .times(len(status_codes)))
+        put_expectation = (flexmock(openshift)
+                           .should_receive('_put')
+                           .times(len(status_codes)))
+        for status_code in status_codes:
+            get_response = HttpResponse(httplib.OK,
+                                        headers={},
+                                        content='{"metadata": {}}')
+            put_response = HttpResponse(status_code,
+                                        headers={},
+                                        content='')
+            get_expectation = get_expectation.and_return(get_response)
+            put_expectation = put_expectation.and_return(put_response)
+
+        (flexmock(time)
+            .should_receive('sleep')
+            .with_args(0.5))
+
+        args = ('any-object-id', {'key': 'value'})
+        if should_raise:
+            with pytest.raises(OsbsResponseException):
+                fn(*args)
+        else:
+            fn(*args)
+
     def test_put_image_stream_tag(self, openshift):
         tag_name = 'spam'
         tag_id = 'maps:' + tag_name