Cleanup ring manager storage relation settings

If local unit is no longer leader, clear rings_url on
storage relations to avoid storage units getting
rings from wrong proxy unit.

Also send broker-timestamp to storage units when providing
rings_url so that we have a means of knowing which is the most
recent. Broker timestamp is the same for peer and storage
sync so this enables identifying most recent.

Change-Id: I2c7e9028f345791bad0a736cb89979284b144e33
Closes-Bug: #1765203
This commit is contained in:
Edward Hope-Morley 2018-12-06 14:08:53 +00:00
parent cff08e7b98
commit f25d2c2d7f
4 changed files with 87 additions and 11 deletions

View File

@ -64,6 +64,7 @@ from lib.swift_utils import (
timestamps_available,
assess_status,
try_initialize_swauth,
clear_storage_rings_available,
)
import charmhelpers.contrib.openstack.utils as openstack
@ -251,6 +252,8 @@ def storage_joined(rid=None):
# possibility of storage nodes getting out-of-date rings by deprecating
# any existing ones from the www dir.
mark_www_rings_deleted()
clear_storage_rings_available()
try_initialize_swauth()
# NOTE(jamespage): Ensure private-address is set to any network-space

View File

@ -261,12 +261,16 @@ class SwiftProxyClusterRPC(object):
rq['peers-only'] = echo_peers_only
return rq
def sync_rings_request(self, broker_token, builders_only=False):
def sync_rings_request(self, broker_token, broker_timestamp,
builders_only=False):
"""Request for peers to sync rings from leader.
NOTE: this action must only be performed by the cluster leader.
:param broker_token: token to identify sync request.
@param broker_timestamp: timestamp for peer and storage sync - this
MUST bethe same as the one used for storage
sync.
:param builders_only: if False, tell peers to sync builders only (not
rings).
"""
@ -281,7 +285,7 @@ class SwiftProxyClusterRPC(object):
rq['sync-only-builders'] = 1
rq['broker-token'] = broker_token
rq['broker-timestamp'] = "{:f}".format(time.time())
rq['broker-timestamp'] = broker_timestamp
rq['builder-broker'] = self._hostname
return rq
@ -893,11 +897,15 @@ def mark_www_rings_deleted():
os.rename(path, "{}.deleted".format(path))
def notify_peers_builders_available(broker_token, builders_only=False):
def notify_peers_builders_available(broker_token, broker_timestamp,
builders_only=False):
"""Notify peer swift-proxy units that they should synchronise ring and
builder files.
Note that this should only be called from the leader unit.
@param broker_timestamp: timestamp for peer and storage sync - this MUST be
the same as the one used for storage sync.
"""
if not is_elected_leader(SWIFT_HA_RES):
log("Ring availability peer broadcast requested by non-leader - "
@ -922,6 +930,7 @@ def notify_peers_builders_available(broker_token, builders_only=False):
log("Notifying peer(s) that {} are ready for sync."
.format(_type), level=INFO)
rq = SwiftProxyClusterRPC().sync_rings_request(broker_token,
broker_timestamp,
builders_only=builders_only)
for rid in cluster_rids:
log("Notifying rid={} ({})".format(rid, rq), level=DEBUG)
@ -935,16 +944,21 @@ def broadcast_rings_available(storage=True, builders_only=False,
We can opt to only notify peer or storage relations if needs be.
"""
# NOTE: this MUST be the same for peer and storage sync
broker_timestamp = "{:f}".format(time.time())
if storage:
# TODO: get ack from storage units that they are synced before
# syncing proxies.
notify_storage_rings_available()
notify_storage_rings_available(broker_timestamp)
else:
log("Skipping notify storage relations", level=DEBUG)
# Always set peer info even if not clustered so that info is present when
# units join
notify_peers_builders_available(broker_token,
broker_timestamp,
builders_only=builders_only)
@ -987,11 +1001,14 @@ def cluster_sync_rings(peers_only=False, builders_only=False, token=None):
relation_set(relation_id=rid, relation_settings=rq)
def notify_storage_rings_available():
def notify_storage_rings_available(broker_timestamp):
"""Notify peer swift-storage relations that they should synchronise ring
and builder files.
Note that this should only be called from the leader unit.
@param broker_timestamp: timestamp for peer and storage sync - this MUST be
the same as the one used for peer sync.
"""
if not is_elected_leader(SWIFT_HA_RES):
log("Ring availability storage-relation broadcast requested by "
@ -1002,13 +1019,26 @@ def notify_storage_rings_available():
hostname = format_ipv6_addr(hostname) or hostname
path = os.path.basename(get_www_dir())
rings_url = 'http://{}/{}'.format(hostname, path)
# TODO(hopem): consider getting rid of this trigger since the timestamp
# should do the job.
trigger = uuid.uuid4()
# Notify storage nodes that there is a new ring to fetch.
log("Notifying storage nodes that new rings are ready for sync.",
level=INFO)
for relid in relation_ids('swift-storage'):
relation_set(relation_id=relid, swift_hash=get_swift_hash(),
rings_url=rings_url, trigger=trigger)
rings_url=rings_url, broker_timestamp=broker_timestamp,
trigger=trigger)
def clear_storage_rings_available():
"""Clear the rings_url setting so that storage units don't accidentally try
to sync from this unit (which is presumably no longer the leader).
"""
for relid in relation_ids('swift-storage'):
relation_set(relation_id=relid, rings_url=None)
def fully_synced():

View File

@ -239,6 +239,7 @@ class SwiftHooksTestCase(unittest.TestCase):
relation_set.assert_called_once_with(
relation_id='rid:23', rel_data='data')
@patch.object(swift_hooks, 'clear_storage_rings_available')
@patch.object(swift_hooks, 'is_elected_leader')
@patch.object(swift_hooks, 'get_relation_ip')
@patch.object(swift_hooks, 'try_initialize_swauth')
@ -249,7 +250,8 @@ class SwiftHooksTestCase(unittest.TestCase):
mark_www_rings_deleted,
try_initialize_swauth,
get_relation_ip,
is_elected_leader):
is_elected_leader,
mock_clear_storage_rings_available):
is_elected_leader.return_value = False
get_relation_ip.return_value = '10.10.20.243'
swift_hooks.storage_joined(rid='swift-storage:23')
@ -259,3 +261,4 @@ class SwiftHooksTestCase(unittest.TestCase):
relation_settings={'private-address': '10.10.20.243'}
)
try_initialize_swauth.assert_called_once()
mock_clear_storage_rings_available.assert_called_once()

View File

@ -311,13 +311,11 @@ class SwiftUtilsTestCase(unittest.TestCase):
@mock.patch('lib.swift_utils.is_elected_leader', lambda arg: True)
@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, mock_time):
mock_time.time = mock.Mock(return_value=float(1.234))
def test_cluster_rpc_sync_request(self, mock_uuid):
mock_uuid.uuid4.return_value = 'token2'
rpc = swift_utils.SwiftProxyClusterRPC()
rq = rpc.sync_rings_request('token1')
rq = rpc.sync_rings_request('token1', '1.234000')
self.assertEqual({'trigger': 'token2',
'broker-token': 'token1',
'broker-timestamp': '1.234000',
@ -512,3 +510,45 @@ class SwiftUtilsTestCase(unittest.TestCase):
'http://localhost:8080/auth',
'-K',
'Test'])
@mock.patch.object(swift_utils.uuid, 'uuid4')
@mock.patch.object(swift_utils, 'relation_set')
@mock.patch.object(swift_utils, 'get_swift_hash')
@mock.patch.object(swift_utils, 'log')
@mock.patch.object(swift_utils, 'relation_ids')
@mock.patch.object(swift_utils, 'get_www_dir')
@mock.patch.object(swift_utils, 'format_ipv6_addr')
@mock.patch.object(swift_utils, 'get_hostaddr')
@mock.patch.object(swift_utils, 'is_elected_leader')
def test_notify_storage_rings_available(self, mock_is_leader,
mock_get_hostaddr,
mock_format_ipv6_addr,
mock_get_www_dir,
mock_relation_ids,
mock_log,
mock_get_swift_hash,
mock_relation_set,
mock_uuid):
mock_is_leader.return_value = True
mock_get_hostaddr.return_value = '10.0.0.1'
mock_format_ipv6_addr.return_value = None
mock_get_www_dir.return_value = 'some/dir'
mock_relation_ids.return_value = ['storage:0']
mock_get_swift_hash.return_value = 'greathash'
mock_uuid.return_value = 'uuid-1234'
swift_utils.notify_storage_rings_available('1.234')
mock_relation_set.assert_called_once_with(
broker_timestamp='1.234',
relation_id='storage:0',
rings_url='http://10.0.0.1/dir',
swift_hash='greathash',
trigger='uuid-1234')
@mock.patch.object(swift_utils, 'relation_set')
@mock.patch.object(swift_utils, 'relation_ids')
def test_clear_notify_storage_rings_available(self, mock_relation_ids,
mock_relation_set):
mock_relation_ids.return_value = ['storage:0']
swift_utils.clear_storage_rings_available()
mock_relation_set.assert_called_once_with(
relation_id='storage:0', rings_url=None)