From 9b9111f81996a22527f9622d0707891e445cbd5f Mon Sep 17 00:00:00 2001 From: Abhishek Kekane Date: Mon, 29 Jan 2024 18:26:10 +0000 Subject: [PATCH] Make `centralized_db` cache driver default Made `centralized_db` cache driver as default driver so that we can test it using tempest jobs in gate. Implements blueprint centralized-cache-db Depends-On: https://review.opendev.org/c/openstack/devstack/+/907110 Change-Id: Id94e93e3ba3fc207b39c7dbff92495805aa0f6f9 --- glance/image_cache/__init__.py | 10 +++--- glance/tests/functional/__init__.py | 7 ++-- glance/tests/functional/v2/test_cache_api.py | 3 ++ glance/tests/functional/v2/test_images.py | 12 +++---- glance/tests/unit/common/test_wsgi.py | 17 +++++++--- glance/tests/unit/common/test_wsgi_app.py | 34 ++++++++++++++++---- glance/tests/unit/test_sqlite_migration.py | 1 + 7 files changed, 60 insertions(+), 24 deletions(-) diff --git a/glance/image_cache/__init__.py b/glance/image_cache/__init__.py index 145932320c..454cf7585b 100644 --- a/glance/image_cache/__init__.py +++ b/glance/image_cache/__init__.py @@ -32,7 +32,7 @@ from glance.i18n import _, _LE, _LI, _LW LOG = logging.getLogger(__name__) image_cache_opts = [ - cfg.StrOpt('image_cache_driver', default='sqlite', + cfg.StrOpt('image_cache_driver', default='centralized_db', choices=('centralized_db', 'sqlite', 'xattr'), ignore_case=True, help=_(""" The driver to use for image cache management. @@ -50,13 +50,13 @@ and prospective) must implement this interface. Currently available drivers are ``sqlite`` and ``xattr``. These drivers primarily differ in the way they store the information about cached images: -* The ``sqlite`` driver uses a sqlite database (which sits on every glance - node locally) to track the usage of cached images. +* The ``centralized_db`` driver uses a central database (which will be common + for all glance nodes) to track the usage of cached images. +* The ``sqlite`` (deprecated) driver uses a sqlite database (which sits on + every glance node locally) to track the usage of cached images. * The ``xattr`` driver uses the extended attributes of files to store this information. It also requires a filesystem that sets ``atime`` on the files when accessed. -* The ``centralized_db`` driver uses a central database (which will be common - for all glance nodes) to track the usage of cached images. Deprecation warning: * As centralized database will now be used for image cache management, the diff --git a/glance/tests/functional/__init__.py b/glance/tests/functional/__init__.py index 7a0d16ee0b..35943df81d 100644 --- a/glance/tests/functional/__init__.py +++ b/glance/tests/functional/__init__.py @@ -830,7 +830,7 @@ class FunctionalTest(test_utils.BaseTestCase): # The clients will try to connect to this address. Let's make sure # we're not using the default '0.0.0.0' self.config(bind_host='127.0.0.1') - + self.config(image_cache_dir=self.test_dir) self.tracecmd = tracecmd_osmap.get(platform.system()) conf_dir = os.path.join(self.test_dir, 'etc') @@ -1630,7 +1630,7 @@ class SynchronousAPIBase(test_utils.BaseTestCase): self.setup_simple_paste() self.setup_stores() - def start_server(self, enable_cache=True): + def start_server(self, enable_cache=True, set_worker_url=True): """Builds and "starts" the API server. Note that this doesn't actually "start" anything like @@ -1643,6 +1643,9 @@ class SynchronousAPIBase(test_utils.BaseTestCase): root_app = 'glance-api-cachemanagement' self.config(image_cache_dir=self._store_dir('cache')) + if set_worker_url: + self.config(worker_self_reference_url='http://workerx') + self.api = config.load_paste_app(root_app, conf_file=self.paste_config) self.config(enforce_new_defaults=True, diff --git a/glance/tests/functional/v2/test_cache_api.py b/glance/tests/functional/v2/test_cache_api.py index 3e2f477d8a..536cb4ec9c 100644 --- a/glance/tests/functional/v2/test_cache_api.py +++ b/glance/tests/functional/v2/test_cache_api.py @@ -34,6 +34,9 @@ class TestImageCache(functional.SynchronousAPIBase): overwrite=True) def start_server(self, enable_cache=True): + # NOTE(abhishekk): Once sqlite driver is removed, fix these tests + # to work with centralized_db driver + self.config(image_cache_driver='sqlite') with mock.patch.object(policy, 'Enforcer') as mock_enf: mock_enf.return_value = self.policy super(TestImageCache, self).start_server(enable_cache=enable_cache) diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py index db05068579..30347413d1 100644 --- a/glance/tests/functional/v2/test_images.py +++ b/glance/tests/functional/v2/test_images.py @@ -6933,7 +6933,7 @@ class TestImportProxy(functional.SynchronousAPIBase): # Stage it on worker1 self.config(worker_self_reference_url='http://worker1') - self.start_server() + self.start_server(set_worker_url=False) image_id = self._create_and_stage() # Make sure we can't see the stage host key @@ -6943,7 +6943,7 @@ class TestImportProxy(functional.SynchronousAPIBase): # Import call goes to worker2 self.config(worker_self_reference_url='http://worker2') - self.start_server() + self.start_server(set_worker_url=False) r = self._import_direct(image_id, ['store1']) # Assert that it was proxied back to worker1 @@ -6966,12 +6966,12 @@ class TestImportProxy(functional.SynchronousAPIBase): # Stage it on worker1 self.config(worker_self_reference_url='http://worker1') - self.start_server() + self.start_server(set_worker_url=False) image_id = self._create_and_stage() # Import call goes to worker2 self.config(worker_self_reference_url='http://worker2') - self.start_server() + self.start_server(set_worker_url=False) r = self._import_direct(image_id, ['store1']) # Make sure we see the relevant details from worker1 @@ -6989,12 +6989,12 @@ class TestImportProxy(functional.SynchronousAPIBase): # Stage it on worker1 self.config(worker_self_reference_url='http://worker1') - self.start_server() + self.start_server(set_worker_url=False) image_id = self._create_and_stage() # Import call goes to worker2 self.config(worker_self_reference_url='http://worker2') - self.start_server() + self.start_server(set_worker_url=False) r = self._import_direct(image_id, ['store1']) self.assertEqual(status, r.status) self.assertIn(b'Stage host is unavailable', r.body) diff --git a/glance/tests/unit/common/test_wsgi.py b/glance/tests/unit/common/test_wsgi.py index b7c6bedefb..86699233c4 100644 --- a/glance/tests/unit/common/test_wsgi.py +++ b/glance/tests/unit/common/test_wsgi.py @@ -577,7 +577,10 @@ class ServerTest(test_utils.BaseTestCase): @mock.patch.object(prefetcher, 'Prefetcher') @mock.patch.object(wsgi.Server, 'configure_socket') - def test_http_keepalive(self, mock_configure_socket, mock_prefetcher): + @mock.patch('glance.sqlite_migration.can_migrate_to_central_db') + def test_http_keepalive(self, mock_migrate_db, mock_configure_socket, + mock_prefetcher): + mock_migrate_db.return_value = False self.config(http_keepalive=False) self.config(workers=0) @@ -599,8 +602,10 @@ class ServerTest(test_utils.BaseTestCase): socket_timeout=900) @mock.patch.object(prefetcher, 'Prefetcher') - def test_number_of_workers_posix(self, mock_prefetcher): + @mock.patch('glance.sqlite_migration.can_migrate_to_central_db') + def test_number_of_workers_posix(self, mock_migrate_db, mock_prefetcher): """Ensure the number of workers matches num cpus limited to 8.""" + mock_migrate_db.return_value = False if os.name == 'nt': raise self.skipException("Unsupported platform.") @@ -637,7 +642,9 @@ class ServerTest(test_utils.BaseTestCase): self.assertEqual(expected_workers, len(server.children)) - def test_invalid_staging_uri(self): + @mock.patch('glance.sqlite_migration.can_migrate_to_central_db') + def test_invalid_staging_uri(self, mock_migrate_db): + mock_migrate_db.return_value = False self.config(node_staging_uri='http://good.luck') server = wsgi.Server() with mock.patch.object(server, 'start_wsgi'): @@ -646,7 +653,9 @@ class ServerTest(test_utils.BaseTestCase): server.start, 'fake-application', 34567) @mock.patch('os.path.exists') - def test_missing_staging_dir(self, mock_exists): + @mock.patch('glance.sqlite_migration.can_migrate_to_central_db') + def test_missing_staging_dir(self, mock_migrate_db, mock_exists): + mock_migrate_db.return_value = False mock_exists.return_value = False server = wsgi.Server() with mock.patch.object(server, 'start_wsgi'): diff --git a/glance/tests/unit/common/test_wsgi_app.py b/glance/tests/unit/common/test_wsgi_app.py index 8dd1460eb2..3f9efc973e 100644 --- a/glance/tests/unit/common/test_wsgi_app.py +++ b/glance/tests/unit/common/test_wsgi_app.py @@ -29,9 +29,12 @@ class TestWsgiAppInit(test_utils.BaseTestCase): @mock.patch('glance.common.config.load_paste_app') @mock.patch('glance.async_.set_threadpool_model') @mock.patch('glance.common.wsgi_app._get_config_files') - def test_wsgi_init_sets_thread_settings(self, mock_config_files, + @mock.patch('glance.sqlite_migration.can_migrate_to_central_db') + def test_wsgi_init_sets_thread_settings(self, mock_migrate_db, + mock_config_files, mock_set_model, mock_load): + mock_migrate_db.return_value = False mock_config_files.return_value = [] self.config(task_pool_threads=123, group='wsgi') common.DEFAULT_POOL_SIZE = 1024 @@ -46,9 +49,12 @@ class TestWsgiAppInit(test_utils.BaseTestCase): @mock.patch('glance.common.config.load_paste_app') @mock.patch('glance.async_.set_threadpool_model') @mock.patch('glance.common.wsgi_app._get_config_files') - def test_wsgi_init_registers_exit_handler(self, mock_config_files, + @mock.patch('glance.sqlite_migration.can_migrate_to_central_db') + def test_wsgi_init_registers_exit_handler(self, mock_migrate_db, + mock_config_files, mock_set_model, mock_load, mock_exit): + mock_migrate_db.return_value = False mock_config_files.return_value = [] wsgi_app.init_app() mock_exit.assert_called_once_with(wsgi_app.drain_workers) @@ -56,9 +62,12 @@ class TestWsgiAppInit(test_utils.BaseTestCase): @mock.patch('glance.common.config.load_paste_app') @mock.patch('glance.async_.set_threadpool_model') @mock.patch('glance.common.wsgi_app._get_config_files') - def test_uwsgi_init_registers_exit_handler(self, mock_config_files, + @mock.patch('glance.sqlite_migration.can_migrate_to_central_db') + def test_uwsgi_init_registers_exit_handler(self, mock_migrate_db, + mock_config_files, mock_set_model, mock_load): + mock_migrate_db.return_value = False mock_config_files.return_value = [] with mock.patch.object(wsgi_app, 'uwsgi') as mock_u: wsgi_app.init_app() @@ -97,8 +106,11 @@ class TestWsgiAppInit(test_utils.BaseTestCase): @mock.patch('glance.common.wsgi_app._get_config_files') @mock.patch('threading.Thread') @mock.patch('glance.housekeeping.StagingStoreCleaner') - def test_runs_staging_cleanup(self, mock_cleaner, mock_Thread, mock_conf, + @mock.patch('glance.sqlite_migration.can_migrate_to_central_db') + def test_runs_staging_cleanup(self, mock_migrate_db, mock_cleaner, + mock_Thread, mock_conf, mock_load): + mock_migrate_db.return_value = False mock_conf.return_value = [] wsgi_app.init_app() mock_Thread.assert_called_once_with( @@ -111,8 +123,11 @@ class TestWsgiAppInit(test_utils.BaseTestCase): @mock.patch('glance.common.wsgi_app._get_config_files') @mock.patch('threading.Timer') @mock.patch('glance.image_cache.prefetcher.Prefetcher') + @mock.patch('glance.sqlite_migration.can_migrate_to_central_db') def test_run_cache_prefetcher_middleware_disabled( - self, mock_prefetcher, mock_Timer, mock_conf, mock_load): + self, mock_migrate_db, mock_prefetcher, mock_Timer, mock_conf, + mock_load): + mock_migrate_db.return_value = False mock_conf.return_value = [] wsgi_app.init_app() mock_Timer.assert_not_called() @@ -120,7 +135,9 @@ class TestWsgiAppInit(test_utils.BaseTestCase): @mock.patch('glance.common.wsgi_app._get_config_files') @mock.patch('glance.async_._THREADPOOL_MODEL', new=None) @mock.patch('glance.common.config.load_paste_app', new=mock.MagicMock()) - def test_staging_store_uri_assertion(self, mock_conf): + @mock.patch('glance.sqlite_migration.can_migrate_to_central_db') + def test_staging_store_uri_assertion(self, mock_migrate_db, mock_conf): + mock_migrate_db.return_value = False self.config(node_staging_uri='http://good.luck') mock_conf.return_value = [] # Make sure a staging URI with a bad scheme will abort startup @@ -130,7 +147,10 @@ class TestWsgiAppInit(test_utils.BaseTestCase): @mock.patch('glance.async_._THREADPOOL_MODEL', new=None) @mock.patch('glance.common.config.load_paste_app', new=mock.MagicMock()) @mock.patch('os.path.exists') - def test_staging_store_path_check(self, mock_exists, mock_conf): + @mock.patch('glance.sqlite_migration.can_migrate_to_central_db') + def test_staging_store_path_check(self, mock_migrate_db, mock_exists, + mock_conf): + mock_migrate_db.return_value = False mock_exists.return_value = False mock_conf.return_value = [] with mock.patch.object(wsgi_app, 'LOG') as mock_log: diff --git a/glance/tests/unit/test_sqlite_migration.py b/glance/tests/unit/test_sqlite_migration.py index 8ffcb4d477..b878eb4b72 100644 --- a/glance/tests/unit/test_sqlite_migration.py +++ b/glance/tests/unit/test_sqlite_migration.py @@ -98,6 +98,7 @@ class TestMigrate(test_utils.BaseTestCase): sq_db.commit() def test_migrate_if_required_false(self): + self.config(image_cache_driver="sqlite") self.assertFalse(sqlite_migration.migrate_if_required()) @mock.patch('os.path.exists')