Merge lp:~hopem/charms/trusty/swift-proxy/lp1448884 into lp:~openstack-charmers-archive/charms/trusty/swift-proxy/next

[Edward Hope-Morley]

[Chris Holcombe (xfactor973)]

Ed, First off this is fantastic work! This will greatly help the swift charm along. I have a few questions though.

Could you talk a little bit about what cases this solves? For example:
1. Adding a new node to the swift cluster
2. Deleting a node from the cluster
3. Reimaging a previous node and putting it back into service
4. A node goes offline for repairs and then comes back later ( say a day or a week )
Can this handle all these cases? It's hard to tell just from sifting through the code.

Are there potential race conditions with this that can leave the cluster is a broken state if the leader changes midway through the sync? Just curious.

I had a few little questions inline in the code about variable names.

[Edward Hope-Morley (hopem)]

Hey Chris, sure thing and thanks for the review.

I've added README.Swift_ring_management with a basic explanation of swift ring
and builder management in the charm. We can always add more overtime as-and-when.

First i'd like to stress that this is mostly a refactor (upgrade safe) that is aimed at making the ring sync process more robust and cleaner to facilitate easier debugging.

With regards to:

   deleting nodes - for storage nodes, this is currently a manual process and
                    something that we need to grow the ability to do in the
                    charm (probably using actions). For proxy units,
                    information about proxy units/hosts is not in the
                    rings/builders so as long as they are synced and the leader
                    switches, removing a proxy unit should not interrupt
                    service.

   repairing nodes - again, same as above

I've responded to your comment inline as well.

[Edward Hope-Morley (hopem)]

(this is a hopefully better formatted version of my previous comment)

Hey Chris, sure thing.

I've added README.Swift_ring_management with a basic explanation of swift ring and builder management in the charm. We can always add more overtime as-and-when.

With regards to:

   deleting nodes - for storage nodes, this is currently a manual process and something that we need to grow the ability to do in the charm (probably using actions). For proxy units, information about proxy units/hosts is not in the rings/builders so as long as they are synced and the leader switches, removing a proxy unit should not interrupt service.

   repairing nodes - again, same as above

I've responded to your comment inline as well.

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #835 swift-proxy-next for hopem mp282296
    LINT OK: passed

Build: http://10.245.162.36:8080/job/charm_lint_check/835/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #739 swift-proxy-next for hopem mp282296
    UNIT OK: passed

Build: http://10.245.162.36:8080/job/charm_unit_test/739/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #337 swift-proxy-next for hopem mp282296
    AMULET OK: passed

Build: http://10.245.162.36:8080/job/charm_amulet_test/337/

[Edward Hope-Morley (hopem)]

Based on the new workflow I've migrated this to git for review - https://review.openstack.org/#/c/287123/

Unmerged revisions

136. By Edward Hope-Morley on 2016-02-18

add README.Swift_ring_management

135. By Edward Hope-Morley on 2016-02-18

sync /next

134. By Edward Hope-Morley on 2016-01-12

[hopem,r=]

Make the ring sync code clearer and implement
tolerance for leader-switch during or after
sync.

Closes-Bug: 1448884

=== added file 'README.Swift_ring_management'
--- README.Swift_ring_management	1970-01-01 00:00:00 +0000
+++ README.Swift_ring_management	2016-02-18 13:23:54 +0000
@@ -0,0 +1,28 @@
+== Swift Charm Ring Management ==
+
+Swift uses rings and builders to manage data distribution across storage devices
+in the cluster. More information on this can be found in the upstream
+documentation [0].
+
+In order to function correctly, the rings and builders need to be the same across
+all swift proxy units and the rings need to be distributed to storage units. The
+swift proxy charm achieves this by electing a leader unit and having that unit
+manage ring and builder files by updating them when new nodes or devices are added
+to the cluster and distrbuting those files to other nodes in the cluster.
+
+Since over time the leader may change, rings syncs use acknowledgements from peer
+units to determin whether a synchronisation has completed. This was if the leader
+swicthes to another unit, we are able to know that that unit has up-to-date
+ring and builder files.
+
+When new devices are added to storage units, the leader proxy unit is notified and
+adds them to the ring. Once complete, the leader unit will broadcast a notification
+that rings and builders are ready to be synced across the cluster (only proxy units
+get builders) and each unit in the cluster should then begin syncing from the leader
+and ackmowledge receipt.
+
+During synchronisation, swift-proxy services are stopped in order to avoid having
+requests being handled while rings are being updated.
+
+
+[0] http://docs.openstack.org/developer/swift/overview_ring.html

=== modified file 'hooks/swift_hooks.py'
--- hooks/swift_hooks.py	2015-11-27 09:48:45 +0000
+++ hooks/swift_hooks.py	2016-02-18 13:23:54 +0000
@@ -33,6 +33,10 @@
     get_first_available_value,
     all_responses_equal,
     ensure_www_dir_permissions,
+    sync_builders_and_rings_if_changed,
+    cluster_sync_rings,
+    is_most_recent_timestamp,
+    timestamps_available,
     is_paused,
     pause_aware_restart_on_change,
     assess_status,
@@ -143,7 +147,7 @@
         do_openstack_upgrade(CONFIGS)
         status_set('maintenance', 'Running openstack upgrade')
 
-    status_set('maintenance', 'Updating and balancing rings')
+    status_set('maintenance', 'Updating and (maybe) balancing rings')
     update_rings(min_part_hours=config('min-hours'))
 
     if not config('disable-ring-balance') and is_elected_leader(SWIFT_HA_RES):
@@ -322,7 +326,7 @@
         private_addr = unit_get('private-address')
 
 
-def all_peers_stopped(responses):
+def is_all_peers_stopped(responses):
     """Establish whether all peers have stopped their proxy services.
 
     Each peer unit will set stop-proxy-service-ack to rq value to indicate that
@@ -336,11 +340,15 @@
     ack_key = SwiftProxyClusterRPC.KEY_STOP_PROXY_SVC_ACK
     token = relation_get(attribute=rq_key, unit=local_unit())
     if not token or token != responses[0].get(ack_key):
-        log("Unmatched token in ack (expected=%s, got=%s)" %
+        log("Token mismatch, rq and ack tokens differ (expected=%s, "
+            "got=%s)" %
             (token, responses[0].get(ack_key)), level=DEBUG)
         return False
 
     if not all_responses_equal(responses, ack_key):
+        log("Not all ack responses are equal. Either we are still waiting "
+            "for responses or we were not the request originator.",
+            level=DEBUG)
         return False
 
     return True
@@ -354,20 +362,44 @@
     log("Cluster changed by unit=%s (local is leader)" % (remote_unit()),
         level=DEBUG)
 
-    # If we have received an ack, check other units
-    settings = relation_get() or {}
-    ack_key = SwiftProxyClusterRPC.KEY_STOP_PROXY_SVC_ACK
-
-    # Protect against leader changing mid-sync
-    if settings.get(SwiftProxyClusterRPC.KEY_STOP_PROXY_SVC):
-        log("Sync request received yet this is leader unit. This would "
-            "indicate that the leader has changed mid-sync - stopping proxy "
-            "and notifying peers", level=ERROR)
-        service_stop('swift-proxy')
-        SwiftProxyClusterRPC().notify_leader_changed()
-        return
-    elif ack_key in settings:
-        token = settings[ack_key]
+    rx_settings = relation_get() or {}
+    tx_settings = relation_get(unit=local_unit()) or {}
+
+    rx_rq_token = rx_settings.get(SwiftProxyClusterRPC.KEY_STOP_PROXY_SVC)
+    rx_ack_token = rx_settings.get(SwiftProxyClusterRPC.KEY_STOP_PROXY_SVC_ACK)
+
+    tx_rq_token = tx_settings.get(SwiftProxyClusterRPC.KEY_STOP_PROXY_SVC)
+    tx_ack_token = tx_settings.get(SwiftProxyClusterRPC.KEY_STOP_PROXY_SVC_ACK)
+
+    rx_leader_changed = \
+        rx_settings.get(SwiftProxyClusterRPC.KEY_NOTIFY_LEADER_CHANGED)
+    if rx_leader_changed:
+        log("Leader change notification received and this is leader so "
+            "retrying sync.", level=INFO)
+        # FIXME: check that we were previously part of a successful sync to
+        #        ensure we have good rings.
+        cluster_sync_rings(peers_only=tx_settings.get('peers-only', False),
+                           token=rx_leader_changed)
+        return
+
+    rx_resync_request = \
+        rx_settings.get(SwiftProxyClusterRPC.KEY_REQUEST_RESYNC)
+    resync_request_ack_key = SwiftProxyClusterRPC.KEY_REQUEST_RESYNC_ACK
+    tx_resync_request_ack = tx_settings.get(resync_request_ack_key)
+    if rx_resync_request and tx_resync_request_ack != rx_resync_request:
+        log("Unit '%s' has requested a resync" % (remote_unit()),
+            level=INFO)
+        cluster_sync_rings(peers_only=True)
+        relation_set(**{resync_request_ack_key: rx_resync_request})
+        return
+
+    # If we have received an ack token ensure it is not associated with a
+    # request we received from another peer. If it is, this would indicate
+    # a leadership change during a sync and this unit will abort the sync or
+    # attempt to restore the original leader so to be able to complete the
+    # sync.
+
+    if rx_ack_token and rx_ack_token == tx_rq_token:
         # Find out if all peer units have been stopped.
         responses = []
         for rid in relation_ids('cluster'):
@@ -375,21 +407,38 @@
                 responses.append(relation_get(rid=rid, unit=unit))
 
         # Ensure all peers stopped before starting sync
-        if all_peers_stopped(responses):
+        if is_all_peers_stopped(responses):
             key = 'peers-only'
             if not all_responses_equal(responses, key, must_exist=False):
                 msg = ("Did not get equal response from every peer unit for "
                        "'%s'" % (key))
                 raise SwiftProxyCharmException(msg)
 
-            peers_only = int(get_first_available_value(responses, key,
-                                                       default=0))
+            peers_only = bool(get_first_available_value(responses, key,
+                                                        default=0))
             log("Syncing rings and builders (peers-only=%s)" % (peers_only),
                 level=DEBUG)
-            broadcast_rings_available(token, storage=not peers_only)
+            broadcast_rings_available(broker_token=rx_ack_token,
+                                      storage=not peers_only)
         else:
             log("Not all peer apis stopped - skipping sync until all peers "
                 "ready (got %s)" % (responses), level=INFO)
+    elif ((rx_ack_token and (rx_ack_token == tx_ack_token)) or
+          (rx_rq_token and (rx_rq_token == rx_ack_token))):
+        log("It appears that the cluster leader has changed mid-sync - "
+            "stopping proxy service", level=WARNING)
+        service_stop('swift-proxy')
+        broker = rx_settings.get('builder-broker')
+        if broker:
+            # If we get here, manual intervention will be required in order
+            # to restore the cluster
+            log("Failed to restore previous broker '%s' as leader" %
+                (broker), level=ERROR)
+        else:
+            log("No builder-broker on rx_settings relation from '%s' - unable "
+                "to attempt leader restore" % (remote_unit()), level=INFO)
+    else:
+        log("Not taking any sync actions", level=DEBUG)
 
     CONFIGS.write_all()
 
@@ -401,31 +450,74 @@
     """
     log("Cluster changed by unit=%s (local is non-leader)" % (remote_unit()),
         level=DEBUG)
-    settings = relation_get() or {}
+    rx_settings = relation_get() or {}
+    tx_settings = relation_get(unit=local_unit()) or {}
+
+    token = rx_settings.get(SwiftProxyClusterRPC.KEY_NOTIFY_LEADER_CHANGED)
+    if token:
+        log("Leader-changed notification received from peer unit. Since "
+            "this most likely occurred during a ring sync proxies will "
+            "be disabled until the leader is restored and a fresh sync "
+            "request is set out", level=WARNING)
+        service_stop("swift-proxy")
+        return
+
+    rx_rq_token = rx_settings.get(SwiftProxyClusterRPC.KEY_STOP_PROXY_SVC)
 
     # Check whether we have been requested to stop proxy service
-    rq_key = SwiftProxyClusterRPC.KEY_STOP_PROXY_SVC
-    token = settings.get(rq_key, None)
-    if token:
+    if rx_rq_token:
         log("Peer request to stop proxy service received (%s) - sending ack" %
-            (token), level=INFO)
+            (rx_rq_token), level=INFO)
         service_stop('swift-proxy')
-        peers_only = settings.get('peers-only', None)
-        rq = SwiftProxyClusterRPC().stop_proxy_ack(echo_token=token,
+        peers_only = rx_settings.get('peers-only', None)
+        rq = SwiftProxyClusterRPC().stop_proxy_ack(echo_token=rx_rq_token,
                                                    echo_peers_only=peers_only)
         relation_set(relation_settings=rq)
         return
 
     # Check if there are any builder files we can sync from the leader.
-    log("Non-leader peer - checking if updated rings available", level=DEBUG)
-    broker = settings.get('builder-broker', None)
+    broker = rx_settings.get('builder-broker', None)
+    broker_token = rx_settings.get('broker-token', None)
+    broker_timestamp = rx_settings.get('broker-timestamp', None)
+    tx_ack_token = tx_settings.get(SwiftProxyClusterRPC.KEY_STOP_PROXY_SVC_ACK)
     if not broker:
-        log("No update available", level=DEBUG)
+        log("No ring/builder update available", level=DEBUG)
         if not is_paused():
             service_start('swift-proxy')
-        return
-
-    builders_only = int(settings.get('sync-only-builders', 0))
+
+        return
+    elif broker_token:
+        if tx_ack_token:
+            if broker_token == tx_ack_token:
+                log("Broker and ACK tokens match (%s)" % (broker_token),
+                    level=DEBUG)
+            else:
+                log("Received ring/builder update notification but tokens do "
+                    "not match (broker-token=%s/ack-token=%s)" %
+                    (broker_token, tx_ack_token), level=WARNING)
+                return
+        else:
+            log("Broker token available without handshake, assuming we just "
+                "joined and rings won't change", level=DEBUG)
+    else:
+        log("Not taking any sync actions", level=DEBUG)
+        return
+
+    # If we upgrade from cluster that did not use timestamps, the new peer will
+    # need to request a re-sync from the leader
+    if not is_most_recent_timestamp(broker_timestamp):
+        if not timestamps_available(excluded_unit=remote_unit()):
+            log("Requesting resync")
+            rq = SwiftProxyClusterRPC().request_resync(broker_token)
+            relation_set(relation_settings=rq)
+        else:
+            log("Did not receive most recent broker timestamp but timestamps "
+                "are available - waiting for next timestamp", level=INFO)
+
+        return
+
+    log("Ring/builder update available", level=DEBUG)
+    builders_only = int(rx_settings.get('sync-only-builders', 0))
     path = os.path.basename(get_www_dir())
     try:
         sync_proxy_rings('http://%s/%s' % (broker, path),
@@ -433,7 +525,7 @@
     except CalledProcessError:
         log("Ring builder sync failed, builders not yet available - "
             "leader not ready?", level=WARNING)
-        return None
+        return
 
     # Re-enable the proxy once all builders and rings are synced
     if fully_synced():
@@ -449,16 +541,6 @@
 @hooks.hook('cluster-relation-changed')
 @pause_aware_restart_on_change(restart_map())
 def cluster_changed():
-    key = SwiftProxyClusterRPC.KEY_NOTIFY_LEADER_CHANGED
-    leader_changed = relation_get(attribute=key)
-    if leader_changed:
-        log("Leader changed notification received from peer unit. Since this "
-            "most likely occurred during a ring sync proxies will be "
-            "disabled until the leader is restored and a fresh sync request "
-            "is set out", level=WARNING)
-        service_stop("swift-proxy")
-        return
-
     if is_elected_leader(SWIFT_HA_RES):
         cluster_leader_actions()
     else:
@@ -466,6 +548,7 @@
 
 
 @hooks.hook('ha-relation-changed')
+@sync_builders_and_rings_if_changed
 def ha_relation_changed():
     clustered = relation_get('clustered')
     if clustered:

=== modified file 'lib/swift_utils.py'
--- lib/swift_utils.py	2015-11-06 18:33:58 +0000
+++ lib/swift_utils.py	2016-02-18 13:23:54 +0000
@@ -7,6 +7,7 @@
 import subprocess
 import tempfile
 import threading
+import time
 import uuid
 
 from collections import OrderedDict
@@ -39,6 +40,7 @@
     INFO,
     WARNING,
     config,
+    local_unit,
     relation_get,
     unit_get,
     relation_set,
@@ -180,40 +182,56 @@
     KEY_STOP_PROXY_SVC = 'stop-proxy-service'
     KEY_STOP_PROXY_SVC_ACK = 'stop-proxy-service-ack'
     KEY_NOTIFY_LEADER_CHANGED = 'leader-changed-notification'
+    KEY_REQUEST_RESYNC = 'resync-request'
+    KEY_REQUEST_RESYNC_ACK = 'resync-request-ack'
 
     def __init__(self, version=1):
         self._version = version
 
+    @property
+    def _hostname(self):
+        hostname = get_hostaddr()
+        return format_ipv6_addr(hostname) or hostname
+
     def template(self):
         # Everything must be None by default so it gets dropped from the
         # relation unless we want it to be set.
         templates = {1: {'trigger': None,
                          'broker-token': None,
                          'builder-broker': None,
+                         'broker-timestamp': None,
                          self.KEY_STOP_PROXY_SVC: None,
                          self.KEY_STOP_PROXY_SVC_ACK: None,
                          self.KEY_NOTIFY_LEADER_CHANGED: None,
+                         self.KEY_REQUEST_RESYNC: None,
                          'peers-only': None,
                          'sync-only-builders': None}}
         return copy.deepcopy(templates[self._version])
 
-    def stop_proxy_request(self, peers_only=False):
+    def stop_proxy_request(self, peers_only=False, token=None):
         """Request to stop peer proxy service.
 
-        NOTE: leader action
+        A token can optionally be supplied in case we want to restart a sync
+        e.g. following a leader change notification.
+
+        NOTE: this action must only be performed by the cluster leader.
         """
         rq = self.template()
-        rq['trigger'] = str(uuid.uuid4())
+        if not token:
+            token = str(uuid.uuid4())
+
+        rq['trigger'] = token
         rq[self.KEY_STOP_PROXY_SVC] = rq['trigger']
         if peers_only:
             rq['peers-only'] = 1
 
+        rq['builder-broker'] = self._hostname
         return rq
 
     def stop_proxy_ack(self, echo_token, echo_peers_only):
         """Ack that peer proxy service is stopped.
 
-        NOTE: non-leader action
+        NOTE: this action must NOT be performed by the cluster leader.
         """
         rq = self.template()
         rq['trigger'] = str(uuid.uuid4())
@@ -222,11 +240,10 @@
         rq['peers-only'] = echo_peers_only
         return rq
 
-    def sync_rings_request(self, broker_host, broker_token,
-                           builders_only=False):
+    def sync_rings_request(self, broker_token, builders_only=False):
         """Request for peer to sync rings.
 
-        NOTE: leader action
+        NOTE: this action must only be performed by the cluster leader.
         """
         rq = self.template()
         rq['trigger'] = str(uuid.uuid4())
@@ -235,17 +252,34 @@
             rq['sync-only-builders'] = 1
 
         rq['broker-token'] = broker_token
-        rq['builder-broker'] = broker_host
+        rq['broker-timestamp'] = "%f" % time.time()
+        rq['builder-broker'] = self._hostname
         return rq
 
-    def notify_leader_changed(self):
+    def notify_leader_changed(self, token):
         """Notify peers that leader has changed.
 
-        NOTE: leader action
-        """
-        rq = self.template()
-        rq['trigger'] = str(uuid.uuid4())
-        rq[self.KEY_NOTIFY_LEADER_CHANGED] = rq['trigger']
+        The token passed in must be that associated with the sync we claim to
+        have been interrupted. It will be re-used by the restored leader once
+        it receives this notification.
+
+        NOTE: this action must only be performed by the cluster leader that
+              has relinquished it's leader status as part of the current hook
+              context.
+        """
+        rq = self.template()
+        rq['trigger'] = str(uuid.uuid4())
+        rq[self.KEY_NOTIFY_LEADER_CHANGED] = token
+        return rq
+
+    def request_resync(self, token):
+        """Request re-sync from leader.
+
+        NOTE: this action must not be performed by the cluster leader.
+        """
+        rq = self.template()
+        rq['trigger'] = str(uuid.uuid4())
+        rq[self.KEY_REQUEST_RESYNC] = token
         return rq
 
 
@@ -704,29 +738,27 @@
     return sha.hexdigest()
 
 
-def get_broker_token():
-    """Get ack token from peers to be used as broker token.
-
-    Must be equal across all peers.
-
-    Returns token or None if not found.
+def non_null_unique(data):
+    """Return True if data is a list containing all non-null values that are
+    all equal.
     """
-    responses = []
-    ack_key = SwiftProxyClusterRPC.KEY_STOP_PROXY_SVC_ACK
+    return (all(data) and len(set(data)) > 1)
+
+
+def previously_synced():
+    """If a full sync is not known to have been performed from this unit, False
+    is returned."""
+    broker = None
     for rid in relation_ids('cluster'):
-        for unit in related_units(rid):
-            responses.append(relation_get(attribute=ack_key, rid=rid,
-                                          unit=unit))
-
-    # If no acks exist we have probably never done a sync so make up a token
-    if len(responses) == 0:
-        return str(uuid.uuid4())
-
-    if not all(responses) or len(set(responses)) != 1:
-        log("Not all ack tokens equal - %s" % (responses), level=DEBUG)
-        return None
-
-    return responses[0]
+        broker = relation_get(attribute='builder-broker', rid=rid,
+                              unit=local_unit())
+        only_builders_synced = relation_get(attribute='sync-only-builders',
+                                            rid=rid, unit=local_unit())
+
+    if broker and not only_builders_synced:
+        return True
+
+    return False
 
 
 def sync_builders_and_rings_if_changed(f):
@@ -757,7 +789,8 @@
             rings_path = os.path.join(SWIFT_CONF_DIR, '*.%s' %
                                       (SWIFT_RING_EXT))
             rings_ready = len(glob.glob(rings_path)) == len(SWIFT_RINGS)
-            rings_changed = rings_after != rings_before
+            rings_changed = ((rings_after != rings_before) or
+                             not previously_synced())
             builders_changed = builders_after != builders_before
             if rings_changed or builders_changed:
                 # Copy builders and rings (if available) to the server dir.
@@ -766,9 +799,9 @@
                     # Trigger sync
                     cluster_sync_rings(peers_only=not rings_changed)
                 else:
-                    cluster_sync_rings(peers_only=True, builders_only=True)
                     log("Rings not ready for sync - syncing builders",
                         level=DEBUG)
+                    cluster_sync_rings(peers_only=True, builders_only=True)
             else:
                 log("Rings/builders unchanged - skipping sync", level=DEBUG)
 
@@ -869,20 +902,31 @@
             "skipping", level=WARNING)
         return
 
-    hostname = get_hostaddr()
-    hostname = format_ipv6_addr(hostname) or hostname
+    if not broker_token:
+        log("No broker token - aborting sync", level=WARNING)
+        return
+
+    cluster_rids = relation_ids('cluster')
+    if not cluster_rids:
+        log("Cluster relation not yet available - skipping sync", level=DEBUG)
+        return
+
+    if builders_only:
+        type = "builders"
+    else:
+        type = "builders & rings"
+
     # Notify peers that builders are available
-    log("Notifying peer(s) that rings are ready for sync.", level=INFO)
-    rq = SwiftProxyClusterRPC().sync_rings_request(hostname,
-                                                   broker_token,
+    log("Notifying peer(s) that %s are ready for sync." % type, level=INFO)
+    rq = SwiftProxyClusterRPC().sync_rings_request(broker_token,
                                                    builders_only=builders_only)
-    for rid in relation_ids('cluster'):
+    for rid in cluster_rids:
         log("Notifying rid=%s (%s)" % (rid, rq), level=DEBUG)
         relation_set(relation_id=rid, relation_settings=rq)
 
 
-def broadcast_rings_available(broker_token, peers=True, storage=True,
-                              builders_only=False):
+def broadcast_rings_available(storage=True, builders_only=False,
+                              broker_token=None):
     """Notify storage relations and cluster (peer) relations that rings and
     builders are availble for sync.
 
@@ -895,14 +939,13 @@
     else:
         log("Skipping notify storage relations", level=DEBUG)
 
-    if peers:
-        notify_peers_builders_available(broker_token,
-                                        builders_only=builders_only)
-    else:
-        log("Skipping notify peer relations", level=DEBUG)
-
-
-def cluster_sync_rings(peers_only=False, builders_only=False):
+    # Always set peer info even if not clustered so that info is present when
+    # units join
+    notify_peers_builders_available(broker_token,
+                                    builders_only=builders_only)
+
+
+def cluster_sync_rings(peers_only=False, builders_only=False, token=None):
     """Notify peer relations that they should stop their proxy services.
 
     Peer units will then be expected to do a relation_set with
@@ -923,24 +966,21 @@
         # If we have no peer units just go ahead and broadcast to storage
         # relations. If we have been instructed to only broadcast to peers this
         # should do nothing.
-        broker_token = get_broker_token()
-        broadcast_rings_available(broker_token, peers=False,
+        broadcast_rings_available(broker_token=str(uuid.uuid4()),
                                   storage=not peers_only)
         return
     elif builders_only:
+        if not token:
+            token = str(uuid.uuid4())
+
         # No need to stop proxies if only syncing builders between peers.
-        broker_token = get_broker_token()
-        broadcast_rings_available(broker_token, storage=False,
-                                  builders_only=builders_only)
+        broadcast_rings_available(storage=False, builders_only=True,
+                                  broker_token=token)
         return
 
-    rel_ids = relation_ids('cluster')
-    trigger = str(uuid.uuid4())
-
-    log("Sending request to stop proxy service to all peers (%s)" % (trigger),
-        level=INFO)
-    rq = SwiftProxyClusterRPC().stop_proxy_request(peers_only)
-    for rid in rel_ids:
+    log("Sending request to stop proxy service to all peers", level=INFO)
+    rq = SwiftProxyClusterRPC().stop_proxy_request(peers_only, token=token)
+    for rid in relation_ids('cluster'):
         relation_set(relation_id=rid, relation_settings=rq)
 
 
@@ -996,6 +1036,34 @@
     return unit_get('private-address')
 
 
+def is_most_recent_timestamp(timestamp):
+    ts = []
+    for rid in relation_ids('cluster'):
+        for unit in related_units(rid):
+            settings = relation_get(rid=rid, unit=unit)
+            t = settings.get('broker-timestamp')
+            if t:
+                ts.append(t)
+
+    if not ts or not timestamp:
+        return False
+
+    return timestamp >= max(ts)
+
+
+def timestamps_available(excluded_unit):
+    for rid in relation_ids('cluster'):
+        for unit in related_units(rid):
+            if unit == excluded_unit:
+                continue
+
+            settings = relation_get(rid=rid, unit=unit)
+            if settings.get('broker-timestamp'):
+                return True
+
+    return False
+
+
 def is_paused(status_get=status_get):
     """Is the unit paused?"""
     with HookData()():

=== modified file 'unit_tests/test_swift_hooks.py'
--- unit_tests/test_swift_hooks.py	2015-11-27 09:48:45 +0000
+++ unit_tests/test_swift_hooks.py	2016-02-18 13:23:54 +0000
@@ -8,41 +8,43 @@
 )
 
 sys.path.append("hooks")
-with patch('charmhelpers.core.hookenv.log'):
-    with patch('lib.swift_utils.is_paused') as is_paused:
-        import swift_hooks
+# swift_hooks.py does a register_configs() on import
+with patch('charmhelpers.contrib.openstack.utils.apt_cache', lambda: None):
+    with patch('charmhelpers.core.hookenv.log'):
+        with patch('lib.swift_utils.is_paused') as is_paused:
+            import swift_hooks
 
 
 class SwiftHooksTestCase(unittest.TestCase):
 
     @patch("swift_hooks.relation_get")
     @patch("swift_hooks.local_unit")
-    def test_all_peers_stopped(self, mock_local_unit, mock_relation_get):
+    def test_is_all_peers_stopped(self, mock_local_unit, mock_relation_get):
         token1 = str(uuid.uuid4())
         token2 = str(uuid.uuid4())
         mock_relation_get.return_value = token1
 
         responses = [{'some-other-key': token1}]
-        self.assertFalse(swift_hooks.all_peers_stopped(responses))
+        self.assertFalse(swift_hooks.is_all_peers_stopped(responses))
 
         responses = [{'stop-proxy-service-ack': token1},
                      {'stop-proxy-service-ack': token2}]
-        self.assertFalse(swift_hooks.all_peers_stopped(responses))
+        self.assertFalse(swift_hooks.is_all_peers_stopped(responses))
 
         responses = [{'stop-proxy-service-ack': token1},
                      {'stop-proxy-service-ack': token1},
                      {'some-other-key': token1}]
-        self.assertFalse(swift_hooks.all_peers_stopped(responses))
+        self.assertFalse(swift_hooks.is_all_peers_stopped(responses))
 
         responses = [{'stop-proxy-service-ack': token1},
                      {'stop-proxy-service-ack': token1}]
-        self.assertTrue(swift_hooks.all_peers_stopped(responses))
+        self.assertTrue(swift_hooks.is_all_peers_stopped(responses))
 
         mock_relation_get.return_value = token2
 
         responses = [{'stop-proxy-service-ack': token1},
                      {'stop-proxy-service-ack': token1}]
-        self.assertFalse(swift_hooks.all_peers_stopped(responses))
+        self.assertFalse(swift_hooks.is_all_peers_stopped(responses))
 
     @patch.object(swift_hooks, 'config')
     @patch('charmhelpers.contrib.openstack.ip.config')

=== modified file 'unit_tests/test_swift_utils.py'
--- unit_tests/test_swift_utils.py	2015-11-03 13:52:37 +0000
+++ unit_tests/test_swift_utils.py	2016-02-18 13:23:54 +0000
@@ -22,7 +22,7 @@
 
 class SwiftUtilsTestCase(unittest.TestCase):
 
-    @mock.patch('lib.swift_utils.get_broker_token')
+    @mock.patch.object(swift_utils, 'previously_synced')
     @mock.patch('lib.swift_utils.update_www_rings')
     @mock.patch('lib.swift_utils.get_builders_checksum')
     @mock.patch('lib.swift_utils.get_rings_checksum')
@@ -38,12 +38,12 @@
                           mock_log, mock_balance_rings,
                           mock_get_rings_checksum,
                           mock_get_builders_checksum, mock_update_www_rings,
-                          mock_get_broker_token):
-        mock_get_broker_token.return_value = "token1"
+                          mock_previously_synced):
 
         # Make sure same is returned for both so that we don't try to sync
         mock_get_rings_checksum.return_value = None
         mock_get_builders_checksum.return_value = None
+        mock_previously_synced.return_value = True
 
         # Test blocker 1
         mock_is_elected_leader.return_value = False
@@ -77,25 +77,23 @@
         self.assertTrue(mock_set_min_hours.called)
         self.assertTrue(mock_balance_rings.called)
 
+    @mock.patch('lib.swift_utils.previously_synced')
     @mock.patch('lib.swift_utils._load_builder')
     @mock.patch('lib.swift_utils.initialize_ring')
-    @mock.patch('lib.swift_utils.get_broker_token')
     @mock.patch('lib.swift_utils.update_www_rings')
     @mock.patch('lib.swift_utils.get_builders_checksum')
     @mock.patch('lib.swift_utils.get_rings_checksum')
     @mock.patch('lib.swift_utils.balance_rings')
     @mock.patch('lib.swift_utils.log')
     @mock.patch('lib.swift_utils.is_elected_leader')
-    def test_update_rings_multiple_devs(self,
-                                        mock_is_elected_leader,
+    def test_update_rings_multiple_devs(self, mock_is_elected_leader,
                                         mock_log, mock_balance_rings,
                                         mock_get_rings_checksum,
                                         mock_get_builders_checksum,
                                         mock_update_www_rings,
-                                        mock_get_broker_token,
                                         mock_initialize_ring,
                                         mock_load_builder,
-                                        ):
+                                        mock_previously_synced):
         # To avoid the need for swift.common.ring library, mock a basic
         # rings dictionary, keyed by path.
         # Each ring has enough logic to hold a dictionary with a single 'devs'
@@ -115,11 +113,13 @@
 
                 def add_dev(self, dev):
                     mock_rings[self.path]['devs'].append(dev)
+
             return mock_ring(path)
 
         def mock_initialize_ring_fn(path, *args):
             mock_rings.setdefault(path, {'devs': []})
 
+        mock_is_elected_leader.return_value = True
         mock_load_builder.side_effect = mock_load_builder_fn
         mock_initialize_ring.side_effect = mock_initialize_ring_fn
 
@@ -153,7 +153,6 @@
             swift_utils.update_rings(nodes)
             self.assertFalse(mock_add_to_ring.called)
 
-    @mock.patch('lib.swift_utils.get_broker_token')
     @mock.patch('lib.swift_utils.balance_rings')
     @mock.patch('lib.swift_utils.log')
     @mock.patch('lib.swift_utils.is_elected_leader')
@@ -165,9 +164,7 @@
                                                 mock_config,
                                                 mock_is_elected_leader,
                                                 mock_log,
-                                                mock_balance_rings,
-                                                mock_get_broker_token):
-        mock_get_broker_token.return_value = "token1"
+                                                mock_balance_rings):
 
         @swift_utils.sync_builders_and_rings_if_changed
         def mock_balance():
@@ -200,6 +197,7 @@
         finally:
             shutil.rmtree(tmpdir)
 
+    @mock.patch.object(swift_utils, 'get_hostaddr', lambda *args: '1.2.3.4')
     @mock.patch('lib.swift_utils.uuid')
     def test_cluster_rpc_stop_proxy_request(self, mock_uuid):
         mock_uuid.uuid4.return_value = 'test-uuid'
@@ -207,9 +205,11 @@
         rq = rpc.stop_proxy_request(peers_only=True)
         self.assertEqual({'trigger': 'test-uuid',
                           'broker-token': None,
-                          'builder-broker': None,
-                          'peers-only': True,
+                          'broker-timestamp': None,
+                          'builder-broker': '1.2.3.4',
+                          'peers-only': 1,
                           'leader-changed-notification': None,
+                          'resync-request': None,
                           'stop-proxy-service': 'test-uuid',
                           'stop-proxy-service-ack': None,
                           'sync-only-builders': None}, rq)
@@ -217,13 +217,18 @@
         rq = rpc.stop_proxy_request()
         self.assertEqual({'trigger': 'test-uuid',
                           'broker-token': None,
-                          'builder-broker': None,
+                          'broker-timestamp': None,
+                          'builder-broker': '1.2.3.4',
                           'peers-only': None,
                           'leader-changed-notification': None,
+                          'resync-request': None,
                           'stop-proxy-service': 'test-uuid',
                           'stop-proxy-service-ack': None,
                           'sync-only-builders': None}, rq)
 
+        template_keys = set(rpc.template())
+        self.assertTrue(set(rq.keys()).issubset(template_keys))
+
     @mock.patch('lib.swift_utils.uuid')
     def test_cluster_rpc_stop_proxy_ack(self, mock_uuid):
         mock_uuid.uuid4.return_value = 'token2'
@@ -232,40 +237,58 @@
         self.assertEqual({'trigger': 'token2',
                           'broker-token': None,
                           'builder-broker': None,
+                          'broker-timestamp': None,
                           'peers-only': '1',
                           'leader-changed-notification': None,
+                          'resync-request': None,
                           'stop-proxy-service': None,
                           'stop-proxy-service-ack': 'token1',
                           'sync-only-builders': None}, rq)
 
+        template_keys = set(rpc.template())
+        self.assertTrue(set(rq.keys()).issubset(template_keys))
+
+    @mock.patch.object(swift_utils, 'get_hostaddr', lambda *args: '1.2.3.4')
+    @mock.patch.object(swift_utils, 'time')
     @mock.patch('lib.swift_utils.uuid')
-    def test_cluster_rpc_sync_request(self, mock_uuid):
+    def test_cluster_rpc_sync_request(self, mock_uuid, mock_time):
+        mock_time.time = mock.Mock(return_value=float(1.234))
         mock_uuid.uuid4.return_value = 'token2'
         rpc = swift_utils.SwiftProxyClusterRPC()
-        rq = rpc.sync_rings_request('HostA', 'token1')
+        rq = rpc.sync_rings_request('token1')
         self.assertEqual({'trigger': 'token2',
                           'broker-token': 'token1',
-                          'builder-broker': 'HostA',
+                          'broker-timestamp': '1.234000',
+                          'builder-broker': '1.2.3.4',
                           'peers-only': None,
                           'leader-changed-notification': None,
+                          'resync-request': None,
                           'stop-proxy-service': None,
                           'stop-proxy-service-ack': None,
                           'sync-only-builders': None}, rq)
 
+        template_keys = set(rpc.template())
+        self.assertTrue(set(rq.keys()).issubset(template_keys))
+
     @mock.patch('lib.swift_utils.uuid')
     def test_cluster_rpc_notify_leader_changed(self, mock_uuid):
-        mock_uuid.uuid4.return_value = 'token1'
+        mock_uuid.uuid4.return_value = 'e4b67426-6cc0-4aa3-829d-227999cd0a75'
         rpc = swift_utils.SwiftProxyClusterRPC()
-        rq = rpc.notify_leader_changed()
-        self.assertEqual({'trigger': 'token1',
+        rq = rpc.notify_leader_changed('token1')
+        self.assertEqual({'trigger': 'e4b67426-6cc0-4aa3-829d-227999cd0a75',
                           'broker-token': None,
                           'builder-broker': None,
+                          'broker-timestamp': None,
                           'peers-only': None,
                           'leader-changed-notification': 'token1',
                           'stop-proxy-service': None,
                           'stop-proxy-service-ack': None,
+                          'resync-request': None,
                           'sync-only-builders': None}, rq)
 
+        template_keys = set(rpc.template().keys())
+        self.assertTrue(set(rq.keys()).issubset(template_keys))
+
     def test_all_responses_equal(self):
         responses = [{'a': 1, 'c': 3}]
         self.assertTrue(swift_utils.all_responses_equal(responses, 'b',
@@ -301,6 +324,42 @@
         rsps = []
         self.assertIsNone(swift_utils.get_first_available_value(rsps, 'key3'))
 
+    @mock.patch.object(swift_utils, 'relation_get')
+    @mock.patch.object(swift_utils, 'related_units', lambda arg: ['proxy/1'])
+    @mock.patch.object(swift_utils, 'relation_ids', lambda arg: ['cluster:1'])
+    def test_is_most_recent_timestamp(self, mock_rel_get):
+        mock_rel_get.return_value = {'broker-timestamp': '1111'}
+        self.assertTrue(swift_utils.is_most_recent_timestamp('1234'))
+        mock_rel_get.return_value = {'broker-timestamp': '2234'}
+        self.assertFalse(swift_utils.is_most_recent_timestamp('1234'))
+        mock_rel_get.return_value = {}
+        self.assertFalse(swift_utils.is_most_recent_timestamp('1234'))
+        mock_rel_get.return_value = {'broker-timestamp': '2234'}
+        self.assertFalse(swift_utils.is_most_recent_timestamp(None))
+
+    @mock.patch.object(swift_utils, 'relation_get')
+    @mock.patch.object(swift_utils, 'related_units', lambda arg: ['proxy/1'])
+    @mock.patch.object(swift_utils, 'relation_ids', lambda arg: ['cluster:1'])
+    def test_timestamps_available(self, mock_rel_get):
+        mock_rel_get.return_value = {}
+        self.assertFalse(swift_utils.timestamps_available('proxy/1'))
+        mock_rel_get.return_value = {'broker-timestamp': '1234'}
+        self.assertFalse(swift_utils.timestamps_available('proxy/1'))
+        mock_rel_get.return_value = {'broker-timestamp': '1234'}
+        self.assertTrue(swift_utils.timestamps_available('proxy/2'))
+
+    def _test_is_paused_unknown(self):
+        fake_status_get = lambda: ("unknown", "")
+        self.assertFalse(swift_utils.is_paused(status_get=fake_status_get))
+
+    def _test_is_paused_paused(self):
+        fake_status_get = lambda: ("maintenance", "Paused")
+        self.assertTrue(swift_utils.is_paused(status_get=fake_status_get))
+
+    def _test_is_paused_other_maintenance(self):
+        fake_status_get = lambda: ("maintenance", "Hook")
+        self.assertFalse(swift_utils.is_paused(status_get=fake_status_get))
+
     @mock.patch('lib.swift_utils.is_paused')
     @mock.patch('lib.swift_utils.config')
     @mock.patch('lib.swift_utils.set_os_workload_status')