[thedac,r=corey.bryant] Enable multiple devices per node

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).

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()

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')