Fix bug in swift-ring-builder list_parts

The number of shown replicas in the partition list might differ from the
actual number of replicas (as shown in the bugreport).

This codes simply iterates for the builder._replica2part2dev and
remembers the number of replicas for each partition.

The code to find the partitions was moved to swift/common/ring/utils.py
to make it easier to test, and a test to ensure the correct number of
replicas is returned was added.

Closes-Bug: 1370070
Change-Id: Id6a3ed437bb86df2f43f8b0b79aa8ccb50bbe13e

swift/cli/ringbuilder.py

@@ -14,10 +14,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from array import array
 from errno import EEXIST
 from itertools import islice, izip
-from math import ceil
 from os import mkdir
 from os.path import basename, abspath, dirname, exists, join as pathjoin
 from sys import argv as sys_argv, exit, stderr
@@ -29,7 +27,7 @@ from swift.common.ring import RingBuilder, Ring
 from swift.common.ring.builder import MAX_BALANCE
 from swift.common.utils import lock_parent_directory
 from swift.common.ring.utils import parse_search_value, parse_args, \
-    build_dev_from_opts, parse_builder_ring_filename_args
+    build_dev_from_opts, parse_builder_ring_filename_args, find_parts
 
 MAJOR_VERSION = 1
 MINOR_VERSION = 3
@@ -329,24 +327,16 @@ swift-ring-builder <builder_file> list_parts <search-value> [<search-value>] ..
             print
             print parse_search_value.__doc__.strip()
             exit(EXIT_ERROR)
-        devs = []
+
-        for arg in argv[3:]:
+        sorted_partition_count = find_parts(builder, argv)
-            devs.extend(builder.search_devs(parse_search_value(arg)) or [])
+
-        if not devs:
+        if not sorted_partition_count:
             print 'No matching devices found'
             exit(EXIT_ERROR)
-        devs = [d['id'] for d in devs]
+
-        max_replicas = int(ceil(builder.replicas))
-        matches = [array('i') for x in xrange(max_replicas)]
-        for part in xrange(builder.parts):
-            count = len([d for d in builder.get_part_devices(part)
-                         if d['id'] in devs])
-            if count:
-                matches[max_replicas - count].append(part)
         print 'Partition   Matches'
-        for index, parts in enumerate(matches):
+        for partition, count in sorted_partition_count:
-            for part in parts:
+            print '%9d   %7d' % (partition, count)
-                print '%9d   %7d' % (part, max_replicas - index)
         exit(EXIT_SUCCESS)
 
     def add():

swift/common/ring/utils.py

@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 from collections import defaultdict
+from operator import itemgetter
 import optparse
 
 
@@ -305,3 +306,28 @@ def build_dev_from_opts(opts):
             'port': opts.port, 'device': opts.device, 'meta': opts.meta,
             'replication_ip': replication_ip,
             'replication_port': replication_port, 'weight': opts.weight}
+
+
+def find_parts(builder, argv):
+        devs = []
+        for arg in argv[3:]:
+            devs.extend(builder.search_devs(parse_search_value(arg)) or [])
+
+        devs = [d['id'] for d in devs]
+
+        if not devs:
+            return None
+
+        partition_count = {}
+        for replica in builder._replica2part2dev:
+            for partition, device in enumerate(replica):
+                if device in devs:
+                    if partition not in partition_count:
+                        partition_count[partition] = 0
+                    partition_count[partition] += 1
+
+        # Sort by number of found replicas to keep the output format
+        sorted_partition_count = sorted(
+            partition_count.iteritems(), key=itemgetter(1), reverse=True)
+
+        return sorted_partition_count

test/unit/common/ring/test_utils.py

@@ -15,9 +15,10 @@
 
 import unittest
 
+from swift.common import ring
 from swift.common.ring.utils import (build_tier_tree, tiers_for_dev,
                                      parse_search_value, parse_args,
-                                     build_dev_from_opts,
+                                     build_dev_from_opts, find_parts,
                                      parse_builder_ring_filename_args)
 
 
@@ -159,6 +160,34 @@ class TestUtils(unittest.TestCase):
             'my.file.name', 'my.file.name.ring.gz'
         ), parse_builder_ring_filename_args(args.split()))
 
+    def test_find_parts(self):
+        rb = ring.RingBuilder(8, 3, 0)
+        rb.add_dev({'id': 0, 'region': 1, 'zone': 0, 'weight': 100,
+                    'ip': '127.0.0.1', 'port': 10000, 'device': 'sda1'})
+        rb.add_dev({'id': 1, 'region': 1, 'zone': 1, 'weight': 100,
+                    'ip': '127.0.0.1', 'port': 10001, 'device': 'sda1'})
+        rb.add_dev({'id': 2, 'region': 1, 'zone': 2, 'weight': 100,
+                    'ip': '127.0.0.1', 'port': 10002, 'device': 'sda1'})
+        rb.rebalance()
+
+        rb.add_dev({'id': 3, 'region': 2, 'zone': 1, 'weight': 10,
+                    'ip': '127.0.0.1', 'port': 10004, 'device': 'sda1'})
+        rb.pretend_min_part_hours_passed()
+        rb.rebalance()
+
+        argv = ['swift-ring-builder', 'object.builder',
+                'list_parts', '127.0.0.1']
+        sorted_partition_count = find_parts(rb, argv)
+
+        # Expect 256 partitions in the output
+        self.assertEqual(256, len(sorted_partition_count))
+
+        # Each partitions should have 3 replicas
+        for partition, count in sorted_partition_count:
+            self.assertEqual(
+                3, count, "Partition %d has only %d replicas" %
+                (partition, count))
+
 
 if __name__ == '__main__':
     unittest.main()