Merge "Basic ring builder validation."

swift/common/ring/builder.py

@@ -39,12 +39,22 @@ class RingBuilder(object):
     a rebalance request is an isolated request or due to added, changed, or
     removed devices.
 
-    :param part_power: number of partitions = 2**part_power
+    :param part_power: number of partitions = 2**part_power.
     :param replicas: number of replicas for each partition
     :param min_part_hours: minimum number of hours between partition changes
     """
 
     def __init__(self, part_power, replicas, min_part_hours):
+        if part_power > 32:
+            raise ValueError("part_power must be at most 32 (was %d)"
+                             % (part_power,))
+        if replicas < 1:
+            raise ValueError("replicas must be at least 1 (was %.6f)"
+                             % (replicas,))
+        if min_part_hours < 0:
+            raise ValueError("min_part_hours must be non-negative (was %d)"
+                             % (min_part_hours,))
+
         self.part_power = part_power
         self.replicas = replicas
         self.min_part_hours = min_part_hours

test/unit/common/ring/test_builder.py

@@ -47,6 +47,18 @@ class TestRingBuilder(unittest.TestCase):
         self.assertEquals(rb.devs_changed, False)
         self.assertEquals(rb.version, 0)
 
+    def test_overlarge_part_powers(self):
+        ring.RingBuilder(32, 3, 1)  # passes by not crashing
+        self.assertRaises(ValueError, ring.RingBuilder, 33, 3, 1)
+
+    def test_insufficient_replicas(self):
+        ring.RingBuilder(8, 1.0, 1)  # passes by not crashing
+        self.assertRaises(ValueError, ring.RingBuilder, 8, 0.999, 1)
+
+    def test_negative_min_part_hours(self):
+        ring.RingBuilder(8, 3, 0)  # passes by not crashing
+        self.assertRaises(ValueError, ring.RingBuilder, 8, 3, -1)
+
     def test_get_ring(self):
         rb = ring.RingBuilder(8, 3, 1)
         rb.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 1,