diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c042c845ed..40cfaf3e5a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -188,6 +188,12 @@ Changed Contributed by @Kami. +* Some of the config option registration code has been refactored to ignore "option already + registered" errors. That was done as a work around for an occasional race in the tests and + also to make all of the config option registration code expose the same consistent API. #5234 + + Contributed by @Kami. + Improvements ~~~~~~~~~~~~ diff --git a/contrib/runners/action_chain_runner/tests/unit/test_actionchain_notifications.py b/contrib/runners/action_chain_runner/tests/unit/test_actionchain_notifications.py index 20dca00f0a..d74efa37f2 100644 --- a/contrib/runners/action_chain_runner/tests/unit/test_actionchain_notifications.py +++ b/contrib/runners/action_chain_runner/tests/unit/test_actionchain_notifications.py @@ -156,7 +156,7 @@ def test_skip_notify_for_task_with_notify(self): ) self.assertIsNone(task1_live.notify) - execution = self._wait_for_children(execution, retries=300) + execution = self._wait_for_children(execution, expected_children=2, retries=300) self.assertEqual(len(execution.children), 2) # Assert task2 notify is not skipped @@ -194,7 +194,7 @@ def test_skip_notify_default_for_task_with_notify(self): ) self.assertEqual(notify, MOCK_NOTIFY) - execution = self._wait_for_children(execution, retries=300) + execution = self._wait_for_children(execution, expected_children=2, retries=300) self.assertEqual(len(execution.children), 2) # Assert task2 notify is not skipped by default. @@ -203,12 +203,31 @@ def test_skip_notify_default_for_task_with_notify(self): self.assertIsNone(task2_live.notify) MockLiveActionPublisherNonBlocking.wait_all() - def _wait_for_children(self, execution, interval=0.1, retries=100): + def _wait_for_children( + self, execution, expected_children=1, interval=0.1, retries=100 + ): # Wait until the execution has children. for i in range(0, retries): execution = ActionExecution.get_by_id(str(execution.id)) - if len(getattr(execution, "children", [])) <= 0: - eventlet.sleep(interval) - continue + found_children = len(getattr(execution, "children", [])) + + if found_children == expected_children: + return execution + + if found_children > expected_children: + raise AssertionError( + "Expected %s children, but got %s" + % (expected_children, found_children) + ) + + eventlet.sleep(interval) + + found_children = len(getattr(execution, "children", [])) + + if found_children != expected_children: + raise AssertionError( + "Expected %s children, but got %s after %s retry attempts" + % (expected_children, found_children, retries) + ) return execution diff --git a/st2actions/st2actions/config.py b/st2actions/st2actions/config.py index 14dc2c4f58..3ed43c3c6e 100644 --- a/st2actions/st2actions/config.py +++ b/st2actions/st2actions/config.py @@ -35,16 +35,16 @@ def parse_args(args=None): ) -def register_opts(): - _register_common_opts() +def register_opts(ignore_errors=False): + _register_common_opts(ignore_errors=ignore_errors) -def _register_common_opts(): - common_config.register_opts() +def _register_common_opts(ignore_errors=False): + common_config.register_opts(ignore_errors=ignore_errors) def get_logging_config_path(): return CONF.actionrunner.logging -register_opts() +register_opts(ignore_errors=True) diff --git a/st2actions/st2actions/notifier/config.py b/st2actions/st2actions/notifier/config.py index 0322179bbc..e5cb03366e 100644 --- a/st2actions/st2actions/notifier/config.py +++ b/st2actions/st2actions/notifier/config.py @@ -34,20 +34,20 @@ def parse_args(args=None): ) -def register_opts(): - _register_common_opts() - _register_notifier_opts() +def register_opts(ignore_errors=False): + _register_common_opts(ignore_errors=ignore_errors) + _register_notifier_opts(ignore_errors=ignore_errors) def get_logging_config_path(): return cfg.CONF.notifier.logging -def _register_common_opts(): +def _register_common_opts(ignore_errors=False): common_config.register_opts() -def _register_notifier_opts(): +def _register_notifier_opts(ignore_errors=False): notifier_opts = [ cfg.StrOpt( "logging", @@ -56,7 +56,9 @@ def _register_notifier_opts(): ) ] - CONF.register_opts(notifier_opts, group="notifier") + common_config.do_register_opts( + notifier_opts, group="notifier", ignore_errors=ignore_errors + ) -register_opts() +register_opts(ignore_errors=True) diff --git a/st2actions/st2actions/scheduler/config.py b/st2actions/st2actions/scheduler/config.py index 8df6c3ff3e..035c030a93 100644 --- a/st2actions/st2actions/scheduler/config.py +++ b/st2actions/st2actions/scheduler/config.py @@ -34,20 +34,20 @@ def parse_args(args=None): ) -def register_opts(): - _register_common_opts() - _register_service_opts() +def register_opts(ignore_errors=False): + _register_common_opts(ignore_errors=ignore_errors) + _register_service_opts(ignore_errors=ignore_errors) def get_logging_config_path(): return cfg.CONF.scheduler.logging -def _register_common_opts(): - common_config.register_opts() +def _register_common_opts(ignore_errors=False): + common_config.register_opts(ignore_errors=ignore_errors) -def _register_service_opts(): +def _register_service_opts(ignore_errors=False): scheduler_opts = [ cfg.StrOpt( "logging", @@ -88,10 +88,9 @@ def _register_service_opts(): ), ] - cfg.CONF.register_opts(scheduler_opts, group="scheduler") + common_config.do_register_opts( + scheduler_opts, group="scheduler", ignore_errors=ignore_errors + ) -try: - register_opts() -except cfg.DuplicateOptError: - LOG.exception("The scheduler configuration options are already parsed and loaded.") +register_opts(ignore_errors=True) diff --git a/st2actions/st2actions/workflows/config.py b/st2actions/st2actions/workflows/config.py index 6854323ddd..bbfa5b22a1 100644 --- a/st2actions/st2actions/workflows/config.py +++ b/st2actions/st2actions/workflows/config.py @@ -30,20 +30,20 @@ def parse_args(args=None): ) -def register_opts(): - _register_common_opts() - _register_service_opts() +def register_opts(ignore_errors=False): + _register_common_opts(ignore_errors=ignore_errors) + _register_service_opts(ignore_errors=ignore_errors) def get_logging_config_path(): return cfg.CONF.workflow_engine.logging -def _register_common_opts(): - common_config.register_opts() +def _register_common_opts(ignore_errors=False): + common_config.register_opts(ignore_errors=ignore_errors) -def _register_service_opts(): +def _register_service_opts(ignore_errors=False): wf_engine_opts = [ cfg.StrOpt( "logging", @@ -52,7 +52,7 @@ def _register_service_opts(): ) ] - cfg.CONF.register_opts(wf_engine_opts, group="workflow_engine") + common_config.do_register_opts(wf_engine_opts, group="workflow_engine") -register_opts() +register_opts(ignore_errors=True) diff --git a/st2api/st2api/config.py b/st2api/st2api/config.py index 35a21d87d5..5009763cdf 100644 --- a/st2api/st2api/config.py +++ b/st2api/st2api/config.py @@ -39,20 +39,20 @@ def parse_args(args=None): ) -def register_opts(): - _register_common_opts() - _register_app_opts() +def register_opts(ignore_errors=False): + _register_common_opts(ignore_errors=ignore_errors) + _register_app_opts(ignore_errors=ignore_errors) -def _register_common_opts(): - common_config.register_opts() +def _register_common_opts(ignore_errors=False): + common_config.register_opts(ignore_errors=ignore_errors) def get_logging_config_path(): return cfg.CONF.api.logging -def _register_app_opts(): +def _register_app_opts(ignore_errors=False): # Note "host", "port", "allow_origin", "mask_secrets" options are registered as part of # st2common config since they are also used outside st2api static_root = os.path.join(cfg.CONF.system.base_path, "static") @@ -72,7 +72,9 @@ def _register_app_opts(): cfg.DictOpt("errors", default={"__force_dict__": True}), ] - CONF.register_opts(pecan_opts, group="api_pecan") + common_config.do_register_opts( + pecan_opts, group="api_pecan", ignore_errors=ignore_errors + ) logging_opts = [ cfg.BoolOpt("debug", default=False), @@ -89,4 +91,6 @@ def _register_app_opts(): ), ] - CONF.register_opts(logging_opts, group="api") + common_config.do_register_opts( + logging_opts, group="api", ignore_errors=ignore_errors + ) diff --git a/st2auth/st2auth/config.py b/st2auth/st2auth/config.py index dee0d2d064..bd57db956f 100644 --- a/st2auth/st2auth/config.py +++ b/st2auth/st2auth/config.py @@ -35,20 +35,20 @@ def parse_args(args=None): ) -def register_opts(): - _register_common_opts() - _register_app_opts() +def register_opts(ignore_errors=False): + _register_common_opts(ignore_errors=ignore_errors) + _register_app_opts(ignore_errors=ignore_errors) def get_logging_config_path(): return cfg.CONF.auth.logging -def _register_common_opts(): - st2cfg.register_opts() +def _register_common_opts(ignore_errors=False): + st2cfg.register_opts(ignore_errors=ignore_errors) -def _register_app_opts(): +def _register_app_opts(ignore_errors=False): available_backends = auth_backends.get_available_backends() auth_opts = [ @@ -110,4 +110,4 @@ def _register_app_opts(): ), ] - cfg.CONF.register_cli_opts(auth_opts, group="auth") + st2cfg.do_register_cli_opts(auth_opts, group="auth", ignore_errors=ignore_errors) diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index d97cf08546..0429c81ea2 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -36,15 +36,19 @@ def do_register_opts(opts, group=None, ignore_errors=False): raise -def do_register_cli_opts(opt, ignore_errors=False): +def do_register_cli_opts(opt, ignore_errors=False, group=None): # TODO: This function has broken name, it should work with lists :/ if not isinstance(opt, (list, tuple)): opts = [opt] else: opts = opt + kwargs = {} + if group: + kwargs["group"] = group + try: - cfg.CONF.register_cli_opts(opts) + cfg.CONF.register_cli_opts(opts, **kwargs) except: if not ignore_errors: raise @@ -454,7 +458,9 @@ def register_opts(ignore_errors=False): ), ] - do_register_opts(action_runner_opts, group="actionrunner") + do_register_opts( + action_runner_opts, group="actionrunner", ignore_errors=ignore_errors + ) dispatcher_pool_opts = [ cfg.IntOpt( @@ -469,7 +475,9 @@ def register_opts(ignore_errors=False): ), ] - do_register_opts(dispatcher_pool_opts, group="actionrunner") + do_register_opts( + dispatcher_pool_opts, group="actionrunner", ignore_errors=ignore_errors + ) ssh_runner_opts = [ cfg.StrOpt( @@ -505,7 +513,7 @@ def register_opts(ignore_errors=False): ), ] - do_register_opts(ssh_runner_opts, group="ssh_runner") + do_register_opts(ssh_runner_opts, group="ssh_runner", ignore_errors=ignore_errors) # Common options (used by action runner and sensor container) action_sensor_opts = [ @@ -521,7 +529,9 @@ def register_opts(ignore_errors=False): ), ] - do_register_opts(action_sensor_opts, group="action_sensor") + do_register_opts( + action_sensor_opts, group="action_sensor", ignore_errors=ignore_errors + ) # Common options for content pack_lib_opts = [ @@ -538,7 +548,7 @@ def register_opts(ignore_errors=False): ) ] - do_register_opts(pack_lib_opts, group="packs") + do_register_opts(pack_lib_opts, group="packs", ignore_errors=ignore_errors) # Coordination options coord_opts = [ @@ -719,8 +729,8 @@ def register_opts(ignore_errors=False): ) -def parse_args(args=None): - register_opts() +def parse_args(args=None, ignore_errors=False): + register_opts(ignore_errors=ignore_errors) cfg.CONF( args=args, version=VERSION_STRING, diff --git a/st2common/st2common/content/bootstrap.py b/st2common/st2common/content/bootstrap.py index 31e60bd75c..9ae04664ad 100644 --- a/st2common/st2common/content/bootstrap.py +++ b/st2common/st2common/content/bootstrap.py @@ -437,6 +437,7 @@ def setup(argv): setup_db=True, register_mq_exchanges=True, register_internal_trigger_types=True, + ignore_register_config_opts_errors=True, ) diff --git a/st2common/st2common/script_setup.py b/st2common/st2common/script_setup.py index 0abb7e8269..5b9d50d1e6 100644 --- a/st2common/st2common/script_setup.py +++ b/st2common/st2common/script_setup.py @@ -49,6 +49,7 @@ def setup( setup_db=True, register_mq_exchanges=True, register_internal_trigger_types=False, + ignore_register_config_opts_errors=False, ): """ Common setup function. @@ -67,7 +68,14 @@ def setup( register_common_cli_options() # Parse args to setup config - config.parse_args() + # NOTE: This code is not the best, but it's only realistic option we have at this point. + # Refactoring all the code and config modules to avoid import time side affects would be a big + # rabbit hole. Luckily registering same options twice is not really a big deal or fatal error + # so we simply ignore such errors. + if config.__name__ == "st2common.config" and ignore_register_config_opts_errors: + config.parse_args(ignore_errors=True) + else: + config.parse_args() if cfg.CONF.debug: cfg.CONF.verbose = True diff --git a/st2exporter/st2exporter/config.py b/st2exporter/st2exporter/config.py index 83f4f45d5d..f5f6444891 100644 --- a/st2exporter/st2exporter/config.py +++ b/st2exporter/st2exporter/config.py @@ -42,16 +42,16 @@ def get_logging_config_path(): return cfg.CONF.exporter.logging -def register_opts(): - _register_common_opts() - _register_app_opts() +def register_opts(ignore_errors=False): + _register_common_opts(ignore_errors=ignore_errors) + _register_app_opts(ignore_errors=ignore_errors) -def _register_common_opts(): - common_config.register_opts() +def _register_common_opts(ignore_errors=False): + common_config.register_opts(ignore_errors=ignore_errors) -def _register_app_opts(): +def _register_app_opts(ignore_errors=False): dump_opts = [ cfg.StrOpt( "dump_dir", @@ -60,7 +60,9 @@ def _register_app_opts(): ) ] - CONF.register_opts(dump_opts, group="exporter") + common_config.do_register_opts( + dump_opts, group="exporter", ignore_errors=ignore_errors + ) logging_opts = [ cfg.StrOpt( @@ -70,4 +72,6 @@ def _register_app_opts(): ) ] - CONF.register_opts(logging_opts, group="exporter") + common_config.do_register_opts( + logging_opts, group="exporter", ignore_errors=ignore_errors + ) diff --git a/st2reactor/st2reactor/garbage_collector/config.py b/st2reactor/st2reactor/garbage_collector/config.py index 9a0faf0dec..af579245f0 100644 --- a/st2reactor/st2reactor/garbage_collector/config.py +++ b/st2reactor/st2reactor/garbage_collector/config.py @@ -36,20 +36,20 @@ def parse_args(args=None): ) -def register_opts(): - _register_common_opts() - _register_garbage_collector_opts() +def register_opts(ignore_errors=False): + _register_common_opts(ignore_errors=ignore_errors) + _register_garbage_collector_opts(ignore_errors=ignore_errors) def get_logging_config_path(): return cfg.CONF.garbagecollector.logging -def _register_common_opts(): - common_config.register_opts() +def _register_common_opts(ignore_errors=False): + common_config.register_opts(ignore_errors=ignore_errors) -def _register_garbage_collector_opts(): +def _register_garbage_collector_opts(ignore_errors=False): logging_opts = [ cfg.StrOpt( "logging", @@ -58,7 +58,9 @@ def _register_garbage_collector_opts(): ) ] - CONF.register_opts(logging_opts, group="garbagecollector") + common_config.do_register_opts( + logging_opts, group="garbagecollector", ignore_errors=ignore_errors + ) common_opts = [ cfg.IntOpt( @@ -74,7 +76,9 @@ def _register_garbage_collector_opts(): ), ] - CONF.register_opts(common_opts, group="garbagecollector") + common_config.do_register_opts( + common_opts, group="garbagecollector", ignore_errors=ignore_errors + ) ttl_opts = [ cfg.IntOpt( @@ -96,7 +100,9 @@ def _register_garbage_collector_opts(): ), ] - CONF.register_opts(ttl_opts, group="garbagecollector") + common_config.do_register_opts( + ttl_opts, group="garbagecollector", ignore_errors=ignore_errors + ) inquiry_opts = [ cfg.BoolOpt( @@ -107,7 +113,9 @@ def _register_garbage_collector_opts(): ) ] - CONF.register_opts(inquiry_opts, group="garbagecollector") + common_config.do_register_opts( + inquiry_opts, group="garbagecollector", ignore_errors=ignore_errors + ) -register_opts() +register_opts(ignore_errors=True) diff --git a/st2reactor/st2reactor/rules/config.py b/st2reactor/st2reactor/rules/config.py index 637ef4e457..49922b2314 100644 --- a/st2reactor/st2reactor/rules/config.py +++ b/st2reactor/st2reactor/rules/config.py @@ -34,20 +34,20 @@ def parse_args(args=None): ) -def register_opts(): - _register_common_opts() - _register_rules_engine_opts() +def register_opts(ignore_errors=False): + _register_common_opts(ignore_errors=ignore_errors) + _register_rules_engine_opts(ignore_errors=ignore_errors) def get_logging_config_path(): return cfg.CONF.rulesengine.logging -def _register_common_opts(): - common_config.register_opts() +def _register_common_opts(ignore_errors=False): + common_config.register_opts(ignore_errors=ignore_errors) -def _register_rules_engine_opts(): +def _register_rules_engine_opts(ignore_errors=False): logging_opts = [ cfg.StrOpt( "logging", @@ -56,7 +56,9 @@ def _register_rules_engine_opts(): ) ] - CONF.register_opts(logging_opts, group="rulesengine") + common_config.do_register_opts( + logging_opts, group="rulesengine", ignore_errors=ignore_errors + ) -register_opts() +register_opts(ignore_errors=True) diff --git a/st2reactor/st2reactor/timer/config.py b/st2reactor/st2reactor/timer/config.py index bbc1020cb9..0301ca4a7f 100644 --- a/st2reactor/st2reactor/timer/config.py +++ b/st2reactor/st2reactor/timer/config.py @@ -32,16 +32,16 @@ def parse_args(args=None): ) -def register_opts(): - _register_common_opts() +def register_opts(ignore_errors=False): + _register_common_opts(ignore_errors=ignore_errors) def get_logging_config_path(): return cfg.CONF.timersengine.logging -def _register_common_opts(): - common_config.register_opts() +def _register_common_opts(ignore_errors=False): + common_config.register_opts(ignore_errors=ignore_errors) -register_opts() +register_opts(ignore_errors=True) diff --git a/st2stream/st2stream/config.py b/st2stream/st2stream/config.py index bc117b556a..eef5c55830 100644 --- a/st2stream/st2stream/config.py +++ b/st2stream/st2stream/config.py @@ -39,20 +39,20 @@ def parse_args(args=None): ) -def register_opts(): - _register_common_opts() - _register_app_opts() +def register_opts(ignore_errors=False): + _register_common_opts(ignore_errors=ignore_errors) + _register_app_opts(ignore_errors=ignore_errors) -def _register_common_opts(): - common_config.register_opts() +def _register_common_opts(ignore_errors=False): + common_config.register_opts(ignore_errors=ignore_errors) def get_logging_config_path(): return cfg.CONF.stream.logging -def _register_app_opts(): +def _register_app_opts(ignore_errors=False): # Note "allow_origin", "mask_secrets", "heartbeat" options are registered as part of st2common # config since they are also used outside st2stream api_opts = [ @@ -68,4 +68,6 @@ def _register_app_opts(): ), ] - CONF.register_opts(api_opts, group="stream") + common_config.do_register_opts( + api_opts, group="stream", ignore_errors=ignore_errors + )