Improve hash ring value conversion

The current method for computing the point on the hash ring converts
the MD5 digest into a 4-byte integer (and assumes big-endian). The
digest is actually a 16-byte value, so we are eliminating much of
the value, thus increasing the possibility of hash collisions.

This patch does two things:

  * Removes the endianness assumption
  * Uses the full value of the digest to create a long integer,
    which has unlimited precision in Python.

Change-Id: I554ec46f506cb8cdfd5878c11a905a3acfe92db0

2 files changed, 27 insertions, 4 deletions

diff --git a/ironic/common/hash_ring.py b/ironic/common/hash_ring.py
index a79ff344d..51ab38aa1 100644
--- a/ironic/common/hash_ring.py
+++ b/ironic/common/hash_ring.py
@@ -15,7 +15,6 @@
 
 import bisect
 import hashlib
-import struct
 import threading
 
 from oslo.config import cfg
@@ -91,16 +90,23 @@ class HashRing(object):
             key_hash = hashlib.md5(key)
             for p in range(2 ** CONF.hash_partition_exponent):
                 key_hash.update(key)
-                hashed_key = struct.unpack_from('>I', key_hash.digest())[0]
+                hashed_key = self._hash2int(key_hash)
                 self._host_hashes[hashed_key] = host
         # Gather the (possibly colliding) resulting hashes into a bisectable
         # list.
         self._partitions = sorted(self._host_hashes.keys())
 
+    def _hash2int(self, key_hash):
+        """Convert the given hash's digest to a numerical value for the ring.
+
+        :returns: An integer equivalent value of the digest.
+        """
+        return int(key_hash.hexdigest(), 16)
+
     def _get_partition(self, data):
         try:
-            hashed_key = struct.unpack_from(
-                '>I', hashlib.md5(data).digest())[0]
+            key_hash = hashlib.md5(data)
+            hashed_key = self._hash2int(key_hash)
             position = bisect.bisect(self._partitions, hashed_key)
             return position if position < len(self._partitions) else 0
         except TypeError:
diff --git a/ironic/tests/test_hash_ring.py b/ironic/tests/test_hash_ring.py
index 8dadb34a7..414da9432 100644
--- a/ironic/tests/test_hash_ring.py
+++ b/ironic/tests/test_hash_ring.py
@@ -13,6 +13,9 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import hashlib
+
+import mock
 from oslo.config import cfg
 from testtools import matchers
 
@@ -33,6 +36,20 @@ class HashRingTestCase(base.TestCase):
     #                fake -> foo, bar, baz
     #                fake-again -> bar, baz, foo
 
+    @mock.patch.object(hashlib, 'md5')
+    def test__hash2int_returns_int(self, mock_md5):
+        CONF.set_override('hash_partition_exponent', 0)
+        r1 = 32 * 'a'
+        r2 = 32 * 'b'
+        mock_md5.return_value.hexdigest.side_effect = [r1, r2]
+
+        hosts = ['foo', 'bar']
+        replicas = 1
+        ring = hash_ring.HashRing(hosts, replicas=replicas)
+
+        self.assertIn(int(r1, 16), ring._host_hashes)
+        self.assertIn(int(r2, 16), ring._host_hashes)
+
     def test_create_ring(self):
         hosts = ['foo', 'bar']
         replicas = 2