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:
parent
cff08e7b98
commit
f25d2c2d7f
|
@ -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
|
||||
|
|
|
@ -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():
|
||||
|
|
|
@ -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()
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue