From da0797a74f49b2e84adc82c31f8da7b6f1a6af14 Mon Sep 17 00:00:00 2001 From: Petr Malik Date: Fri, 8 Jan 2016 17:51:23 -0500 Subject: [PATCH] Move 'enable root on prepare' to the base When given password on prepare, the 'root' should be enabled in the base class, similarly to how the configuration overrides get applied. It is achieved by calling already existing 'enable_root_with_password' manager method. The call is encapsulated so that it can be easily overriden if necessary. The action is performed in the block where other users/databases get created. Note I placed user/database creation and root action into two separate try-blocks. This is to ensure that failure in one does not stop the other. It also allows for more accurate error message. I made the above actions log, but not re-raise exceptions so that the 'post_prepare' can still run in case of an error. Datastores that do not implement root actions will have an exception recorded in the log and the root password will be ignored (i.e. the instance will come up without 'root' user). This corresponds to the current behavior (except the error gets logged rather than ignored). Also extended the base manager test accordingly. Related-Bug: 1532583 Change-Id: I982d94d20c9ef317d83827f898b236d0e44e95a5 --- .../experimental/couchbase/manager.py | 5 +- .../datastore/experimental/mongodb/manager.py | 7 +- .../experimental/postgresql/manager.py | 3 - .../experimental/postgresql/service/root.py | 3 + trove/guestagent/datastore/manager.py | 21 ++++- .../datastore/mysql_common/manager.py | 8 +- .../unittests/guestagent/test_manager.py | 81 ++++++++++--------- 7 files changed, 72 insertions(+), 56 deletions(-) diff --git a/trove/guestagent/datastore/experimental/couchbase/manager.py b/trove/guestagent/datastore/experimental/couchbase/manager.py index 7b1cea184d..de6fdaccdf 100644 --- a/trove/guestagent/datastore/experimental/couchbase/manager.py +++ b/trove/guestagent/datastore/experimental/couchbase/manager.py @@ -61,8 +61,6 @@ class Manager(manager.Manager): LOG.debug('Mounted the volume (%s).' % device_path) self.app.start_db_with_conf_changes(config_contents) LOG.debug('Securing couchbase now.') - if root_password: - self.app.enable_root(root_password) self.app.initial_setup() if backup_info: LOG.debug('Now going to perform restore.') @@ -93,6 +91,9 @@ class Manager(manager.Manager): LOG.debug("Enabling root.") return self.app.enable_root() + def enable_root_with_password(self, context, root_password=None): + return self.app.enable_root(root_password) + def is_root_enabled(self, context): LOG.debug("Checking if root is enabled.") return os.path.exists(system.pwd_file) diff --git a/trove/guestagent/datastore/experimental/mongodb/manager.py b/trove/guestagent/datastore/experimental/mongodb/manager.py index 31ad66e455..9105b207a3 100644 --- a/trove/guestagent/datastore/experimental/mongodb/manager.py +++ b/trove/guestagent/datastore/experimental/mongodb/manager.py @@ -93,10 +93,6 @@ class Manager(manager.Manager): if service.MongoDBAdmin().is_root_enabled(): self.app.status.report_root(context, 'root') - if not cluster_config and root_password: - LOG.debug('Root password provided. Enabling root.') - service.MongoDBAdmin().enable_root(root_password) - def restart(self, context): LOG.debug("Restarting MongoDB.") self.app.restart() @@ -170,6 +166,9 @@ class Manager(manager.Manager): LOG.debug("Enabling root.") return service.MongoDBAdmin().enable_root() + def enable_root_with_password(self, context, root_password=None): + return service.MongoDBAdmin().enable_root(root_password) + def is_root_enabled(self, context): LOG.debug("Checking if root is enabled.") return service.MongoDBAdmin().is_root_enabled() diff --git a/trove/guestagent/datastore/experimental/postgresql/manager.py b/trove/guestagent/datastore/experimental/postgresql/manager.py index ad13f5e6d6..53fdb17e60 100644 --- a/trove/guestagent/datastore/experimental/postgresql/manager.py +++ b/trove/guestagent/datastore/experimental/postgresql/manager.py @@ -113,9 +113,6 @@ class Manager( else: self._secure(context) - if root_password and not backup_info: - self.enable_root(context, root_password) - def _secure(self, context): # Create a new administrative user for Trove and also # disable the built-in superuser. diff --git a/trove/guestagent/datastore/experimental/postgresql/service/root.py b/trove/guestagent/datastore/experimental/postgresql/service/root.py index e83db66fbf..be96642254 100644 --- a/trove/guestagent/datastore/experimental/postgresql/service/root.py +++ b/trove/guestagent/datastore/experimental/postgresql/service/root.py @@ -94,3 +94,6 @@ class PgSqlRoot(PgSqlUsers): stay that way. """ self.enable_root(context) + + def enable_root_with_password(self, context, root_password=None): + self.enable_root(context, root_password) diff --git a/trove/guestagent/datastore/manager.py b/trove/guestagent/datastore/manager.py index 4cca839f45..74e1ab941f 100644 --- a/trove/guestagent/datastore/manager.py +++ b/trove/guestagent/datastore/manager.py @@ -280,8 +280,9 @@ class Manager(periodic_task.PeriodicTasks): LOG.info(_("Completed setup of '%s' datastore successfully.") % self.manager) - # We only create databases and users automatically for non-cluster - # instances. + # The following block performs single-instance initialization. + # Failures will be recorded, but won't stop the provisioning + # or change the instance state. if not cluster_config: try: if databases: @@ -295,7 +296,18 @@ class Manager(periodic_task.PeriodicTasks): except Exception as ex: LOG.exception(_("An error occurred creating databases/users: " "%s") % ex.message) - raise + + # We only enable-root automatically if not restoring a backup + # that may already have root enabled in which case we keep it + # unchanged. + if root_password and not backup_info: + try: + LOG.info(_("Enabling root user (with password).")) + self.enable_root_on_prepare(context, root_password) + LOG.info(_('Root enabled successfully.')) + except Exception as ex: + LOG.exception(_("An error occurred enabling root user: " + "%s") % ex.message) try: LOG.info(_("Calling post_prepare for '%s' datastore.") % @@ -315,6 +327,9 @@ class Manager(periodic_task.PeriodicTasks): self.update_overrides(context, overrides) self.restart(context) + def enable_root_on_prepare(self, context, root_password): + self.enable_root_with_password(context, root_password) + @abc.abstractmethod def do_prepare(self, context, packages, databases, memory_mb, users, device_path, mount_point, backup_info, config_contents, diff --git a/trove/guestagent/datastore/mysql_common/manager.py b/trove/guestagent/datastore/mysql_common/manager.py index a70e9b7136..347469b7e6 100644 --- a/trove/guestagent/datastore/mysql_common/manager.py +++ b/trove/guestagent/datastore/mysql_common/manager.py @@ -169,8 +169,7 @@ class MySqlManager(manager.Manager): return self.mysql_admin().enable_root() def enable_root_with_password(self, context, root_password=None): - raise exception.DatastoreOperationNotSupported( - operation='enable_root_with_password', datastore=self.manager) + return self.mysql_admin().enable_root(root_password) def is_root_enabled(self, context): return self.mysql_admin().is_root_enabled() @@ -228,10 +227,7 @@ class MySqlManager(manager.Manager): app.secure(config_contents) enable_root_on_restore = (backup_info and self.mysql_admin().is_root_enabled()) - if root_password and not backup_info: - app.secure_root(secure_remote_root=True) - self.mysql_admin().enable_root(root_password) - elif enable_root_on_restore: + if enable_root_on_restore: app.secure_root(secure_remote_root=False) self.mysql_app_status.get().report_root(context, 'root') else: diff --git a/trove/tests/unittests/guestagent/test_manager.py b/trove/tests/unittests/guestagent/test_manager.py index a2cfdaf252..bff443fec2 100644 --- a/trove/tests/unittests/guestagent/test_manager.py +++ b/trove/tests/unittests/guestagent/test_manager.py @@ -320,41 +320,30 @@ class ManagerTest(trove_testtools.TestCase): "%s not called" % key) def test_prepare_single(self): - packages = Mock() - databases = Mock() - memory_mb = Mock() - users = Mock() - device_path = Mock() - mount_point = Mock() - backup_info = Mock() - config_contents = Mock() - root_password = Mock() - overrides = Mock() - snapshot = Mock() + self.run_prepare_test(cluster_config=None) - self._assert_prepare( - self.context, packages, databases, memory_mb, users, device_path, - mount_point, backup_info, config_contents, root_password, - overrides, None, snapshot) + def test_prepare_single_no_users(self): + self.run_prepare_test(cluster_config=None, users=None) + + def test_prepare_single_no_databases(self): + self.run_prepare_test(cluster_config=None, databases=None) + + def test_prepare_single_no_root_password(self): + self.run_prepare_test(cluster_config=None, root_password=None) def test_prepare_cluster(self): - packages = Mock() - databases = Mock() - memory_mb = Mock() - users = Mock() - device_path = Mock() - mount_point = Mock() - backup_info = Mock() - config_contents = Mock() - root_password = Mock() - overrides = Mock() - cluster_config = Mock() - snapshot = Mock() + self.run_prepare_test() - self._assert_prepare( - self.context, packages, databases, memory_mb, users, device_path, - mount_point, backup_info, config_contents, root_password, - overrides, cluster_config, snapshot) + def run_prepare_test(self, packages=Mock(), databases=Mock(), + memory_mb=Mock(), users=Mock(), + device_path=Mock(), mount_point=Mock(), + backup_info=Mock(), config_contents=Mock(), + root_password=Mock(), overrides=Mock(), + cluster_config=Mock(), snapshot=Mock()): + self._assert_prepare(self.context, packages, databases, memory_mb, + users, device_path, mount_point, backup_info, + config_contents, root_password, overrides, + cluster_config, snapshot) def _assert_prepare(self, context, packages, databases, memory_mb, users, device_path, mount_point, backup_info, config_contents, @@ -366,6 +355,7 @@ class ManagerTest(trove_testtools.TestCase): with patch.multiple(self.manager, do_prepare=DEFAULT, post_prepare=DEFAULT, apply_overrides_on_prepare=DEFAULT, + enable_root_on_prepare=DEFAULT, create_database=DEFAULT, create_user=DEFAULT): self.manager.prepare( context, packages, databases, memory_mb, users, @@ -409,15 +399,30 @@ class ManagerTest(trove_testtools.TestCase): snapshot) if not is_post_process_expected: - self.manager.create_database.assert_called_once_with( - context, - databases) - self.manager.create_user.assert_called_once_with( - context, - users) + if databases: + self.manager.create_database.assert_called_once_with( + context, + databases) + else: + self.assertEqual( + 0, self.manager.create_database.call_count) + if users: + self.manager.create_user.assert_called_once_with( + context, + users) + else: + self.assertEqual(0, self.manager.create_user.call_count) + if not backup_info and root_password: + (self.manager.enable_root_on_prepare. + assert_called_once_with(context, root_password)) + else: + self.assertEqual( + 0, self.manager.enable_root_on_prepare.call_count) else: self.assertEqual(0, self.manager.create_database.call_count) self.assertEqual(0, self.manager.create_user.call_count) + self.assertEqual( + 0, self.manager.enable_root_on_prepare.call_count) def test_apply_overrides_on_prepare(self): overrides = Mock() @@ -451,7 +456,7 @@ class ManagerTest(trove_testtools.TestCase): side_effect=expected_failure )): self.assertRaisesRegexp( - Exception, "Error in 'apply_overrides_on_prepare'.", + Exception, expected_failure.message, self.manager.prepare, self.context, packages, databases, memory_mb, users, device_path, mount_point, backup_info, config_contents,