From 821b9641661a363148a17d6952bed959c070a669 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Fri, 6 Mar 2020 16:48:59 -0800 Subject: [PATCH] ring: Flag region, zone, and device as required in add_dev They effectively already *were*, but if you used the RingBuilder API directly (rather than the CLI) you could previously write down builders that would hit KeyErrors on load. Change-Id: I1de895d4571f7464be920345881789d47659729f --- swift/common/ring/builder.py | 10 +++++----- test/unit/common/ring/test_builder.py | 8 +++++--- test/unit/common/ring/test_ring.py | 9 ++++++--- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index e3ccf9b59b..691374389a 100644 --- a/swift/common/ring/builder.py +++ b/swift/common/ring/builder.py @@ -419,11 +419,11 @@ class RingBuilder(object): # Add holes to self.devs to ensure self.devs[dev['id']] will be the dev while dev['id'] >= len(self.devs): self.devs.append(None) - required_keys = ('ip', 'port', 'weight') - if any(required not in dev for required in required_keys): - raise ValueError( - '%r is missing at least one the required key %r' % ( - dev, required_keys)) + required_keys = ('region', 'zone', 'ip', 'port', 'device', 'weight') + missing = tuple(key for key in required_keys if key not in dev) + if missing: + raise ValueError('%r is missing required key(s): %s' % ( + dev, ', '.join(missing))) dev['weight'] = float(dev['weight']) dev['parts'] = 0 dev.setdefault('meta', '') diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py index 3a6a2cc145..42a1994d49 100644 --- a/test/unit/common/ring/test_builder.py +++ b/test/unit/common/ring/test_builder.py @@ -231,19 +231,21 @@ class TestRingBuilder(unittest.TestCase): def test_add_dev(self): rb = ring.RingBuilder(8, 3, 1) dev = {'id': 0, 'region': 0, 'zone': 0, 'weight': 1, - 'ip': '127.0.0.1', 'port': 10000} + 'ip': '127.0.0.1', 'port': 10000, 'device': 'sda1'} dev_id = rb.add_dev(dev) self.assertRaises(exceptions.DuplicateDeviceError, rb.add_dev, dev) self.assertEqual(dev_id, 0) rb = ring.RingBuilder(8, 3, 1) # test add new dev with no id dev_id = rb.add_dev({'zone': 0, 'region': 1, 'weight': 1, - 'ip': '127.0.0.1', 'port': 6200}) + 'ip': '127.0.0.1', 'port': 6200, + 'device': 'sda2'}) self.assertEqual(rb.devs[0]['id'], 0) self.assertEqual(dev_id, 0) # test add another dev with no id dev_id = rb.add_dev({'zone': 3, 'region': 2, 'weight': 1, - 'ip': '127.0.0.1', 'port': 6200}) + 'ip': '127.0.0.1', 'port': 6200, + 'device': 'sda3'}) self.assertEqual(rb.devs[1]['id'], 1) self.assertEqual(dev_id, 1) # some keys are required diff --git a/test/unit/common/ring/test_ring.py b/test/unit/common/ring/test_ring.py index 3b4b1467a3..8ebe0f9437 100644 --- a/test/unit/common/ring/test_ring.py +++ b/test/unit/common/ring/test_ring.py @@ -559,7 +559,8 @@ class TestRing(TestRingBase): 'ip': '1.2.%d.%d' % (zone, server), 'port': 1234 + device, 'zone': zone, 'region': 0, - 'weight': 1.0}) + 'weight': 1.0, + 'device': "d%s" % device}) next_dev_id += 1 rb.rebalance(seed=2) rb.get_ring().save(self.testgz) @@ -604,7 +605,8 @@ class TestRing(TestRingBase): server = 0 rb.add_dev({'id': next_dev_id, 'ip': '1.2.%d.%d' % (zone, server), - 'port': 1234, 'zone': zone, 'region': 0, 'weight': 1.0}) + 'port': 1234, 'zone': zone, 'region': 0, 'weight': 1.0, + 'device': 'xd0'}) next_dev_id += 1 rb.pretend_min_part_hours_passed() num_parts_changed, _balance, _removed_dev = rb.rebalance(seed=2) @@ -836,7 +838,8 @@ class TestRing(TestRingBase): # 108.0 is the weight of all devices created prior to # this test in region 0; this way all regions have # equal combined weight - 'zone': 1, 'region': region, 'weight': 108.0}) + 'zone': 1, 'region': region, 'weight': 108.0, + 'device': 'sdx'}) next_dev_id += 1 rb.pretend_min_part_hours_passed() rb.rebalance(seed=1)