From 98f9781096f4f8f989e11f52fee0ec145fb5a173 Mon Sep 17 00:00:00 2001 From: Christian Schwede Date: Sat, 12 Mar 2016 06:36:46 +0000 Subject: [PATCH] Add commands to ring-builder to change region / zone Currently one has to remove and re-add devices when the region or zone needs to be changed. Adding the subcommands set_region and set_zone simplifies this, now it is possible to change the region and/or zone easily. Note that there is no change to the required rebalancing; it is likely that data still needs to be moved within the cluster. This is mostly copy-n-paste of the existing set_weight subcommand and adopting tests accordingly. Some duplicated code in the tests has been aggregated as well. Change-Id: I37dd8e8ac24e2b0bb196758449a1d91a769b2e96 --- swift/cli/ringbuilder.py | 165 +++++++++++++++ swift/common/ring/builder.py | 40 ++++ test/unit/cli/test_ringbuilder.py | 324 +++++++++++++++++++++++++++++- 3 files changed, 523 insertions(+), 6 deletions(-) diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py index 9688efe327..59e0fb28f8 100644 --- a/swift/cli/ringbuilder.py +++ b/swift/cli/ringbuilder.py @@ -212,6 +212,32 @@ def _set_weight_values(devs, weight, opts): dev['weight'])) +def _set_region_values(devs, region, opts): + + input_question = 'Are you sure you want to update the region for these ' \ + '%s devices? (y/N) ' % len(devs) + abort_msg = 'Aborting device modifications' + check_devs(devs, input_question, opts, abort_msg) + + for dev in devs: + builder.set_dev_region(dev['id'], region) + print('%s region set to %s' % (format_device(dev), + dev['region'])) + + +def _set_zone_values(devs, zone, opts): + + input_question = 'Are you sure you want to update the zone for these ' \ + '%s devices? (y/N) ' % len(devs) + abort_msg = 'Aborting device modifications' + check_devs(devs, input_question, opts, abort_msg) + + for dev in devs: + builder.set_dev_zone(dev['id'], zone) + print('%s zone set to %s' % (format_device(dev), + dev['zone'])) + + def _parse_set_weight_values(argvish): new_cmd_format, opts, args = validate_args(argvish) @@ -301,6 +327,76 @@ def calculate_change_value(change_value, change, v_name, v_name_port): return change_value +def _parse_set_region_values(argvish): + + new_cmd_format, opts, args = validate_args(argvish) + + # We'll either parse the all-in-one-string format or the + # --options format, + # but not both. If both are specified, raise an error. + try: + devs = [] + if not new_cmd_format: + if len(args) % 2 != 0: + print(Commands.set_region.__doc__.strip()) + exit(EXIT_ERROR) + + devs_and_regions = izip(islice(argvish, 0, len(argvish), 2), + islice(argvish, 1, len(argvish), 2)) + for devstr, regionstr in devs_and_regions: + devs.extend(builder.search_devs( + parse_search_value(devstr)) or []) + region = int(regionstr) + _set_region_values(devs, region, opts) + else: + if len(args) != 1: + print(Commands.set_region.__doc__.strip()) + exit(EXIT_ERROR) + + devs.extend(builder.search_devs( + parse_search_values_from_opts(opts)) or []) + region = int(args[0]) + _set_region_values(devs, region, opts) + except ValueError as e: + print(e) + exit(EXIT_ERROR) + + +def _parse_set_zone_values(argvish): + + new_cmd_format, opts, args = validate_args(argvish) + + # We'll either parse the all-in-one-string format or the + # --options format, + # but not both. If both are specified, raise an error. + try: + devs = [] + if not new_cmd_format: + if len(args) % 2 != 0: + print(Commands.set_zone.__doc__.strip()) + exit(EXIT_ERROR) + + devs_and_zones = izip(islice(argvish, 0, len(argvish), 2), + islice(argvish, 1, len(argvish), 2)) + for devstr, zonestr in devs_and_zones: + devs.extend(builder.search_devs( + parse_search_value(devstr)) or []) + zone = int(zonestr) + _set_zone_values(devs, zone, opts) + else: + if len(args) != 1: + print(Commands.set_zone.__doc__.strip()) + exit(EXIT_ERROR) + + devs.extend(builder.search_devs( + parse_search_values_from_opts(opts)) or []) + zone = int(args[0]) + _set_zone_values(devs, zone, opts) + except ValueError as e: + print(e) + exit(EXIT_ERROR) + + def _parse_set_info_values(argvish): new_cmd_format, opts, args = validate_args(argvish) @@ -745,6 +841,75 @@ swift-ring-builder set_weight builder.save(builder_file) exit(EXIT_SUCCESS) + @staticmethod + def set_region(): + """ +swift-ring-builder set_region + [ set_region + --region --zone --ip --port + --replication-ip --replication-port + --device --meta [--yes] + + Where , and are replication ip, hostname + and port. + Any of the options are optional in both cases. + + Resets the devices' regions. No partitions will be reassigned to or from + the device until after running 'rebalance'. This is so you can make + multiple device changes and rebalance them all just once. + + Option --yes assume a yes response to all questions. + """ + if len(argv) < 5: + print(Commands.set_region.__doc__.strip()) + print() + print(parse_search_value.__doc__.strip()) + exit(EXIT_ERROR) + + _parse_set_region_values(argv[3:]) + + builder.save(builder_file) + exit(EXIT_SUCCESS) + + @staticmethod + def set_zone(): + """ +swift-ring-builder set_zone + [ set_zone + --region --zone --ip --port + --replication-ip --replication-port + --device --meta [--yes] + + Where , and are replication ip, hostname + and port. + Any of the options are optional in both cases. + + Resets the devices' zones. No partitions will be reassigned to or from + the device until after running 'rebalance'. This is so you can make + multiple device changes and rebalance them all just once. + + Option --yes assume a yes response to all questions. + """ + # if len(argv) < 5 or len(argv) % 2 != 1: + if len(argv) < 5: + print(Commands.set_zone.__doc__.strip()) + print() + print(parse_search_value.__doc__.strip()) + exit(EXIT_ERROR) + + _parse_set_zone_values(argv[3:]) + + builder.save(builder_file) + exit(EXIT_SUCCESS) + @staticmethod def set_info(): """ diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index 06976e4c77..cdee16a2f5 100644 --- a/swift/common/ring/builder.py +++ b/swift/common/ring/builder.py @@ -452,6 +452,46 @@ class RingBuilder(object): self.devs_changed = True self.version += 1 + def set_dev_region(self, dev_id, region): + """ + Set the region of a device. This should be called rather than just + altering the region key in the device dict directly, as the builder + will need to rebuild some internal state to reflect the change. + + .. note:: + This will not rebalance the ring immediately as you may want to + make multiple changes for a single rebalance. + + :param dev_id: device id + :param region: new region for device + """ + if any(dev_id == d['id'] for d in self._remove_devs): + raise ValueError("Can not set region of dev_id %s because it " + "is marked for removal" % (dev_id,)) + self.devs[dev_id]['region'] = region + self.devs_changed = True + self.version += 1 + + def set_dev_zone(self, dev_id, zone): + """ + Set the zone of a device. This should be called rather than just + altering the zone key in the device dict directly, as the builder + will need to rebuild some internal state to reflect the change. + + .. note:: + This will not rebalance the ring immediately as you may want to + make multiple changes for a single rebalance. + + :param dev_id: device id + :param zone: new zone for device + """ + if any(dev_id == d['id'] for d in self._remove_devs): + raise ValueError("Can not set zone of dev_id %s because it " + "is marked for removal" % (dev_id,)) + self.devs[dev_id]['zone'] = zone + self.devs_changed = True + self.version += 1 + def remove_dev(self, dev_id): """ Remove a device from the ring. diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py index 4d64305387..7f9f73606a 100644 --- a/test/unit/cli/test_ringbuilder.py +++ b/test/unit/cli/test_ringbuilder.py @@ -299,6 +299,36 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): self.assertSystemExit( EXIT_ERROR, ringbuilder._parse_set_weight_values, argv) + def test_set_region_values_no_devices(self): + # Test no devices + self.assertSystemExit( + EXIT_ERROR, ringbuilder._set_region_values, [], 100, {}) + + def test_parse_set_region_values_number_of_arguments(self): + # Test Number of arguments abnormal + argv = ["r1", "100", "r2"] + self.assertSystemExit( + EXIT_ERROR, ringbuilder._parse_set_region_values, argv) + + argv = ["--region", "2"] + self.assertSystemExit( + EXIT_ERROR, ringbuilder._parse_set_region_values, argv) + + def test_set_zone_values_no_devices(self): + # Test no devices + self.assertSystemExit( + EXIT_ERROR, ringbuilder._set_zone_values, [], 100, {}) + + def test_parse_set_zone_values_number_of_arguments(self): + # Test Number of arguments abnormal + argv = ["r1", "100", "r2"] + self.assertSystemExit( + EXIT_ERROR, ringbuilder._parse_set_zone_values, argv) + + argv = ["--region", "2"] + self.assertSystemExit( + EXIT_ERROR, ringbuilder._parse_set_zone_values, argv) + def test_set_info_values_no_devices(self): # Test no devices # _set_info_values doesn't take argv-like arguments @@ -1006,6 +1036,288 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): "--ip", "unknown"] self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + def _check_region(self, ring, dev_id, expected_region): + for dev in ring.devs: + if dev['id'] != dev_id: + self.assertNotEqual(dev['region'], expected_region) + else: + self.assertEqual(dev['region'], expected_region) + + # Final check, rebalance and check ring is ok + ring.rebalance() + self.assertTrue(ring.validate()) + + def test_set_region(self): + for search_value in self.search_values: + self.create_sample_ring() + + argv = ["", self.tmpfile, "set_region", + search_value, "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + + ring = RingBuilder.load(self.tmpfile) + self._check_region(ring, 0, 314) + + def test_set_region_ipv4_old_format(self): + self.create_sample_ring() + # Test ipv4(old format) + argv = ["", self.tmpfile, "set_region", + "d0r0z0-127.0.0.1:6200R127.0.0.1:6200/sda1_some meta data", + "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + + self._check_region(ring, 0, 314) + + def test_set_region_ipv6_old_format(self): + self.create_sample_ring() + # add IPV6 + argv = \ + ["", self.tmpfile, "add", + "--region", "2", "--zone", "3", + "--ip", "[2001:0000:1234:0000:0000:C1C0:ABCD:0876]", + "--port", "6000", + "--replication-ip", "[2::10]", + "--replication-port", "7000", + "--device", "sda3", "--meta", "some meta data", + "--weight", "100"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + + # Test ipv6(old format) + argv = ["", self.tmpfile, "set_region", + "d4r2z3-[2001:0000:1234:0000:0000:C1C0:ABCD:0876]:6000" + "R[2::10]:7000/sda3_some meta data", "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + + self._check_region(ring, 4, 314) + + def test_set_region_ipv4_new_format(self): + self.create_sample_ring() + # Test ipv4(new format) + argv = \ + ["", self.tmpfile, "set_region", + "--id", "0", "--region", "0", "--zone", "0", + "--ip", "127.0.0.1", + "--port", "6200", + "--replication-ip", "127.0.0.1", + "--replication-port", "6200", + "--device", "sda1", "--meta", "some meta data", "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + + self._check_region(ring, 0, 314) + + def test_set_region_ipv6_new_format(self): + self.create_sample_ring() + # add IPV6 + argv = \ + ["", self.tmpfile, "add", + "--region", "2", "--zone", "3", + "--ip", "[2001:0000:1234:0000:0000:C1C0:ABCD:0876]", + "--port", "6000", + "--replication-ip", "[2::10]", + "--replication-port", "7000", + "--device", "sda3", "--meta", "some meta data", + "--weight", "100"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + + # Test ipv6(new format) + argv = \ + ["", self.tmpfile, "set_region", + "--id", "4", "--region", "2", "--zone", "3", + "--ip", "[2001:0000:1234:0000:0000:C1C0:ABCD:0876]", + "--port", "6000", + "--replication-ip", "[2::10]", + "--replication-port", "7000", + "--device", "sda3", "--meta", "some meta data", "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + + self._check_region(ring, 4, 314) + + def test_set_region_domain_new_format(self): + self.create_sample_ring() + # add domain name + argv = \ + ["", self.tmpfile, "add", + "--region", "2", "--zone", "3", + "--ip", "test.test.com", + "--port", "6000", + "--replication-ip", "r.test.com", + "--replication-port", "7000", + "--device", "sda3", "--meta", "some meta data", + "--weight", "100"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + + # Test domain name + argv = \ + ["", self.tmpfile, "set_region", + "--id", "4", "--region", "2", "--zone", "3", + "--ip", "test.test.com", + "--port", "6000", + "--replication-ip", "r.test.com", + "--replication-port", "7000", + "--device", "sda3", "--meta", "some meta data", "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + + self._check_region(ring, 4, 314) + + def test_set_region_number_of_arguments(self): + self.create_sample_ring() + # Test Number of arguments abnormal + argv = ["", self.tmpfile, "set_region"] + self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + + def test_set_region_no_matching(self): + self.create_sample_ring() + # Test No matching devices + argv = ["", self.tmpfile, "set_region", + "--ip", "unknown"] + self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + + def test_set_zone(self): + for search_value in self.search_values: + self.create_sample_ring() + + argv = ["", self.tmpfile, "set_zone", + search_value, "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + + self._check_zone(ring, 0, 314) + + def test_set_zone_ipv4_old_format(self): + self.create_sample_ring() + # Test ipv4(old format) + argv = ["", self.tmpfile, "set_zone", + "d0r0z0-127.0.0.1:6200R127.0.0.1:6200/sda1_some meta data", + "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + + self._check_zone(ring, 0, 314) + + def _check_zone(self, ring, dev_id, expected_zone): + for dev in ring.devs: + if dev['id'] != dev_id: + self.assertFalse(dev['zone'] == expected_zone) + else: + self.assertEqual(dev['zone'], expected_zone) + + # Final check, rebalance and check ring is ok + ring.rebalance() + self.assertTrue(ring.validate()) + + def test_set_zone_ipv6_old_format(self): + self.create_sample_ring() + # add IPV6 + argv = \ + ["", self.tmpfile, "add", + "--region", "2", "--zone", "3", + "--ip", "[2001:0000:1234:0000:0000:C1C0:ABCD:0876]", + "--port", "6000", + "--replication-ip", "[2::10]", + "--replication-port", "7000", + "--device", "sda3", "--meta", "some meta data", + "--weight", "100"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + + # Test ipv6(old format) + argv = ["", self.tmpfile, "set_zone", + "d4r2z3-[2001:0000:1234:0000:0000:C1C0:ABCD:0876]:6000" + "R[2::10]:7000/sda3_some meta data", "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + + self._check_zone(ring, 4, 314) + + def test_set_zone_ipv4_new_format(self): + self.create_sample_ring() + # Test ipv4(new format) + argv = \ + ["", self.tmpfile, "set_zone", + "--id", "0", "--region", "0", "--zone", "0", + "--ip", "127.0.0.1", + "--port", "6200", + "--replication-ip", "127.0.0.1", + "--replication-port", "6200", + "--device", "sda1", "--meta", "some meta data", "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + + self._check_zone(ring, 0, 314) + + def test_set_zone_ipv6_new_format(self): + self.create_sample_ring() + # add IPV6 + argv = \ + ["", self.tmpfile, "add", + "--region", "2", "--zone", "3", + "--ip", "[2001:0000:1234:0000:0000:C1C0:ABCD:0876]", + "--port", "6000", + "--replication-ip", "[2::10]", + "--replication-port", "7000", + "--device", "sda3", "--meta", "some meta data", + "--weight", "100"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + + # Test ipv6(new format) + argv = \ + ["", self.tmpfile, "set_zone", + "--id", "4", "--region", "2", + "--ip", "[2001:0000:1234:0000:0000:C1C0:ABCD:0876]", + "--port", "6000", + "--replication-ip", "[2::10]", + "--replication-port", "7000", + "--device", "sda3", "--meta", "some meta data", "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + + self._check_zone(ring, 4, 314) + + def test_set_zone_domain_new_format(self): + self.create_sample_ring() + # add domain name + argv = \ + ["", self.tmpfile, "add", + "--region", "2", "--zone", "3", + "--ip", "test.test.com", + "--port", "6000", + "--replication-ip", "r.test.com", + "--replication-port", "7000", + "--device", "sda3", "--meta", "some meta data", + "--weight", "100"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + + # Test domain name + argv = \ + ["", self.tmpfile, "set_zone", + "--id", "4", "--region", "2", "--zone", "3", + "--ip", "test.test.com", + "--port", "6000", + "--replication-ip", "r.test.com", + "--replication-port", "7000", + "--device", "sda3", "--meta", "some meta data", "314"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + + self._check_zone(ring, 4, 314) + + def test_set_zone_number_of_arguments(self): + self.create_sample_ring() + # Test Number of arguments abnormal + argv = ["", self.tmpfile, "set_zone"] + self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + + def test_set_zone_no_matching(self): + self.create_sample_ring() + # Test No matching devices + argv = ["", self.tmpfile, "set_zone", + "--ip", "unknown"] + self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + def test_set_info(self): for search_value in self.search_values: @@ -1846,13 +2158,13 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): "--replication-ip", "127.0.0.5", "--replication-port", "6004", "--device", "sda5", "--weight", "100.0"] - self.assertRaises(SystemExit, ringbuilder.main, argv) + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) argv = ["", self.tmpfile, "remove", "--id", "0"] - self.assertRaises(SystemExit, ringbuilder.main, argv) + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) argv = ["", self.tmpfile, "remove", "--id", "3"] - self.assertRaises(SystemExit, ringbuilder.main, argv) + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) argv = ["", self.tmpfile, "rebalance"] - self.assertRaises(SystemExit, ringbuilder.main, argv) + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) argv = \ ["", self.tmpfile, "add", "--region", "2", "--zone", "1", @@ -1860,14 +2172,14 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): "--replication-ip", "127.0.0.6", "--replication-port", "6005", "--device", "sdb6", "--weight", "100.0"] - self.assertRaises(SystemExit, ringbuilder.main, argv) + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) # Check the order of the devices listed the output. argv = ["", self.tmpfile] with mock.patch("sys.stdout", mock_stdout), mock.patch( "sys.stderr", mock_stderr), mock.patch( 'swift.common.ring.builder.time', return_value=now): - self.assertRaises(SystemExit, ringbuilder.main, argv) + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) self.assertOutputStub(mock_stdout.getvalue(), builder_id=ring.id) def test_default_ringfile_check(self):