From d052ce57b5cfc66a4b3b8030a14db3d46c312990 Mon Sep 17 00:00:00 2001 From: Abhishek Kekane Date: Thu, 20 Feb 2020 07:21:14 +0000 Subject: [PATCH] Image upload fails if cinder multipath is enabled Image upload is failing with NoFibreChannelVolumeDeviceFound error after configuring cinder backend (HP3Par FC storage) for the glance service. The issue here is that usually cinder calls os_brick with multipath information but in this case cinder driver of glance is doing the call (without passing any multipath information). with hard-coded' False to let os_brick library know that multipath is not configured. In order to fix this, added two new boolean config option for cinder driver default to False; 'cinder_use_multipath' 'cinder_enforce_multipath' Change-Id: I07064f3cb1a33ac4ac7e4b572f8e1b5c688bd9a3 Closes-Bug: #1863983 (cherry picked from commit ae73287c7b4db8d5894f72d8a5b1b89b42697545) --- glance_store/_drivers/cinder.py | 41 ++++++++++++++++- glance_store/tests/unit/test_cinder_store.py | 44 ++++++++++++------ .../tests/unit/test_multistore_cinder.py | 45 +++++++++++++------ glance_store/tests/unit/test_opts.py | 2 + 4 files changed, 104 insertions(+), 28 deletions(-) diff --git a/glance_store/_drivers/cinder.py b/glance_store/_drivers/cinder.py index b6bd2e17..e8572b6b 100644 --- a/glance_store/_drivers/cinder.py +++ b/glance_store/_drivers/cinder.py @@ -304,6 +304,34 @@ Possible values: Related options: * None +"""), + cfg.BoolOpt('cinder_enforce_multipath', + default=False, + help=""" +If this is set to True, attachment of volumes for image transfer will +be aborted when multipathd is not running. Otherwise, it will fallback +to single path. + +Possible values: + * True or False + +Related options: + * cinder_use_multipath + +"""), + cfg.BoolOpt('cinder_use_multipath', + default=False, + help=""" +Flag to identify mutipath is supported or not in the deployment. + +Set it to False if multipath is not supported. + +Possible values: + * True or False + +Related options: + * cinder_enforce_multipath + """), ] @@ -498,8 +526,17 @@ class Store(glance_store.driver.Store): root_helper = get_root_helper(backend=self.backend_group) priv_context.init(root_helper=shlex.split(root_helper)) host = socket.gethostname() - properties = connector.get_connector_properties(root_helper, host, - False, False) + if self.backend_group: + use_multipath = getattr( + self.conf, self.backend_group).cinder_use_multipath + enforce_multipath = getattr( + self.conf, self.backend_group).cinder_enforce_multipath + else: + use_multipath = self.conf.glance_store.cinder_use_multipath + enforce_multipath = self.conf.glance_store.cinder_enforce_multipath + + properties = connector.get_connector_properties( + root_helper, host, use_multipath, enforce_multipath) try: volume.reserve(volume) diff --git a/glance_store/tests/unit/test_cinder_store.py b/glance_store/tests/unit/test_cinder_store.py index 49f86a7a..8850cef7 100644 --- a/glance_store/tests/unit/test_cinder_store.py +++ b/glance_store/tests/unit/test_cinder_store.py @@ -145,7 +145,9 @@ class TestCinderStore(base.StoreBaseTest, volume_available, 'available', 'in-use') fake_manager.get.assert_called_with('fake-id') - def _test_open_cinder_volume(self, open_mode, attach_mode, error): + def _test_open_cinder_volume(self, open_mode, attach_mode, error, + multipath_supported=False, + enforce_multipath=False): fake_volume = mock.MagicMock(id=str(uuid.uuid4()), status='available') fake_volumes = FakeObject(get=lambda id: fake_volume, detach=mock.Mock()) @@ -179,22 +181,26 @@ class TestCinderStore(base.StoreBaseTest, side_effect=fake_chown), \ mock.patch.object(cinder, 'get_root_helper', return_value=root_helper), \ - mock.patch.object(connector, 'get_connector_properties'), \ mock.patch.object(connector.InitiatorConnector, 'factory', side_effect=fake_factory): - if error: - self.assertRaises(error, do_open) - else: - do_open() + with mock.patch.object(connector, + 'get_connector_properties') as mock_conn: + if error: + self.assertRaises(error, do_open) + else: + do_open() - fake_connector.connect_volume.assert_called_once_with(mock.ANY) - fake_connector.disconnect_volume.assert_called_once_with( - mock.ANY, fake_devinfo) - fake_volume.attach.assert_called_once_with( - None, 'glance_store', attach_mode, - host_name=socket.gethostname()) - fake_volumes.detach.assert_called_once_with(fake_volume) + mock_conn.assert_called_once_with( + root_helper, socket.gethostname(), multipath_supported, + enforce_multipath) + fake_connector.connect_volume.assert_called_once_with(mock.ANY) + fake_connector.disconnect_volume.assert_called_once_with( + mock.ANY, fake_devinfo) + fake_volume.attach.assert_called_once_with( + None, 'glance_store', attach_mode, + host_name=socket.gethostname()) + fake_volumes.detach.assert_called_once_with(fake_volume) def test_open_cinder_volume_rw(self): self._test_open_cinder_volume('wb', 'rw', None) @@ -205,6 +211,18 @@ class TestCinderStore(base.StoreBaseTest, def test_open_cinder_volume_error(self): self._test_open_cinder_volume('wb', 'rw', IOError) + def test_open_cinder_volume_multipath_supported(self): + self.config(cinder_use_multipath=True) + self._test_open_cinder_volume('wb', 'rw', None, + multipath_supported=True) + + def test_open_cinder_volume_enforce_multipath(self): + self.config(cinder_use_multipath=True) + self.config(cinder_enforce_multipath=True) + self._test_open_cinder_volume('wb', 'rw', None, + multipath_supported=True, + enforce_multipath=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 acb9a29b..12ae7133 100644 --- a/glance_store/tests/unit/test_multistore_cinder.py +++ b/glance_store/tests/unit/test_multistore_cinder.py @@ -165,7 +165,9 @@ class TestMultiCinderStore(base.MultiStoreBaseTest, volume_available, 'available', 'in-use') fake_manager.get.assert_called_with('fake-id') - def _test_open_cinder_volume(self, open_mode, attach_mode, error): + def _test_open_cinder_volume(self, open_mode, attach_mode, error, + multipath_supported=False, + enforce_multipath=False): fake_volume = mock.MagicMock(id=str(uuid.uuid4()), status='available') fake_volumes = FakeObject(get=lambda id: fake_volume, detach=mock.Mock()) @@ -199,22 +201,26 @@ class TestMultiCinderStore(base.MultiStoreBaseTest, side_effect=fake_chown), \ mock.patch.object(cinder, 'get_root_helper', return_value=root_helper), \ - mock.patch.object(connector, 'get_connector_properties'), \ mock.patch.object(connector.InitiatorConnector, 'factory', side_effect=fake_factory): - if error: - self.assertRaises(error, do_open) - else: - do_open() + with mock.patch.object(connector, + 'get_connector_properties') as mock_conn: + if error: + self.assertRaises(error, do_open) + else: + do_open() - fake_connector.connect_volume.assert_called_once_with(mock.ANY) - fake_connector.disconnect_volume.assert_called_once_with( - mock.ANY, fake_devinfo) - fake_volume.attach.assert_called_once_with( - None, 'glance_store', attach_mode, - host_name=socket.gethostname()) - fake_volumes.detach.assert_called_once_with(fake_volume) + mock_conn.assert_called_once_with( + root_helper, socket.gethostname(), multipath_supported, + enforce_multipath) + fake_connector.connect_volume.assert_called_once_with(mock.ANY) + fake_connector.disconnect_volume.assert_called_once_with( + mock.ANY, fake_devinfo) + fake_volume.attach.assert_called_once_with( + None, 'glance_store', attach_mode, + host_name=socket.gethostname()) + fake_volumes.detach.assert_called_once_with(fake_volume) def test_open_cinder_volume_rw(self): self._test_open_cinder_volume('wb', 'rw', None) @@ -225,6 +231,19 @@ class TestMultiCinderStore(base.MultiStoreBaseTest, def test_open_cinder_volume_error(self): self._test_open_cinder_volume('wb', 'rw', IOError) + def test_open_cinder_volume_multipath_disabled(self): + self.config(cinder_use_multipath=False, group='cinder1') + self._test_open_cinder_volume('wb', 'rw', None, + multipath_supported=False) + + def test_open_cinder_volume_enforce_multipath(self): + + self.config(cinder_use_multipath=True, group='cinder1') + self.config(cinder_enforce_multipath=True, group='cinder1') + self._test_open_cinder_volume('wb', 'rw', None, + multipath_supported=True, + enforce_multipath=True) + def test_cinder_configure_add(self): self.assertRaises(exceptions.BadStoreConfiguration, self.store._check_context, None) diff --git a/glance_store/tests/unit/test_opts.py b/glance_store/tests/unit/test_opts.py index 9536f393..8243ec90 100644 --- a/glance_store/tests/unit/test_opts.py +++ b/glance_store/tests/unit/test_opts.py @@ -82,6 +82,8 @@ class OptsTestCase(base.StoreBaseTest): 'cinder_store_password', 'cinder_store_project_name', 'cinder_volume_type', + 'cinder_use_multipath', + 'cinder_enforce_multipath', 'default_swift_reference', 'https_insecure', 'filesystem_store_chunk_size',