get_part_nodes should raise error on invalid part

[clayg]

When you ask a ring to `get_part_nodes` for a partition number > partition_count - 1 you get back an empty list [1].

This is almost guaranteed to have strange behaviors for code that is expecting `get_part_nodes` to return a list of devices equal to the length of the rings replicas (or at least >= int(replica_count)).

It should raise a specific Error (ValueError? IndexError?).

1. https://gist.github.com/clayg/9aef468776d410a52e37bb40b9745590

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/319988

[Michael Glaser (mikeg451)]

Releasing assignment to give someone else a chance.
pep8 error was easy to fix, but new condition causing a failure in test_replicator.py during the tox test.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Michael Glaser (<email address hidden>) on branch: master
Review: https://review.openstack.org/319988
Reason: Releasing bug for someone else to have a try. Thanks for reviewing.

[Sachin (sacpatil)]

Is the bug fixed? as I see that the list is not empty(or I'm doing something wrong)

>>> from swift.common.storage_policy import POLICIES
>>> p = POLICIES[1]
>>> p
StoragePolicy(1, ['silver'], is_default=False, is_deprecated=False, policy_type='replication')
>>> p.load_ring('/etc/swift')
>>> p.object_ring.get_part_nodes(63)
[{'index': 0, u'replication_port': 6020, u'weight': 1.0, u'zone': 2, u'ip': u'127.0.0.1', u'region': 1, u'id': 1, u'replication_ip': u'127.0.0.1', u'meta': u'', u'device': u'sdb2', u'port': 6020}, {'index': 1, u
'replication_port': 6030, u'weight': 1.0, u'zone': 3, u'ip': u'127.0.0.1', u'region': 1, u'id': 2, u'replication_ip': u'127.0.0.1', u'meta': u'', u'device': u'sdb3', u'port': 6030}]
>>>

[clayg (clay-gerrard)]

I don't really know how many parts you have in your ring, 64 should fail on part-power of 6, but would work as you show above on anything higher.... try a ridiculously large part number like 2 ** 32 [1]

FWIW someone took a stab at this and discovered that fixing some of replication/reconstructor code would hit the new error in a unittest and fixing it might have been a bit of a hozer [2].

But if you want to take over that patch, add in some new simple unittests, and maybe try and suss out the issues with replication/reconstructor code/tests someone in #openstack-swift on freenode might could probably try and suggest a few pointers if you get stuck.

1. https://gist.github.com/clayg/fd3c9e95224cf51a3c4bb813bc8cf89c
2. https://review.openstack.org/#/c/319988/

[Sachin (sacpatil)]

>>> from swift.common.storage_policy import POLICIES
>>> p = POLICIES[1]
>>> p
StoragePolicy(1, ['silver'], is_default=False, is_deprecated=False, policy_type='replication')
>>> p.load_ring('/etc/swift')
>>> p.object_ring.get_part_nodes(457293)
[]

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/402522