From cebee2c4d715b83e86b1b06e4960e4ae471700ec Mon Sep 17 00:00:00 2001 From: Dantali0n Date: Wed, 26 Jun 2019 06:44:56 +0000 Subject: [PATCH] improve OptGroup consistency across configuration This is a follow-up to: https://review.opendev.org/#/c/666897/ and makes sure titles and help information get rendered in the configuration documentation and configuration samples. The options for the placement_client group are already changed and left untouched as a result. The changes to grafana_client are already done in another patch and also untouched. Change-Id: Ia33cd4576e4b55e651f3f3779a01f2867126138d --- watcher/conf/_opts.py | 30 ++++++++++++---------- watcher/conf/api.py | 2 +- watcher/conf/applier.py | 2 +- watcher/conf/ceilometer_client.py | 2 +- watcher/conf/cinder_client.py | 2 +- watcher/conf/clients_auth.py | 2 +- watcher/conf/collector.py | 2 +- watcher/conf/datasources.py | 2 +- watcher/conf/db.py | 2 +- watcher/conf/decision_engine.py | 11 +++------ watcher/conf/glance_client.py | 2 +- watcher/conf/gnocchi_client.py | 2 +- watcher/conf/grafana_translators.py | 2 +- watcher/conf/ironic_client.py | 2 +- watcher/conf/keystone_client.py | 2 +- watcher/conf/monasca_client.py | 2 +- watcher/conf/neutron_client.py | 2 +- watcher/conf/nova_client.py | 2 +- watcher/conf/planner.py | 2 +- watcher/tests/conf/test_list_opts.py | 37 +++++++++++++++++++--------- 20 files changed, 62 insertions(+), 50 deletions(-) diff --git a/watcher/conf/_opts.py b/watcher/conf/_opts.py index 250c6f03e..2616701bb 100644 --- a/watcher/conf/_opts.py +++ b/watcher/conf/_opts.py @@ -22,6 +22,7 @@ from watcher.conf import api as conf_api from watcher.conf import applier as conf_applier from watcher.conf import ceilometer_client as conf_ceilometer_client from watcher.conf import cinder_client as conf_cinder_client +from watcher.conf import clients_auth as conf_auth from watcher.conf import db from watcher.conf import decision_engine as conf_de from watcher.conf import exception @@ -39,19 +40,22 @@ def list_opts(): (conf_api.AUTH_OPTS + exception.EXC_LOG_OPTS + paths.PATH_OPTS)), - ('api', conf_api.API_SERVICE_OPTS), - ('database', db.SQL_OPTS), - ('watcher_planner', conf_planner.WATCHER_PLANNER_OPTS), - ('watcher_applier', conf_applier.APPLIER_MANAGER_OPTS), - ('watcher_decision_engine', - (conf_de.WATCHER_DECISION_ENGINE_OPTS + - conf_de.WATCHER_CONTINUOUS_OPTS)), - ('nova_client', conf_nova_client.NOVA_CLIENT_OPTS), - ('glance_client', conf_glance_client.GLANCE_CLIENT_OPTS), - ('cinder_client', conf_cinder_client.CINDER_CLIENT_OPTS), - ('ceilometer_client', conf_ceilometer_client.CEILOMETER_CLIENT_OPTS), - ('neutron_client', conf_neutron_client.NEUTRON_CLIENT_OPTS), - ('watcher_clients_auth', + (conf_api.api, conf_api.API_SERVICE_OPTS), + (db.database, db.SQL_OPTS), + (conf_planner.watcher_planner, conf_planner.WATCHER_PLANNER_OPTS), + (conf_applier.watcher_applier, conf_applier.APPLIER_MANAGER_OPTS), + (conf_de.watcher_decision_engine, + (conf_de.WATCHER_DECISION_ENGINE_OPTS)), + (conf_nova_client.nova_client, conf_nova_client.NOVA_CLIENT_OPTS), + (conf_glance_client.glance_client, + conf_glance_client.GLANCE_CLIENT_OPTS), + (conf_cinder_client.cinder_client, + conf_cinder_client.CINDER_CLIENT_OPTS), + (conf_ceilometer_client.ceilometer_client, + conf_ceilometer_client.CEILOMETER_CLIENT_OPTS), + (conf_neutron_client.neutron_client, + conf_neutron_client.NEUTRON_CLIENT_OPTS), + (conf_auth.WATCHER_CLIENTS_AUTH, (ka_loading.get_auth_common_conf_options() + ka_loading.get_auth_plugin_conf_options('password') + ka_loading.get_session_conf_options())) diff --git a/watcher/conf/api.py b/watcher/conf/api.py index 4531eca8e..0fb48beab 100644 --- a/watcher/conf/api.py +++ b/watcher/conf/api.py @@ -65,4 +65,4 @@ def register_opts(conf): def list_opts(): - return [('api', API_SERVICE_OPTS), ('DEFAULT', AUTH_OPTS)] + return [(api, API_SERVICE_OPTS), ('DEFAULT', AUTH_OPTS)] diff --git a/watcher/conf/applier.py b/watcher/conf/applier.py index 8aa6adc60..818540d45 100644 --- a/watcher/conf/applier.py +++ b/watcher/conf/applier.py @@ -50,4 +50,4 @@ def register_opts(conf): def list_opts(): - return [('watcher_applier', APPLIER_MANAGER_OPTS)] + return [(watcher_applier, APPLIER_MANAGER_OPTS)] diff --git a/watcher/conf/ceilometer_client.py b/watcher/conf/ceilometer_client.py index 6d78a2662..efe12ba4f 100644 --- a/watcher/conf/ceilometer_client.py +++ b/watcher/conf/ceilometer_client.py @@ -60,4 +60,4 @@ def register_opts(conf): def list_opts(): - return [('ceilometer_client', CEILOMETER_CLIENT_OPTS)] + return [(ceilometer_client, CEILOMETER_CLIENT_OPTS)] diff --git a/watcher/conf/cinder_client.py b/watcher/conf/cinder_client.py index a06e41a05..bb72c53b0 100644 --- a/watcher/conf/cinder_client.py +++ b/watcher/conf/cinder_client.py @@ -41,4 +41,4 @@ def register_opts(conf): def list_opts(): - return [('cinder_client', CINDER_CLIENT_OPTS)] + return [(cinder_client, CINDER_CLIENT_OPTS)] diff --git a/watcher/conf/clients_auth.py b/watcher/conf/clients_auth.py index 8e959fcdf..257d1abd1 100644 --- a/watcher/conf/clients_auth.py +++ b/watcher/conf/clients_auth.py @@ -27,5 +27,5 @@ def register_opts(conf): def list_opts(): - return [('watcher_clients_auth', ka_loading.get_session_conf_options() + + return [(WATCHER_CLIENTS_AUTH, ka_loading.get_session_conf_options() + ka_loading.get_auth_common_conf_options())] diff --git a/watcher/conf/collector.py b/watcher/conf/collector.py index 63762c459..a585f6af9 100644 --- a/watcher/conf/collector.py +++ b/watcher/conf/collector.py @@ -45,4 +45,4 @@ def register_opts(conf): def list_opts(): - return [('collector', COLLECTOR_OPTS)] + return [(collector, COLLECTOR_OPTS)] diff --git a/watcher/conf/datasources.py b/watcher/conf/datasources.py index 21b6da0a7..6c2751288 100644 --- a/watcher/conf/datasources.py +++ b/watcher/conf/datasources.py @@ -56,4 +56,4 @@ def register_opts(conf): def list_opts(): - return [('watcher_datasources', DATASOURCES_OPTS)] + return [(datasources, DATASOURCES_OPTS)] diff --git a/watcher/conf/db.py b/watcher/conf/db.py index 1f22c1e09..53de882a0 100644 --- a/watcher/conf/db.py +++ b/watcher/conf/db.py @@ -41,4 +41,4 @@ def register_opts(conf): def list_opts(): - return [('database', SQL_OPTS)] + return [(database, SQL_OPTS)] diff --git a/watcher/conf/decision_engine.py b/watcher/conf/decision_engine.py index 962fd8cde..51a2b2313 100644 --- a/watcher/conf/decision_engine.py +++ b/watcher/conf/decision_engine.py @@ -68,24 +68,19 @@ WATCHER_DECISION_ENGINE_OPTS = [ ' instance_cpu_usage: VM_CPU\n' ' gnocchi:\n' ' instance_cpu_usage: cpu_vm_util\n\n' - 'This file is optional.')] - -WATCHER_CONTINUOUS_OPTS = [ + 'This file is optional.'), cfg.IntOpt('continuous_audit_interval', default=10, mutable=True, help='Interval (in seconds) for checking newly created ' - 'continuous audits.') -] + 'continuous audits.')] def register_opts(conf): conf.register_group(watcher_decision_engine) conf.register_opts(WATCHER_DECISION_ENGINE_OPTS, group=watcher_decision_engine) - conf.register_opts(WATCHER_CONTINUOUS_OPTS, group=watcher_decision_engine) def list_opts(): - return [('watcher_decision_engine', WATCHER_DECISION_ENGINE_OPTS), - ('watcher_decision_engine', WATCHER_CONTINUOUS_OPTS)] + return [(watcher_decision_engine, WATCHER_DECISION_ENGINE_OPTS)] diff --git a/watcher/conf/glance_client.py b/watcher/conf/glance_client.py index a6d96adc2..801e40bfb 100644 --- a/watcher/conf/glance_client.py +++ b/watcher/conf/glance_client.py @@ -41,4 +41,4 @@ def register_opts(conf): def list_opts(): - return [('glance_client', GLANCE_CLIENT_OPTS)] + return [(glance_client, GLANCE_CLIENT_OPTS)] diff --git a/watcher/conf/gnocchi_client.py b/watcher/conf/gnocchi_client.py index 592ef7459..d7950d57e 100644 --- a/watcher/conf/gnocchi_client.py +++ b/watcher/conf/gnocchi_client.py @@ -42,4 +42,4 @@ def register_opts(conf): def list_opts(): - return [('gnocchi_client', GNOCCHI_CLIENT_OPTS)] + return [(gnocchi_client, GNOCCHI_CLIENT_OPTS)] diff --git a/watcher/conf/grafana_translators.py b/watcher/conf/grafana_translators.py index 8755eee9b..f8920e5f9 100644 --- a/watcher/conf/grafana_translators.py +++ b/watcher/conf/grafana_translators.py @@ -41,4 +41,4 @@ def register_opts(conf): def list_opts(): - return [('grafana_translators', GRAFANA_TRANSLATOR_INFLUX_OPTS)] + return [(grafana_translators, GRAFANA_TRANSLATOR_INFLUX_OPTS)] diff --git a/watcher/conf/ironic_client.py b/watcher/conf/ironic_client.py index c97043eb5..1cb64a447 100755 --- a/watcher/conf/ironic_client.py +++ b/watcher/conf/ironic_client.py @@ -41,4 +41,4 @@ def register_opts(conf): def list_opts(): - return [('ironic_client', IRONIC_CLIENT_OPTS)] + return [(ironic_client, IRONIC_CLIENT_OPTS)] diff --git a/watcher/conf/keystone_client.py b/watcher/conf/keystone_client.py index e2ada3861..3de4f699a 100644 --- a/watcher/conf/keystone_client.py +++ b/watcher/conf/keystone_client.py @@ -35,4 +35,4 @@ def register_opts(conf): def list_opts(): - return [('keystone_client', KEYSTONE_CLIENT_OPTS)] + return [(keystone_client, KEYSTONE_CLIENT_OPTS)] diff --git a/watcher/conf/monasca_client.py b/watcher/conf/monasca_client.py index 3e0094ff7..de2ecd7d3 100644 --- a/watcher/conf/monasca_client.py +++ b/watcher/conf/monasca_client.py @@ -41,4 +41,4 @@ def register_opts(conf): def list_opts(): - return [('monasca_client', MONASCA_CLIENT_OPTS)] + return [(monasca_client, MONASCA_CLIENT_OPTS)] diff --git a/watcher/conf/neutron_client.py b/watcher/conf/neutron_client.py index 94b49c378..e203b4a90 100644 --- a/watcher/conf/neutron_client.py +++ b/watcher/conf/neutron_client.py @@ -41,4 +41,4 @@ def register_opts(conf): def list_opts(): - return [('neutron_client', NEUTRON_CLIENT_OPTS)] + return [(neutron_client, NEUTRON_CLIENT_OPTS)] diff --git a/watcher/conf/nova_client.py b/watcher/conf/nova_client.py index 952130e25..07c1dbed6 100755 --- a/watcher/conf/nova_client.py +++ b/watcher/conf/nova_client.py @@ -52,4 +52,4 @@ def register_opts(conf): def list_opts(): - return [('nova_client', NOVA_CLIENT_OPTS)] + return [(nova_client, NOVA_CLIENT_OPTS)] diff --git a/watcher/conf/planner.py b/watcher/conf/planner.py index 1386c2f89..45e12dfd1 100644 --- a/watcher/conf/planner.py +++ b/watcher/conf/planner.py @@ -38,4 +38,4 @@ def register_opts(conf): def list_opts(): - return [('watcher_planner', WATCHER_PLANNER_OPTS)] + return [(watcher_planner, WATCHER_PLANNER_OPTS)] diff --git a/watcher/tests/conf/test_list_opts.py b/watcher/tests/conf/test_list_opts.py index fb478c83e..192d9a57c 100755 --- a/watcher/tests/conf/test_list_opts.py +++ b/watcher/tests/conf/test_list_opts.py @@ -28,6 +28,11 @@ from watcher.tests.decision_engine import fake_strategies class TestListOpts(base.TestCase): def setUp(self): super(TestListOpts, self).setUp() + # These option groups will be registered using strings instead of + # OptGroup objects this should be avoided if possible. + self.none_objects = ['DEFAULT', 'watcher_clients_auth', + 'watcher_strategies.strategy_1'] + self.base_sections = [ 'DEFAULT', 'api', 'database', 'watcher_decision_engine', 'watcher_applier', 'watcher_datasources', 'watcher_planner', @@ -38,6 +43,24 @@ class TestListOpts(base.TestCase): 'placement_client'] self.opt_sections = list(dict(opts.list_opts()).keys()) + def _assert_name_or_group(self, actual_sections, expected_sections): + for name_or_group, options in actual_sections: + section_name = name_or_group + if isinstance(name_or_group, cfg.OptGroup): + section_name = name_or_group.name + elif section_name in self.none_objects: + pass + else: + # All option groups should be added to list_otps with an + # OptGroup object for some exceptions this is not possible but + # new groups should use OptGroup + raise Exception( + "Invalid option group: {0} should be of type OptGroup not " + "string.".format(section_name)) + + self.assertIn(section_name, expected_sections) + self.assertTrue(len(options)) + def test_run_list_opts(self): expected_sections = self.opt_sections @@ -73,14 +96,9 @@ class TestListOpts(base.TestCase): with mock.patch.object(extension, "ExtensionManager") as m_ext_manager: m_ext_manager.side_effect = m_list_available result = opts.list_opts() + self._assert_name_or_group(result, expected_sections) self.assertIsNotNone(result) - for name_or_group, options in result: - section_name = name_or_group - if isinstance(name_or_group, cfg.OptGroup): - section_name = name_or_group.name - self.assertIn(section_name, expected_sections) - self.assertTrue(len(options)) def test_list_opts_with_opts(self): expected_sections = self.base_sections + [ @@ -110,12 +128,7 @@ class TestListOpts(base.TestCase): result = opts.list_opts() self.assertIsNotNone(result) - for name_or_group, options in result: - section_name = name_or_group - if isinstance(name_or_group, cfg.OptGroup): - section_name = name_or_group.name - self.assertIn(section_name, expected_sections) - self.assertTrue(len(options)) + self._assert_name_or_group(result, expected_sections) result_map = dict(result) strategy_opts = result_map['watcher_strategies.strategy_1']