diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py index aab22accea..bc7d1f9593 100644 --- a/swift/cli/ringbuilder.py +++ b/swift/cli/ringbuilder.py @@ -852,15 +852,8 @@ swift-ring-builder rebalance [options] handler.setFormatter(formatter) logger.addHandler(handler) - if builder.min_part_seconds_left > 0 and not options.force: - print('No partitions could be reassigned.') - print('The time between rebalances must be at least ' - 'min_part_hours: %s hours (%s remaining)' % ( - builder.min_part_hours, - timedelta(seconds=builder.min_part_seconds_left))) - exit(EXIT_WARNING) - devs_changed = builder.devs_changed + min_part_seconds_left = builder.min_part_seconds_left try: last_balance = builder.get_balance() parts, balance, removed_devs = builder.rebalance(seed=get_seed(3)) @@ -875,7 +868,13 @@ swift-ring-builder rebalance [options] exit(EXIT_ERROR) if not (parts or options.force or removed_devs): print('No partitions could be reassigned.') - print('There is no need to do so at this time') + if min_part_seconds_left > 0: + print('The time between rebalances must be at least ' + 'min_part_hours: %s hours (%s remaining)' % ( + builder.min_part_hours, + timedelta(seconds=builder.min_part_seconds_left))) + else: + print('There is no need to do so at this time') exit(EXIT_WARNING) # If we set device's weight to zero, currently balance will be set # special value(MAX_BALANCE) until zero weighted device return all diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py index b3e33acb54..ba8247ccfa 100644 --- a/test/unit/cli/test_ringbuilder.py +++ b/test/unit/cli/test_ringbuilder.py @@ -25,6 +25,7 @@ import unittest import uuid import shlex import shutil +import time from swift.cli import ringbuilder from swift.cli.ringbuilder import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR @@ -1886,6 +1887,27 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): ring = RingBuilder.load(self.tmpfile) self.assertEqual(ring.min_part_seconds_left, 3600) + def test_time_remaining(self): + self.create_sample_ring() + self.run_srb('rebalance') + now = time.time() + with mock.patch('swift.common.ring.builder.time', return_value=now): + out, err = self.run_srb('rebalance') + self.assertIn('No partitions could be reassigned', out) + self.assertIn('must be at least min_part_hours', out) + self.assertIn('1 hours (1:00:00 remaining)', out) + the_future = now + 3600 + with mock.patch('swift.common.ring.builder.time', + return_value=the_future): + out, err = self.run_srb('rebalance') + self.assertIn('No partitions could be reassigned', out) + self.assertIn('There is no need to do so at this time', out) + # or you can pretend_min_part_hours_passed + self.run_srb('pretend_min_part_hours_passed') + out, err = self.run_srb('rebalance') + self.assertIn('No partitions could be reassigned', out) + self.assertIn('There is no need to do so at this time', out) + def test_rebalance_failure_does_not_reset_last_moves_epoch(self): ring = RingBuilder(8, 3, 1) ring.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 1, @@ -1922,6 +1944,40 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): argv = ["", self.tmpfile, "rebalance", "--seed", "2"] self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + def test_rebalance_removed_devices(self): + self.create_sample_ring() + argvs = [ + ["", self.tmpfile, "rebalance", "3"], + ["", self.tmpfile, "remove", "d0"], + ["", self.tmpfile, "rebalance", "3"]] + for argv in argvs: + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + + def test_rebalance_min_part_hours_not_passed(self): + self.create_sample_ring() + argvs = [ + ["", self.tmpfile, "rebalance", "3"], + ["", self.tmpfile, "set_weight", "d0", "1000"]] + for argv in argvs: + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + + ring = RingBuilder.load(self.tmpfile) + last_replica2part2dev = ring._replica2part2dev + + mock_stdout = six.StringIO() + argv = ["", self.tmpfile, "rebalance", "3"] + with mock.patch("sys.stdout", mock_stdout): + self.assertSystemExit(EXIT_WARNING, ringbuilder.main, argv) + expected = "No partitions could be reassigned.\n" + \ + "The time between rebalances must be " + \ + "at least min_part_hours: 1 hours" + self.assertTrue(expected in mock_stdout.getvalue()) + + # Messages can be faked, so let's assure that the partition assignment + # did not change at all, despite the warning + ring = RingBuilder.load(self.tmpfile) + self.assertEqual(last_replica2part2dev, ring._replica2part2dev) + def test_write_ring(self): self.create_sample_ring() argv = ["", self.tmpfile, "rebalance"]