Rationalize and clarify database configuration
Discussion in IRC, hangouts and summarized in email [1] led to a decision that in placement the use of a default data connection string was dangerous for people migrating from placement-in-nova to placement by itself. Instead what we want are explicit but small required actions for operators/deployers and a hard fail if they do not do them. A critical first step to make that work is removing the default connection string and requiring an explicit [placement_database]/connection value. To whit, this patch: * removes api_database configuration settings * removes the default connection value * make it the value required (which causes config parsing to fail) * updates test fixtures to remove knowledge of other databases besides 'placement_database' * adds a unit tests to confirm cfg.RequiredOptError is raised when expected (when the setting is not set) * Comments in the wsgi app that a hard fail will happen when config and required settings are missing In local testing I have confirmed two different hard failures that we want: * If there is no configuration file at all (either in the default /etc/placement/placement.conf location or the same file set by OS_PLACEMENT_CONFIG_DIR), fail * If there is a config file and it has no [placement_database]/connection, fail. Note that all these changes are not fully meaningful until the change in Ibed4c64fa4e74c830d0d2fa040089bf9dc7b8f29, which changes the placement/wsgi.py app. That's currently the only external entry point to using placement, and will not work until those changes are in place. [1] http://lists.openstack.org/pipermail/openstack-dev/2018-September/134444.html Change-Id: Iecf5006b32ca7bc2d46b7a59e78b4941a2e98122
This commit is contained in:
Chris Dent
parent
fb6c99feec
commit
cf586f599e
|
|
@ -17,81 +17,9 @@ from __future__ import absolute_import
|
|||
from oslo_config import cfg
|
||||
from oslo_db import options as oslo_db_options
|
||||
|
||||
from placement.conf import paths
|
||||
|
||||
_DEFAULT_SQL_CONNECTION = 'sqlite:///' + paths.state_path_def(
|
||||
'placement.sqlite')
|
||||
_ENRICHED = False
|
||||
|
||||
|
||||
# NOTE(markus_z): We cannot simply do:
|
||||
# conf.register_opts(oslo_db_options.database_opts, 'api_database')
|
||||
# If we reuse a db config option for two different groups ("api_database"
|
||||
# and "database") and deprecate or rename a config option in one of these
|
||||
# groups, "oslo.config" cannot correctly determine which one to update.
|
||||
# That's why we copied & pasted these config options for the "api_database"
|
||||
# group here. See commit ba407e3 ("Add support for multiple database engines")
|
||||
# for more details.
|
||||
api_db_group = cfg.OptGroup('api_database',
|
||||
title='API Database Options',
|
||||
help="""
|
||||
The *Nova API Database* is a separate database which is used for information
|
||||
which is used across *cells*. This database is mandatory since the Mitaka
|
||||
release (13.0.0).
|
||||
""")
|
||||
|
||||
api_db_opts = [
|
||||
# TODO(markus_z): This should probably have a required=True attribute
|
||||
cfg.StrOpt('connection',
|
||||
secret=True,
|
||||
help=''),
|
||||
cfg.StrOpt('connection_parameters',
|
||||
default='',
|
||||
help=''),
|
||||
cfg.BoolOpt('sqlite_synchronous',
|
||||
default=True,
|
||||
help=''),
|
||||
cfg.StrOpt('slave_connection',
|
||||
secret=True,
|
||||
help=''),
|
||||
cfg.StrOpt('mysql_sql_mode',
|
||||
default='TRADITIONAL',
|
||||
help=''),
|
||||
cfg.IntOpt('connection_recycle_time',
|
||||
default=3600,
|
||||
deprecated_name='idle_timeout',
|
||||
help=''),
|
||||
# TODO(markus_z): We should probably default this to 5 to not rely on the
|
||||
# SQLAlchemy default. Otherwise we wouldn't provide a stable default.
|
||||
cfg.IntOpt('max_pool_size',
|
||||
help=''),
|
||||
cfg.IntOpt('max_retries',
|
||||
default=10,
|
||||
help=''),
|
||||
# TODO(markus_z): This should have a minimum attribute of 0
|
||||
cfg.IntOpt('retry_interval',
|
||||
default=10,
|
||||
help=''),
|
||||
# TODO(markus_z): We should probably default this to 10 to not rely on the
|
||||
# SQLAlchemy default. Otherwise we wouldn't provide a stable default.
|
||||
cfg.IntOpt('max_overflow',
|
||||
help=''),
|
||||
# TODO(markus_z): This should probably make use of the "choices" attribute.
|
||||
# "oslo.db" uses only the values [<0, 0, 50, 100] see module
|
||||
# /oslo_db/sqlalchemy/engines.py method "_setup_logging"
|
||||
cfg.IntOpt('connection_debug',
|
||||
default=0,
|
||||
help=''),
|
||||
cfg.BoolOpt('connection_trace',
|
||||
default=False,
|
||||
help=''),
|
||||
# TODO(markus_z): We should probably default this to 30 to not rely on the
|
||||
# SQLAlchemy default. Otherwise we wouldn't provide a stable default.
|
||||
cfg.IntOpt('pool_timeout',
|
||||
help='')
|
||||
] # noqa
|
||||
|
||||
|
||||
def enrich_help_text(alt_db_opts):
|
||||
|
||||
def get_db_opts():
|
||||
|
|
@ -107,20 +35,30 @@ def enrich_help_text(alt_db_opts):
|
|||
# texts here if needed.
|
||||
alt_db_opt.help = db_opt.help + alt_db_opt.help
|
||||
|
||||
# NOTE(cdent): See the note above on api_db_group. The same issues
|
||||
# apply here.
|
||||
|
||||
# NOTE(markus_z): We cannot simply do:
|
||||
# conf.register_opts(oslo_db_options.database_opts, 'placement_database')
|
||||
# If we reuse a db config option for two different groups ("placement_database"
|
||||
# and "database") and deprecate or rename a config option in one of these
|
||||
# groups, "oslo.config" cannot correctly determine which one to update.
|
||||
# That's why we copied & pasted these config options for the
|
||||
# "placement_database" group here. See nova commit ba407e3 ("Add support
|
||||
# for multiple database engines") for more details.
|
||||
# TODO(cdent): Consider our future options of using 'database' instead of
|
||||
# 'placement_database' for the group. This is already loose in the wild,
|
||||
# explicit, and safe if there will ever be more than one database, so may
|
||||
# be good to leave it.
|
||||
placement_db_group = cfg.OptGroup('placement_database',
|
||||
title='Placement API database options',
|
||||
help="""
|
||||
The *Placement API Database* is a separate database which can be used with the
|
||||
placement service. This database is optional: if the connection option is not
|
||||
set, the nova api database will be used instead.
|
||||
The *Placement API Database* is a the database used with the placement
|
||||
service. If the connection option is not set, the placement service will
|
||||
not start.
|
||||
""")
|
||||
|
||||
placement_db_opts = [
|
||||
cfg.StrOpt('connection',
|
||||
help='',
|
||||
required=True,
|
||||
secret=True),
|
||||
cfg.StrOpt('connection_parameters',
|
||||
default='',
|
||||
|
|
@ -159,8 +97,6 @@ placement_db_opts = [
|
|||
|
||||
|
||||
def register_opts(conf):
|
||||
oslo_db_options.set_defaults(conf, connection=_DEFAULT_SQL_CONNECTION)
|
||||
conf.register_opts(api_db_opts, group=api_db_group)
|
||||
conf.register_opts(placement_db_opts, group=placement_db_group)
|
||||
|
||||
|
||||
|
|
@ -174,10 +110,8 @@ def list_opts():
|
|||
# here.
|
||||
global _ENRICHED
|
||||
if not _ENRICHED:
|
||||
enrich_help_text(api_db_opts)
|
||||
enrich_help_text(placement_db_opts)
|
||||
_ENRICHED = True
|
||||
return {
|
||||
api_db_group: api_db_opts,
|
||||
placement_db_group: placement_db_opts,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,10 +13,11 @@
|
|||
own file so the nova db_api (which has cascading imports) is not imported.
|
||||
"""
|
||||
|
||||
|
||||
from oslo_db.sqlalchemy import enginefacade
|
||||
from oslo_log import log as logging
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
placement_context_manager = enginefacade.transaction_context()
|
||||
|
||||
|
||||
|
|
@ -25,14 +26,8 @@ def _get_db_conf(conf_group):
|
|||
|
||||
|
||||
def configure(conf):
|
||||
# If [placement_database]/connection is not set in conf, then placement
|
||||
# data will be stored in the nova_api database.
|
||||
if conf.placement_database.connection is None:
|
||||
placement_context_manager.configure(
|
||||
**_get_db_conf(conf.api_database))
|
||||
else:
|
||||
placement_context_manager.configure(
|
||||
**_get_db_conf(conf.placement_database))
|
||||
placement_context_manager.configure(
|
||||
**_get_db_conf(conf.placement_database))
|
||||
|
||||
|
||||
def get_placement_engine():
|
||||
|
|
|
|||
|
|
@ -39,13 +39,10 @@ class TestCase(testtools.TestCase):
|
|||
|
||||
# Manage required configuration
|
||||
conf_fixture = self.useFixture(config_fixture.Config(CONF))
|
||||
# The Database fixture will get confused if only one of the databases
|
||||
# is configured.
|
||||
for group in ('placement_database', 'api_database', 'database'):
|
||||
conf_fixture.config(
|
||||
group=group,
|
||||
connection='sqlite://',
|
||||
sqlite_synchronous=False)
|
||||
conf_fixture.config(
|
||||
group='placement_database',
|
||||
connection='sqlite://',
|
||||
sqlite_synchronous=False)
|
||||
CONF([], default_config_files=[])
|
||||
|
||||
self.useFixture(policy_fixture.PolicyFixture())
|
||||
|
|
|
|||
|
|
@ -0,0 +1,32 @@
|
|||
# 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 testtools
|
||||
|
||||
from oslo_config import cfg
|
||||
from oslo_config import fixture as config_fixture
|
||||
|
||||
|
||||
CONF = cfg.CONF
|
||||
|
||||
|
||||
class TestPlacementDBConf(testtools.TestCase):
|
||||
"""Test cases for Placement DB Setup."""
|
||||
|
||||
def setUp(self):
|
||||
super(TestPlacementDBConf, self).setUp()
|
||||
self.conf_fixture = self.useFixture(config_fixture.Config(CONF))
|
||||
|
||||
def test_missing_config_raises(self):
|
||||
self.assertRaises(
|
||||
cfg.RequiredOptError, self.conf_fixture.conf,
|
||||
[], default_config_files=[])
|
||||
|
|
@ -34,6 +34,9 @@ class PlacementPolicyTestCase(testtools.TestCase):
|
|||
self.conf = self.useFixture(config_fixture.Config(CONF)).conf
|
||||
self.ctxt = context.RequestContext(user_id='fake', project_id='fake')
|
||||
self.target = {'user_id': 'fake', 'project_id': 'fake'}
|
||||
# A value is required in the database connection opt for CONF to
|
||||
# parse.
|
||||
CONF.set_default('connection', 'stub', group='placement_database')
|
||||
CONF([], default_config_files=[])
|
||||
|
||||
def test_modified_policy_reloads(self):
|
||||
|
|
|
|||
|
|
@ -99,12 +99,19 @@ def _set_middleware_defaults():
|
|||
def init_application():
|
||||
# initialize the config system
|
||||
conffile = _get_config_file()
|
||||
# This will raise cfg.ConfigFilesNotFoundError and cfg.RequiredOptError
|
||||
# when either conffile is not there or some required option is not set
|
||||
# (notably the database connection string). We want both of these to
|
||||
# be a hard fail and prevent the application from starting so we hard
|
||||
# fail here. The error will show up in the wsgi server's logs and the
|
||||
# app will not start.
|
||||
_parse_args([], default_config_files=[conffile])
|
||||
db_api.configure(conf.CONF)
|
||||
|
||||
# initialize the logging system
|
||||
setup_logging(conf.CONF)
|
||||
|
||||
# configure database
|
||||
db_api.configure(conf.CONF)
|
||||
|
||||
# dump conf at debug if log_options
|
||||
if conf.CONF.log_options:
|
||||
conf.CONF.log_opt_values(
|
||||
|
|
|
|||
Loading…
Reference in New Issue