diff options
author | Paul Luse <paul.e.luse@intel.com> | 2014-07-02 14:39:42 -0700 |
---|---|---|
committer | John Dickinson <me@not.mn> | 2014-07-02 21:18:10 -0700 |
commit | bfa9139649c0ac7f75e8c9503cb04d06f7bae0b9 (patch) | |
tree | e01b12d2f9061cb7345701727736327bed7c236d | |
parent | 7dbf8324569d45897f2b4f05cd97299366eff23d (diff) | |
download | swift-proposed/2.0.0.tar.gz |
Fix potential missing key error in container_info2.0.0.rc22.0.0proposed/2.0.0
If upgrading from a non-storage policy enabled version of
swift to a storage policy enabled version its possible that
memcached will have an info structure that does not contain
the 'storage_policy" key resulting in an unhandled exception
during the lookup. The fix is to simply make sure we never
return the dict without a storage_policy key defined; if it
doesn't exist its safe to make it '0' as this means you're
in the update scenario and there's xno other possibility.
Change-Id: If8c88f67ba7a3180ad06b586372fe35c65807aac
-rw-r--r-- | swift/proxy/controllers/base.py | 2 | ||||
-rw-r--r-- | test/unit/proxy/controllers/test_base.py | 2 | ||||
-rwxr-xr-x | test/unit/proxy/controllers/test_obj.py | 67 |
3 files changed, 50 insertions, 21 deletions
diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index 5d65e3c78..f3cf4f290 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -295,6 +295,7 @@ def get_container_info(env, app, swift_source=None): swift_source=swift_source) if not info: info = headers_to_container_info({}, 0) + info.setdefault('storage_policy', '0') return info @@ -987,6 +988,7 @@ class Controller(object): else: info['partition'] = part info['nodes'] = nodes + info.setdefault('storage_policy', '0') return info def _make_request(self, nodes, part, method, path, headers, query, diff --git a/test/unit/proxy/controllers/test_base.py b/test/unit/proxy/controllers/test_base.py index 2c150e1c0..51b104f15 100644 --- a/test/unit/proxy/controllers/test_base.py +++ b/test/unit/proxy/controllers/test_base.py @@ -315,6 +315,7 @@ class TestFuncs(unittest.TestCase): req = Request.blank("/v1/AUTH_account/cont", environ={'swift.cache': FakeCache({})}) resp = get_container_info(req.environ, FakeApp()) + self.assertEquals(resp['storage_policy'], '0') self.assertEquals(resp['bytes'], 6666) self.assertEquals(resp['object_count'], 1000) @@ -342,6 +343,7 @@ class TestFuncs(unittest.TestCase): req = Request.blank("/v1/account/cont", environ={'swift.cache': FakeCache(cache_stub)}) resp = get_container_info(req.environ, FakeApp()) + self.assertEquals(resp['storage_policy'], '0') self.assertEquals(resp['bytes'], 3333) self.assertEquals(resp['object_count'], 10) self.assertEquals(resp['status'], 404) diff --git a/test/unit/proxy/controllers/test_obj.py b/test/unit/proxy/controllers/test_obj.py index eaee22aed..645b2742d 100755 --- a/test/unit/proxy/controllers/test_obj.py +++ b/test/unit/proxy/controllers/test_obj.py @@ -119,6 +119,20 @@ class TestObjControllerWriteAffinity(unittest.TestCase): StoragePolicy(2, 'two'), ]) class TestObjController(unittest.TestCase): + container_info = { + 'partition': 1, + 'nodes': [ + {'ip': '127.0.0.1', 'port': '1', 'device': 'sda'}, + {'ip': '127.0.0.1', 'port': '2', 'device': 'sda'}, + {'ip': '127.0.0.1', 'port': '3', 'device': 'sda'}, + ], + 'write_acl': None, + 'read_acl': None, + 'storage_policy': None, + 'sync_key': None, + 'versions': None, + } + def setUp(self): # setup fake rings with handoffs self.obj_ring = FakeRing(max_more_nodes=3) @@ -132,21 +146,15 @@ class TestObjController(unittest.TestCase): container_ring=FakeRing(), logger=logger) class FakeContainerInfoObjController(proxy_server.ObjectController): - pass - self.controller = FakeContainerInfoObjController - self.controller.container_info = mock.MagicMock(return_value={ - 'partition': 1, - 'nodes': [ - {'ip': '127.0.0.1', 'port': '1', 'device': 'sda'}, - {'ip': '127.0.0.1', 'port': '2', 'device': 'sda'}, - {'ip': '127.0.0.1', 'port': '3', 'device': 'sda'}, - ], - 'write_acl': None, - 'read_acl': None, - 'storage_policy': None, - 'sync_key': None, - 'versions': None}) - self.app.object_controller = self.controller + + def container_info(controller, *args, **kwargs): + patch_path = 'swift.proxy.controllers.base.get_info' + with mock.patch(patch_path) as mock_get_info: + mock_get_info.return_value = dict(self.container_info) + return super(FakeContainerInfoObjController, + controller).container_info(*args, **kwargs) + + self.app.object_controller = FakeContainerInfoObjController def test_PUT_simple(self): req = swift.common.swob.Request.blank('/v1/a/c/o', method='PUT') @@ -244,8 +252,7 @@ class TestObjController(unittest.TestCase): def test_container_sync_put_x_timestamp_not_found(self): test_indexes = [None] + [int(p) for p in POLICIES] for policy_index in test_indexes: - self.controller.container_info.return_value['storage_policy'] = \ - policy_index + self.container_info['storage_policy'] = policy_index put_timestamp = utils.Timestamp(time.time()).normal req = swob.Request.blank( '/v1/a/c/o', method='PUT', headers={ @@ -263,8 +270,7 @@ class TestObjController(unittest.TestCase): def test_container_sync_put_x_timestamp_match(self): test_indexes = [None] + [int(p) for p in POLICIES] for policy_index in test_indexes: - self.controller.container_info.return_value['storage_policy'] = \ - policy_index + self.container_info['storage_policy'] = policy_index put_timestamp = utils.Timestamp(time.time()).normal req = swob.Request.blank( '/v1/a/c/o', method='PUT', headers={ @@ -282,8 +288,7 @@ class TestObjController(unittest.TestCase): ts = (utils.Timestamp(t) for t in itertools.count(int(time.time()))) test_indexes = [None] + [int(p) for p in POLICIES] for policy_index in test_indexes: - self.controller.container_info.return_value['storage_policy'] = \ - policy_index + self.container_info['storage_policy'] = policy_index req = swob.Request.blank( '/v1/a/c/o', method='PUT', headers={ 'Content-Length': 0, @@ -392,5 +397,25 @@ class TestObjController(unittest.TestCase): self.assertEquals(req.environ.get('swift.log_info'), None) +@patch_policies([ + StoragePolicy(0, 'zero', True), + StoragePolicy(1, 'one'), + StoragePolicy(2, 'two'), +]) +class TestObjControllerLegacyCache(TestObjController): + """ + This test pretends like memcache returned a stored value that should + resemble whatever "old" format. It catches KeyErrors you'd get if your + code was expecting some new format during a rolling upgrade. + """ + + container_info = { + 'read_acl': None, + 'write_acl': None, + 'sync_key': None, + 'versions': None, + } + + if __name__ == '__main__': unittest.main() |