summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlex Wheeler <wheelear@gmail.com>2013-03-29 10:56:17 -0400
committerPeter Portante <peter.portante@redhat.com>2013-04-29 16:35:57 -0400
commitce1fd47287787ea9c90a712337d709245cb3445d (patch)
treed1cc41711621117a7cf160bf6f59f7506955439b
parent287a52aa098427fd87fb8a81fd735f14c4714a85 (diff)
Adds missing functions to ring.py, and more thorough tests.
Situation: The function get_part_nodes is being called by Openstack-Swift's proxy/controllers/base.py: https://github.com/openstack/swift/blob/1.7.4/swift/proxy/controllers/base.py#L410 https://github.com/openstack/swift/blob/1.7.6/swift/proxy/controllers/base.py#L447 As this was not implemented in the current GlusterFS version of ring.py, it was calling swift's original get_part_nodes, which would often return the incorrect node, resulting in the incorrect drive being associated with a request. There is another function that the original ring.py implements -- get_other_nodes, which has to do with replication. Since GlusterFS is handling replication, this function should never be called. However, in the interest of completeness, that function is also being replaced. Code changes: The two functions, get_part_nodes, and get_other_nodes have been implemented to override the default functions, and get_nodes has been updated to store information in self vars, about the account being operated on, as the two new functions are not called with that info, and get_nodes appears to always be called first. The code has be refactored to all call _get_part_nodes, much like swift has refactored their code. Reason for implementation this way: I didn't see a better way to do it, but am open to suggestions. Test cases: A mock ring is created with two different devices, test and iops test_first_device: Ensure that the first device, test, is returned for both get_nodes, and get_part_node, and get_more_nodes returns volume_not_in_ring. test_invalid_device: Ensure that a request for a non-existant device returns volume_not_in_ring. test_second_device: Same as test_first_device, but for the second device, iops instead of test. test_second_device_with_reseller_prefix: Test that calling with the reseller prefix, AUTH_ will still return the correct data. Change-Id: I2f3d526934a47b01e9c065d0edf0fbf06f300369 BUG: 924792 Signed-off-by: Alex Wheeler <wheelear@gmail.com> Reviewed-on: http://review.gluster.org/4748 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com> Reviewed-by: Jeff Darcy <jdarcy@redhat.com> Reviewed-by: Anand Avati <avati@redhat.com>
-rw-r--r--ufo/gluster/swift/common/ring.py53
-rw-r--r--ufo/test/unit/common/test_ring.py42
2 files changed, 81 insertions, 14 deletions
diff --git a/ufo/gluster/swift/common/ring.py b/ufo/gluster/swift/common/ring.py
index 9bac39c..06aab8d 100644
--- a/ufo/gluster/swift/common/ring.py
+++ b/ufo/gluster/swift/common/ring.py
@@ -37,6 +37,29 @@ if not reseller_prefix.endswith('_'):
reseller_prefix = reseller_prefix + '_'
class Ring(ring.Ring):
+ def _get_part_nodes(self, part):
+ seen_ids = set()
+ nodes = [dev for dev in self._devs \
+ if dev['device'] == self.acc_name \
+ and not (dev['id'] in seen_ids \
+ or seen_ids.add(dev['id']))]
+ if not nodes:
+ nodes = [self.false_node]
+ return nodes
+
+ def get_part_nodes(self, part):
+ """
+ Get the nodes that are responsible for the partition. If one
+ node is responsible for more than one replica of the same
+ partition, it will only appear in the output once.
+
+ :param part: partition to get nodes for
+ :returns: list of node dicts
+
+ See :func:`get_nodes` for a description of the node dicts.
+ """
+ return self._get_part_nodes(part)
+
def get_nodes(self, account, container=None, obj=None):
"""
Get the partition and nodes for an account/container/object.
@@ -63,20 +86,26 @@ class Ring(ring.Ring):
hardware description
====== ===============================================================
"""
- false_node = [{'zone': 1, 'weight': 100.0, 'ip': '127.0.0.1', 'id': 0, \
+ self.false_node = {'zone': 1, 'weight': 100.0, 'ip': '127.0.0.1', 'id': 0, \
'meta': '', 'device': 'volume_not_in_ring', \
- 'port': 6012}]
+ 'port': 6012}
if account.startswith(reseller_prefix):
- acc_name = account.replace(reseller_prefix, '', 1)
+ self.acc_name = account.replace(reseller_prefix, '', 1)
else:
- acc_name = account
+ self.acc_name = account
part = 0
- seen_ids = set()
- nodes = [dev for dev in self._devs \
- if dev['device'] == acc_name \
- and not (dev['id'] in seen_ids \
- or seen_ids.add(dev['id']))]
- if not nodes:
- nodes = false_node
- return part, nodes
+ return part, self._get_part_nodes(part)
+
+
+ def get_more_nodes(self, part):
+ """
+ Generator to get extra nodes for a partition for hinted handoff.
+
+ :param part: partition to get handoff nodes for
+ :returns: generator of node dicts
+
+ See :func:`get_nodes` for a description of the node dicts.
+ Should never be called in the swift UFO environment, so yield nothing
+ """
+ yield self.false_node
diff --git a/ufo/test/unit/common/test_ring.py b/ufo/test/unit/common/test_ring.py
index 4fb964e..48ed952 100644
--- a/ufo/test/unit/common/test_ring.py
+++ b/ufo/test/unit/common/test_ring.py
@@ -30,14 +30,52 @@ class TestRing(unittest.TestCase):
def setUp(self):
self.ring = Ring(SWIFT_DIR, ring_name='object')
- def test_get_notes(self):
+ def test_first_device(self):
try:
__devs = self.ring._devs
self.ring._devs = _mock_ring_data()
+
part, node = self.ring.get_nodes('test')
assert node[0]['device'] == 'test'
+ node = self.ring.get_part_nodes(0)
+ assert node[0]['device'] == 'test'
+ for node in self.ring.get_more_nodes(0):
+ assert node['device'] == 'volume_not_in_ring'
+ finally:
+ self.ring._devs = __devs
+
+ def test_invalid_device(self):
+ try:
+ __devs = self.ring._devs
+ self.ring._devs = _mock_ring_data()
+
part, node = self.ring.get_nodes('test2')
- assert node
assert node[0]['device'] == 'volume_not_in_ring'
+ node = self.ring.get_part_nodes(0)
+ assert node[0]['device'] == 'volume_not_in_ring'
+ finally:
+ self.ring._devs = __devs
+
+ def test_second_device(self):
+ try:
+ __devs = self.ring._devs
+ self.ring._devs = _mock_ring_data()
+
+ part, node = self.ring.get_nodes('iops')
+ assert node[0]['device'] == 'iops'
+ node = self.ring.get_part_nodes(0)
+ assert node[0]['device'] == 'iops'
+ for node in self.ring.get_more_nodes(0):
+ assert node['device'] == 'volume_not_in_ring'
+ finally:
+ self.ring._devs = __devs
+
+ def test_second_device_with_reseller_prefix(self):
+ try:
+ __devs = self.ring._devs
+ self.ring._devs = _mock_ring_data()
+
+ part, node = self.ring.get_nodes('AUTH_iops')
+ assert node[0]['device'] == 'iops'
finally:
self.ring._devs = __devs