From fd222f31283db66a640a1e0802ccc7e386f7a6a4 Mon Sep 17 00:00:00 2001 From: Abhishek Kekane Date: Fri, 23 Jun 2023 09:53:07 +0000 Subject: [PATCH] Sort locations based on store weight Related to blueprint store-weight Change-Id: I2383a476cb7e79c7efecdf33203cff0b50ef3bbb --- api-ref/source/v2/discovery-parameters.yaml | 3 + .../samples/stores-list-detail-response.json | 4 + glance/api/v2/discovery.py | 1 + glance/api/v2/images.py | 3 +- glance/common/utils.py | 25 +++- glance/db/__init__.py | 4 +- glance/tests/functional/v2/test_discovery.py | 3 + glance/tests/functional/v2/test_images.py | 35 +++++ glance/tests/unit/common/test_utils.py | 133 ++++++++++++++++++ glance/tests/unit/v2/test_discovery_stores.py | 15 ++ .../notes/store-weight-3ed3ee612579bc25.yaml | 7 + 11 files changed, 228 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/store-weight-3ed3ee612579bc25.yaml diff --git a/api-ref/source/v2/discovery-parameters.yaml b/api-ref/source/v2/discovery-parameters.yaml index 205d6378e0..a979c2a7e4 100644 --- a/api-ref/source/v2/discovery-parameters.yaml +++ b/api-ref/source/v2/discovery-parameters.yaml @@ -37,6 +37,9 @@ stores-detail: for more information.) ``read-only`` (optional) Included only when the store is read only. + ``weight`` (default 0) + Contains weight (positive integer) to sort image locations for + preference. ``properties`` Contains store specific properties in: body diff --git a/api-ref/source/v2/samples/stores-list-detail-response.json b/api-ref/source/v2/samples/stores-list-detail-response.json index 50f3065156..4f2db586f5 100644 --- a/api-ref/source/v2/samples/stores-list-detail-response.json +++ b/api-ref/source/v2/samples/stores-list-detail-response.json @@ -5,6 +5,7 @@ "type": "rbd", "description": "More expensive store with data redundancy", "default": true, + "weight": 100, "properties": { "pool": "pool1", "chunk_size": 65536, @@ -15,6 +16,7 @@ "id":"cheap", "type": "file", "description": "Less expensive store for seldom-used images", + "weight": 200, "properties": { "datadir": "fdir", "chunk_size": 65536, @@ -25,6 +27,7 @@ "id":"fast", "type": "cinder", "description": "Reasonably-priced fast store", + "weight": 300, "properties": { "volume_type": "volume1", "use_multipath": false @@ -34,6 +37,7 @@ "id":"slow", "type": "swift", "description": "Entry-level store balancing price and speed", + "weight": 400, "properties": { "container": "container1", "large_object_size": 52428, diff --git a/glance/api/v2/discovery.py b/glance/api/v2/discovery.py index 720dde8096..0a259950e7 100644 --- a/glance/api/v2/discovery.py +++ b/glance/api/v2/discovery.py @@ -149,6 +149,7 @@ class InfoController(object): store['id']) store['properties'] = store_mapper.get(store_type)( store_detail) + store['weight'] = getattr(CONF, store['id']).weight except exception.Forbidden as e: LOG.debug("User not permitted to view details") diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index 95cfe7b9c3..90776a5e1f 100644 --- a/glance/api/v2/images.py +++ b/glance/api/v2/images.py @@ -36,7 +36,6 @@ from glance.api import common from glance.api import policy from glance.api.v2 import policy as api_policy from glance.common import exception -from glance.common import location_strategy from glance.common import store_utils from glance.common import timeutils from glance.common import utils @@ -1610,7 +1609,7 @@ class ResponseSerializer(wsgi.JSONResponseSerializer): locations = _get_image_locations(image) if locations: # Choose best location configured strategy - loc = location_strategy.choose_best_location(locations) + loc = utils.sort_image_locations(locations)[0] image_view['direct_url'] = loc['url'] else: LOG.debug("The 'locations' list of image %s is empty, " diff --git a/glance/common/utils.py b/glance/common/utils.py index ec10dc59a1..feb2c81565 100644 --- a/glance/common/utils.py +++ b/glance/common/utils.py @@ -42,9 +42,10 @@ from oslo_utils import strutils from webob import exc from glance.common import exception +from glance.common import location_strategy from glance.common import timeutils from glance.common import wsgi -from glance.i18n import _, _LE +from glance.i18n import _, _LE, _LW CONF = cfg.CONF @@ -712,3 +713,25 @@ def get_stores_from_request(req, body): for store in stores: glance_store.get_store_from_store_identifier(store) return stores + + +def sort_image_locations(locations): + if not CONF.enabled_backends: + return location_strategy.get_ordered_locations(locations) + + def get_store_weight(location): + store_id = location['metadata'].get('store') + if not store_id: + return 0 + try: + store = glance_store.get_store_from_store_identifier(store_id) + except glance_store.exceptions.UnknownScheme: + msg = (_LW("Unable to find store '%s', returning " + "default weight '0'") % store_id) + LOG.warning(msg) + return 0 + return store.weight if store is not None else 0 + + sorted_locations = sorted(locations, key=get_store_weight, reverse=True) + LOG.debug(('Sorted locations: %s'), sorted_locations) + return sorted_locations diff --git a/glance/db/__init__.py b/glance/db/__init__.py index d1e8c24000..12c04c6f4b 100644 --- a/glance/db/__init__.py +++ b/glance/db/__init__.py @@ -24,7 +24,7 @@ from wsme.rest import json from glance.api.v2.model.metadef_property_type import PropertyType from glance.common import crypt from glance.common import exception -from glance.common import location_strategy +from glance.common import utils as common_utils import glance.domain import glance.domain.proxy from glance.i18n import _ @@ -127,7 +127,7 @@ class ImageRepo(object): min_disk=db_image['min_disk'], min_ram=db_image['min_ram'], protected=db_image['protected'], - locations=location_strategy.get_ordered_locations(locations), + locations=common_utils.sort_image_locations(locations), checksum=db_image['checksum'], os_hash_algo=db_image['os_hash_algo'], os_hash_value=db_image['os_hash_value'], diff --git a/glance/tests/functional/v2/test_discovery.py b/glance/tests/functional/v2/test_discovery.py index 026a9e1ba4..ce633f82f1 100644 --- a/glance/tests/functional/v2/test_discovery.py +++ b/glance/tests/functional/v2/test_discovery.py @@ -131,6 +131,7 @@ class TestDiscovery(functional.SynchronousAPIBase): "id": "store1", "default": "true", "type": "file", + "weight": 0, "properties": { "data_dir": self._store_dir('store1'), "chunk_size": 65536, @@ -140,6 +141,7 @@ class TestDiscovery(functional.SynchronousAPIBase): { "id": "store2", "type": "file", + "weight": 0, "properties": { "data_dir": self._store_dir('store2'), "chunk_size": 65536, @@ -149,6 +151,7 @@ class TestDiscovery(functional.SynchronousAPIBase): { "id": "store3", "type": "file", + "weight": 0, "properties": { "data_dir": self._store_dir('store3'), "chunk_size": 65536, diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py index bbe1a00909..5009e92be2 100644 --- a/glance/tests/functional/v2/test_images.py +++ b/glance/tests/functional/v2/test_images.py @@ -7318,3 +7318,38 @@ class TestKeystoneQuotas(functional.SynchronousAPIBase): # Make sure we can still import. self._create_and_import(stores=['store1']) + + +class TestStoreWeight(functional.SynchronousAPIBase): + def setUp(self): + super(TestStoreWeight, self).setUp() + + def test_store_weight_combinations(self): + self.start_server() + # Import image in all available stores + image_id = self._create_and_import(stores=['store1', 'store2', + 'store3']) + # make sure as weight is default, we will get locations based + # on insertion order + image = self.api_get('/v2/images/%s' % image_id).json + self.assertEqual("store1,store2,store3", image['stores']) + + # give highest weight to store2 then store3 and then store1 + self.config(weight=200, group='store2') + self.config(weight=100, group='store3') + self.config(weight=50, group='store1') + self.start_server() + # make sure as per store weight locations will be sorted + # as store2,store3,store1 + image = self.api_get('/v2/images/%s' % image_id).json + self.assertEqual("store2,store3,store1", image['stores']) + + # give highest weight to store3 then store1 and then store2 + self.config(weight=20, group='store2') + self.config(weight=100, group='store3') + self.config(weight=50, group='store1') + self.start_server() + # make sure as per store weight locations will be sorted + # as store3,store1,store2 + image = self.api_get('/v2/images/%s' % image_id).json + self.assertEqual("store3,store1,store2", image['stores']) diff --git a/glance/tests/unit/common/test_utils.py b/glance/tests/unit/common/test_utils.py index a07b2709d7..88b4880be0 100644 --- a/glance/tests/unit/common/test_utils.py +++ b/glance/tests/unit/common/test_utils.py @@ -155,6 +155,139 @@ class TestCinderStoreUtils(base.MultiStoreClearingUnitTest): class TestUtils(test_utils.BaseTestCase): """Test routines in glance.utils""" + def test_sort_image_locations_multistore_disabled(self): + self.config(enabled_backends=None) + locations = [{ + 'url': 'rbd://aaaaaaaa/images/id', + 'metadata': {'store': 'rbd1'} + }, { + 'url': 'rbd://bbbbbbbb/images/id', + 'metadata': {'store': 'rbd2'} + }, { + 'url': 'rbd://cccccccc/images/id', + 'metadata': {'store': 'rbd3'} + }] + mp = "glance.common.utils.glance_store.get_store_from_store_identifier" + with mock.patch(mp) as mock_get_store: + utils.sort_image_locations(locations) + + # Since multistore is not enabled, it will not sort the locations + self.assertEqual(0, mock_get_store.call_count) + + def test_sort_image_locations(self): + enabled_backends = { + "rbd1": "rbd", + "rbd2": "rbd", + "rbd3": "rbd" + } + self.config(enabled_backends=enabled_backends) + store.register_store_opts(CONF) + self.config(default_backend="rbd1", group="glance_store") + locations = [{ + 'url': 'rbd://aaaaaaaa/images/id', + 'metadata': {'store': 'rbd1'} + }, { + 'url': 'rbd://bbbbbbbb/images/id', + 'metadata': {'store': 'rbd2'} + }, { + 'url': 'rbd://cccccccc/images/id', + 'metadata': {'store': 'rbd3'} + }] + mp = "glance.common.utils.glance_store.get_store_from_store_identifier" + with mock.patch(mp) as mock_get_store: + mock_store = mock_get_store.return_value + mock_store.weight = 100 + utils.sort_image_locations(locations) + + # Since 3 stores are configured, internal method will be called 3 times + self.assertEqual(3, mock_get_store.call_count) + + def test_sort_image_locations_without_metadata(self): + enabled_backends = { + "rbd1": "rbd", + "rbd2": "rbd", + "rbd3": "rbd" + } + self.config(enabled_backends=enabled_backends) + store.register_store_opts(CONF) + self.config(default_backend="rbd1", group="glance_store") + locations = [{ + 'url': 'rbd://aaaaaaaa/images/id', + 'metadata': {} + }, { + 'url': 'rbd://bbbbbbbb/images/id', + 'metadata': {} + }, { + 'url': 'rbd://cccccccc/images/id', + 'metadata': {} + }] + mp = "glance.common.utils.glance_store.get_store_from_store_identifier" + with mock.patch(mp) as mock_get_store: + utils.sort_image_locations(locations) + + # Since 3 stores are configured, without store in metadata the internal + # method will be called 0 times + self.assertEqual(0, mock_get_store.call_count) + + def test_sort_image_locations_with_partial_metadata(self): + enabled_backends = { + "rbd1": "rbd", + "rbd2": "rbd", + "rbd3": "rbd" + } + self.config(enabled_backends=enabled_backends) + store.register_store_opts(CONF) + self.config(default_backend="rbd1", group="glance_store") + locations = [{ + 'url': 'rbd://aaaaaaaa/images/id', + 'metadata': {'store': 'rbd1'} + }, { + 'url': 'rbd://bbbbbbbb/images/id', + 'metadata': {} + }, { + 'url': 'rbd://cccccccc/images/id', + 'metadata': {} + }] + mp = "glance.common.utils.glance_store.get_store_from_store_identifier" + with mock.patch(mp) as mock_get_store: + mock_store = mock_get_store.return_value + mock_store.weight = 100 + utils.sort_image_locations(locations) + + # Since 3 stores are configured, but only one location has + # store in metadata the internal + # method will be called 1 time only + self.assertEqual(1, mock_get_store.call_count) + + def test_sort_image_locations_unknownscheme(self): + enabled_backends = { + "rbd1": "rbd", + "rbd2": "rbd", + "rbd3": "rbd" + } + self.config(enabled_backends=enabled_backends) + store.register_store_opts(CONF) + self.config(default_backend="rbd1", group="glance_store") + locations = [{ + 'url': 'rbd://aaaaaaaa/images/id', + 'metadata': {'store': 'rbd1'} + }, { + 'url': 'rbd://bbbbbbbb/images/id', + 'metadata': {'store': 'rbd2'} + }, { + 'url': 'rbd://cccccccc/images/id', + 'metadata': {'store': 'rbd3'} + }] + mp = "glance.common.utils.glance_store.get_store_from_store_identifier" + with mock.patch(mp) as mock_get_store: + mock_get_store.side_effect = store.UnknownScheme() + sorted_locations = utils.sort_image_locations(locations) + + # Even though UnknownScheme exception is raised, processing continues + self.assertEqual(3, mock_get_store.call_count) + # Since we return 0 weight, original location order should be preserved + self.assertEqual(locations, sorted_locations) + def test_cooperative_reader(self): """Ensure cooperative reader class accesses all bytes of file""" BYTES = 1024 diff --git a/glance/tests/unit/v2/test_discovery_stores.py b/glance/tests/unit/v2/test_discovery_stores.py index 6afdf751ae..4e08a29780 100644 --- a/glance/tests/unit/v2/test_discovery_stores.py +++ b/glance/tests/unit/v2/test_discovery_stores.py @@ -46,6 +46,7 @@ class TestInfoControllers(base.MultiStoreClearingUnitTest): self.assertIn('stores', output) for stores in output['stores']: self.assertIn('id', stores) + self.assertNotIn('weight', stores) self.assertIn(stores['id'], available_stores) def test_get_stores_read_only_store(self): @@ -108,6 +109,20 @@ class TestInfoControllers(base.MultiStoreClearingUnitTest): expected_attribute = store_attributes[store['type']] self.assertEqual(actual_attribute, expected_attribute) + def test_get_stores_detail_with_store_weight(self): + self.config(weight=100, group='fast') + self.config(weight=200, group='cheap') + self.config(weight=300, group='fast-rbd') + self.config(weight=400, group='fast-cinder') + self.config(weight=500, group='reliable') + + req = unit_test_utils.get_fake_request(roles=['admin']) + output = self.controller.get_stores_detail(req) + self.assertEqual(len(CONF.enabled_backends), len(output['stores'])) + self.assertIn('stores', output) + for store in output['stores']: + self.assertIn('weight', store) + def test_get_stores_detail_non_admin(self): req = unit_test_utils.get_fake_request() self.assertRaises(webob.exc.HTTPForbidden, diff --git a/releasenotes/notes/store-weight-3ed3ee612579bc25.yaml b/releasenotes/notes/store-weight-3ed3ee612579bc25.yaml new file mode 100644 index 0000000000..e625564085 --- /dev/null +++ b/releasenotes/notes/store-weight-3ed3ee612579bc25.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + "GET" images API will now sort image locations based on store weight + configured for each store in configuration files. Image will be + downloaded from the store having highest weight configured. For default + weight scenario the locations will remain same as per insertion order.