Unify and improve config option registration code

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
 ~~~~~~~~~~~~
 

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

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)

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)

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)

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)

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
+    )

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)

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,

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,
     )