openstack/swift
Code Issues Proposed changes

add byteorder information and logic to ring files

Browse Source 
On-disk, serialized ring files are byteorder dependent, which makes then
unportable between different endian architectures. Add a field to the
ring dictionary in the file indicating the byteorder used to generate
the file, and then byteswap if necessary when deserializing the file.

This patch only allows newly created ring files to be byteoder
agnostic. Previously generated ring files will still fail on different
endian architectures, and will need to be regenerated with this patch.

Change-Id: I23b5e0a8082b30ca257aeb1fab03ab74e6f0b2d4
Closes-Bug: #1639980
This commit is contained in:
Drew Balfour 2016-11-08 14:17:31 -08:00
parent a30351bdfc
commit 1ec6e2bb0a
3 changed files with 120 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
swift/common/ring/ring.py
View File

@ -26,6 +26,7 @@ from io import BufferedReader
from hashlib import md5 from hashlib import md5
from itertools import chain from itertools import chain
from tempfile import NamedTemporaryFile from tempfile import NamedTemporaryFile
import sys
from six.moves import range from six.moves import range
@ -70,10 +71,15 @@ class RingData(object):
if metadata_only: if metadata_only:
return ring_dict return ring_dict
byteswap = (ring_dict.get('byteorder', sys.byteorder) != sys.byteorder)
partition_count = 1 << (32 - ring_dict['part_shift']) partition_count = 1 << (32 - ring_dict['part_shift'])
for x in range(ring_dict['replica_count']): for x in range(ring_dict['replica_count']):
ring_dict['replica2part2dev_id'].append( part2dev = array.array('H', gz_file.read(2 * partition_count))
array.array('H', gz_file.read(2 * partition_count))) if byteswap:
part2dev.byteswap()
ring_dict['replica2part2dev_id'].append(part2dev)
return ring_dict return ring_dict
@classmethod @classmethod
@ -117,7 +123,8 @@ class RingData(object):
json_encoder = json.JSONEncoder(sort_keys=True) json_encoder = json.JSONEncoder(sort_keys=True)
json_text = json_encoder.encode( json_text = json_encoder.encode(
{'devs': ring['devs'], 'part_shift': ring['part_shift'], {'devs': ring['devs'], 'part_shift': ring['part_shift'],
'replica_count': len(ring['replica2part2dev_id'])}) 'replica_count': len(ring['replica2part2dev_id']),
'byteorder': sys.byteorder})
json_len = len(json_text) json_len = len(json_text)
file_obj.write(struct.pack('!I', json_len)) file_obj.write(struct.pack('!I', json_len))
file_obj.write(json_text) file_obj.write(json_text)

98
test/unit/common/middleware/test_recon.py
View File

@ -21,6 +21,7 @@ from posix import stat_result, statvfs_result
from shutil import rmtree from shutil import rmtree
import unittest import unittest
from unittest import TestCase from unittest import TestCase
import sys
from swift import __version__ as swiftver from swift import __version__ as swiftver
from swift.common import ring, utils from swift.common import ring, utils
@ -313,23 +314,33 @@ class TestReconSuccess(TestCase):
def test_get_ring_md5(self): def test_get_ring_md5(self):
# We should only see configured and present rings, so to handle the # We should only see configured and present rings, so to handle the
# "normal" case just patch the policies to match the existing rings. # "normal" case just patch the policies to match the existing rings.
expt_out = {'%s/account.ring.gz' % self.tempdir: expt_out = {'little': {'%s/account.ring.gz' % self.tempdir:
'11e0c98abb209474d40d6a9a8a523803', '672c6c50dfcb77e04f5a2124aef87596',
'%s/container.ring.gz' % self.tempdir: '%s/container.ring.gz' % self.tempdir:
'6685496a4045ce0be123068e0165a64d', '4c4392f8bf816596990ca7cd4d4b6e50',
'%s/object.ring.gz' % self.tempdir: '%s/object.ring.gz' % self.tempdir:
'782728be98644fb725e165d4bf5728d4', 'a34178f7399706e41395eb3ac8b2c4f3',
'%s/object-1.ring.gz' % self.tempdir: '%s/object-1.ring.gz' % self.tempdir:
'7c3a4bc9f724d4eb69c9b797cdc28b8c', 'cd54c473676ce5e3103e68f0e9f2326d',
'%s/object-2.ring.gz' % self.tempdir: '%s/object-2.ring.gz' % self.tempdir:
'324b9c4da20cf7ef097edbd219d296e0'} '8783ec76f29bbcfd3f51acc63b2fc337'},
'big': {'%s/account.ring.gz' % self.tempdir:
'70d2dde8144c09e5b42858e0fa17ab7e',
'%s/container.ring.gz' % self.tempdir:
'0bd14f4327cbea88bde7ab7850d4d77d',
'%s/object.ring.gz' % self.tempdir:
'2d17555687af36fd8c7ee5b0c492c582',
'%s/object-1.ring.gz' % self.tempdir:
'bc1145d31771d7957d939077fe40e2e8',
'%s/object-2.ring.gz' % self.tempdir:
'cd95a01ae1ab158f2e9e4c207aeb1769'}}
# We need to instantiate app after overriding the configured policies. # We need to instantiate app after overriding the configured policies.
# object-{1,2}.ring.gz should both appear as they are present on disk # object-{1,2}.ring.gz should both appear as they are present on disk
# and were configured as policies. # and were configured as policies.
app = recon.ReconMiddleware(FakeApp(), {'swift_dir': self.tempdir}) app = recon.ReconMiddleware(FakeApp(), {'swift_dir': self.tempdir})
self.assertEqual(sorted(app.get_ring_md5().items()), self.assertEqual(sorted(app.get_ring_md5().items()),
sorted(expt_out.items())) sorted(expt_out[sys.byteorder].items()))
def test_get_ring_md5_ioerror_produces_none_hash(self): def test_get_ring_md5_ioerror_produces_none_hash(self):
# Ring files that are present but produce an IOError on read should # Ring files that are present but produce an IOError on read should
@ -369,13 +380,18 @@ class TestReconSuccess(TestCase):
raise IOError raise IOError
return open(fn, fmode) return open(fn, fmode)
expt_out = {'%s/account.ring.gz' % self.tempdir: None, expt_out = {'little': {'%s/account.ring.gz' % self.tempdir: None,
'%s/container.ring.gz' % self.tempdir: None, '%s/container.ring.gz' % self.tempdir: None,
'%s/object.ring.gz' % self.tempdir: '%s/object.ring.gz' % self.tempdir:
'782728be98644fb725e165d4bf5728d4'} 'a34178f7399706e41395eb3ac8b2c4f3'},
'big': {'%s/account.ring.gz' % self.tempdir: None,
'%s/container.ring.gz' % self.tempdir: None,
'%s/object.ring.gz' % self.tempdir:
'2d17555687af36fd8c7ee5b0c492c582'}}
ringmd5 = self.app.get_ring_md5(openr=fake_open_objonly) ringmd5 = self.app.get_ring_md5(openr=fake_open_objonly)
self.assertEqual(sorted(ringmd5.items()), self.assertEqual(sorted(ringmd5.items()),
sorted(expt_out.items())) sorted(expt_out[sys.byteorder].items()))
@patch_policies([ @patch_policies([
StoragePolicy(0, 'stagecoach'), StoragePolicy(0, 'stagecoach'),
@ -386,14 +402,22 @@ class TestReconSuccess(TestCase):
# If a configured ring is missing when the app is instantiated, but is # If a configured ring is missing when the app is instantiated, but is
# later moved into place, we shouldn't need to restart object-server # later moved into place, we shouldn't need to restart object-server
# for it to appear in recon. # for it to appear in recon.
expt_out = {'%s/account.ring.gz' % self.tempdir: expt_out = {'little': {'%s/account.ring.gz' % self.tempdir:
'11e0c98abb209474d40d6a9a8a523803', '672c6c50dfcb77e04f5a2124aef87596',
'%s/container.ring.gz' % self.tempdir: '%s/container.ring.gz' % self.tempdir:
'6685496a4045ce0be123068e0165a64d', '4c4392f8bf816596990ca7cd4d4b6e50',
'%s/object.ring.gz' % self.tempdir: '%s/object.ring.gz' % self.tempdir:
'782728be98644fb725e165d4bf5728d4', 'a34178f7399706e41395eb3ac8b2c4f3',
'%s/object-2.ring.gz' % self.tempdir: '%s/object-2.ring.gz' % self.tempdir:
'324b9c4da20cf7ef097edbd219d296e0'} '8783ec76f29bbcfd3f51acc63b2fc337'},
'big': {'%s/account.ring.gz' % self.tempdir:
'70d2dde8144c09e5b42858e0fa17ab7e',
'%s/container.ring.gz' % self.tempdir:
'0bd14f4327cbea88bde7ab7850d4d77d',
'%s/object.ring.gz' % self.tempdir:
'2d17555687af36fd8c7ee5b0c492c582',
'%s/object-2.ring.gz' % self.tempdir:
'cd95a01ae1ab158f2e9e4c207aeb1769'}}
# We need to instantiate app after overriding the configured policies. # We need to instantiate app after overriding the configured policies.
# object-1.ring.gz should not appear as it's present but unconfigured. # object-1.ring.gz should not appear as it's present but unconfigured.
@ -401,7 +425,7 @@ class TestReconSuccess(TestCase):
# present. # present.
app = recon.ReconMiddleware(FakeApp(), {'swift_dir': self.tempdir}) app = recon.ReconMiddleware(FakeApp(), {'swift_dir': self.tempdir})
self.assertEqual(sorted(app.get_ring_md5().items()), self.assertEqual(sorted(app.get_ring_md5().items()),
sorted(expt_out.items())) sorted(expt_out[sys.byteorder].items()))
# Simulate the configured policy's missing ringfile being moved into # Simulate the configured policy's missing ringfile being moved into
# place during runtime # place during runtime
@ -412,12 +436,14 @@ class TestReconSuccess(TestCase):
array.array('H', [1, 1, 0, 3])] array.array('H', [1, 1, 0, 3])]
self._create_ring(os.path.join(self.tempdir, ringfn), self._create_ring(os.path.join(self.tempdir, ringfn),
ringmap, self.ring_devs, self.ring_part_shift) ringmap, self.ring_devs, self.ring_part_shift)
expt_out[ringpath] = 'a7e591642beea6933f64aebd56f357d9' expt_out[sys.byteorder][ringpath] = \
'77f752964f3bd4719e1b9b6cee47659f' if sys.byteorder == 'little' \
else '3e189e6c668a4d159707438dfb87589a'
# We should now see it in the ringmd5 response, without a restart # We should now see it in the ringmd5 response, without a restart
# (using the same app instance) # (using the same app instance)
self.assertEqual(sorted(app.get_ring_md5().items()), self.assertEqual(sorted(app.get_ring_md5().items()),
sorted(expt_out.items())) sorted(expt_out[sys.byteorder].items()))
@patch_policies([ @patch_policies([
StoragePolicy(0, 'stagecoach', is_default=True), StoragePolicy(0, 'stagecoach', is_default=True),
@ -427,14 +453,22 @@ class TestReconSuccess(TestCase):
def test_get_ring_md5_excludes_configured_missing_obj_rings(self): def test_get_ring_md5_excludes_configured_missing_obj_rings(self):
# Object rings that are configured but missing aren't meant to appear # Object rings that are configured but missing aren't meant to appear
# in the ringmd5 response. # in the ringmd5 response.
expt_out = {'%s/account.ring.gz' % self.tempdir: expt_out = {'little': {'%s/account.ring.gz' % self.tempdir:
'11e0c98abb209474d40d6a9a8a523803', '672c6c50dfcb77e04f5a2124aef87596',
'%s/container.ring.gz' % self.tempdir: '%s/container.ring.gz' % self.tempdir:
'6685496a4045ce0be123068e0165a64d', '4c4392f8bf816596990ca7cd4d4b6e50',
'%s/object.ring.gz' % self.tempdir: '%s/object.ring.gz' % self.tempdir:
'782728be98644fb725e165d4bf5728d4', 'a34178f7399706e41395eb3ac8b2c4f3',
'%s/object-2.ring.gz' % self.tempdir: '%s/object-2.ring.gz' % self.tempdir:
'324b9c4da20cf7ef097edbd219d296e0'} '8783ec76f29bbcfd3f51acc63b2fc337'},
'big': {'%s/account.ring.gz' % self.tempdir:
'70d2dde8144c09e5b42858e0fa17ab7e',
'%s/container.ring.gz' % self.tempdir:
'0bd14f4327cbea88bde7ab7850d4d77d',
'%s/object.ring.gz' % self.tempdir:
'2d17555687af36fd8c7ee5b0c492c582',
'%s/object-2.ring.gz' % self.tempdir:
'cd95a01ae1ab158f2e9e4c207aeb1769'}}
# We need to instantiate app after overriding the configured policies. # We need to instantiate app after overriding the configured policies.
# object-1.ring.gz should not appear as it's present but unconfigured. # object-1.ring.gz should not appear as it's present but unconfigured.
@ -442,7 +476,7 @@ class TestReconSuccess(TestCase):
# present. # present.
app = recon.ReconMiddleware(FakeApp(), {'swift_dir': self.tempdir}) app = recon.ReconMiddleware(FakeApp(), {'swift_dir': self.tempdir})
self.assertEqual(sorted(app.get_ring_md5().items()), self.assertEqual(sorted(app.get_ring_md5().items()),
sorted(expt_out.items())) sorted(expt_out[sys.byteorder].items()))
@patch_policies([ @patch_policies([
StoragePolicy(0, 'zero', is_default=True), StoragePolicy(0, 'zero', is_default=True),
@ -450,19 +484,25 @@ class TestReconSuccess(TestCase):
def test_get_ring_md5_excludes_unconfigured_present_obj_rings(self): def test_get_ring_md5_excludes_unconfigured_present_obj_rings(self):
# Object rings that are present but not configured in swift.conf # Object rings that are present but not configured in swift.conf
# aren't meant to appear in the ringmd5 response. # aren't meant to appear in the ringmd5 response.
expt_out = {'%s/account.ring.gz' % self.tempdir: expt_out = {'little': {'%s/account.ring.gz' % self.tempdir:
'11e0c98abb209474d40d6a9a8a523803', '672c6c50dfcb77e04f5a2124aef87596',
'%s/container.ring.gz' % self.tempdir: '%s/container.ring.gz' % self.tempdir:
'6685496a4045ce0be123068e0165a64d', '4c4392f8bf816596990ca7cd4d4b6e50',
'%s/object.ring.gz' % self.tempdir: '%s/object.ring.gz' % self.tempdir:
'782728be98644fb725e165d4bf5728d4'} 'a34178f7399706e41395eb3ac8b2c4f3'},
'big': {'%s/account.ring.gz' % self.tempdir:
'70d2dde8144c09e5b42858e0fa17ab7e',
'%s/container.ring.gz' % self.tempdir:
'0bd14f4327cbea88bde7ab7850d4d77d',
'%s/object.ring.gz' % self.tempdir:
'2d17555687af36fd8c7ee5b0c492c582'}}
# We need to instantiate app after overriding the configured policies. # We need to instantiate app after overriding the configured policies.
# object-{1,2}.ring.gz should not appear as they are present on disk # object-{1,2}.ring.gz should not appear as they are present on disk
# but were not configured as policies. # but were not configured as policies.
app = recon.ReconMiddleware(FakeApp(), {'swift_dir': self.tempdir}) app = recon.ReconMiddleware(FakeApp(), {'swift_dir': self.tempdir})
self.assertEqual(sorted(app.get_ring_md5().items()), self.assertEqual(sorted(app.get_ring_md5().items()),
sorted(expt_out.items())) sorted(expt_out[sys.byteorder].items()))
def test_from_recon_cache(self): def test_from_recon_cache(self):
oart = OpenAndReadTester(['{"notneeded": 5, "testkey1": "canhazio"}']) oart = OpenAndReadTester(['{"notneeded": 5, "testkey1": "canhazio"}'])

27
test/unit/common/ring/test_ring.py
View File

@ -24,6 +24,9 @@ from tempfile import mkdtemp
from shutil import rmtree from shutil import rmtree
from time import sleep, time from time import sleep, time
import random import random
import sys
import copy
import mock
from six.moves import range from six.moves import range
@ -107,6 +110,30 @@ class TestRingData(unittest.TestCase):
rd2 = ring.RingData.load(ring_fname) rd2 = ring.RingData.load(ring_fname)
self.assert_ring_data_equal(rd, rd2) self.assert_ring_data_equal(rd, rd2)
def test_byteswapped_serialization(self):
# Manually byte swap a ring and write it out, claiming it was written
# on a different endian machine. Then read it back in and see if it's
# the same as the non-byte swapped original.
ring_fname = os.path.join(self.testdir, 'foo.ring.gz')
data = [array.array('H', [0, 1, 0, 1]), array.array('H', [0, 1, 0, 1])]
swapped_data = copy.deepcopy(data)
for x in swapped_data:
x.byteswap()
with mock.patch.object(sys, 'byteorder',
'big' if sys.byteorder == 'little'
else 'little'):
rds = ring.RingData(swapped_data,
[{'id': 0, 'zone': 0}, {'id': 1, 'zone': 1}],
30)
rds.save(ring_fname)
rd1 = ring.RingData(data, [{'id': 0, 'zone': 0}, {'id': 1, 'zone': 1}],
30)
rd2 = ring.RingData.load(ring_fname)
self.assert_ring_data_equal(rd1, rd2)
def test_deterministic_serialization(self): def test_deterministic_serialization(self):
""" """
Two identical rings should produce identical .gz files on disk. Two identical rings should produce identical .gz files on disk.