diff --git a/trove/guestagent/common/configuration.py b/trove/guestagent/common/configuration.py index 26d0ca499d..772acb6ba8 100644 --- a/trove/guestagent/common/configuration.py +++ b/trove/guestagent/common/configuration.py @@ -85,9 +85,6 @@ class ConfigurationManager(object): revision_dir = guestagent_utils.build_file_path( os.path.dirname(base_config_path), self.DEFAULT_STRATEGY_OVERRIDES_SUB_DIR) - operating_system.create_directory( - revision_dir, user=owner, group=group, force=True, - as_root=requires_root) self._override_strategy = OneFileOverrideStrategy(revision_dir) else: self._override_strategy = override_strategy @@ -313,6 +310,7 @@ class ImportOverrideStrategy(ConfigurationOverrideStrategy): return self._find_revision_file(group_name, change_id) is not None def apply(self, group_name, change_id, options): + self._initialize_import_directory() revision_file = self._find_revision_file(group_name, change_id) if revision_file is None: # Create a new file. @@ -337,6 +335,14 @@ class ImportOverrideStrategy(ConfigurationOverrideStrategy): operating_system.chmod( revision_file, FileMode.ADD_READ_ALL, as_root=self._requires_root) + def _initialize_import_directory(self): + """Lazy-initialize the directory for imported revision files. + """ + if not os.path.exists(self._revision_dir): + operating_system.create_directory( + self._revision_dir, user=self._owner, group=self._group, + force=True, as_root=self._requires_root) + def remove(self, group_name, change_id=None): removed = set() if change_id: diff --git a/trove/guestagent/datastore/experimental/mongodb/service.py b/trove/guestagent/datastore/experimental/mongodb/service.py index e9ed52369d..958853afd7 100644 --- a/trove/guestagent/datastore/experimental/mongodb/service.py +++ b/trove/guestagent/datastore/experimental/mongodb/service.py @@ -50,26 +50,12 @@ CONFIGSVR_PORT = CONF.mongodb.configsvr_port class MongoDBApp(object): """Prepares DBaaS on a Guest container.""" - @classmethod - def _init_overrides_dir(cls): - """Initialize a directory for configuration overrides. - """ - revision_dir = guestagent_utils.build_file_path( - os.path.dirname(CONFIG_FILE), - ConfigurationManager.DEFAULT_STRATEGY_OVERRIDES_SUB_DIR) - - if not os.path.exists(revision_dir): - operating_system.create_directory( - revision_dir, - user=system.MONGO_USER, group=system.MONGO_USER, - force=True, as_root=True) - - return revision_dir - def __init__(self): self.state_change_wait_time = CONF.state_change_wait_time - revision_dir = self._init_overrides_dir() + revision_dir = guestagent_utils.build_file_path( + os.path.dirname(CONFIG_FILE), + ConfigurationManager.DEFAULT_STRATEGY_OVERRIDES_SUB_DIR) self.configuration_manager = ConfigurationManager( CONFIG_FILE, system.MONGO_USER, system.MONGO_USER, SafeYamlCodec(default_flow_style=False), diff --git a/trove/guestagent/datastore/experimental/redis/service.py b/trove/guestagent/datastore/experimental/redis/service.py index 2508c93ba8..addb225048 100644 --- a/trove/guestagent/datastore/experimental/redis/service.py +++ b/trove/guestagent/datastore/experimental/redis/service.py @@ -72,22 +72,6 @@ class RedisApp(object): on a trove instance. """ - @classmethod - def _init_overrides_dir(cls): - """Initialize a directory for configuration overrides. - """ - revision_dir = guestagent_utils.build_file_path( - os.path.dirname(system.REDIS_CONFIG), - ConfigurationManager.DEFAULT_STRATEGY_OVERRIDES_SUB_DIR) - - if not os.path.exists(revision_dir): - operating_system.create_directory( - revision_dir, - user=system.REDIS_OWNER, group=system.REDIS_OWNER, - force=True, as_root=True) - - return revision_dir - def __init__(self, state_change_wait_time=None): """ Sets default status and state_change_wait_time @@ -97,7 +81,9 @@ class RedisApp(object): else: self.state_change_wait_time = CONF.state_change_wait_time - revision_dir = self._init_overrides_dir() + revision_dir = guestagent_utils.build_file_path( + os.path.dirname(system.REDIS_CONFIG), + ConfigurationManager.DEFAULT_STRATEGY_OVERRIDES_SUB_DIR) config_value_mappings = {'yes': True, 'no': False, "''": None} self._value_converter = StringConverter(config_value_mappings) self.configuration_manager = ConfigurationManager( diff --git a/trove/tests/unittests/backup/test_backupagent.py b/trove/tests/unittests/backup/test_backupagent.py index a7f34f92bf..d76c9c8b87 100644 --- a/trove/tests/unittests/backup/test_backupagent.py +++ b/trove/tests/unittests/backup/test_backupagent.py @@ -26,7 +26,7 @@ from trove.common import utils from trove.conductor import api as conductor_api from trove.guestagent.backup import backupagent from trove.guestagent.common import configuration -from trove.guestagent.datastore.experimental.mongodb.service import MongoDBApp +from trove.guestagent.common.configuration import ImportOverrideStrategy from trove.guestagent.strategies.backup.base import BackupRunner from trove.guestagent.strategies.backup.base import UnknownBackupType from trove.guestagent.strategies.backup.experimental import couchbase_impl @@ -251,7 +251,7 @@ class BackupAgentTest(trove_testtools.TestCase): self.assertIsNotNone(cbbackup.manifest) self.assertIn('gz.enc', cbbackup.manifest) - @mock.patch.object(MongoDBApp, '_init_overrides_dir', return_value='') + @mock.patch.object(ImportOverrideStrategy, '_initialize_import_directory') def test_backup_impl_MongoDump(self, _): netutils.get_my_ipv4 = Mock(return_value="1.1.1.1") utils.execute_with_timeout = Mock(return_value=None) diff --git a/trove/tests/unittests/guestagent/test_backups.py b/trove/tests/unittests/guestagent/test_backups.py index 05e6aad582..f5ab0ed8b4 100644 --- a/trove/tests/unittests/guestagent/test_backups.py +++ b/trove/tests/unittests/guestagent/test_backups.py @@ -17,8 +17,8 @@ from testtools.testcase import ExpectedException from trove.common import exception from trove.common import utils from trove.guestagent.common import configuration +from trove.guestagent.common.configuration import ImportOverrideStrategy from trove.guestagent.common import operating_system -from trove.guestagent.datastore.experimental.mongodb.service import MongoDBApp from trove.guestagent.strategies.backup import base as backupBase from trove.guestagent.strategies.backup.mysql_impl import MySqlApp from trove.guestagent.strategies.restore import base as restoreBase @@ -105,10 +105,16 @@ class GuestAgentBackupTest(trove_testtools.TestCase): MySqlApp, 'get_auth_password', mock.Mock(return_value='password')) self.get_auth_pwd_mock = self.get_auth_pwd_patch.start() self.addCleanup(self.get_auth_pwd_patch.stop) - self.orig_exec_with_to = utils.execute_with_timeout - self.get_data_dir_patcher = patch.object( + + self.exec_timeout_patch = patch.object(utils, 'execute_with_timeout') + self.exec_timeout_mock = self.exec_timeout_patch.start() + self.addCleanup(self.exec_timeout_patch.stop) + + self.get_data_dir_patch = patch.object( MySqlApp, 'get_data_dir', return_value='/var/lib/mysql/data') - self.mock_get_datadir = self.get_data_dir_patcher.start() + self.get_datadir_mock = self.get_data_dir_patch.start() + self.addCleanup(self.get_data_dir_patch.stop) + backupBase.BackupRunner.is_zipped = True backupBase.BackupRunner.is_encrypted = True restoreBase.RestoreRunner.is_zipped = True @@ -116,8 +122,6 @@ class GuestAgentBackupTest(trove_testtools.TestCase): def tearDown(self): super(GuestAgentBackupTest, self).tearDown() - utils.execute_with_timeout = self.orig_exec_with_to - self.get_data_dir_patcher.stop() def test_backup_decrypted_xtrabackup_command(self): backupBase.BackupRunner.is_encrypted = False @@ -317,8 +321,7 @@ class GuestAgentBackupTest(trove_testtools.TestCase): # (see bug/1423759). remove.assert_called_once_with(ANY, force=True, as_root=True) - @mock.patch.object(MongoDBApp, '_init_overrides_dir', - return_value='') + @mock.patch.object(ImportOverrideStrategy, '_initialize_import_directory') def test_backup_encrypted_mongodump_command(self, _): backupBase.BackupRunner.encrypt_key = CRYPTO_KEY RunnerClass = utils.import_class(BACKUP_MONGODUMP_CLS) @@ -329,8 +332,7 @@ class GuestAgentBackupTest(trove_testtools.TestCase): MONGODUMP_CMD + PIPE + ZIP + PIPE + ENCRYPT, bkp.command) self.assertIn("gz.enc", bkp.manifest) - @mock.patch.object(MongoDBApp, '_init_overrides_dir', - return_value='') + @mock.patch.object(ImportOverrideStrategy, '_initialize_import_directory') def test_backup_not_encrypted_mongodump_command(self, _): backupBase.BackupRunner.is_encrypted = False backupBase.BackupRunner.encrypt_key = CRYPTO_KEY @@ -341,8 +343,7 @@ class GuestAgentBackupTest(trove_testtools.TestCase): self.assertEqual(MONGODUMP_CMD + PIPE + ZIP, bkp.command) self.assertIn("gz", bkp.manifest) - @mock.patch.object(MongoDBApp, '_init_overrides_dir', - return_value='') + @mock.patch.object(ImportOverrideStrategy, '_initialize_import_directory') def test_restore_decrypted_mongodump_command(self, _): restoreBase.RestoreRunner.is_encrypted = False RunnerClass = utils.import_class(RESTORE_MONGODUMP_CLS) @@ -350,8 +351,7 @@ class GuestAgentBackupTest(trove_testtools.TestCase): location="filename", checksum="md5") self.assertEqual(restr.restore_cmd, UNZIP + PIPE + MONGODUMP_RESTORE) - @mock.patch.object(MongoDBApp, '_init_overrides_dir', - return_value='') + @mock.patch.object(ImportOverrideStrategy, '_initialize_import_directory') def test_restore_encrypted_mongodump_command(self, _): restoreBase.RestoreRunner.decrypt_key = CRYPTO_KEY RunnerClass = utils.import_class(RESTORE_MONGODUMP_CLS) @@ -496,51 +496,51 @@ class MongodbBackupTests(trove_testtools.TestCase): def setUp(self): super(MongodbBackupTests, self).setUp() + self.exec_timeout_patch = patch.object(utils, 'execute_with_timeout') - self.exec_timeout_patch.start() - self.mongodb_init_overrides_dir_patch = patch.object( - MongoDBApp, - '_init_overrides_dir', - return_value='') - self.mongodb_init_overrides_dir_patch.start() - self.backup_runner = utils.import_class( - BACKUP_MONGODUMP_CLS) + self.exec_timeout_mock = self.exec_timeout_patch.start() + self.addCleanup(self.exec_timeout_patch.stop) + + self.init_overrides_dir_patch = patch.object( + ImportOverrideStrategy, '_initialize_import_directory') + self.init_overrides_dir_mock = self.init_overrides_dir_patch.start() + self.addCleanup(self.init_overrides_dir_patch.stop) + + self.backup_runner = utils.import_class(BACKUP_MONGODUMP_CLS) + self.backup_runner_patch = patch.multiple( self.backup_runner, _run=DEFAULT, _run_pre_backup=DEFAULT, _run_post_backup=DEFAULT) + self.backup_runner_mocks = self.backup_runner_patch.start() + self.addCleanup(self.backup_runner_patch.stop) def tearDown(self): super(MongodbBackupTests, self).tearDown() - self.backup_runner_patch.stop() - self.exec_timeout_patch.stop() - self.mongodb_init_overrides_dir_patch.stop() def test_backup_success(self): - backup_runner_mocks = self.backup_runner_patch.start() with self.backup_runner(12345): pass - backup_runner_mocks['_run_pre_backup'].assert_called_once_with() - backup_runner_mocks['_run'].assert_called_once_with() - backup_runner_mocks['_run_post_backup'].assert_called_once_with() + self.backup_runner_mocks['_run_pre_backup'].assert_called_once_with() + self.backup_runner_mocks['_run'].assert_called_once_with() + self.backup_runner_mocks['_run_post_backup'].assert_called_once_with() def test_backup_failed_due_to_run_backup(self): - backup_runner_mocks = self.backup_runner_patch.start() - backup_runner_mocks['_run'].configure_mock( + self.backup_runner_mocks['_run'].configure_mock( side_effect=exception.TroveError('test') ) with ExpectedException(exception.TroveError, 'test'): with self.backup_runner(12345): pass - backup_runner_mocks['_run_pre_backup'].assert_called_once_with() - backup_runner_mocks['_run'].assert_called_once_with() - self.assertEqual(0, backup_runner_mocks['_run_post_backup'].call_count) + self.backup_runner_mocks['_run_pre_backup'].assert_called_once_with() + self.backup_runner_mocks['_run'].assert_called_once_with() + self.assertEqual( + 0, self.backup_runner_mocks['_run_post_backup'].call_count) class MongodbRestoreTests(trove_testtools.TestCase): - @patch.object(MongoDBApp, '_init_overrides_dir', - return_value='') + @mock.patch.object(ImportOverrideStrategy, '_initialize_import_directory') def setUp(self, _): super(MongodbRestoreTests, self).setUp() diff --git a/trove/tests/unittests/guestagent/test_dbaas.py b/trove/tests/unittests/guestagent/test_dbaas.py index bc7dcf4a8f..b67cce8564 100644 --- a/trove/tests/unittests/guestagent/test_dbaas.py +++ b/trove/tests/unittests/guestagent/test_dbaas.py @@ -42,6 +42,7 @@ from trove.common.exception import ProcessExecutionError from trove.common import instance as rd_instance from trove.common import utils from trove.conductor import api as conductor_api +from trove.guestagent.common.configuration import ImportOverrideStrategy from trove.guestagent.common import operating_system from trove.guestagent.common.operating_system import FileMode from trove.guestagent.datastore.experimental.cassandra import ( @@ -2188,9 +2189,10 @@ class TestRedisApp(testtools.TestCase): self.orig_os_path_eu = os.path.expanduser os.path.expanduser = Mock(return_value='/tmp/.file') - with patch.multiple(RedisApp, _build_admin_client=DEFAULT, - _init_overrides_dir=DEFAULT): - self.app = RedisApp(state_change_wait_time=0) + with patch.object(RedisApp, '_build_admin_client'): + with patch.object(ImportOverrideStrategy, + '_initialize_import_directory'): + self.app = RedisApp(state_change_wait_time=0) self.orig_os_path_isfile = os.path.isfile self.orig_utils_execute_with_timeout = utils.execute_with_timeout @@ -2784,7 +2786,7 @@ class MongoDBAppTest(testtools.TestCase): 'cmd_disable': 'disable' } - @patch.object(mongo_service.MongoDBApp, '_init_overrides_dir') + @patch.object(ImportOverrideStrategy, '_initialize_import_directory') def setUp(self, _): super(MongoDBAppTest, self).setUp() self.orig_utils_execute_with_timeout = (mongo_service. diff --git a/trove/tests/unittests/guestagent/test_mongodb_cluster_manager.py b/trove/tests/unittests/guestagent/test_mongodb_cluster_manager.py index 1189ef40af..e4c3b77802 100644 --- a/trove/tests/unittests/guestagent/test_mongodb_cluster_manager.py +++ b/trove/tests/unittests/guestagent/test_mongodb_cluster_manager.py @@ -20,6 +20,7 @@ import pymongo import trove.common.context as context import trove.common.instance as ds_instance import trove.common.utils as utils +from trove.guestagent.common.configuration import ImportOverrideStrategy from trove.guestagent.common import operating_system import trove.guestagent.datastore.experimental.mongodb.manager as manager import trove.guestagent.datastore.experimental.mongodb.service as service @@ -30,8 +31,7 @@ import trove.tests.unittests.trove_testtools as trove_testtools class GuestAgentMongoDBClusterManagerTest(trove_testtools.TestCase): - @mock.patch.object(service.MongoDBApp, '_init_overrides_dir', - return_value='') + @mock.patch.object(ImportOverrideStrategy, '_initialize_import_directory') def setUp(self, _): super(GuestAgentMongoDBClusterManagerTest, self).setUp() self.context = context.TroveContext() diff --git a/trove/tests/unittests/guestagent/test_mongodb_manager.py b/trove/tests/unittests/guestagent/test_mongodb_manager.py index a7b47d8147..9e1877cd73 100644 --- a/trove/tests/unittests/guestagent/test_mongodb_manager.py +++ b/trove/tests/unittests/guestagent/test_mongodb_manager.py @@ -18,6 +18,7 @@ import pymongo import trove.common.context as context import trove.common.utils as utils import trove.guestagent.backup as backup +from trove.guestagent.common.configuration import ImportOverrideStrategy import trove.guestagent.datastore.experimental.mongodb.manager as manager import trove.guestagent.datastore.experimental.mongodb.service as service import trove.guestagent.db.models as models @@ -27,8 +28,7 @@ import trove.tests.unittests.trove_testtools as trove_testtools class GuestAgentMongoDBManagerTest(trove_testtools.TestCase): - @mock.patch.object(service.MongoDBApp, '_init_overrides_dir', - return_value='') + @mock.patch.object(ImportOverrideStrategy, '_initialize_import_directory') def setUp(self, _): super(GuestAgentMongoDBManagerTest, self).setUp() self.context = context.TroveContext() diff --git a/trove/tests/unittests/guestagent/test_redis_manager.py b/trove/tests/unittests/guestagent/test_redis_manager.py index d7881365c7..0e219a5562 100644 --- a/trove/tests/unittests/guestagent/test_redis_manager.py +++ b/trove/tests/unittests/guestagent/test_redis_manager.py @@ -17,6 +17,7 @@ from mock import DEFAULT, MagicMock, patch from trove.common.context import TroveContext from trove.guestagent import backup from trove.guestagent.common import configuration +from trove.guestagent.common.configuration import ImportOverrideStrategy from trove.guestagent.common import operating_system from trove.guestagent.datastore.experimental.redis import ( service as redis_service) @@ -28,8 +29,8 @@ from trove.tests.unittests import trove_testtools class RedisGuestAgentManagerTest(trove_testtools.TestCase): - @patch.multiple(redis_service.RedisApp, - _build_admin_client=DEFAULT, _init_overrides_dir=DEFAULT) + @patch.object(redis_service.RedisApp, '_build_admin_client') + @patch.object(ImportOverrideStrategy, '_initialize_import_directory') def setUp(self, *args, **kwargs): super(RedisGuestAgentManagerTest, self).setUp() self.patch_ope = patch('os.path.expanduser') @@ -168,8 +169,7 @@ class RedisGuestAgentManagerTest(trove_testtools.TestCase): self.manager.stop_db(self.context) redis_mock.assert_any_call(do_not_start_on_reboot=False) - @patch.object(redis_service.RedisApp, '_init_overrides_dir', - return_value='') + @patch.object(ImportOverrideStrategy, '_initialize_import_directory') @patch.object(backup, 'backup') @patch.object(configuration.ConfigurationManager, 'parse_configuration', MagicMock(return_value={'dir': '/var/lib/redis',