diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py index fd4461b91f..d4119f6c55 100644 --- a/swift/cli/ringbuilder.py +++ b/swift/cli/ringbuilder.py @@ -35,6 +35,7 @@ from six.moves import input from swift.common import exceptions from swift.common.ring import RingBuilder, Ring, RingData from swift.common.ring.builder import MAX_BALANCE +from swift.common.ring.composite_builder import CompositeRingBuilder from swift.common.ring.utils import validate_args, \ validate_and_normalize_ip, build_dev_from_opts, \ parse_builder_ring_filename_args, parse_search_value, \ @@ -1483,7 +1484,13 @@ def main(arguments=None): try: builder = RingBuilder.load(builder_file) except exceptions.UnPicklingError as e: - print(e) + msg = str(e) + try: + CompositeRingBuilder.load(builder_file) + msg += ' (it appears to be a composite ring builder file?)' + except Exception: # noqa + pass + print(msg) exit(EXIT_ERROR) except (exceptions.FileNotFoundError, exceptions.PermissionError) as e: if len(argv) < 3 or argv[2] not in('create', 'write_builder'): diff --git a/test/unit/__init__.py b/test/unit/__init__.py index 278c55a4ca..8cbff3d994 100644 --- a/test/unit/__init__.py +++ b/test/unit/__init__.py @@ -44,7 +44,7 @@ from swift.common.utils import Timestamp, NOTICE from test import get_config from swift.common import utils from swift.common.header_key_dict import HeaderKeyDict -from swift.common.ring import Ring, RingData +from swift.common.ring import Ring, RingData, RingBuilder from swift.obj import server from hashlib import md5 import logging.handlers @@ -304,6 +304,31 @@ def write_fake_ring(path, *devs): pickle.dump(RingData(replica2part2dev_id, devs, part_shift), f) +def write_stub_builder(tmpdir, region=1, name=''): + """ + Pretty much just a three node, three replica, 8 part power builder... + + :param tmpdir: a place to write the builder, be sure to clean it up! + :param region: an integer, fills in region and ip + :param name: the name of the builder (i.e. .builder) + """ + name = name or str(region) + replicas = 3 + builder = RingBuilder(8, replicas, 1) + for i in range(replicas): + dev = {'weight': 100, + 'region': '%d' % region, + 'zone': '1', + 'ip': '10.0.0.%d' % region, + 'port': '3600', + 'device': 'sdb%d' % i} + builder.add_dev(dev) + builder.rebalance() + builder_file = os.path.join(tmpdir, '%s.builder' % name) + builder.save(builder_file) + return builder, builder_file + + class FabricatedRing(Ring): """ When a FakeRing just won't do - you can fabricate one to meet diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py index 65a6cf72ef..67eb259095 100644 --- a/test/unit/cli/test_ringbuilder.py +++ b/test/unit/cli/test_ringbuilder.py @@ -31,8 +31,9 @@ from swift.cli import ringbuilder from swift.cli.ringbuilder import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR from swift.common import exceptions from swift.common.ring import RingBuilder +from swift.common.ring.composite_builder import CompositeRingBuilder -from test.unit import Timeout +from test.unit import Timeout, write_stub_builder try: from itertools import zip_longest @@ -1411,10 +1412,31 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): argv = ["", self.tmpfile, "validate"] self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + def test_validate_composite_builder_file(self): + b1, b1_file = write_stub_builder(self.tmpdir, 1) + b2, b2_file = write_stub_builder(self.tmpdir, 2) + cb = CompositeRingBuilder([b1_file, b2_file]) + cb.compose() + cb_file = os.path.join(self.tmpdir, 'composite.builder') + cb.save(cb_file) + argv = ["", cb_file, "validate"] + with mock.patch("sys.stdout", six.StringIO()) as mock_stdout: + self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + lines = mock_stdout.getvalue().strip().split('\n') + self.assertIn("Ring Builder file is invalid", lines[0]) + self.assertIn("appears to be a composite ring builder file", lines[0]) + self.assertFalse(lines[1:]) + def test_validate_empty_file(self): open(self.tmpfile, 'a').close argv = ["", self.tmpfile, "validate"] - self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + with mock.patch("sys.stdout", six.StringIO()) as mock_stdout: + self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + lines = mock_stdout.getvalue().strip().split('\n') + self.assertIn("Ring Builder file is invalid", lines[0]) + self.assertNotIn("appears to be a composite ring builder file", + lines[0]) + self.assertFalse(lines[1:]) def test_validate_corrupted_file(self): self.create_sample_ring() @@ -1427,19 +1449,35 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): # corrupt the file with open(self.tmpfile, 'wb') as f: f.write(os.urandom(1024)) - self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + with mock.patch("sys.stdout", six.StringIO()) as mock_stdout: + self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + lines = mock_stdout.getvalue().strip().split('\n') + self.assertIn("Ring Builder file is invalid", lines[0]) + self.assertNotIn("appears to be a composite ring builder file", + lines[0]) + self.assertFalse(lines[1:]) def test_validate_non_existent_file(self): rand_file = '%s/%s' % (tempfile.gettempdir(), str(uuid.uuid4())) argv = ["", rand_file, "validate"] - self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + with mock.patch("sys.stdout", six.StringIO()) as mock_stdout: + self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + lines = mock_stdout.getvalue().strip().split('\n') + self.assertIn("Ring Builder file does not exist", lines[0]) + self.assertNotIn("appears to be a composite ring builder file", + lines[0]) + self.assertFalse(lines[1:]) def test_validate_non_accessible_file(self): with mock.patch.object( RingBuilder, 'load', - mock.Mock(side_effect=exceptions.PermissionError)): + mock.Mock(side_effect=exceptions.PermissionError("boom"))): argv = ["", self.tmpfile, "validate"] - self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + with mock.patch("sys.stdout", six.StringIO()) as mock_stdout: + self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv) + lines = mock_stdout.getvalue().strip().split('\n') + self.assertIn("boom", lines[0]) + self.assertFalse(lines[1:]) def test_validate_generic_error(self): with mock.patch.object( diff --git a/test/unit/cli/test_ringcomposer.py b/test/unit/cli/test_ringcomposer.py index 3022d3702f..bacff3ebfc 100644 --- a/test/unit/cli/test_ringcomposer.py +++ b/test/unit/cli/test_ringcomposer.py @@ -21,7 +21,7 @@ import six from mock import mock from swift.cli import ringcomposer -from swift.common.ring import RingBuilder +from test.unit import write_stub_builder class TestCommands(unittest.TestCase): @@ -36,22 +36,6 @@ class TestCommands(unittest.TestCase): def tearDown(self): shutil.rmtree(self.tmpdir) - def _write_stub_builder(self, region): - replicas = 3 - builder = RingBuilder(8, replicas, 1) - for i in range(replicas): - dev = {'weight': 100, - 'region': '%d' % region, - 'zone': '1', - 'ip': '10.0.0.%d' % region, - 'port': '3600', - 'device': 'sdb%d' % i} - builder.add_dev(dev) - builder.rebalance() - builder_file = os.path.join(self.tmpdir, '%d.builder' % region) - builder.save(builder_file) - return builder, builder_file - def _run_composer(self, args): mock_stdout = six.StringIO() mock_stderr = six.StringIO() @@ -92,8 +76,8 @@ class TestCommands(unittest.TestCase): self.fail('Failed testing command %r due to: %s' % (cmd, err)) def test_compose(self): - b1, b1_file = self._write_stub_builder(1) - b2, b2_file = self._write_stub_builder(2) + b1, b1_file = write_stub_builder(self.tmpdir, 1) + b2, b2_file = write_stub_builder(self.tmpdir, 2) args = ('', self.composite_builder_file, 'compose', b1_file, b2_file, '--output', self.composite_ring_file) exit_code, stdout, stderr = self._run_composer(args) @@ -102,8 +86,8 @@ class TestCommands(unittest.TestCase): self.assertTrue(os.path.exists(self.composite_ring_file)) def test_compose_existing(self): - b1, b1_file = self._write_stub_builder(1) - b2, b2_file = self._write_stub_builder(2) + b1, b1_file = write_stub_builder(self.tmpdir, 1) + b2, b2_file = write_stub_builder(self.tmpdir, 2) args = ('', self.composite_builder_file, 'compose', b1_file, b2_file, '--output', self.composite_ring_file) exit_code, stdout, stderr = self._run_composer(args) @@ -123,7 +107,7 @@ class TestCommands(unittest.TestCase): self.assertTrue(os.path.exists(self.composite_ring_file)) def test_compose_insufficient_component_builder_files(self): - b1, b1_file = self._write_stub_builder(1) + b1, b1_file = write_stub_builder(self.tmpdir, 1) args = ('', self.composite_builder_file, 'compose', b1_file, '--output', self.composite_ring_file) exit_code, stdout, stderr = self._run_composer(args) @@ -134,7 +118,7 @@ class TestCommands(unittest.TestCase): self.assertFalse(os.path.exists(self.composite_ring_file)) def test_compose_nonexistent_component_builder_file(self): - b1, b1_file = self._write_stub_builder(1) + b1, b1_file = write_stub_builder(self.tmpdir, 1) bad_file = os.path.join(self.tmpdir, 'non-existent-file') args = ('', self.composite_builder_file, 'compose', b1_file, bad_file, '--output', self.composite_ring_file) @@ -146,8 +130,8 @@ class TestCommands(unittest.TestCase): self.assertFalse(os.path.exists(self.composite_ring_file)) def test_compose_fails_to_write_composite_ring_file(self): - b1, b1_file = self._write_stub_builder(1) - b2, b2_file = self._write_stub_builder(2) + b1, b1_file = write_stub_builder(self.tmpdir, 1) + b2, b2_file = write_stub_builder(self.tmpdir, 2) args = ('', self.composite_builder_file, 'compose', b1_file, b2_file, '--output', self.composite_ring_file) with mock.patch('swift.common.ring.RingData.save', @@ -161,8 +145,8 @@ class TestCommands(unittest.TestCase): self.assertFalse(os.path.exists(self.composite_ring_file)) def test_compose_fails_to_write_composite_builder_file(self): - b1, b1_file = self._write_stub_builder(1) - b2, b2_file = self._write_stub_builder(2) + b1, b1_file = write_stub_builder(self.tmpdir, 1) + b2, b2_file = write_stub_builder(self.tmpdir, 2) args = ('', self.composite_builder_file, 'compose', b1_file, b2_file, '--output', self.composite_ring_file) func = 'swift.common.ring.composite_builder.CompositeRingBuilder.save' @@ -177,8 +161,8 @@ class TestCommands(unittest.TestCase): self.assertTrue(os.path.exists(self.composite_ring_file)) def test_show(self): - b1, b1_file = self._write_stub_builder(1) - b2, b2_file = self._write_stub_builder(2) + b1, b1_file = write_stub_builder(self.tmpdir, 1) + b2, b2_file = write_stub_builder(self.tmpdir, 2) args = ('', self.composite_builder_file, 'compose', b1_file, b2_file, '--output', self.composite_ring_file) exit_code, stdout, stderr = self._run_composer(args)