From d1af33f17b0994ac1d0ca5acca91f2f29bc82ce9 Mon Sep 17 00:00:00 2001 From: Lingxian Kong Date: Tue, 13 Oct 2020 23:51:46 +1300 Subject: [PATCH] Support mysql 8.0 * MySQL 5.7 and MySQL 8.0 need different percona-xtrabackup package version. Added Percona XtraBackup 8 support for MySQL 8.x backup and restore. * Construct different backup container image names for MySQL 5.7 and MySQL 8.0 based on the default option value. * Two docker images are uploaded for backup/restore: openstacktrove/db-backup-mysql5.7:1.0.0 and openstacktrove/db-backup-mysql8.0:1.0.0. Trove guest agent can automatically choose the approriate one based on the datastore version. * Added option "secure-file-priv=NULL" in MySQL config template to fix https://github.com/docker-library/mysql/issues/541. * Stop using IDENTIFIED BY in GRANT clause (also REVOKE). Starting with MySQL 8 creating a user implicitly using the GRANT command is not supported. Story: #2008275 Task: #41143 Change-Id: Ibdec63324b1b39ba9b8a38dbe529da17bbb06767 --- backup/Dockerfile | 5 +- backup/drivers/xtrabackup.py | 133 ++++++++++++++++++ backup/install.sh | 12 +- backup/main.py | 4 +- devstack/plugin.sh | 5 +- integration/scripts/trovestack | 6 +- lower-constraints.txt | 1 + requirements.txt | 1 + trove/common/cfg.py | 4 +- trove/guestagent/common/sql_query.py | 48 +++---- trove/guestagent/datastore/mariadb/manager.py | 10 +- trove/guestagent/datastore/mysql/manager.py | 25 ++++ trove/guestagent/datastore/mysql/service.py | 35 +++++ .../datastore/mysql_common/manager.py | 19 +-- .../datastore/mysql_common/service.py | 31 ++-- trove/guestagent/datastore/service.py | 13 +- trove/instance/models.py | 42 +++--- trove/templates/mysql/config.template | 1 + trove/tests/api/configurations.py | 3 +- trove/tests/api/users.py | 4 +- trove/tests/config.py | 2 +- 21 files changed, 312 insertions(+), 92 deletions(-) create mode 100644 backup/drivers/xtrabackup.py diff --git a/backup/Dockerfile b/backup/Dockerfile index a5e4e7e01a..f60149c70c 100644 --- a/backup/Dockerfile +++ b/backup/Dockerfile @@ -1,9 +1,8 @@ FROM ubuntu:18.04 LABEL maintainer="anlin.kong@gmail.com" -ARG DATASTORE="mysql" +ARG DATASTORE="mysql5.7" ARG APTOPTS="-y -qq --no-install-recommends --allow-unauthenticated" -ARG PERCONA_XTRABACKUP_VERSION=24 RUN export DEBIAN_FRONTEND="noninteractive" \ && export APT_KEY_DONT_WARN_ON_DANGEROUS_USAGE=1 @@ -16,7 +15,7 @@ RUN apt-get update \ COPY . /opt/trove/backup WORKDIR /opt/trove/backup -RUN ./install.sh $DATASTORE ${PERCONA_XTRABACKUP_VERSION} +RUN ./install.sh $DATASTORE RUN apt-get update \ && apt-get install $APTOPTS build-essential python3-setuptools python3-all python3-all-dev python3-pip libffi-dev libssl-dev libxml2-dev libxslt1-dev libyaml-dev \ diff --git a/backup/drivers/xtrabackup.py b/backup/drivers/xtrabackup.py new file mode 100644 index 0000000000..6874326f63 --- /dev/null +++ b/backup/drivers/xtrabackup.py @@ -0,0 +1,133 @@ +# Copyright 2020 Catalyst Cloud +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import re + +from oslo_concurrency import processutils +from oslo_config import cfg +from oslo_log import log as logging + +from backup.drivers import mysql_base + +LOG = logging.getLogger(__name__) +CONF = cfg.CONF + + +class XtraBackup(mysql_base.MySQLBaseRunner): + """Implementation of Backup and Restore for XtraBackup 8.0. + + According to + https://www.percona.com/doc/percona-xtrabackup/8.0/index.html#user-s-manual, + Percona XtraBackup 8.0 does not support making backups of databases created + in versions prior to 8.0 of MySQL. + + Percona XtraBackup 8.0.12 supports backup and restore processing for + versions of MySQL 8.x. + + innobackupex was removed in Percona XtraBackup 8.0. + """ + backup_log = '/tmp/xtrabackup.log' + prepare_log = '/tmp/prepare.log' + restore_cmd = ('xbstream -x -C %(restore_location)s --parallel=2' + ' 2>/tmp/xbstream_extract.log') + prepare_cmd = (f'xtrabackup ' + f'--target-dir=%(restore_location)s ' + f'--prepare 2>{prepare_log}') + + @property + def cmd(self): + cmd = (f'xtrabackup --backup --stream=xbstream --parallel=2 ' + f'--datadir={self.datadir} {self.user_and_pass} ' + f'2>{self.backup_log}') + return cmd + self.zip_cmd + self.encrypt_cmd + + def check_restore_process(self): + """Check whether xbstream restore is successful.""" + LOG.info('Checking return code of xbstream restore process.') + return_code = self.process.wait() + if return_code != 0: + LOG.error('xbstream exited with %s', return_code) + return False + + with open('/tmp/xbstream_extract.log', 'r') as xbstream_log: + for line in xbstream_log: + # Ignore empty lines + if not line.strip(): + continue + + LOG.error('xbstream restore failed with: %s', + line.rstrip('\n')) + return False + + return True + + def post_restore(self): + """Prepare after data restore.""" + LOG.info("Running prepare command: %s.", self.prepare_command) + processutils.execute(self.prepare_command, shell=True) + + LOG.info("Checking prepare log") + with open(self.prepare_log, 'r') as prepare_log: + output = prepare_log.read() + if not output: + msg = "Empty prepare log file" + raise Exception(msg) + + last_line = output.splitlines()[-1].strip() + if not re.search('completed OK!', last_line): + msg = "Prepare did not complete successfully" + raise Exception(msg) + + +class XtraBackupIncremental(XtraBackup): + """XtraBackup incremental backup.""" + prepare_log = '/tmp/prepare.log' + incremental_prep = (f'xtrabackup --prepare --apply-log-only' + f' --target-dir=%(restore_location)s' + f' %(incremental_args)s' + f' 2>{prepare_log}') + + def __init__(self, *args, **kwargs): + if not kwargs.get('lsn'): + raise AttributeError('lsn attribute missing') + self.parent_location = kwargs.pop('parent_location', '') + self.parent_checksum = kwargs.pop('parent_checksum', '') + + super(XtraBackupIncremental, self).__init__(*args, **kwargs) + + @property + def cmd(self): + cmd = (f'xtrabackup --backup --stream=xbstream ' + f'--incremental --incremental-lsn=%(lsn)s ' + f'--datadir={self.datadir} {self.user_and_pass} ' + f'2>{self.backup_log}') + return cmd + self.zip_cmd + self.encrypt_cmd + + def get_metadata(self): + _meta = super(XtraBackupIncremental, self).get_metadata() + + _meta.update({ + 'parent_location': self.parent_location, + 'parent_checksum': self.parent_checksum, + }) + return _meta + + def run_restore(self): + """Run incremental restore. + + https://www.percona.com/doc/percona-xtrabackup/8.0/backup_scenarios/incremental_backup.html + """ + LOG.debug('Running incremental restore') + self.incremental_restore(self.location, self.checksum) + return self.restore_content_length diff --git a/backup/install.sh b/backup/install.sh index a2aad105ce..26f5a51e4a 100755 --- a/backup/install.sh +++ b/backup/install.sh @@ -5,12 +5,20 @@ export APT_KEY_DONT_WARN_ON_DANGEROUS_USAGE=1 APTOPTS="-y -qq --no-install-recommends --allow-unauthenticated" case "$1" in -"mysql") +"mysql5.7") curl -sSL https://repo.percona.com/apt/percona-release_latest.$(lsb_release -sc)_all.deb -o percona-release.deb dpkg -i percona-release.deb percona-release enable-only tools release apt-get update - apt-get install $APTOPTS percona-xtrabackup-$2 + apt-get install $APTOPTS percona-xtrabackup-24 + rm -f percona-release.deb + ;; +"mysql8.0") + curl -sSL https://repo.percona.com/apt/percona-release_latest.$(lsb_release -sc)_all.deb -o percona-release.deb + dpkg -i percona-release.deb + percona-release enable-only tools release + apt-get update + apt-get install $APTOPTS percona-xtrabackup-80 rm -f percona-release.deb ;; "mariadb") diff --git a/backup/main.py b/backup/main.py index a42dc4bad3..04b38ecad0 100644 --- a/backup/main.py +++ b/backup/main.py @@ -36,7 +36,7 @@ cli_opts = [ cfg.StrOpt( 'driver', default='innobackupex', - choices=['innobackupex', 'mariabackup', 'pg_basebackup'] + choices=['innobackupex', 'mariabackup', 'pg_basebackup', 'xtrabackup'] ), cfg.BoolOpt('backup'), cfg.StrOpt('backup-encryption-key'), @@ -68,6 +68,8 @@ driver_mapping = { 'mariabackup_inc': 'backup.drivers.mariabackup.MariaBackupIncremental', 'pg_basebackup': 'backup.drivers.postgres.PgBasebackup', 'pg_basebackup_inc': 'backup.drivers.postgres.PgBasebackupIncremental', + 'xtrabackup': 'backup.drivers.xtrabackup.XtraBackup', + 'xtrabackup_inc': 'backup.drivers.xtrabackup.XtraBackupIncremental' } storage_mapping = { 'swift': 'backup.storage.swift.SwiftStorage', diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 2a753f4be3..3d45bf844e 100644 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -476,7 +476,10 @@ function create_guest_image { glance_image_id=$(openstack --os-region-name RegionOne --os-password ${SERVICE_PASSWORD} \ --os-project-name service --os-username trove \ image create ${image_name} \ - --disk-format qcow2 --container-format bare --property hw_rng_model='virtio' --file ${image_file} \ + --disk-format qcow2 --container-format bare \ + --tag trove \ + --property hw_rng_model='virtio' \ + --file ${image_file} \ -c id -f value) echo "Register the image in datastore" diff --git a/integration/scripts/trovestack b/integration/scripts/trovestack index 1c121f9dde..4e6742a152 100755 --- a/integration/scripts/trovestack +++ b/integration/scripts/trovestack @@ -525,7 +525,7 @@ function cmd_set_datastore() { rd_manage datastore_update "$datastore" "" # trove-manage datastore_version_update - rd_manage datastore_version_update "${DATASTORE_TYPE}" "${DATASTORE_VERSION}" "${DATASTORE_TYPE}" $IMAGEID "" "" 1 + rd_manage datastore_version_update "${DATASTORE_TYPE}" "${DATASTORE_VERSION}" "${DATASTORE_TYPE}" ${IMAGEID} "trove" "" 1 rd_manage datastore_update "${DATASTORE_TYPE}" "${DATASTORE_VERSION}" if [[ -f "$PATH_TROVE"/trove/templates/${DATASTORE_TYPE}/validation-rules.json ]]; then @@ -766,7 +766,9 @@ function cmd_build_and_upload_image() { local output_dir=${5:-"$HOME/images"} name=trove-guest-${guest_os}-${guest_release} - glance_imageid=$(openstack ${CLOUD_ADMIN_ARG} image list --name $name -f value -c ID) + glance_imageid=$(openstack ${CLOUD_ADMIN_ARG} image list \ + --tag trove --sort created_at:desc \ + -f value -c ID | awk 'NR==1 {print}') if [[ -z ${glance_imageid} ]]; then mkdir -p ${output_dir} output=${output_dir}/${name}.qcow2 diff --git a/lower-constraints.txt b/lower-constraints.txt index 764efffa54..ebd0d7076d 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -133,6 +133,7 @@ requestsexceptions==1.4.0 restructuredtext-lint==1.1.3 rfc3986==1.1.0 Routes==2.3.1 +semantic-version==2.7.0 simplejson==3.13.2 smmap2==2.0.3 snowballstemmer==1.2.1 diff --git a/requirements.txt b/requirements.txt index e58dd2a5af..7738bcfc50 100644 --- a/requirements.txt +++ b/requirements.txt @@ -48,3 +48,4 @@ oslo.policy>=1.30.0 # Apache-2.0 diskimage-builder!=1.6.0,!=1.7.0,!=1.7.1,>=1.1.2 # Apache-2.0 docker>=4.2.0 # Apache-2.0 psycopg2-binary>=2.6.2 # LGPL/ZPL +semantic-version>=2.7.0 # BSD diff --git a/trove/common/cfg.py b/trove/common/cfg.py index 4d8d6078dc..7368010dad 100644 --- a/trove/common/cfg.py +++ b/trove/common/cfg.py @@ -617,7 +617,9 @@ mysql_opts = [ ), cfg.StrOpt( 'backup_docker_image', default='openstacktrove/db-backup-mysql:1.0.0', - help='The docker image used for backup and restore.' + help='The docker image used for backup and restore. For mysql, ' + 'the minor version is added to the image name as a suffix before ' + 'creating container, e.g. openstacktrove/db-backup-mysql5.7:1.0.0' ), ] diff --git a/trove/guestagent/common/sql_query.py b/trove/guestagent/common/sql_query.py index 2143668260..508a99907c 100644 --- a/trove/guestagent/common/sql_query.py +++ b/trove/guestagent/common/sql_query.py @@ -20,6 +20,7 @@ Do not hard-code strings into the guest agent; use this module to build them for you. """ +import semantic_version class Query(object): @@ -159,14 +160,6 @@ class Grant(object): def _user(self): return self.user or "" - @property - def _identity(self): - if self.clear: - return "IDENTIFIED BY '%s'" % self.clear - if self.hashed: - return "IDENTIFIED BY PASSWORD '%s'" % self.hashed - return "" - @property def _host(self): return self.host or "%" @@ -187,12 +180,7 @@ class Grant(object): @property def _whom(self): - # User and host to be granted permission. Optionally, password, too. - whom = [("TO %s" % self._user_host), - self._identity, - ] - whom = [w for w in whom if w] - return " ".join(whom) + return f"TO {self._user_host}" @property def _with(self): @@ -256,12 +244,7 @@ class Revoke(Grant): @property def _whom(self): # User and host from whom to revoke permission. - # Optionally, password, too. - whom = [("FROM %s" % self._user_host), - self._identity, - ] - whom = [w for w in whom if w] - return " ".join(whom) + return f"FROM {self._user_host}" class CreateDatabase(object): @@ -368,21 +351,32 @@ class RenameUser(object): class SetPassword(object): - - def __init__(self, user, host=None, new_password=None): + def __init__(self, user, host=None, new_password=None, ds=None, + ds_version=None): self.user = user self.host = host or '%' self.new_password = new_password or '' + self.ds = ds or 'mysql' + self.ds_version = ds_version or '5.7' def __repr__(self): return str(self) def __str__(self): - properties = {'user_name': self.user, - 'user_host': self.host, - 'new_password': self.new_password} - return ("SET PASSWORD FOR '%(user_name)s'@'%(user_host)s' = " - "PASSWORD('%(new_password)s');" % properties) + if self.ds == 'mysql': + cur_version = semantic_version.Version.coerce(self.ds_version) + mysql_575 = semantic_version.Version('5.7.5') + if cur_version <= mysql_575: + return (f"SET PASSWORD FOR '{self.user}'@'{self.host}' = " + f"PASSWORD('{self.new_password}');") + + return (f"ALTER USER '{self.user}'@'{self.host}' " + f"IDENTIFIED WITH mysql_native_password " + f"BY '{self.new_password}';") + elif self.ds == 'mariadb': + return (f"ALTER USER '{self.user}'@'{self.host}' IDENTIFIED VIA " + f"mysql_native_password USING " + f"PASSWORD('{self.new_password}');") class DropUser(object): diff --git a/trove/guestagent/datastore/mariadb/manager.py b/trove/guestagent/datastore/mariadb/manager.py index 87d502f8f8..59bb44a671 100644 --- a/trove/guestagent/datastore/mariadb/manager.py +++ b/trove/guestagent/datastore/mariadb/manager.py @@ -11,7 +11,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - from trove.guestagent.datastore.mariadb import service from trove.guestagent.datastore.mysql_common import manager from trove.guestagent.datastore.mysql_common import service as mysql_service @@ -24,3 +23,12 @@ class Manager(manager.MySqlManager): adm = service.MariaDBAdmin(app) super(Manager, self).__init__(app, status, adm) + + def get_start_db_params(self, data_dir): + """Get parameters for starting database. + + Cinder volume initialization(after formatted) may leave a lost+found + folder. + """ + return (f'--ignore-db-dir=lost+found --ignore-db-dir=conf.d ' + f'--datadir={data_dir}') diff --git a/trove/guestagent/datastore/mysql/manager.py b/trove/guestagent/datastore/mysql/manager.py index 6704984543..db8ae83a7d 100644 --- a/trove/guestagent/datastore/mysql/manager.py +++ b/trove/guestagent/datastore/mysql/manager.py @@ -11,10 +11,14 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import semantic_version +from trove.common import cfg from trove.guestagent.datastore.mysql import service from trove.guestagent.datastore.mysql_common import manager +CONF = cfg.CONF + class Manager(manager.MySqlManager): def __init__(self): @@ -23,3 +27,24 @@ class Manager(manager.MySqlManager): adm = service.MySqlAdmin(app) super(Manager, self).__init__(app, status, adm) + + def get_start_db_params(self, data_dir): + """Get parameters for starting database. + + Cinder volume initialization(after formatted) may leave a lost+found + folder. + + The --ignore-db-dir option is deprecated in MySQL 5.7. With the + introduction of the data dictionary in MySQL 8.0, it became + superfluous and was removed in that version. + """ + params = f'--datadir={data_dir}' + + mysql_8 = semantic_version.Version('8.0.0') + cur_ver = semantic_version.Version.coerce(CONF.datastore_version) + params = f'--datadir={data_dir}' + if cur_ver < mysql_8: + params = (f"{params} --ignore-db-dir=lost+found " + f"--ignore-db-dir=conf.d") + + return params diff --git a/trove/guestagent/datastore/mysql/service.py b/trove/guestagent/datastore/mysql/service.py index cbe59721e8..891996cee8 100644 --- a/trove/guestagent/datastore/mysql/service.py +++ b/trove/guestagent/datastore/mysql/service.py @@ -11,9 +11,14 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import semantic_version + +from trove.common import cfg from trove.guestagent.datastore.mysql_common import service from trove.guestagent.utils import mysql as mysql_util +CONF = cfg.CONF + class MySqlAppStatus(service.BaseMySqlAppStatus): def __init__(self, docker_client): @@ -55,6 +60,36 @@ class MySqlApp(service.BaseMySqlApp): client.execute("SELECT WAIT_UNTIL_SQL_THREAD_AFTER_GTIDS('%s')" % txn) + def get_backup_image(self): + """Get the actual container image based on datastore version. + + For example, this method converts openstacktrove/db-backup-mysql:1.0.0 + to openstacktrove/db-backup-mysql5.7:1.0.0 + """ + image = cfg.get_configuration_property('backup_docker_image') + name, tag = image.split(':', 1) + + # Get minor version + cur_ver = semantic_version.Version.coerce(CONF.datastore_version) + minor_ver = f"{cur_ver.major}.{cur_ver.minor}" + + return f"{name}{minor_ver}:{tag}" + + def get_backup_strategy(self): + """Get backup strategy. + + innobackupex was removed in Percona XtraBackup 8.0, use xtrabackup + instead. + """ + strategy = cfg.get_configuration_property('backup_strategy') + + mysql_8 = semantic_version.Version('8.0.0') + cur_ver = semantic_version.Version.coerce(CONF.datastore_version) + if cur_ver >= mysql_8: + strategy = 'xtrabackup' + + return strategy + class MySqlRootAccess(service.BaseMySqlRootAccess): def __init__(self, app): diff --git a/trove/guestagent/datastore/mysql_common/manager.py b/trove/guestagent/datastore/mysql_common/manager.py index 31172f2028..a81431a16c 100644 --- a/trove/guestagent/datastore/mysql_common/manager.py +++ b/trove/guestagent/datastore/mysql_common/manager.py @@ -60,6 +60,9 @@ class MySqlManager(manager.Manager): except Exception: return super(MySqlManager, self).get_service_status() + def get_start_db_params(self, data_dir): + return f'--datadir={data_dir}' + def do_prepare(self, context, packages, databases, memory_mb, users, device_path, mount_point, backup_info, config_contents, root_password, overrides, @@ -86,13 +89,7 @@ class MySqlManager(manager.Manager): data_dir=data_dir) # Start database service. - # Cinder volume initialization(after formatted) may leave a - # lost+found folder - # The --ignore-db-dir option is deprecated in MySQL 5.7. With the - # introduction of the data dictionary in MySQL 8.0, it became - # superfluous and was removed in that version. - command = (f'--ignore-db-dir=lost+found --ignore-db-dir=conf.d ' - f'--datadir={data_dir}') + command = self.get_start_db_params(data_dir) self.app.start_db(ds_version=ds_version, command=command) self.app.secure() @@ -315,13 +312,7 @@ class MySqlManager(manager.Manager): self.app.update_overrides(config_overrides) # Start database service. - # Cinder volume initialization(after formatted) may leave a - # lost+found folder - # The --ignore-db-dir option is deprecated in MySQL 5.7. With the - # introduction of the data dictionary in MySQL 8.0, it became - # superfluous and was removed in that version. - command = (f'--ignore-db-dir=lost+found --ignore-db-dir=conf.d ' - f'--datadir={data_dir}') + command = self.get_start_db_params(data_dir) self.app.start_db(ds_version=ds_version, command=command) except Exception as e: LOG.error(f"Failed to restore database service after rebuild, " diff --git a/trove/guestagent/datastore/mysql_common/service.py b/trove/guestagent/datastore/mysql_common/service.py index 6e29ec7b8a..d0f9236e25 100644 --- a/trove/guestagent/datastore/mysql_common/service.py +++ b/trove/guestagent/datastore/mysql_common/service.py @@ -126,9 +126,10 @@ class BaseMySqlAdmin(object, metaclass=abc.ABCMeta): '_host': item['host'], '_password': item['password']} user = models.MySQLUser.deserialize(user_dict) - LOG.debug("\tDeserialized: %s.", user.__dict__) uu = sql_query.SetPassword(user.name, host=user.host, - new_password=user.password) + new_password=user.password, + ds=CONF.datastore_manager, + ds_version=CONF.datastore_version) t = text(str(uu)) client.execute(t) @@ -142,13 +143,13 @@ class BaseMySqlAdmin(object, metaclass=abc.ABCMeta): new_password = user_attrs.get('password') if new_name or new_host or new_password: - with mysql_util.SqlClient(self.mysql_app.get_engine()) as client: - if new_password is not None: - uu = sql_query.SetPassword(user.name, host=user.host, - new_password=new_password) - + uu = sql_query.SetPassword( + user.name, host=user.host, + new_password=new_password, + ds=CONF.datastore_manager, + ds_version=CONF.datastore_version) t = text(str(uu)) client.execute(t) @@ -481,8 +482,10 @@ class BaseMySqlApp(service.BaseDbApp): # Ignore, user is already created, just reset the password # (user will already exist in a restore from backup) LOG.debug(err) - uu = sql_query.SetPassword(ADMIN_USER_NAME, host=host, - new_password=password) + uu = sql_query.SetPassword( + ADMIN_USER_NAME, host=host, new_password=password, + ds=CONF.datastore_manager, ds_version=CONF.datastore_version + ) t = text(str(uu)) client.execute(t) @@ -659,11 +662,11 @@ class BaseMySqlApp(service.BaseDbApp): def restore_backup(self, context, backup_info, restore_location): backup_id = backup_info['id'] storage_driver = CONF.storage_strategy - backup_driver = cfg.get_configuration_property('backup_strategy') + backup_driver = self.get_backup_strategy() user_token = context.auth_token auth_url = CONF.service_credentials.auth_url user_tenant = context.project_id - image = cfg.get_configuration_property('backup_docker_image') + image = self.get_backup_image() name = 'db_restore' volumes = {'/var/lib/mysql': {'bind': '/var/lib/mysql', 'mode': 'rw'}} @@ -834,8 +837,10 @@ class BaseMySqlRootAccess(object): # TODO(rnirmal): More fine grained error checking later on LOG.debug(err) with mysql_util.SqlClient(self.mysql_app.get_engine()) as client: - uu = sql_query.SetPassword(user.name, host=user.host, - new_password=user.password) + uu = sql_query.SetPassword( + user.name, host=user.host, new_password=user.password, + ds=CONF.datastore_manager, ds_version=CONF.datastore_version + ) t = text(str(uu)) client.execute(t) diff --git a/trove/guestagent/datastore/service.py b/trove/guestagent/datastore/service.py index f1c3c3ca69..d016ab5d9f 100644 --- a/trove/guestagent/datastore/service.py +++ b/trove/guestagent/datastore/service.py @@ -406,10 +406,16 @@ class BaseDbApp(object): self.reset_configuration(config_contents) self.start_db(update_db=True, ds_version=ds_version) + def get_backup_image(self): + return cfg.get_configuration_property('backup_docker_image') + + def get_backup_strategy(self): + return cfg.get_configuration_property('backup_strategy') + def create_backup(self, context, backup_info, volumes_mapping={}, need_dbuser=True, extra_params=''): storage_driver = CONF.storage_strategy - backup_driver = cfg.get_configuration_property('backup_strategy') + backup_driver = self.get_backup_strategy() incremental = '' backup_type = 'full' if backup_info.get('parent'): @@ -419,10 +425,9 @@ class BaseDbApp(object): f'--parent-checksum={backup_info["parent"]["checksum"]}') backup_type = 'incremental' - backup_id = backup_info["id"] - image = cfg.get_configuration_property('backup_docker_image') name = 'db_backup' - + backup_id = backup_info["id"] + image = self.get_backup_image() os_cred = (f"--os-token={context.auth_token} " f"--os-auth-url={CONF.service_credentials.auth_url} " f"--os-tenant-id={context.project_id}") diff --git a/trove/instance/models.py b/trove/instance/models.py index cc39279807..37e607c9a1 100644 --- a/trove/instance/models.py +++ b/trove/instance/models.py @@ -332,23 +332,29 @@ class SimpleInstance(object): @property def status(self): + LOG.info(f"Getting instance status for {self.id}, " + f"task status: {self.db_info.task_status}, " + f"datastore status: {self.datastore_status.status}, " + f"server status: {self.db_info.server_status}") + + task_status = self.db_info.task_status + server_status = self.db_info.server_status + ds_status = self.datastore_status.status + # Check for taskmanager errors. - if self.db_info.task_status.is_error: + if task_status.is_error: return InstanceStatus.ERROR - action = self.db_info.task_status.action + action = task_status.action # Check if we are resetting status or force deleting - if (srvstatus.ServiceStatuses.UNKNOWN == self.datastore_status.status - and action == InstanceTasks.DELETING.action): + if (srvstatus.ServiceStatuses.UNKNOWN == ds_status + and action == InstanceTasks.DELETING.action): return InstanceStatus.SHUTDOWN - elif (srvstatus.ServiceStatuses.UNKNOWN == - self.datastore_status.status): - return InstanceStatus.ERROR # Check for taskmanager status. if InstanceTasks.BUILDING.action == action: - if 'ERROR' == self.db_info.server_status: + if 'ERROR' == server_status: return InstanceStatus.ERROR return InstanceStatus.BUILD if InstanceTasks.REBOOTING.action == action: @@ -369,13 +375,12 @@ class SimpleInstance(object): return InstanceStatus.DETACH # Check for server status. - if self.db_info.server_status in ["BUILD", "ERROR", "REBOOT", - "RESIZE"]: - return self.db_info.server_status + if server_status in ["BUILD", "ERROR", "REBOOT", "RESIZE"]: + return server_status # As far as Trove is concerned, Nova instances in VERIFY_RESIZE should # still appear as though they are in RESIZE. - if self.db_info.server_status in ["VERIFY_RESIZE"]: + if server_status in ["VERIFY_RESIZE"]: return InstanceStatus.RESIZE # Check if there is a backup running for this instance @@ -384,23 +389,22 @@ class SimpleInstance(object): # Report as Shutdown while deleting, unless there's an error. if 'DELETING' == action: - if self.db_info.server_status in ["ACTIVE", "SHUTDOWN", "DELETED", - "HEALTHY"]: + if server_status in ["ACTIVE", "SHUTDOWN", "DELETED", "HEALTHY"]: return InstanceStatus.SHUTDOWN else: LOG.error("While shutting down instance (%(instance)s): " "server had status (%(status)s).", - {'instance': self.id, - 'status': self.db_info.server_status}) + {'instance': self.id, 'status': server_status}) return InstanceStatus.ERROR # Check against the service status. # The service is only paused during a reboot. - if srvstatus.ServiceStatuses.PAUSED == self.datastore_status.status: + if ds_status == srvstatus.ServiceStatuses.PAUSED: return InstanceStatus.REBOOT - # If the service status is NEW, then we are building. - if srvstatus.ServiceStatuses.NEW == self.datastore_status.status: + elif ds_status == srvstatus.ServiceStatuses.NEW: return InstanceStatus.BUILD + elif ds_status == srvstatus.ServiceStatuses.UNKNOWN: + return InstanceStatus.ERROR # For everything else we can look at the service status mapping. return self.datastore_status.status.api_status diff --git a/trove/templates/mysql/config.template b/trove/templates/mysql/config.template index b005c406e4..acfa5e04fc 100644 --- a/trove/templates/mysql/config.template +++ b/trove/templates/mysql/config.template @@ -11,6 +11,7 @@ nice = 0 port = 3306 basedir = /usr datadir = /var/lib/mysql/data +secure-file-priv = NULL tmpdir = /var/tmp pid-file = /var/run/mysqld/mysqld.pid socket = /var/run/mysqld/mysqld.sock diff --git a/trove/tests/api/configurations.py b/trove/tests/api/configurations.py index e01f13e095..ebb692adf0 100644 --- a/trove/tests/api/configurations.py +++ b/trove/tests/api/configurations.py @@ -269,7 +269,7 @@ class CreateConfigurations(ConfigurationsTestBase): @test def test_valid_configurations_create(self): - # create a configuration with valid parameters + """create a configuration with valid parameters from config.""" expected_configs = self.expected_default_datastore_configs() values = json.dumps(expected_configs.get('valid_values')) expected_values = json.loads(values) @@ -296,6 +296,7 @@ class CreateConfigurations(ConfigurationsTestBase): @test(runs_after=[test_valid_configurations_create]) def test_appending_to_existing_configuration(self): + """test_appending_to_existing_configuration""" # test being able to update and insert new parameter name and values # to an existing configuration expected_configs = self.expected_default_datastore_configs() diff --git a/trove/tests/api/users.py b/trove/tests/api/users.py index 6282a8b160..a1acc02929 100644 --- a/trove/tests/api/users.py +++ b/trove/tests/api/users.py @@ -199,8 +199,7 @@ class TestUsers(object): @test(depends_on=[test_create_users_list, test_delete_users]) def test_hostnames_make_users_unique(self): - # These tests rely on test_delete_users as they create users only - # they use. + """test_hostnames_make_users_unique.""" username = "testuser_unique" hostnames = ["192.168.0.1", "192.168.0.2"] users = [{"name": username, "password": "password", "databases": [], @@ -210,6 +209,7 @@ class TestUsers(object): # Nothing wrong with creating two users with the same name, so long # as their hosts are different. self.dbaas.users.create(instance_info.id, users) + for hostname in hostnames: self.dbaas.users.delete(instance_info.id, username, hostname=hostname) diff --git a/trove/tests/config.py b/trove/tests/config.py index 85181075ec..2b720226ee 100644 --- a/trove/tests/config.py +++ b/trove/tests/config.py @@ -98,7 +98,7 @@ class TestConfig(object): "valid_values": { "connect_timeout": 120, "local_infile": 0, - "collation_server": "latin1_swedish_ci" + "innodb_log_checksums": False }, "appending_values": { "join_buffer_size": 1048576,