diff options
| -rw-r--r-- | glance_store/_drivers/cinder.py | 20 | ||||
| -rw-r--r-- | glance_store/common/attachment_state_manager.py | 261 | ||||
| -rw-r--r-- | glance_store/common/cinder_utils.py | 25 | ||||
| -rw-r--r-- | glance_store/tests/unit/common/test_attachment_state_manager.py | 108 | ||||
| -rw-r--r-- | glance_store/tests/unit/common/test_cinder_utils.py | 21 | ||||
| -rw-r--r-- | glance_store/tests/unit/test_cinder_store.py | 28 | ||||
| -rw-r--r-- | glance_store/tests/unit/test_multistore_cinder.py | 28 | ||||
| -rw-r--r-- | releasenotes/notes/multiattach-volume-handling-1a8446a64463f2cf.yaml | 12 |
8 files changed, 482 insertions, 21 deletions
diff --git a/glance_store/_drivers/cinder.py b/glance_store/_drivers/cinder.py index d62feb3..3ccfe29 100644 --- a/glance_store/_drivers/cinder.py +++ b/glance_store/_drivers/cinder.py @@ -33,6 +33,7 @@ from oslo_config import cfg from oslo_utils import units from glance_store import capabilities +from glance_store.common import attachment_state_manager from glance_store.common import cinder_utils from glance_store.common import utils import glance_store.driver @@ -699,8 +700,13 @@ class Store(glance_store.driver.Store): connector_prop = connector.get_connector_properties( root_helper, host, use_multipath, enforce_multipath) - attachment = self.volume_api.attachment_create(client, volume_id, - mode=attach_mode) + if volume.multiattach: + attachment = attachment_state_manager.attach(client, volume_id, + host, + mode=attach_mode) + else: + attachment = self.volume_api.attachment_create(client, volume_id, + mode=attach_mode) attachment = self.volume_api.attachment_update( client, attachment['id'], connector_prop, mountpoint='glance_store') @@ -767,13 +773,19 @@ class Store(glance_store.driver.Store): root_helper) disconnect_volume_nfs() else: - conn.disconnect_volume(connection_info, device) + if volume.multiattach: + attachment_state_manager.detach( + client, attachment.id, volume_id, host, conn, + connection_info, device) + else: + conn.disconnect_volume(connection_info, device) except Exception: LOG.exception(_LE('Failed to disconnect volume ' '%(volume_id)s.'), {'volume_id': volume.id}) - self.volume_api.attachment_delete(client, attachment.id) + if not volume.multiattach: + self.volume_api.attachment_delete(client, attachment.id) def _cinder_volume_data_iterator(self, client, volume, max_size, offset=0, chunk_size=None, partial_length=None): diff --git a/glance_store/common/attachment_state_manager.py b/glance_store/common/attachment_state_manager.py new file mode 100644 index 0000000..187a24c --- /dev/null +++ b/glance_store/common/attachment_state_manager.py @@ -0,0 +1,261 @@ +# Copyright 2021 RedHat Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import collections +import contextlib +import logging +import socket +import threading + +from oslo_config import cfg + +from glance_store.common import cinder_utils +from glance_store import exceptions +from glance_store.i18n import _LE, _LW + + +LOG = logging.getLogger(__name__) + +HOST = socket.gethostname() +CONF = cfg.CONF + + +class AttachmentStateManagerMeta(type): + _instance = {} + + def __call__(cls, *args, **kwargs): + if cls not in cls._instance: + cls._instance[cls] = super( + AttachmentStateManagerMeta, cls).__call__(*args, **kwargs) + return cls._instance[cls] + + +class _AttachmentStateManager(metaclass=AttachmentStateManagerMeta): + """A global manager of a volume's multiple attachments. + + _AttachmentStateManager manages a _AttachmentState object for the current + glance node. Primarily it creates one on object initialization and returns + it via get_state(). + + _AttachmentStateManager manages concurrency itself. Independent callers do + not need to consider interactions between multiple _AttachmentStateManager + calls when designing their own locking. + + """ + # Reset state of global _AttachmentStateManager + state = None + use_count = 0 + + # Guards both state and use_count + cond = threading.Condition() + + def __init__(self, host): + """Initialise a new _AttachmentState + + We will block before creating a new state until all operations + using a previous state have completed. + + :param host: host + """ + # Wait until all operations using a previous state are + # complete before initialising a new one. Note that self.state is + # already None, set either by initialisation or by host_down. This + # means the current state will not be returned to any new callers, + # and use_count will eventually reach zero. + # We do this to avoid a race between _AttachmentState initialisation + # and an on-going attach/detach operation + self.host = host + while self.use_count != 0: + self.cond.wait() + + # Another thread might have initialised state while we were + # waiting + if self.state is None: + LOG.debug('Initialising _HostMountState') + self.state = _AttachmentState() + + @contextlib.contextmanager + def get_state(self): + """Return the current attachment state. + + _AttachmentStateManager will not permit a new state object to be + created while any previous state object is still in use. + + :rtype: _AttachmentState + """ + + # We hold the instance lock here so that if a _AttachmentState is + # currently initialising we'll wait for it to complete rather than + # fail. + with self.cond: + state = self.state + if state is None: + LOG.error('Host not initialized') + raise exceptions.HostNotInitialized(host=self.host) + self.use_count += 1 + try: + LOG.debug('Got _AttachmentState') + yield state + finally: + with self.cond: + self.use_count -= 1 + self.cond.notify_all() + + +class _AttachmentState(object): + """A data structure recording all managed attachments. _AttachmentState + ensures that the glance node only attempts to a single multiattach volume + in use by multiple attachments once, and that it is not disconnected until + it is no longer in use by any attachments. + + Callers should not create a _AttachmentState directly, but should obtain + it via: + + with attachment.get_manager().get_state() as state: + state.attach(...) + + _AttachmentState manages concurrency itself. Independent callers do not + need to consider interactions between multiple _AttachmentState calls when + designing their own locking. + """ + + class _Attachment(object): + # A single multiattach volume, and the set of attachments in use + # on it. + def __init__(self): + # A guard for operations on this volume + self.lock = threading.Lock() + + # The set of attachments on this volume + self.attachments = set() + + def add_attachment(self, attachment_id, host): + self.attachments.add((attachment_id, host)) + + def remove_attachment(self, attachment_id, host): + self.attachments.remove((attachment_id, host)) + + def in_use(self): + return len(self.attachments) > 0 + + def __init__(self): + """Initialise _AttachmentState""" + + self.volumes = collections.defaultdict(self._Attachment) + self.volume_api = cinder_utils.API() + + @contextlib.contextmanager + def _get_locked(self, volume): + """Get a locked attachment object + + :param mountpoint: The path of the volume whose attachment we should + return. + :rtype: _AttachmentState._Attachment + """ + while True: + vol = self.volumes[volume] + with vol.lock: + if self.volumes[volume] is vol: + yield vol + break + + def attach(self, client, volume_id, host, mode=None): + """Ensure a volume is available for an attachment and create an + attachment + + :param client: Cinderclient object + :param volume_id: ID of the volume to attach + :param host: The host the volume will be attached to + :param mode: The attachment mode + """ + + LOG.debug('_AttachmentState.attach(volume_id=%(volume_id)s, ' + 'host=%(host)s, mode=%(mode)s)', + {'volume_id': volume_id, 'host': host, 'mode': mode}) + with self._get_locked(volume_id) as vol_attachment: + + try: + attachment = self.volume_api.attachment_create( + client, volume_id, mode=mode) + except Exception: + LOG.exception(_LE('Error attaching volume %(volume_id)s', + {'volume_id': volume_id})) + del self.volumes[volume_id] + raise + + vol_attachment.add_attachment(attachment['id'], host) + + LOG.debug('_AttachmentState.attach for volume_id=%(volume_id)s ' + 'and attachment_id=%(attachment_id)s completed successfully', + {'volume_id': volume_id, 'attachment_id': attachment['id']}) + return attachment + + def detach(self, client, attachment_id, volume_id, host, conn, + connection_info, device): + """Delete the attachment no longer in use, and disconnect volume + if necessary. + + :param client: Cinderclient object + :param attachment_id: ID of the attachment between volume and host + :param volume_id: ID of the volume to attach + :param host: The host the volume was attached to + :param conn: connector object + :param connection_info: connection information of the volume we are + detaching + :device: device used to write image + + """ + LOG.debug('_AttachmentState.detach(vol_id=%(volume_id)s, ' + 'attachment_id=%(attachment_id)s)', + {'volume_id': volume_id, 'attachment_id': attachment_id}) + with self._get_locked(volume_id) as vol_attachment: + try: + vol_attachment.remove_attachment(attachment_id, host) + except KeyError: + LOG.warning(_LW("Request to remove attachment " + "(%(volume_id)s, %(host)s) but we " + "don't think it's in use."), + {'volume_id': volume_id, 'host': host}) + + if not vol_attachment.in_use(): + conn.disconnect_volume(connection_info, device) + del self.volumes[volume_id] + self.volume_api.attachment_delete(client, attachment_id) + + LOG.debug('_AttachmentState.detach for volume %(volume_id)s ' + 'and attachment_id=%(attachment_id)s completed ' + 'successfully', + {'volume_id': volume_id, + 'attachment_id': attachment_id}) + + +__manager__ = _AttachmentStateManager(HOST) + + +def attach(client, volume_id, host, mode=None): + """A convenience wrapper around _AttachmentState.attach()""" + + with __manager__.get_state() as attach_state: + attachment = attach_state.attach(client, volume_id, host, mode=mode) + return attachment + + +def detach(client, attachment_id, volume_id, host, conn, connection_info, + device): + """A convenience wrapper around _AttachmentState.detach()""" + + with __manager__.get_state() as attach_state: + attach_state.detach(client, attachment_id, volume_id, host, conn, + connection_info, device) diff --git a/glance_store/common/cinder_utils.py b/glance_store/common/cinder_utils.py index d66b783..b3739a8 100644 --- a/glance_store/common/cinder_utils.py +++ b/glance_store/common/cinder_utils.py @@ -47,6 +47,12 @@ def _retry_on_internal_server_error(e): return False +def _retry_on_bad_request(e): + if isinstance(e, cinder_exception.BadRequest): + return True + return False + + class API(object): """API for interacting with the cinder.""" @@ -64,6 +70,8 @@ class API(object): def delete(self, client, volume_id): client.volumes.delete(volume_id) + @retrying.retry(stop_max_attempt_number=5, + retry_on_exception=_retry_on_bad_request) @handle_exceptions def attachment_create(self, client, volume_id, connector=None, mountpoint=None, mode=None): @@ -95,11 +103,18 @@ class API(object): return attachment_ref except cinder_exception.ClientException as ex: with excutils.save_and_reraise_exception(): - LOG.error(_LE('Create attachment failed for volume ' - '%(volume_id)s. Error: %(msg)s Code: %(code)s'), - {'volume_id': volume_id, - 'msg': str(ex), - 'code': getattr(ex, 'code', None)}) + # While handling simultaneous requests, the volume can be + # in different states and we retry on attachment_create + # until the volume reaches a valid state for attachment. + # Hence, it is better to not log 400 cases as no action + # from users is needed in this case + if getattr(ex, 'code', None) != 400: + LOG.error(_LE('Create attachment failed for volume ' + '%(volume_id)s. Error: %(msg)s ' + 'Code: %(code)s'), + {'volume_id': volume_id, + 'msg': str(ex), + 'code': getattr(ex, 'code', None)}) @handle_exceptions def attachment_get(self, client, attachment_id): diff --git a/glance_store/tests/unit/common/test_attachment_state_manager.py b/glance_store/tests/unit/common/test_attachment_state_manager.py new file mode 100644 index 0000000..f019c9b --- /dev/null +++ b/glance_store/tests/unit/common/test_attachment_state_manager.py @@ -0,0 +1,108 @@ +# Copyright 2021 RedHat Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from unittest import mock + +from oslo_config import cfg +from oslotest import base + +from glance_store.common import attachment_state_manager as attach_manager +from glance_store.common import cinder_utils +from glance_store import exceptions + +CONF = cfg.CONF + + +class AttachmentStateManagerTestCase(base.BaseTestCase): + + class FakeAttachmentState: + def __init__(self): + self.attachments = {mock.sentinel.attachments} + + def setUp(self): + super(AttachmentStateManagerTestCase, self).setUp() + self.__manager__ = attach_manager.__manager__ + + def get_state(self): + with self.__manager__.get_state() as state: + return state + + def test_get_state_host_not_initialized(self): + self.__manager__.state = None + self.assertRaises(exceptions.HostNotInitialized, + self.get_state) + + def test_get_state(self): + self.__manager__.state = self.FakeAttachmentState() + state = self.get_state() + self.assertEqual({mock.sentinel.attachments}, state.attachments) + + +class AttachmentStateTestCase(base.BaseTestCase): + + def setUp(self): + super(AttachmentStateTestCase, self).setUp() + self.attachments = set() + self.m = attach_manager._AttachmentState() + self.attach_call_1 = [mock.sentinel.client, mock.sentinel.volume_id] + self.attach_call_2 = {'mode': mock.sentinel.mode} + self.disconnect_vol_call = [mock.sentinel.connection_info, + mock.sentinel.device] + self.detach_call = [mock.sentinel.client, mock.sentinel.attachment_id] + self.attachment_dict = {'id': mock.sentinel.attachment_id} + + def _sentinel_attach(self): + attachment_id = self.m.attach( + mock.sentinel.client, mock.sentinel.volume_id, + mock.sentinel.host, mode=mock.sentinel.mode) + return attachment_id + + def _sentinel_detach(self, conn): + self.m.detach(mock.sentinel.client, mock.sentinel.attachment_id, + mock.sentinel.volume_id, mock.sentinel.host, + conn, mock.sentinel.connection_info, + mock.sentinel.device) + + @mock.patch.object(cinder_utils.API, 'attachment_create') + def test_attach(self, mock_attach_create): + mock_attach_create.return_value = self.attachment_dict + attachment = self._sentinel_attach() + mock_attach_create.assert_called_once_with( + *self.attach_call_1, **self.attach_call_2) + self.assertEqual(mock.sentinel.attachment_id, attachment['id']) + + @mock.patch.object(cinder_utils.API, 'attachment_delete') + def test_detach_without_attach(self, mock_attach_delete): + ex = exceptions.BackendException + conn = mock.MagicMock() + mock_attach_delete.side_effect = ex() + self.assertRaises(ex, self._sentinel_detach, conn) + conn.disconnect_volume.assert_called_once_with( + *self.disconnect_vol_call) + + @mock.patch.object(cinder_utils.API, 'attachment_create') + @mock.patch.object(cinder_utils.API, 'attachment_delete') + def test_detach_with_attach(self, mock_attach_delete, mock_attach_create): + conn = mock.MagicMock() + mock_attach_create.return_value = self.attachment_dict + attachment = self._sentinel_attach() + self._sentinel_detach(conn) + mock_attach_create.assert_called_once_with( + *self.attach_call_1, **self.attach_call_2) + self.assertEqual(mock.sentinel.attachment_id, attachment['id']) + conn.disconnect_volume.assert_called_once_with( + *self.disconnect_vol_call) + mock_attach_delete.assert_called_once_with( + *self.detach_call) diff --git a/glance_store/tests/unit/common/test_cinder_utils.py b/glance_store/tests/unit/common/test_cinder_utils.py index 2acfaac..2f68afb 100644 --- a/glance_store/tests/unit/common/test_cinder_utils.py +++ b/glance_store/tests/unit/common/test_cinder_utils.py @@ -74,6 +74,27 @@ class CinderUtilsTestCase(base.BaseTestCase): self.volume_api.attachment_create, self.fake_client, self.fake_vol_id) + def test_attachment_create_retries(self): + + fake_attach_id = 'fake-attach-id' + # Make create fail two times and succeed on the third attempt. + self.fake_client.attachments.create.side_effect = [ + cinder_exception.BadRequest(400), + cinder_exception.BadRequest(400), + fake_attach_id] + + # Make sure we get a clean result. + fake_attachment_id = self.volume_api.attachment_create( + self.fake_client, self.fake_vol_id) + + self.assertEqual(fake_attach_id, fake_attachment_id) + # Assert that we called attachment create three times due to the retry + # decorator. + self.fake_client.attachments.create.assert_has_calls([ + mock.call(self.fake_vol_id, None, mode=None), + mock.call(self.fake_vol_id, None, mode=None), + mock.call(self.fake_vol_id, None, mode=None)]) + def test_attachment_get(self): self.volume_api.attachment_get(self.fake_client, self.fake_attach_id) self.fake_client.attachments.show.assert_called_once_with( diff --git a/glance_store/tests/unit/test_cinder_store.py b/glance_store/tests/unit/test_cinder_store.py index ad25486..437cfa0 100644 --- a/glance_store/tests/unit/test_cinder_store.py +++ b/glance_store/tests/unit/test_cinder_store.py @@ -31,6 +31,7 @@ from oslo_concurrency import processutils from oslo_utils.secretutils import md5 from oslo_utils import units +from glance_store.common import attachment_state_manager from glance_store.common import cinder_utils from glance_store import exceptions from glance_store import location @@ -152,9 +153,11 @@ class TestCinderStore(base.StoreBaseTest, def _test_open_cinder_volume(self, open_mode, attach_mode, error, multipath_supported=False, enforce_multipath=False, - encrypted_nfs=False, qcow2_vol=False): + encrypted_nfs=False, qcow2_vol=False, + multiattach=False): self.config(cinder_mount_point_base=None) - fake_volume = mock.MagicMock(id=str(uuid.uuid4()), status='available') + fake_volume = mock.MagicMock(id=str(uuid.uuid4()), status='available', + multiattach=multiattach) fake_volume.manager.get.return_value = fake_volume fake_volumes = FakeObject(get=lambda id: fake_volume) fake_attachment_id = str(uuid.uuid4()) @@ -178,10 +181,20 @@ class TestCinderStore(base.StoreBaseTest, yield def do_open(): - with self.store._open_cinder_volume( - fake_client, fake_volume, open_mode): - if error: - raise error + if multiattach: + with mock.patch.object( + attachment_state_manager._AttachmentStateManager, + 'get_state') as mock_get_state: + mock_get_state.return_value.__enter__.return_value = ( + attachment_state_manager._AttachmentState()) + with self.store._open_cinder_volume( + fake_client, fake_volume, open_mode): + pass + else: + with self.store._open_cinder_volume( + fake_client, fake_volume, open_mode): + if error: + raise error def fake_factory(protocol, root_helper, **kwargs): return fake_connector @@ -298,6 +311,9 @@ class TestCinderStore(base.StoreBaseTest, def test_open_cinder_volume_nfs_qcow2_volume(self): self._test_open_cinder_volume('rb', 'ro', None, qcow2_vol=True) + def test_open_cinder_volume_multiattach_volume(self): + self._test_open_cinder_volume('rb', 'ro', None, multiattach=True) + def test_cinder_configure_add(self): self.assertRaises(exceptions.BadStoreConfiguration, self.store._check_context, None) diff --git a/glance_store/tests/unit/test_multistore_cinder.py b/glance_store/tests/unit/test_multistore_cinder.py index ebe69ee..167b379 100644 --- a/glance_store/tests/unit/test_multistore_cinder.py +++ b/glance_store/tests/unit/test_multistore_cinder.py @@ -33,6 +33,7 @@ from oslo_utils.secretutils import md5 from oslo_utils import units import glance_store as store +from glance_store.common import attachment_state_manager from glance_store.common import cinder_utils from glance_store import exceptions from glance_store import location @@ -184,9 +185,11 @@ class TestMultiCinderStore(base.MultiStoreBaseTest, def _test_open_cinder_volume(self, open_mode, attach_mode, error, multipath_supported=False, enforce_multipath=False, - encrypted_nfs=False, qcow2_vol=False): + encrypted_nfs=False, qcow2_vol=False, + multiattach=False): self.config(cinder_mount_point_base=None, group='cinder1') - fake_volume = mock.MagicMock(id=str(uuid.uuid4()), status='available') + fake_volume = mock.MagicMock(id=str(uuid.uuid4()), status='available', + multiattach=multiattach) fake_volume.manager.get.return_value = fake_volume fake_attachment_id = str(uuid.uuid4()) fake_attachment_create = {'id': fake_attachment_id} @@ -210,10 +213,20 @@ class TestMultiCinderStore(base.MultiStoreBaseTest, yield def do_open(): - with self.store._open_cinder_volume( - fake_client, fake_volume, open_mode): - if error: - raise error + if multiattach: + with mock.patch.object( + attachment_state_manager._AttachmentStateManager, + 'get_state') as mock_get_state: + mock_get_state.return_value.__enter__.return_value = ( + attachment_state_manager._AttachmentState()) + with self.store._open_cinder_volume( + fake_client, fake_volume, open_mode): + pass + else: + with self.store._open_cinder_volume( + fake_client, fake_volume, open_mode): + if error: + raise error def fake_factory(protocol, root_helper, **kwargs): return fake_connector @@ -332,6 +345,9 @@ class TestMultiCinderStore(base.MultiStoreBaseTest, def test_open_cinder_volume_nfs_qcow2_volume(self): self._test_open_cinder_volume('rb', 'ro', None, qcow2_vol=True) + def test_open_cinder_volume_multiattach_volume(self): + self._test_open_cinder_volume('rb', 'ro', None, multiattach=True) + def test_cinder_check_context(self): self.assertRaises(exceptions.BadStoreConfiguration, self.store._check_context, None) diff --git a/releasenotes/notes/multiattach-volume-handling-1a8446a64463f2cf.yaml b/releasenotes/notes/multiattach-volume-handling-1a8446a64463f2cf.yaml new file mode 100644 index 0000000..09c01a9 --- /dev/null +++ b/releasenotes/notes/multiattach-volume-handling-1a8446a64463f2cf.yaml @@ -0,0 +1,12 @@ +--- +prelude: > + This release adds support for handling cinder's multiattach volumes + in glance cinder store. +features: + - | + Glance cinder store now supports handling of multiattach volumes. +fixes: + - | + `Bug #1904546 <https://bugs.launchpad.net/cinder/+bug/1904546>`_: + Fixed creating multiple instances/volumes from image if multiattach + volumes are used.
\ No newline at end of file |