diff options
| -rw-r--r-- | glance_store/common/cinder_utils.py | 4 | ||||
| -rw-r--r-- | glance_store/tests/unit/test_cinder_base.py | 270 | ||||
| -rw-r--r-- | glance_store/tests/unit/test_cinder_store.py | 17 | ||||
| -rw-r--r-- | glance_store/tests/unit/test_multistore_cinder.py | 18 | ||||
| -rw-r--r-- | releasenotes/notes/fix-interval-in-retries-471155ff34d9f0e9.yaml | 7 |
5 files changed, 287 insertions, 29 deletions
diff --git a/glance_store/common/cinder_utils.py b/glance_store/common/cinder_utils.py index b3739a8..b14aa23 100644 --- a/glance_store/common/cinder_utils.py +++ b/glance_store/common/cinder_utils.py @@ -71,7 +71,9 @@ class API(object): client.volumes.delete(volume_id) @retrying.retry(stop_max_attempt_number=5, - retry_on_exception=_retry_on_bad_request) + retry_on_exception=_retry_on_bad_request, + wait_exponential_multiplier=1000, + wait_exponential_max=10000) @handle_exceptions def attachment_create(self, client, volume_id, connector=None, mountpoint=None, mode=None): diff --git a/glance_store/tests/unit/test_cinder_base.py b/glance_store/tests/unit/test_cinder_base.py index 0fd8294..d9e6c2d 100644 --- a/glance_store/tests/unit/test_cinder_base.py +++ b/glance_store/tests/unit/test_cinder_base.py @@ -437,11 +437,42 @@ class TestCinderStoreBase(object): self.assertEqual(expected_num_chunks, num_chunks) self.assertEqual(expected_file_contents, data) + def _test_cinder_volume_not_found(self, method_call, mock_method): + fake_volume_uuid = str(uuid.uuid4()) + loc = mock.MagicMock(volume_id=fake_volume_uuid) + mock_not_found = {mock_method: mock.MagicMock( + side_effect=cinder.cinder_exception.NotFound(code=404))} + fake_volumes = mock.MagicMock(**mock_not_found) + + with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc: + mocked_cc.return_value = mock.MagicMock(volumes=fake_volumes) + self.assertRaises(exceptions.NotFound, method_call, loc, + context=self.context) + + def test_cinder_get_volume_not_found(self): + self._test_cinder_volume_not_found(self.store.get, 'get') + + def test_cinder_get_size_volume_not_found(self): + self._test_cinder_volume_not_found(self.store.get_size, 'get') + + def test_cinder_delete_volume_not_found(self): + self._test_cinder_volume_not_found(self.store.delete, 'delete') + + def test_cinder_get_client_exception(self): + fake_volume_uuid = str(uuid.uuid4()) + loc = mock.MagicMock(volume_id=fake_volume_uuid) + + with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc: + mock_cc.side_effect = ( + cinder.cinder_exception.ClientException(code=500)) + self.assertRaises(exceptions.BackendException, self.store.get, loc, + context=self.context) + def _test_cinder_get_size(self, is_multi_store=False): fake_client = mock.MagicMock(auth_token=None, management_url=None) fake_volume_uuid = str(uuid.uuid4()) fake_volume = mock.MagicMock(size=5, metadata={}) - fake_volumes = {fake_volume_uuid: fake_volume} + fake_volumes = mock.MagicMock(get=lambda fake_volume_uuid: fake_volume) with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc: mocked_cc.return_value = mock.MagicMock(client=fake_client, @@ -471,6 +502,17 @@ class TestCinderStoreBase(object): image_size = self.store.get_size(loc, context=self.context) self.assertEqual(expected_image_size, image_size) + def test_cinder_get_size_generic_exception(self): + fake_volume_uuid = str(uuid.uuid4()) + loc = mock.MagicMock(volume_id=fake_volume_uuid) + fake_volumes = mock.MagicMock( + get=mock.MagicMock(side_effect=Exception())) + + with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc: + mocked_cc.return_value = mock.MagicMock(volumes=fake_volumes) + image_size = self.store.get_size(loc, context=self.context) + self.assertEqual(0, image_size) + def _test_cinder_add(self, fake_volume, volume_file, size_kb=5, verifier=None, backend='glance_store', fail_resize=False, is_multi_store=False): @@ -528,6 +570,199 @@ class TestCinderStoreBase(object): if is_multi_store: self.assertEqual(backend, metadata["store"]) + def test_cinder_add_volume_not_found(self): + image_file = mock.MagicMock() + fake_image_id = str(uuid.uuid4()) + expected_size = 0 + fake_volumes = mock.MagicMock(create=mock.MagicMock( + side_effect=cinder.cinder_exception.NotFound(code=404))) + + with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc: + mock_cc.return_value = mock.MagicMock(volumes=fake_volumes) + self.assertRaises( + exceptions.BackendException, self.store.add, + fake_image_id, image_file, expected_size, self.hash_algo, + self.context, None) + + def _test_cinder_add_extend(self, is_multi_store=False): + + expected_volume_size = 2 * units.Gi + expected_multihash = 'fake_hash' + + fakebuffer = mock.MagicMock() + fakebuffer.__len__.return_value = expected_volume_size + + def get_fake_hash(type, secure=False): + if type == 'md5': + return mock.MagicMock(hexdigest=lambda: expected_checksum) + else: + return mock.MagicMock(hexdigest=lambda: expected_multihash) + + expected_image_id = str(uuid.uuid4()) + expected_volume_id = str(uuid.uuid4()) + expected_size = 0 + image_file = mock.MagicMock( + read=mock.MagicMock(side_effect=[fakebuffer, None])) + fake_volume = mock.MagicMock(id=expected_volume_id, status='available', + size=1) + expected_checksum = 'fake_checksum' + verifier = None + backend = 'glance_store' + + expected_location = 'cinder://%s' % fake_volume.id + if is_multi_store: + # Default backend is 'glance_store' for single store but in case + # of multi store, if the backend option is not passed, we should + # assign it to the default i.e. 'cinder1' + backend = 'cinder1' + expected_location = 'cinder://%s/%s' % (backend, fake_volume.id) + self.config(cinder_volume_type='some_type', group=backend) + + fake_client = mock.MagicMock(auth_token=None, management_url=None) + fake_volume.manager.get.return_value = fake_volume + fake_volumes = mock.MagicMock(create=mock.Mock( + return_value=fake_volume)) + + @contextlib.contextmanager + def fake_open(client, volume, mode): + self.assertEqual('wb', mode) + yield mock.MagicMock() + + with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \ + mock.patch.object(self.store, '_open_cinder_volume', + side_effect=fake_open), \ + mock.patch.object(cinder.Store, '_wait_resize_device'), \ + mock.patch.object(cinder.utils, 'get_hasher') as fake_hasher, \ + mock.patch.object(cinder.Store, '_wait_volume_status', + return_value=fake_volume) as mock_wait: + mock_cc.return_value = mock.MagicMock(client=fake_client, + volumes=fake_volumes) + + fake_hasher.side_effect = get_fake_hash + loc, size, checksum, multihash, metadata = self.store.add( + expected_image_id, image_file, expected_size, self.hash_algo, + self.context, verifier) + self.assertEqual(expected_location, loc) + self.assertEqual(expected_volume_size, size) + self.assertEqual(expected_checksum, checksum) + self.assertEqual(expected_multihash, multihash) + fake_volumes.create.assert_called_once_with( + 1, + name='image-%s' % expected_image_id, + metadata={'image_owner': self.context.project_id, + 'glance_image_id': expected_image_id, + 'image_size': str(expected_volume_size)}, + volume_type='some_type') + if is_multi_store: + self.assertEqual(backend, metadata["store"]) + fake_volume.extend.assert_called_once_with( + fake_volume, expected_volume_size // units.Gi) + mock_wait.assert_has_calls( + [mock.call(fake_volume, 'creating', 'available'), + mock.call(fake_volume, 'extending', 'available')]) + + def test_cinder_add_extend_storage_full(self): + + expected_volume_size = 2 * units.Gi + + fakebuffer = mock.MagicMock() + fakebuffer.__len__.return_value = expected_volume_size + + expected_image_id = str(uuid.uuid4()) + expected_volume_id = str(uuid.uuid4()) + expected_size = 0 + image_file = mock.MagicMock( + read=mock.MagicMock(side_effect=[fakebuffer, None])) + fake_volume = mock.MagicMock(id=expected_volume_id, status='available', + size=1) + verifier = None + + fake_client = mock.MagicMock() + fake_volume.manager.get.return_value = fake_volume + fake_volumes = mock.MagicMock(create=mock.Mock( + return_value=fake_volume)) + + with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \ + mock.patch.object(self.store, '_open_cinder_volume'), \ + mock.patch.object(cinder.Store, '_wait_resize_device'), \ + mock.patch.object(cinder.utils, 'get_hasher'), \ + mock.patch.object( + cinder.Store, '_wait_volume_status') as mock_wait: + + mock_cc.return_value = mock.MagicMock(client=fake_client, + volumes=fake_volumes) + + mock_wait.side_effect = [fake_volume, exceptions.BackendException] + self.assertRaises( + exceptions.StorageFull, self.store.add, expected_image_id, + image_file, expected_size, self.hash_algo, self.context, + verifier) + + def test_cinder_add_extend_volume_delete_exception(self): + + expected_volume_size = 2 * units.Gi + + fakebuffer = mock.MagicMock() + fakebuffer.__len__.return_value = expected_volume_size + + expected_image_id = str(uuid.uuid4()) + expected_volume_id = str(uuid.uuid4()) + expected_size = 0 + image_file = mock.MagicMock( + read=mock.MagicMock(side_effect=[fakebuffer, None])) + fake_volume = mock.MagicMock( + id=expected_volume_id, status='available', size=1, + delete=mock.MagicMock(side_effect=Exception())) + + fake_client = mock.MagicMock() + fake_volume.manager.get.return_value = fake_volume + fake_volumes = mock.MagicMock(create=mock.Mock( + return_value=fake_volume)) + verifier = None + + with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \ + mock.patch.object(self.store, '_open_cinder_volume'), \ + mock.patch.object(cinder.Store, '_wait_resize_device'), \ + mock.patch.object(cinder.utils, 'get_hasher'), \ + mock.patch.object( + cinder.Store, '_wait_volume_status') as mock_wait: + + mock_cc.return_value = mock.MagicMock(client=fake_client, + volumes=fake_volumes) + + mock_wait.side_effect = [fake_volume, exceptions.BackendException] + self.assertRaises( + Exception, self.store.add, expected_image_id, # noqa + image_file, expected_size, self.hash_algo, self.context, + verifier) + fake_volume.delete.assert_called_once() + + def _test_cinder_delete(self, is_multi_store=False): + fake_client = mock.MagicMock(auth_token=None, management_url=None) + fake_volume_uuid = str(uuid.uuid4()) + fake_volumes = mock.MagicMock(delete=mock.Mock()) + + with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc: + mocked_cc.return_value = mock.MagicMock(client=fake_client, + volumes=fake_volumes) + + loc = self._get_uri_loc(fake_volume_uuid, + is_multi_store=is_multi_store) + + self.store.delete(loc, context=self.context) + fake_volumes.delete.assert_called_once_with(fake_volume_uuid) + + def test_cinder_delete_client_exception(self): + fake_volume_uuid = str(uuid.uuid4()) + loc = mock.MagicMock(volume_id=fake_volume_uuid) + fake_volumes = mock.MagicMock(delete=mock.MagicMock( + side_effect=cinder.cinder_exception.ClientException(code=500))) + + with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc: + mocked_cc.return_value = mock.MagicMock(volumes=fake_volumes) + self.assertRaises(exceptions.BackendException, self.store.delete, + loc, context=self.context) + def test__get_device_size(self): fake_data = b"fake binary data" fake_len = int(math.ceil(float(len(fake_data)) / units.Gi)) @@ -577,3 +812,36 @@ class TestCinderStoreBase(object): self.config(rootwrap_config=fake_rootwrap, group=group) res = self.store.get_root_helper() self.assertEqual(expected, res) + + def test_get_hash_str(self): + test_str = 'test_str' + with mock.patch.object(cinder.hashlib, 'sha256') as fake_hashlib: + self.store.get_hash_str(test_str) + test_str = test_str.encode('utf-8') + fake_hashlib.assert_called_once_with(test_str) + + def test__get_mount_path(self): + fake_hex = 'fake_hex_digest' + fake_share = 'fake_share' + fake_path = 'fake_mount_path' + expected_path = os.path.join(fake_path, fake_hex) + with mock.patch.object(self.store, 'get_hash_str') as fake_hash: + fake_hash.return_value = fake_hex + res = self.store._get_mount_path(fake_share, fake_path) + self.assertEqual(expected_path, res) + + def test__get_host_ip_v6(self): + fake_ipv6 = '2001:0db8:85a3:0000:0000:8a2e:0370' + fake_socket_return = [[0, 1, 2, 3, [fake_ipv6]]] + with mock.patch.object(cinder.socket, 'getaddrinfo') as fake_socket: + fake_socket.return_value = fake_socket_return + res = self.store._get_host_ip('fake_host') + self.assertEqual(fake_ipv6, res) + + def test__get_host_ip_v4(self): + fake_ip = '127.0.0.1' + fake_socket_return = [[0, 1, 2, 3, [fake_ip]]] + with mock.patch.object(cinder.socket, 'getaddrinfo') as fake_socket: + fake_socket.side_effect = [socket.gaierror, fake_socket_return] + res = self.store._get_host_ip('fake_host') + self.assertEqual(fake_ip, res) diff --git a/glance_store/tests/unit/test_cinder_store.py b/glance_store/tests/unit/test_cinder_store.py index 60fba6f..efd5419 100644 --- a/glance_store/tests/unit/test_cinder_store.py +++ b/glance_store/tests/unit/test_cinder_store.py @@ -23,7 +23,6 @@ import uuid from oslo_utils import units from glance_store import exceptions -from glance_store import location from glance_store.tests import base from glance_store.tests.unit import test_cinder_base from glance_store.tests.unit import test_store_capabilities @@ -146,19 +145,11 @@ class TestCinderStore(base.StoreBaseTest, fail_resize=True) fake_volume.delete.assert_called_once() + def test_cinder_add_extend(self): + self._test_cinder_add_extend() + def test_cinder_delete(self): - fake_client = mock.MagicMock(auth_token=None, management_url=None) - fake_volume_uuid = str(uuid.uuid4()) - fake_volumes = mock.MagicMock(delete=mock.Mock()) - - with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc: - mocked_cc.return_value = mock.MagicMock(client=fake_client, - volumes=fake_volumes) - - uri = 'cinder://%s' % fake_volume_uuid - loc = location.get_location_from_uri(uri, conf=self.conf) - self.store.delete(loc, context=self.context) - fake_volumes.delete.assert_called_once_with(fake_volume_uuid) + self._test_cinder_delete() def test_set_url_prefix(self): self.assertEqual('cinder://', self.store._url_prefix) diff --git a/glance_store/tests/unit/test_multistore_cinder.py b/glance_store/tests/unit/test_multistore_cinder.py index e92f86c..9161275 100644 --- a/glance_store/tests/unit/test_multistore_cinder.py +++ b/glance_store/tests/unit/test_multistore_cinder.py @@ -283,21 +283,11 @@ class TestMultiCinderStore(base.MultiStoreBaseTest, fail_resize=True, is_multi_store=True) fake_volume.delete.assert_called_once() + def test_cinder_add_extend(self): + self._test_cinder_add_extend(is_multi_store=True) + def test_cinder_delete(self): - fake_client = mock.MagicMock(auth_token=None, management_url=None) - fake_volume_uuid = str(uuid.uuid4()) - fake_volumes = mock.MagicMock(delete=mock.Mock()) - - with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc: - mocked_cc.return_value = mock.MagicMock(client=fake_client, - volumes=fake_volumes) - - uri = 'cinder://cinder1/%s' % fake_volume_uuid - loc = location.get_location_from_uri_and_backend(uri, - "cinder1", - conf=self.conf) - self.store.delete(loc, context=self.context) - fake_volumes.delete.assert_called_once_with(fake_volume_uuid) + self._test_cinder_delete(is_multi_store=True) def test_set_url_prefix(self): self.assertEqual('cinder://cinder1', self.store._url_prefix) diff --git a/releasenotes/notes/fix-interval-in-retries-471155ff34d9f0e9.yaml b/releasenotes/notes/fix-interval-in-retries-471155ff34d9f0e9.yaml new file mode 100644 index 0000000..00f28da --- /dev/null +++ b/releasenotes/notes/fix-interval-in-retries-471155ff34d9f0e9.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + `Bug #1969373 <https://bugs.launchpad.net/glance-store/+bug/1969373>`_: + Cinder Driver: Correct the retry interval from fixed 1 second to + exponential backoff for attaching a volume during image create/save + operation. |