Allow set_overload to take value as percent

...and output overload as a percent like dispersion and balance.

Also raise a warning if someone tries to set overload higher than 100%
(unless the specifically requested a percent value great than 100).

Change-Id: Id030123153ea746671a8f1ca306d4b86e903fa22

2 files changed, 80 insertions, 25 deletions

diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py
index 3bf073550..b02e33aa1 100755
--- a/swift/cli/ringbuilder.py
+++ b/swift/cli/ringbuilder.py
@@ -254,7 +254,8 @@ swift-ring-builder <builder_file>
                   balance, builder.dispersion)
         print 'The minimum number of hours before a partition can be ' \
               'reassigned is %s' % builder.min_part_hours
-        print 'The overload factor is %.6f' % builder.overload
+        print 'The overload factor is %0.2f%% (%.6f)' % (
+            builder.overload * 100, builder.overload)
         if builder.devs:
             print 'Devices:    id  region  zone      ip address  port  ' \
                   'replication ip  replication port      name ' \
@@ -740,7 +741,8 @@ swift-ring-builder <builder_file> dispersion <search_filter> [options]
             search_filter = None
         report = dispersion_report(builder, search_filter=search_filter,
                                    verbose=options.verbose)
-        print 'Dispersion is %.06f' % builder.dispersion
+        print 'Dispersion is %.06f, Balance is %.06f, Overload is %0.2f%%' % (
+            builder.dispersion, builder.get_balance(), builder.overload * 100)
         if report['worst_tier']:
             status = EXIT_WARNING
             print 'Worst tier is %.06f (%s)' % (report['max_dispersion'],
@@ -902,7 +904,7 @@ swift-ring-builder <builder_file> set_replicas <replicas>
 
     def set_overload():
         """
-swift-ring-builder <builder_file> set_overload <overload>
+swift-ring-builder <builder_file> set_overload <overload>[%]
     Changes the overload factor to the given <overload>.
 
     A rebalance is needed to make the change take effect.
@@ -912,22 +914,36 @@ swift-ring-builder <builder_file> set_overload <overload>
             exit(EXIT_ERROR)
 
         new_overload = argv[3]
+        if new_overload.endswith('%'):
+            percent = True
+            new_overload = new_overload.rstrip('%')
+        else:
+            percent = False
         try:
             new_overload = float(new_overload)
         except ValueError:
             print Commands.set_overload.__doc__.strip()
-            print "\"%s\" is not a valid number." % new_overload
+            print "%r is not a valid number." % new_overload
             exit(EXIT_ERROR)
 
+        if percent:
+            new_overload *= 0.01
         if new_overload < 0:
             print "Overload must be non-negative."
             exit(EXIT_ERROR)
 
+        if new_overload > 1 and not percent:
+            print "!?! Warning overload is greater than 100% !?!"
+            status = EXIT_WARNING
+        else:
+            status = EXIT_SUCCESS
+
         builder.set_overload(new_overload)
-        print 'The overload is now %.6f.' % builder.overload
+        print 'The overload factor is now %0.2f%% (%.6f)' % (
+            builder.overload * 100, builder.overload)
         print 'The change will take effect after the next rebalance.'
         builder.save(argv[1])
-        exit(EXIT_SUCCESS)
+        exit(status)
 
 
 def main(arguments=None):
diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py
index 2fb231f9d..ebe2a2639 100644
--- a/test/unit/cli/test_ringbuilder.py
+++ b/test/unit/cli/test_ringbuilder.py
@@ -25,7 +25,25 @@ from swift.common import exceptions
 from swift.common.ring import RingBuilder
 
 
-class TestCommands(unittest.TestCase):
+class RunSwiftRingBuilderMixin(object):
+
+    def run_srb(self, *argv):
+        mock_stdout = StringIO.StringIO()
+        mock_stderr = StringIO.StringIO()
+
+        srb_args = ["", self.tempfile] + [str(s) for s in argv]
+
+        try:
+            with mock.patch("sys.stdout", mock_stdout):
+                with mock.patch("sys.stderr", mock_stderr):
+                    swift.cli.ringbuilder.main(srb_args)
+        except SystemExit as err:
+            if err.code not in (0, 1):  # (success, warning)
+                raise
+        return (mock_stdout.getvalue(), mock_stderr.getvalue())
+
+
+class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin):
 
     def __init__(self, *args, **kwargs):
         super(TestCommands, self).__init__(*args, **kwargs)
@@ -38,7 +56,7 @@ class TestCommands(unittest.TestCase):
                               "127.0.0.1R127.0.0.1", "R:6000",
                               "_some meta data"]
         tmpf = tempfile.NamedTemporaryFile()
-        self.tmpfile = tmpf.name
+        self.tempfile = self.tmpfile = tmpf.name
 
     def tearDown(self):
         try:
@@ -235,6 +253,42 @@ class TestCommands(unittest.TestCase):
         ring = RingBuilder.load(self.tmpfile)
         self.assertEqual(ring.overload, 0.0)
 
+    def test_set_overload_percent(self):
+        self.create_sample_ring()
+        argv = "set_overload 10%".split()
+        out, err = self.run_srb(*argv)
+        ring = RingBuilder.load(self.tmpfile)
+        self.assertEqual(ring.overload, 0.1)
+        self.assertTrue('10.00%' in out)
+        self.assertTrue('0.100000' in out)
+
+    def test_set_overload_percent_strange_input(self):
+        self.create_sample_ring()
+        argv = "set_overload 26%%%%".split()
+        out, err = self.run_srb(*argv)
+        ring = RingBuilder.load(self.tmpfile)
+        self.assertEqual(ring.overload, 0.26)
+        self.assertTrue('26.00%' in out)
+        self.assertTrue('0.260000' in out)
+
+    def test_server_overload_crazy_high(self):
+        self.create_sample_ring()
+        argv = "set_overload 10".split()
+        out, err = self.run_srb(*argv)
+        ring = RingBuilder.load(self.tmpfile)
+        self.assertEqual(ring.overload, 10.0)
+        self.assertTrue('Warning overload is greater than 100%' in out)
+        self.assertTrue('1000.00%' in out)
+        self.assertTrue('10.000000' in out)
+        # but it's cool if you do it on purpose
+        argv[-1] = '1000%'
+        out, err = self.run_srb(*argv)
+        ring = RingBuilder.load(self.tmpfile)
+        self.assertEqual(ring.overload, 10.0)
+        self.assertTrue('Warning overload is greater than 100%' not in out)
+        self.assertTrue('1000.00%' in out)
+        self.assertTrue('10.000000' in out)
+
     def test_validate(self):
         self.create_sample_ring()
         ring = RingBuilder.load(self.tmpfile)
@@ -307,12 +361,12 @@ class TestCommands(unittest.TestCase):
             self.assertEquals(e.code, 1)
 
 
-class TestRebalanceCommand(unittest.TestCase):
+class TestRebalanceCommand(unittest.TestCase, RunSwiftRingBuilderMixin):
 
     def __init__(self, *args, **kwargs):
         super(TestRebalanceCommand, self).__init__(*args, **kwargs)
         tmpf = tempfile.NamedTemporaryFile()
-        self.tempfile = tmpf.name
+        self.tempfile = self.tmpfile = tmpf.name
 
     def tearDown(self):
         try:
@@ -320,21 +374,6 @@ class TestRebalanceCommand(unittest.TestCase):
         except OSError:
             pass
 
-    def run_srb(self, *argv):
-        mock_stdout = StringIO.StringIO()
-        mock_stderr = StringIO.StringIO()
-
-        srb_args = ["", self.tempfile] + [str(s) for s in argv]
-
-        try:
-            with mock.patch("sys.stdout", mock_stdout):
-                with mock.patch("sys.stderr", mock_stderr):
-                    swift.cli.ringbuilder.main(srb_args)
-        except SystemExit as err:
-            if err.code not in (0, 1):  # (success, warning)
-                raise
-        return (mock_stdout.getvalue(), mock_stderr.getvalue())
-
     def test_rebalance_warning_appears(self):
         self.run_srb("create", 8, 3, 24)
         # all in one machine: totally balanceable