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

@@ -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 <builder_file> set_weight
         builder.save(builder_file)
         exit(EXIT_SUCCESS)
 
+    @staticmethod
+    def set_region():
+        """
+swift-ring-builder <builder_file> set_region <search-value> <region>
+    [<search-value> <region] ...
+
+or
+
+swift-ring-builder <builder_file> set_region
+    --region <region> --zone <zone> --ip <ip or hostname> --port <port>
+    --replication-ip <r_ip or r_hostname> --replication-port <r_port>
+    --device <device_name> --meta <meta> <new region> [--yes]
+
+    Where <r_ip>, <r_hostname> and <r_port> 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 <builder_file> set_zone <search-value> <zone>
+    [<search-value> <zone] ...
+
+or
+
+swift-ring-builder <builder_file> set_zone
+    --region <region> --zone <zone> --ip <ip or hostname> --port <port>
+    --replication-ip <r_ip or r_hostname> --replication-port <r_port>
+    --device <device_name> --meta <meta> <new zone> [--yes]
+
+    Where <r_ip>, <r_hostname> and <r_port> 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():
         """

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.

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