From f8e80496dc5cd3aa04f15a37d811de08c837b66b Mon Sep 17 00:00:00 2001 From: Brian Rosmaita Date: Tue, 18 Apr 2023 11:22:27 -0400 Subject: [PATCH] Add force to os-brick disconnect In order to be sure that devices are being removed from the host, we should be using the 'force' parameter with os-brick's disconnect_volume() method. Closes-bug: #2004555 Change-Id: I63d09ad9ef465bc154c85a9ea125449c039d1b90 (cherry picked from commit ed85e00ccfd8b103778d43dc1583ae92ae678d09) Conflicts: glance_store/_drivers/cinder/base.py glance_store/tests/unit/cinder/test_base.py (cherry picked from commit 0b337d575babdb1e0532d78b729c1cbd678c6b1a) Conflicts: glance_store/tests/unit/test_cinder_base.py --- glance_store/_drivers/cinder.py | 5 ++++- glance_store/common/attachment_state_manager.py | 3 ++- .../unit/common/test_attachment_state_manager.py | 4 ++-- glance_store/tests/unit/test_cinder_store.py | 2 +- glance_store/tests/unit/test_multistore_cinder.py | 2 +- releasenotes/notes/bug-2004555-4fd67fce86c07461.yaml | 11 +++++++++++ 6 files changed, 21 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/bug-2004555-4fd67fce86c07461.yaml diff --git a/glance_store/_drivers/cinder.py b/glance_store/_drivers/cinder.py index 8a81543..4817ba9 100644 --- a/glance_store/_drivers/cinder.py +++ b/glance_store/_drivers/cinder.py @@ -819,7 +819,10 @@ class Store(glance_store.driver.Store): client, attachment.id, volume_id, host, conn, connection_info, device) else: - conn.disconnect_volume(connection_info, device) + # Bug #2004555: use force so there aren't any + # leftovers + conn.disconnect_volume(connection_info, device, + force=True) except Exception: LOG.exception(_LE('Failed to disconnect volume ' '%(volume_id)s.'), diff --git a/glance_store/common/attachment_state_manager.py b/glance_store/common/attachment_state_manager.py index 984fcb8..948ebd1 100644 --- a/glance_store/common/attachment_state_manager.py +++ b/glance_store/common/attachment_state_manager.py @@ -230,7 +230,8 @@ class _AttachmentState(object): {'volume_id': volume_id, 'host': host}) if not vol_attachment.in_use(): - conn.disconnect_volume(connection_info, device) + # Bug #2004555: use force so there aren't any leftovers + conn.disconnect_volume(connection_info, device, force=True) del self.volumes[volume_id] self.volume_api.attachment_delete(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 index d8c5189..4d1c26a 100644 --- a/glance_store/tests/unit/common/test_attachment_state_manager.py +++ b/glance_store/tests/unit/common/test_attachment_state_manager.py @@ -91,7 +91,7 @@ class AttachmentStateTestCase(base.BaseTestCase): mock_attach_delete.side_effect = ex() self.assertRaises(ex, self._sentinel_detach, conn) conn.disconnect_volume.assert_called_once_with( - *self.disconnect_vol_call) + *self.disconnect_vol_call, force=True) @mock.patch.object(cinder_utils.API, 'attachment_create') @mock.patch.object(cinder_utils.API, 'attachment_delete') @@ -104,7 +104,7 @@ class AttachmentStateTestCase(base.BaseTestCase): *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) + *self.disconnect_vol_call, force=True) mock_attach_delete.assert_called_once_with( *self.detach_call) diff --git a/glance_store/tests/unit/test_cinder_store.py b/glance_store/tests/unit/test_cinder_store.py index d59cf07..fe738ca 100644 --- a/glance_store/tests/unit/test_cinder_store.py +++ b/glance_store/tests/unit/test_cinder_store.py @@ -265,7 +265,7 @@ class TestCinderStore(base.StoreBaseTest, fake_connector.connect_volume.assert_called_once_with( mock.ANY) fake_connector.disconnect_volume.assert_called_once_with( - mock.ANY, fake_devinfo) + mock.ANY, fake_devinfo, force=True) fake_conn_obj.assert_called_once_with( mock.ANY, root_helper, conn=mock.ANY, use_multipath=multipath_supported) diff --git a/glance_store/tests/unit/test_multistore_cinder.py b/glance_store/tests/unit/test_multistore_cinder.py index 66770c7..c8ee25e 100644 --- a/glance_store/tests/unit/test_multistore_cinder.py +++ b/glance_store/tests/unit/test_multistore_cinder.py @@ -298,7 +298,7 @@ class TestMultiCinderStore(base.MultiStoreBaseTest, fake_connector.connect_volume.assert_called_once_with( mock.ANY) fake_connector.disconnect_volume.assert_called_once_with( - mock.ANY, fake_devinfo) + mock.ANY, fake_devinfo, force=True) fake_conn_obj.assert_called_once_with( mock.ANY, root_helper, conn=mock.ANY, use_multipath=multipath_supported) diff --git a/releasenotes/notes/bug-2004555-4fd67fce86c07461.yaml b/releasenotes/notes/bug-2004555-4fd67fce86c07461.yaml new file mode 100644 index 0000000..8d982c6 --- /dev/null +++ b/releasenotes/notes/bug-2004555-4fd67fce86c07461.yaml @@ -0,0 +1,11 @@ +security: + - | + Cinder glance_store driver: in order to avoid a situation where a + leftover device could be mapped to a different volume than the one + intended, the cinder glance_store driver now instructs the os-brick + library to force detach volumes, which ensures that devices are + removed from the host. + + See `Bug #2004555 + `_ for more + information about this issue. -- 2.38.1