diff --git a/glance_store/_drivers/cinder/store.py b/glance_store/_drivers/cinder/store.py index ad0f1b16..7a399824 100644 --- a/glance_store/_drivers/cinder/store.py +++ b/glance_store/_drivers/cinder/store.py @@ -555,10 +555,29 @@ class Store(glance_store.driver.Store): If above both conditions doesn't meet, it returns false. """ try: - cinder_client = self.get_cinderclient(context=context, - legacy_update=True) + # We will use either the service credentials defined in + # config file or the user context credentials + cinder_client = self.get_cinderclient(context=context) cinder_volume_type = self.store_conf.cinder_volume_type - volume = cinder_client.volumes.get(volume_id) + # Here we are assuming that the volume is stored in the + # service project or context user's project else this + # will return NotFound exception. + # Ideally we should be using service user's credentials + # defined in the config and the volume should be stored + # in the service (internal) project else we are opening the + # image-volume to modification by users which might lead + # to corruption of image. + try: + volume = cinder_client.volumes.get(volume_id) + except cinder_exception.NotFound: + reason = (_LW("Image-Volume %s not found. If you have " + "upgraded your environment from single store " + "to multi store, transfer all your " + "Image-Volumes from user projects to service " + "project." + % volume_id)) + LOG.warning(reason) + return False if cinder_volume_type and volume.volume_type == cinder_volume_type: return True elif not cinder_volume_type: @@ -584,17 +603,8 @@ class Store(glance_store.driver.Store): for key in ['user_name', 'password', 'project_name', 'auth_address']]) - def get_cinderclient(self, context=None, legacy_update=False, - version='3.0'): - # NOTE: For legacy image update from single store to multiple - # stores we need to use admin context rather than user provided - # credentials - if legacy_update: - user_overriden = False - context = context.elevated() - else: - user_overriden = self.is_user_overriden() - + def get_cinderclient(self, context=None, version='3.0'): + user_overriden = self.is_user_overriden() session = get_cinder_session(self.store_conf) if user_overriden: diff --git a/glance_store/tests/unit/cinder/test_multistore_cinder.py b/glance_store/tests/unit/cinder/test_multistore_cinder.py index f40965bf..e637a28c 100644 --- a/glance_store/tests/unit/cinder/test_multistore_cinder.py +++ b/glance_store/tests/unit/cinder/test_multistore_cinder.py @@ -112,10 +112,12 @@ class TestMultiCinderStore(base.MultiStoreBaseTest, self._test_get_cinderclient_with_ca_certificates(group='cinder1') def test_get_cinderclient_legacy_update(self): - cc = self.store.get_cinderclient(self.fake_admin_context, - legacy_update=True) - self.assertEqual('admin_token', cc.client.auth.token) - self.assertEqual('http://foo/public_url', cc.client.auth.endpoint) + fake_endpoint = 'http://cinder.openstack.example.com/v2/fake_project' + self.config(cinder_endpoint_template=fake_endpoint, group='cinder1') + cc = self.store.get_cinderclient(self.context) + self.assertEqual(self.context.auth_token, + cc.client.auth.token) + self.assertEqual(fake_endpoint, cc.client.auth.endpoint) def test_open_cinder_volume_multipath_enabled(self): self.config(cinder_use_multipath=True, group='cinder1') @@ -221,6 +223,19 @@ class TestMultiCinderStore(base.MultiStoreBaseTest, type_match = self.store.is_image_associated_with_store( self.context, fake_vol_id) self.assertFalse(type_match) + # When the Image-Volume is not found + mocked_cc.return_value.volumes.get = mock.MagicMock( + side_effect=cinder.cinder_exception.NotFound(code=404)) + with mock.patch.object(cinder, 'LOG') as mock_log: + type_match = self.store.is_image_associated_with_store( + self.context, fake_vol_id) + mock_log.warning.assert_called_with( + "Image-Volume %s not found. If you have " + "upgraded your environment from single store " + "to multi store, transfer all your " + "Image-Volumes from user projects to service " + "project." % fake_vol_id) + self.assertFalse(type_match) def test_cinder_get(self): self._test_cinder_get(is_multi_store=True) diff --git a/releasenotes/notes/fix-legacy-image-update-49a149ec267dccb6.yaml b/releasenotes/notes/fix-legacy-image-update-49a149ec267dccb6.yaml new file mode 100644 index 00000000..e9372006 --- /dev/null +++ b/releasenotes/notes/fix-legacy-image-update-49a149ec267dccb6.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + `Bug #2056179 `_: + Cinder Store: Fix issue when updating legacy image location. + Previously we only used the user context's credentials to make request + to cinder which we have now updated to use the service credentials + configured in the config file else use the user context's credentials.