From 99a6f915ffc3aba2086e22dfba1f33b4fee46e81 Mon Sep 17 00:00:00 2001 From: Nandini Tata Date: Fri, 20 May 2016 17:41:29 +0000 Subject: [PATCH] swift-ring-builder output corrected for ipv6 Adjusted width of ip and port columns in swift-ring-builder command output to dynamically span to the longest ip or the longest port in the devices list. Also combined the port and ip address columns for better visual clarity. Took care of ipv6 format [ipv6]:port Modified the corresponding test case with expected output. Change-Id: I65837f8fed70be60b53d5a817a4ce529ad0f070e Closes-Bug: #1567105 --- swift/cli/ringbuilder.py | 69 ++++++++++++++--- test/unit/cli/test_default_output.stub | 11 +++ test/unit/cli/test_ipv6_output.stub | 10 +++ test/unit/cli/test_ringbuilder.py | 103 ++++++++++++++++++++++--- 4 files changed, 171 insertions(+), 22 deletions(-) create mode 100644 test/unit/cli/test_default_output.stub create mode 100644 test/unit/cli/test_ipv6_output.stub diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py index 0c35553185..5b7f8108d5 100644 --- a/swift/cli/ringbuilder.py +++ b/swift/cli/ringbuilder.py @@ -39,7 +39,7 @@ from swift.common.ring.utils import validate_args, \ parse_builder_ring_filename_args, parse_search_value, \ parse_search_values_from_opts, parse_change_values_from_opts, \ dispersion_report, parse_add_value -from swift.common.utils import lock_parent_directory +from swift.common.utils import lock_parent_directory, is_valid_ipv6 MAJOR_VERSION = 1 MINOR_VERSION = 3 @@ -376,6 +376,58 @@ def _parse_remove_values(argvish): exit(EXIT_ERROR) +def _make_display_device_table(builder): + ip_width = 10 + port_width = 4 + rep_ip_width = 14 + rep_port_width = 4 + ip_ipv6 = rep_ipv6 = False + for dev in builder._iter_devs(): + if is_valid_ipv6(dev['ip']): + ip_ipv6 = True + if is_valid_ipv6(dev['replication_ip']): + rep_ipv6 = True + ip_width = max(len(dev['ip']), ip_width) + rep_ip_width = max(len(dev['replication_ip']), rep_ip_width) + port_width = max(len(str(dev['port'])), port_width) + rep_port_width = max(len(str(dev['replication_port'])), + rep_port_width) + if ip_ipv6: + ip_width += 2 + if rep_ipv6: + rep_ip_width += 2 + header_line = ('Devices:%5s %6s %4s %' + str(ip_width) + + 's:%-' + str(port_width) + 's %' + + str(rep_ip_width) + 's:%-' + str(rep_port_width) + + 's %5s %6s %10s %7s %5s %s') % ( + 'id', 'region', 'zone', 'ip address', + 'port', 'replication ip', 'port', 'name', + 'weight', 'partitions', 'balance', 'flags', + 'meta') + + def print_dev_f(dev, balance_per_dev=0.00, flags=''): + def get_formated_ip(key): + value = dev[key] + if ':' in value: + value = '[%s]' % value + return value + dev_ip = get_formated_ip('ip') + dev_replication_ip = get_formated_ip('replication_ip') + format_string = ''.join(['%13d %6d %4d ', + '%', str(ip_width), 's:%-', + str(port_width), 'd ', '%', + str(rep_ip_width), 's', ':%-', + str(rep_port_width), 'd %5s %6.02f' + ' %10s %7.02f %5s %s']) + args = (dev['id'], dev['region'], dev['zone'], dev_ip, dev['port'], + dev_replication_ip, dev['replication_port'], dev['device'], + dev['weight'], dev['parts'], balance_per_dev, flags, + dev['meta']) + print(format_string % args) + + return header_line, print_dev_f + + class Commands(object): @staticmethod def unknown(): @@ -458,18 +510,11 @@ swift-ring-builder if builder.devs: balance_per_dev = builder._build_balance_per_dev() - print('Devices: id region zone ip address port ' - 'replication ip replication port name ' - 'weight partitions balance flags meta') + header_line, print_dev_f = _make_display_device_table(builder) + print(header_line) for dev in builder._iter_devs(): flags = 'DEL' if dev in builder._remove_devs else '' - print(' %5d %7d %5d %15s %5d %15s %17d %9s %6.02f ' - '%10s %7.02f %5s %s' % - (dev['id'], dev['region'], dev['zone'], dev['ip'], - dev['port'], dev['replication_ip'], - dev['replication_port'], dev['device'], dev['weight'], - dev['parts'], balance_per_dev[dev['id']], flags, - dev['meta'])) + print_dev_f(dev, balance_per_dev[dev['id']], flags) exit(EXIT_SUCCESS) @staticmethod @@ -905,7 +950,7 @@ swift-ring-builder dispersion [options] --verbose option will display dispersion graph broken down by tier You can filter which tiers are evaluated to drill down using a regex - in the optional search_filter arguemnt. i.e. + in the optional search_filter argument. i.e. swift-ring-builder dispersion "r\d+z\d+$" -v diff --git a/test/unit/cli/test_default_output.stub b/test/unit/cli/test_default_output.stub new file mode 100644 index 0000000000..3b4786b479 --- /dev/null +++ b/test/unit/cli/test_default_output.stub @@ -0,0 +1,11 @@ +__RINGFILE__, build version 4 +64 partitions, 3.000000 replicas, 4 regions, 4 zones, 4 devices, 100.00 balance, 0.00 dispersion +The minimum number of hours before a partition can be reassigned is 1 (0:00:00 remaining) +The overload factor is 0.00% (0.000000) +Ring file __RINGFILE__.ring.gz not found, probably it hasn't been written yet +Devices: id region zone ip address:port replication ip:port name weight partitions balance flags meta + 0 0 0 127.0.0.1:6200 127.0.0.1:6200 sda1 100.00 0 -100.00 some meta data + 1 1 1 127.0.0.2:6201 127.0.0.2:6201 sda2 100.00 0 -100.00 + 2 2 2 127.0.0.3:6202 127.0.0.3:6202 sdc3 100.00 0 -100.00 + 3 3 3 127.0.0.4:6203 127.0.0.4:6203 sdd4 100.00 0 -100.00 + diff --git a/test/unit/cli/test_ipv6_output.stub b/test/unit/cli/test_ipv6_output.stub new file mode 100644 index 0000000000..1691bb43af --- /dev/null +++ b/test/unit/cli/test_ipv6_output.stub @@ -0,0 +1,10 @@ +__RINGFILE__, build version 4 +256 partitions, 3.000000 replicas, 4 regions, 4 zones, 4 devices, 100.00 balance, 0.00 dispersion +The minimum number of hours before a partition can be reassigned is 1 (0:00:00 remaining) +The overload factor is 0.00% (0.000000) +Ring file __RINGFILE__.ring.gz not found, probably it hasn't been written yet +Devices: id region zone ip address:port replication ip:port name weight partitions balance flags meta + 0 0 0 [2001:db8:85a3::8a2e:370:7334]:6200 [2001:db8:85a3::8a2e:370:7334]:6200 sda1 100.00 0 -100.00 some meta data + 1 1 1 127.0.0.1:66201 127.0.0.1:66201 sda2 100.00 0 -100.00 + 2 2 2 [2001:db8:85a3::8a2e:370:7336]:6202 127.0.10.127:7070 sdc3 100.00 0 -100.00 + 3 3 3 [2001:db8:85a3::8a2e:370:7337]:6203 [7001:db8:85a3::8a2e:370:7337]:11664 sdd4 100.00 0 -100.00 diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py index 5e86b6fea1..bf994012a1 100644 --- a/test/unit/cli/test_ringbuilder.py +++ b/test/unit/cli/test_ringbuilder.py @@ -13,6 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import errno +import itertools import logging import mock import os @@ -92,6 +94,43 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): except OSError: pass + def assertOutputStub(self, output, ext='stub'): + """ + assert that the given output string is equal to a in-tree stub file, + if a test needs to check multiple outputs it can use custom ext's + """ + filepath = os.path.abspath( + os.path.join(os.path.dirname(__file__), self.id().split('.')[-1])) + print(filepath) + filepath = '%s.%s' % (filepath, ext) + try: + with open(filepath, 'r') as f: + stub = f.read() + except (IOError, OSError) as e: + if e.errno == errno.ENOENT: + self.fail('%r does not exist' % filepath) + else: + self.fail('%r could not be read (%s)' % (filepath, e)) + output = output.replace(self.tempfile, '__RINGFILE__') + for i, (value, expected) in enumerate( + itertools.izip_longest( + output.splitlines(), stub.splitlines())): + # N.B. differences in trailing whitespace are ignored! + value = (value or '').rstrip() + expected = (expected or '').rstrip() + try: + self.assertEqual(value, expected) + except AssertionError: + msg = 'Line #%s value is not like expected:\n%r\n%r' % ( + i, value, expected) + msg += '\n\nFull output was:\n' + for i, line in enumerate(output.splitlines()): + msg += '%3d: %s\n' % (i, line) + msg += '\n\nCompared to stub:\n' + for i, line in enumerate(stub.splitlines()): + msg += '%3d: %s\n' % (i, line) + self.fail(msg) + def create_sample_ring(self, part_power=6): """ Create a sample ring with four devices @@ -1614,6 +1653,50 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): argv = ["", self.tmpfile] self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + def test_default_output(self): + self.create_sample_ring() + out, err = self.run_srb('') + self.assertOutputStub(out) + + def test_ipv6_output(self): + ring = RingBuilder(8, 3, 1) + ring.add_dev({'weight': 100.0, + 'region': 0, + 'zone': 0, + 'ip': '2001:db8:85a3::8a2e:370:7334', + 'port': 6200, + 'device': 'sda1', + 'meta': 'some meta data', + }) + ring.add_dev({'weight': 100.0, + 'region': 1, + 'zone': 1, + 'ip': '127.0.0.1', + 'port': 66201, + 'device': 'sda2', + }) + ring.add_dev({'weight': 100.0, + 'region': 2, + 'zone': 2, + 'ip': '2001:db8:85a3::8a2e:370:7336', + 'port': 6202, + 'device': 'sdc3', + 'replication_ip': '127.0.10.127', + 'replication_port': 7070, + }) + ring.add_dev({'weight': 100.0, + 'region': 3, + 'zone': 3, + 'ip': '2001:db8:85a3::8a2e:370:7337', + 'port': 6203, + 'device': 'sdd4', + 'replication_ip': '7001:db8:85a3::8a2e:370:7337', + 'replication_port': 11664, + }) + ring.save(self.tmpfile) + out, err = self.run_srb('') + self.assertOutputStub(out) + def test_default_show_removed(self): mock_stdout = six.StringIO() mock_stderr = six.StringIO() @@ -1642,20 +1725,20 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): "The overload factor is 0.00%% (0.000000)\n" \ "Ring file %s.ring.gz not found, probably " \ "it hasn't been written yet\n" \ - "Devices: id region zone ip address port " \ - "replication ip replication port name weight " \ + "Devices: id region zone ip address:port " \ + "replication ip:port name weight " \ "partitions balance flags meta\n" \ - " 0 0 0 127.0.0.1 6200 " \ - "127.0.0.1 6200 sda1 100.00" \ + " 0 0 0 127.0.0.1:6200 " \ + " 127.0.0.1:6200 sda1 100.00" \ " 0 -100.00 some meta data\n" \ - " 1 1 1 127.0.0.2 6201 " \ - "127.0.0.2 6201 sda2 0.00" \ + " 1 1 1 127.0.0.2:6201 " \ + " 127.0.0.2:6201 sda2 0.00" \ " 0 0.00 DEL \n" \ - " 2 2 2 127.0.0.3 6202 " \ - "127.0.0.3 6202 sdc3 100.00" \ + " 2 2 2 127.0.0.3:6202 " \ + " 127.0.0.3:6202 sdc3 100.00" \ " 0 -100.00 \n" \ - " 3 3 3 127.0.0.4 6203 " \ - "127.0.0.4 6203 sdd4 0.00" \ + " 3 3 3 127.0.0.4:6203 " \ + " 127.0.0.4:6203 sdd4 0.00" \ " 0 0.00 \n" % (self.tmpfile, self.tmpfile) self.assertEqual(expected, mock_stdout.getvalue())