Merge "ring: Centralize device normalization"

2 files changed, 32 insertions, 25 deletions

diff --git a/swift/common/ring/ring.py b/swift/common/ring/ring.py
index 98bc591f0..c3f726df6 100644
--- a/swift/common/ring/ring.py
+++ b/swift/common/ring/ring.py
@@ -48,6 +48,23 @@ def calc_replica_count(replica2part2dev_id):
     return base + extra
 
 
+def normalize_devices(devs):
+    # NOTE(akscram): Replication parameters like replication_ip
+    #                and replication_port are required for
+    #                replication process. An old replication
+    #                ring doesn't contain this parameters into
+    #                device. Old-style pickled rings won't have
+    #                region information.
+    for dev in devs:
+        if dev is None:
+            continue
+        dev.setdefault('region', 1)
+        if 'ip' in dev:
+            dev.setdefault('replication_ip', dev['ip'])
+        if 'port' in dev:
+            dev.setdefault('replication_port', dev['port'])
+
+
 class RingReader(object):
     chunk_size = 2 ** 16
 
@@ -118,6 +135,7 @@ class RingData(object):
 
     def __init__(self, replica2part2dev_id, devs, part_shift,
                  next_part_power=None, version=None):
+        normalize_devices(devs)
         self.devs = devs
         self._replica2part2dev_id = replica2part2dev_id
         self._part_shift = part_shift
@@ -125,10 +143,6 @@ class RingData(object):
         self.version = version
         self.md5 = self.size = self.raw_size = None
 
-        for dev in self.devs:
-            if dev is not None:
-                dev.setdefault("region", 1)
-
     @property
     def replica_count(self):
         """Number of replicas (full or partial) used in the ring."""
@@ -194,7 +208,10 @@ class RingData(object):
                 gz_file.seek(0)
                 ring_data = pickle.load(gz_file)
 
-        if not hasattr(ring_data, 'devs'):
+        if hasattr(ring_data, 'devs'):
+            # pickled RingData; make sure we've got region/replication info
+            normalize_devices(ring_data.devs)
+        else:
             ring_data = RingData(ring_data['replica2part2dev_id'],
                                  ring_data['devs'], ring_data['part_shift'],
                                  ring_data.get('next_part_power'),
@@ -306,20 +323,6 @@ class Ring(object):
 
             self._mtime = getmtime(self.serialized_path)
             self._devs = ring_data.devs
-            # NOTE(akscram): Replication parameters like replication_ip
-            #                and replication_port are required for
-            #                replication process. An old replication
-            #                ring doesn't contain this parameters into
-            #                device. Old-style pickled rings won't have
-            #                region information.
-            for dev in self._devs:
-                if dev:
-                    dev.setdefault('region', 1)
-                    if 'ip' in dev:
-                        dev.setdefault('replication_ip', dev['ip'])
-                    if 'port' in dev:
-                        dev.setdefault('replication_port', dev['port'])
-
             self._replica2part2dev_id = ring_data._replica2part2dev_id
             self._part_shift = ring_data._part_shift
             self._rebuild_tier_data()
diff --git a/test/unit/common/ring/test_ring.py b/test/unit/common/ring/test_ring.py
index 0f7e58e0c..55f45862e 100644
--- a/test/unit/common/ring/test_ring.py
+++ b/test/unit/common/ring/test_ring.py
@@ -68,8 +68,10 @@ class TestRingData(unittest.TestCase):
 
     def test_attrs(self):
         r2p2d = [[0, 1, 0, 1], [0, 1, 0, 1]]
-        d = [{'id': 0, 'zone': 0, 'region': 0, 'ip': '10.1.1.0', 'port': 7000},
-             {'id': 1, 'zone': 1, 'region': 1, 'ip': '10.1.1.1', 'port': 7000}]
+        d = [{'id': 0, 'zone': 0, 'region': 0, 'ip': '10.1.1.0', 'port': 7000,
+              'replication_ip': '10.1.1.0', 'replication_port': 7000},
+             {'id': 1, 'zone': 1, 'region': 1, 'ip': '10.1.1.1', 'port': 7000,
+              'replication_ip': '10.1.1.1', 'replication_port': 7000}]
         s = 30
         rd = ring.RingData(r2p2d, d, s)
         self.assertEqual(rd._replica2part2dev_id, r2p2d)
@@ -88,10 +90,12 @@ class TestRingData(unittest.TestCase):
                 pickle.dump(rd, f, protocol=p)
             meta_only = ring.RingData.load(ring_fname, metadata_only=True)
             self.assertEqual([
-                {'id': 0, 'zone': 0, 'region': 1, 'ip': '10.1.1.0',
-                 'port': 7000},
-                {'id': 1, 'zone': 1, 'region': 1, 'ip': '10.1.1.1',
-                 'port': 7000},
+                {'id': 0, 'zone': 0, 'region': 1,
+                 'ip': '10.1.1.0', 'port': 7000,
+                 'replication_ip': '10.1.1.0', 'replication_port': 7000},
+                {'id': 1, 'zone': 1, 'region': 1,
+                 'ip': '10.1.1.1', 'port': 7000,
+                 'replication_ip': '10.1.1.1', 'replication_port': 7000},
             ], meta_only.devs)
             # Pickled rings can't load only metadata, so you get it all
             self.assert_ring_data_equal(rd, meta_only)