diff --git a/hooks/swift_hooks.py b/hooks/swift_hooks.py index ed2aa7d..e3b4b25 100755 --- a/hooks/swift_hooks.py +++ b/hooks/swift_hooks.py @@ -235,11 +235,15 @@ def storage_changed(): # Allow for multiple devs per unit, passed along as a : separated list # Update and balance rings. + nodes = [] devs = relation_get('device') if devs: - node_settings['devices'] = devs.split(':') + for dev in devs.split(':'): + node = {k: v for k, v in node_settings.items()} + node['device'] = dev + nodes.append(node) - update_rings(node_settings) + update_rings(nodes) if not is_paused(): # Restart proxy here in case no config changes made (so # pause_aware_restart_on_change() ineffective). diff --git a/lib/swift_utils.py b/lib/swift_utils.py index 76c73d7..472758f 100644 --- a/lib/swift_utils.py +++ b/lib/swift_utils.py @@ -424,7 +424,7 @@ def exists_in_ring(ring_path, node): return False -def add_to_ring(ring_path, node, device): +def add_to_ring(ring_path, node): ring = _load_builder(ring_path) port = ring_port(ring_path, node) @@ -438,7 +438,7 @@ def add_to_ring(ring_path, node, device): 'zone': node['zone'], 'ip': node['ip'], 'port': port, - 'device': device, + 'device': node['device'], 'weight': 100, 'meta': '', } @@ -779,7 +779,7 @@ def sync_builders_and_rings_if_changed(f): @sync_builders_and_rings_if_changed -def update_rings(node_settings=None, min_part_hours=None): +def update_rings(nodes=[], min_part_hours=None): """Update builder with node settings and balance rings if necessary. Also update min_part_hours if provided. @@ -811,12 +811,11 @@ def update_rings(node_settings=None, min_part_hours=None): else: balance_required = True - if node_settings: - for dev in node_settings.get('devices', []): - for ring in SWIFT_RINGS.itervalues(): - if not exists_in_ring(ring, node_settings): - add_to_ring(ring, node_settings, dev) - balance_required = True + for node in nodes: + for ring in SWIFT_RINGS.itervalues(): + if not exists_in_ring(ring, node): + add_to_ring(ring, node) + balance_required = True if balance_required: balance_rings() diff --git a/unit_tests/test_swift_utils.py b/unit_tests/test_swift_utils.py index 0ff380c..a792cdb 100644 --- a/unit_tests/test_swift_utils.py +++ b/unit_tests/test_swift_utils.py @@ -77,6 +77,82 @@ class SwiftUtilsTestCase(unittest.TestCase): self.assertTrue(mock_set_min_hours.called) self.assertTrue(mock_balance_rings.called) + @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, + 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, + ): + # 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' + # key, which stores the list of passed dev(s) by add_dev(). + # + # If swift (actual) ring representation diverges (see _load_builder), + # this mock will need to be adapted. + mock_rings = {} + + def mock_load_builder_fn(path): + class mock_ring(object): + def __init__(self, path): + self.path = path + + def to_dict(self): + return mock_rings[self.path] + + 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_load_builder.side_effect = mock_load_builder_fn + mock_initialize_ring.side_effect = mock_initialize_ring_fn + + init_ring_paths(tempfile.mkdtemp()) + devices = ['sdb', 'sdc'] + node_settings = { + 'object_port': 6000, + 'container_port': 6001, + 'account_port': 6002, + 'zone': 1, + 'ip': '1.2.3.4', + } + for path in swift_utils.SWIFT_RINGS.itervalues(): + swift_utils.initialize_ring(path, 8, 3, 0) + + # verify all devices added to each ring + nodes = [] + for dev in devices: + node = {k: v for k, v in node_settings.items()} + node['device'] = dev + nodes.append(node) + + swift_utils.update_rings(nodes) + for path in swift_utils.SWIFT_RINGS.itervalues(): + devs = swift_utils._load_builder(path).to_dict()['devs'] + added_devices = [dev['device'] for dev in devs] + self.assertEqual(devices, added_devices) + + # try re-adding, assert add_to_ring was not called + with mock.patch('swift_utils.add_to_ring') as mock_add_to_ring: + 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')