From 2716a7e177abb35964021794563a6a75afe15dcb Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Wed, 14 Aug 2024 11:54:02 +0000 Subject: [PATCH 01/15] Beer.conf plugin requires --- brewtils/models.py | 3 +++ brewtils/plugin.py | 4 ++++ brewtils/rest/easy_client.py | 5 +++++ brewtils/specification.py | 7 +++++++ brewtils/test/fixtures.py | 1 + test/plugin_test.py | 8 ++++++++ 6 files changed, 28 insertions(+) diff --git a/brewtils/models.py b/brewtils/models.py index fa594a49..fd001a4e 100644 --- a/brewtils/models.py +++ b/brewtils/models.py @@ -228,6 +228,7 @@ class Instance(BaseModel): "STARTING", "STOPPING", "UNKNOWN", + "AWAITING_SYSTEM" } def __init__( @@ -761,6 +762,7 @@ def __init__( template=None, groups=None, prefix_topic=None, + requires=None, ): self.name = name self.description = description @@ -777,6 +779,7 @@ def __init__( self.template = template self.groups = groups or [] self.prefix_topic = prefix_topic + self.requires = requires or [] def __str__(self): return "%s:%s-%s" % (self.namespace, self.name, self.version) diff --git a/brewtils/plugin.py b/brewtils/plugin.py index a4724c02..0bc1ddd4 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -85,6 +85,7 @@ class Plugin(object): - ``display_name`` - ``group`` - ``groups`` + - ``requires`` Connection information tells the Plugin how to communicate with Beer-garden. The most important of these is the ``bg_host`` (to tell the plugin where to find the @@ -171,6 +172,7 @@ class Plugin(object): metadata (dict): System metadata instance_name (str): Instance name namespace (str): Namespace name + requires (list): Required systems dependencies group (str): Grouping label applied to plugin groups (list): Grouping labels applied to plugin @@ -540,6 +542,7 @@ def _initialize_system(self): "icon_name": self._system.icon_name, "template": self._system.template, "groups": self._system.groups, + "requires": self._system.requires, } # And if this particular instance doesn't exist we want to add it @@ -860,6 +863,7 @@ def _setup_system(self, system, plugin_kwargs): template=self._config.template, groups=self._config.groups, prefix_topic=self._config.prefix_topic, + requires=self._config.requires, ) return system diff --git a/brewtils/rest/easy_client.py b/brewtils/rest/easy_client.py index be5234d9..0f0a73d7 100644 --- a/brewtils/rest/easy_client.py +++ b/brewtils/rest/easy_client.py @@ -398,6 +398,7 @@ def update_system(self, system_id, new_commands=None, **kwargs): icon_name (str): New System icon name template (str): New System template groups (list): New System groups + requires (list): New System dependencies Returns: System: The updated system @@ -424,6 +425,10 @@ def update_system(self, system_id, new_commands=None, **kwargs): if groups: operations.append(PatchOperation("replace", "/groups", groups)) + requires = kwargs.pop("requires", []) + if requires: + operations.append(PatchOperation("replace", "/requires", requires)) + # The remaining kwargs are all strings # Sending an empty string (instead of None) ensures they're actually cleared for key, value in kwargs.items(): diff --git a/brewtils/specification.py b/brewtils/specification.py index acea8de6..cb2ba77b 100644 --- a/brewtils/specification.py +++ b/brewtils/specification.py @@ -171,6 +171,13 @@ def _is_json_dict(s): "....." "if a prefix is provided, then it is `.`", }, + "requires": { + "type": "list", + "description": "The required system dependencies", + "items": {"name": {"type": "str"}}, + "required": False, + "default": [], + }, } _PLUGIN_SPEC = { diff --git a/brewtils/test/fixtures.py b/brewtils/test/fixtures.py index 3996cc8b..f27488a9 100644 --- a/brewtils/test/fixtures.py +++ b/brewtils/test/fixtures.py @@ -263,6 +263,7 @@ def system_dict(instance_dict, command_dict, command_dict_2, system_id): "template": "template", "groups": ["GroupB", "GroupA"], "prefix_topic": "custom_topic", + "requires": ["SystemA"], } diff --git a/test/plugin_test.py b/test/plugin_test.py index c0b83d36..1545831d 100644 --- a/test/plugin_test.py +++ b/test/plugin_test.py @@ -152,6 +152,7 @@ def test_kwargs(self, client, bg_system): group="GroupA", groups=["GroupB"], prefix_topic="custom.topic", + requires=["SystemA"], ) assert plugin._logger == logger @@ -164,6 +165,7 @@ def test_kwargs(self, client, bg_system): assert "GroupA" == plugin._config.group assert "GroupB" in plugin._config.groups assert "GroupA" not in plugin._config.groups + assert "SystemA" in plugin._config.requires def test_env(self, client, bg_system): os.environ["BG_HOST"] = "remotehost" @@ -173,6 +175,7 @@ def test_env(self, client, bg_system): os.environ["BG_CA_VERIFY"] = "False" os.environ["BG_GROUP"] = "GroupA" os.environ["BG_PREFIX_TOPIC"] = "custom.topic" + os.environ["BG_REQUIRES"] = "['SystemA']" plugin = Plugin(client, system=bg_system, max_concurrent=1) @@ -183,6 +186,7 @@ def test_env(self, client, bg_system): assert plugin._config.ca_verify is False assert plugin._config.prefix_topic == "custom.topic" assert "GroupA" == plugin._config.group + assert "SystemA" in plugin._config.requires def test_conflicts(self, client, bg_system): os.environ["BG_HOST"] = "remotehost" @@ -504,6 +508,7 @@ def test_system_exists( display_name=bg_system.display_name, template="template", groups=bg_system.groups, + requires=bg_system.requires, ) # assert ez_client.create_system.return_value == plugin.system @@ -534,6 +539,7 @@ def test_new_instance(self, plugin, ez_client, bg_system, bg_instance): template="template", add_instance=ANY, groups=["GroupB", "GroupA"], + requires=["SystemA"], ) assert ez_client.update_system.call_args[1]["add_instance"].name == new_name @@ -811,6 +817,7 @@ def test_construct_system(self, plugin): "icon_name": "icon", "display_name": "display_name", "metadata": '{"foo": "bar"}', + "requires": [] } ) @@ -825,6 +832,7 @@ def _validate_system(new_system): assert new_system.icon_name == "icon" assert new_system.metadata == {"foo": "bar"} assert new_system.display_name == "display_name" + assert new_system.requires == [] class TestValidateSystem(object): From a7b3f4fd20d60a00b5d6a7fde818822ef7e48a1e Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Wed, 14 Aug 2024 12:09:56 +0000 Subject: [PATCH 02/15] Add requires to system schema --- brewtils/schemas.py | 1 + 1 file changed, 1 insertion(+) diff --git a/brewtils/schemas.py b/brewtils/schemas.py index 92427abd..dbad8da1 100644 --- a/brewtils/schemas.py +++ b/brewtils/schemas.py @@ -257,6 +257,7 @@ class SystemSchema(BaseSchema): template = fields.Str(allow_none=True) groups = fields.List(fields.Str(), allow_none=True) prefix_topic = fields.Str(allow_none=True) + requires = fields.List(fields.Str(), allow_none=True) class SystemDomainIdentifierSchema(BaseSchema): From a57ff2a2820249ca180465958d74a9aad5888cab Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Thu, 15 Aug 2024 11:05:38 +0000 Subject: [PATCH 03/15] Add str require to fix requires env plugin test --- brewtils/plugin.py | 6 ++++++ brewtils/specification.py | 5 +++++ test/plugin_test.py | 28 ++++++++++++++++++++++++---- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/brewtils/plugin.py b/brewtils/plugin.py index 0bc1ddd4..c0c2fbc5 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -85,6 +85,7 @@ class Plugin(object): - ``display_name`` - ``group`` - ``groups`` + - ``require`` - ``requires`` Connection information tells the Plugin how to communicate with Beer-garden. The @@ -172,6 +173,8 @@ class Plugin(object): metadata (dict): System metadata instance_name (str): Instance name namespace (str): Namespace name + + require (str): Required system dependency requires (list): Required systems dependencies group (str): Grouping label applied to plugin @@ -850,6 +853,9 @@ def _setup_system(self, system, plugin_kwargs): if self._config.group: self._config.groups.append(self._config.group) + if self._config.require: + self._config.requires.append(self._config.require) + system = System( name=self._config.name, version=self._config.version, diff --git a/brewtils/specification.py b/brewtils/specification.py index cb2ba77b..32e17497 100644 --- a/brewtils/specification.py +++ b/brewtils/specification.py @@ -171,6 +171,11 @@ def _is_json_dict(s): "....." "if a prefix is provided, then it is `.`", }, + "require": { + "type": "str", + "description": "A requires system dependency", + "required": False, + }, "requires": { "type": "list", "description": "The required system dependencies", diff --git a/test/plugin_test.py b/test/plugin_test.py index 1545831d..ef699ed7 100644 --- a/test/plugin_test.py +++ b/test/plugin_test.py @@ -136,6 +136,23 @@ def test_groups(self, client): assert plugin._system.groups == ["GroupB", "GroupA"] + def test_requires(self, client): + logger = Mock() + plugin = Plugin( + client, + bg_host="host1", + bg_port=2338, + bg_url_prefix="/beer/", + ssl_enabled=False, + ca_verify=False, + logger=logger, + max_concurrent=1, + require="SystemA", + requires=["SystemB"], + ) + + assert plugin._system.requires == ["SystemB", "SystemA"] + def test_kwargs(self, client, bg_system): logger = Mock() @@ -152,7 +169,8 @@ def test_kwargs(self, client, bg_system): group="GroupA", groups=["GroupB"], prefix_topic="custom.topic", - requires=["SystemA"], + require="SystemA", + requires=["SystemB"], ) assert plugin._logger == logger @@ -165,7 +183,9 @@ def test_kwargs(self, client, bg_system): assert "GroupA" == plugin._config.group assert "GroupB" in plugin._config.groups assert "GroupA" not in plugin._config.groups - assert "SystemA" in plugin._config.requires + assert "SystemA" == plugin._config.require + assert "SystemB" in plugin._config.requires + assert "SystemA" not in plugin._config.requires def test_env(self, client, bg_system): os.environ["BG_HOST"] = "remotehost" @@ -175,7 +195,7 @@ def test_env(self, client, bg_system): os.environ["BG_CA_VERIFY"] = "False" os.environ["BG_GROUP"] = "GroupA" os.environ["BG_PREFIX_TOPIC"] = "custom.topic" - os.environ["BG_REQUIRES"] = "['SystemA']" + os.environ["BG_REQUIRE"] = "SystemA" plugin = Plugin(client, system=bg_system, max_concurrent=1) @@ -186,7 +206,7 @@ def test_env(self, client, bg_system): assert plugin._config.ca_verify is False assert plugin._config.prefix_topic == "custom.topic" assert "GroupA" == plugin._config.group - assert "SystemA" in plugin._config.requires + assert "SystemA" in plugin._config.require def test_conflicts(self, client, bg_system): os.environ["BG_HOST"] = "remotehost" From e3e5a90ed965128848ea218530bc6d13e8231a69 Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Fri, 16 Aug 2024 12:11:52 +0000 Subject: [PATCH 04/15] Add dependency check to plugin run method --- brewtils/models.py | 2 +- brewtils/plugin.py | 41 +++++++++++++++++++++++++++++++++++++++-- test/plugin_test.py | 2 +- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/brewtils/models.py b/brewtils/models.py index fd001a4e..3a067f44 100644 --- a/brewtils/models.py +++ b/brewtils/models.py @@ -228,7 +228,7 @@ class Instance(BaseModel): "STARTING", "STOPPING", "UNKNOWN", - "AWAITING_SYSTEM" + "AWAITING_SYSTEM", } def __init__( diff --git a/brewtils/plugin.py b/brewtils/plugin.py index c0c2fbc5..146e453f 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -254,6 +254,40 @@ def __init__(self, client=None, system=None, logger=None, **kwargs): # And with _system and _ez_client we can ask for the real logging config self._initialize_logging() + def get_system_dependency(self, require, timeout=300): + wait_time = 0.1 + while timeout > 0: + system = self._ez_client.find_unique_system(name=require) + if ( + system + and system.instances + and any("RUNNING" == instance.status for instance in system.instances) + ): + return system + self.logger.warning( + f"Waiting {wait_time:.1f} seconds before next attempt for {self._system} " + f"dependency for {require}" + ) + timeout = timeout - wait_time + wait_time = min(wait_time * 2, 30) + + # TODO: Raise better exception + raise ValueError(f"Failed to resolve {self._system} dependency for {require}") + + def await_dependencies(self, config): + for req in config.requires: + try: + system = self.get_system_dependency(req) + self.logger.info( + f"Resolved system {system} for {req}: {config.name} {config.instance_name}" + ) + except Exception: + # TODO: Shutdown the plugin + self.logger.error( + f"Failed to find a matching system for {req}: " + f"{config.name} {config.instance_name}" + ) + def run(self): if not self._client: raise AttributeError( @@ -261,6 +295,9 @@ def run(self): "attribute to an instance of a class decorated with @brewtils.system" ) + if self._config.requires: + self.await_dependencies(self._config) + self._startup() self._logger.info("Plugin %s has started", self.unique_name) @@ -596,13 +633,13 @@ def _initialize_processors(self): thread_name="Admin Consumer", queue_name=self._instance.queue_info["admin"]["name"], max_concurrent=1, - **common_args + **common_args, ) request_consumer = RequestConsumer.create( thread_name="Request Consumer", queue_name=self._instance.queue_info["request"]["name"], max_concurrent=self._config.max_concurrent, - **common_args + **common_args, ) # Both RequestProcessors need an updater diff --git a/test/plugin_test.py b/test/plugin_test.py index ef699ed7..2cb251a7 100644 --- a/test/plugin_test.py +++ b/test/plugin_test.py @@ -837,7 +837,7 @@ def test_construct_system(self, plugin): "icon_name": "icon", "display_name": "display_name", "metadata": '{"foo": "bar"}', - "requires": [] + "requires": [], } ) From de33d39d97287b5b5e67162e96c9b7acf3afc6b2 Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Mon, 19 Aug 2024 13:15:27 +0000 Subject: [PATCH 05/15] Update status on backoff and shutdown after timeout --- brewtils/plugin.py | 106 +++++++++++++++++++++++++-------------------- 1 file changed, 60 insertions(+), 46 deletions(-) diff --git a/brewtils/plugin.py b/brewtils/plugin.py index 146e453f..be2469fc 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -254,40 +254,6 @@ def __init__(self, client=None, system=None, logger=None, **kwargs): # And with _system and _ez_client we can ask for the real logging config self._initialize_logging() - def get_system_dependency(self, require, timeout=300): - wait_time = 0.1 - while timeout > 0: - system = self._ez_client.find_unique_system(name=require) - if ( - system - and system.instances - and any("RUNNING" == instance.status for instance in system.instances) - ): - return system - self.logger.warning( - f"Waiting {wait_time:.1f} seconds before next attempt for {self._system} " - f"dependency for {require}" - ) - timeout = timeout - wait_time - wait_time = min(wait_time * 2, 30) - - # TODO: Raise better exception - raise ValueError(f"Failed to resolve {self._system} dependency for {require}") - - def await_dependencies(self, config): - for req in config.requires: - try: - system = self.get_system_dependency(req) - self.logger.info( - f"Resolved system {system} for {req}: {config.name} {config.instance_name}" - ) - except Exception: - # TODO: Shutdown the plugin - self.logger.error( - f"Failed to find a matching system for {req}: " - f"{config.name} {config.instance_name}" - ) - def run(self): if not self._client: raise AttributeError( @@ -295,11 +261,11 @@ def run(self): "attribute to an instance of a class decorated with @brewtils.system" ) - if self._config.requires: - self.await_dependencies(self._config) - - self._startup() - self._logger.info("Plugin %s has started", self.unique_name) + try: + self._startup() + self._logger.info("Plugin %s has started", self.unique_name) + except PluginValidationError: + self._shutdown(status="ERROR") try: # Need the timeout param so this works correctly in Python 2 @@ -410,6 +376,35 @@ def _hook(exc_type, exc_value, traceback): sys.excepthook = _hook + def get_system_dependency(self, require, timeout=300): + wait_time = 0.1 + while timeout > 0: + system = self._ez_client.find_unique_system(name=require) + if ( + system + and system.instances + and any("RUNNING" == instance.status for instance in system.instances) + ): + return system + self._wait() + self.logger.error( + f"Waiting {wait_time:.1f} seconds before next attempt for {self._system} " + f"dependency for {require}" + ) + timeout = timeout - wait_time + wait_time = min(wait_time * 2, 30) + + raise PluginValidationError( + f"Failed to resolve {self._system} dependency for {require}" + ) + + def await_dependencies(self, config): + for req in config.requires: + system = self.get_system_dependency(req) + self.logger.info( + f"Resolved system {system} for {req}: {config.name} {config.instance_name}" + ) + def _startup(self): """Plugin startup procedure @@ -448,12 +443,20 @@ def _startup(self): self._logger.debug("Initializing and starting processors") self._admin_processor, self._request_processor = self._initialize_processors() self._admin_processor.startup() - self._request_processor.startup() - self._logger.debug("Setting signal handlers") - self._set_signal_handlers() + try: + if self._config.requires: + self.await_dependencies(self._config) + except PluginValidationError: + raise + else: + self._start() + self._request_processor.startup() + finally: + self._logger.debug("Setting signal handlers") + self._set_signal_handlers() - def _shutdown(self): + def _shutdown(self, status="STOPPED"): """Plugin shutdown procedure This method gracefully stops the plugin. When it completes the plugin should be @@ -464,14 +467,18 @@ def _shutdown(self): self._shutdown_event.set() self._logger.debug("Shutting down processors") - self._request_processor.shutdown() + # Join will cause an exception if processor thread wasn't started + try: + self._request_processor.shutdown() + except RuntimeError: + pass self._admin_processor.shutdown() try: - self._ez_client.update_instance(self._instance.id, new_status="STOPPED") + self._ez_client.update_instance(self._instance.id, new_status=status) except Exception: self._logger.warning( - "Unable to notify Beer-garden that this plugin is STOPPED, so this " + f"Unable to notify Beer-garden that this plugin is {status}, so this " "plugin's status may be incorrect in Beer-garden" ) @@ -678,6 +685,13 @@ def _start(self): self._instance.id, new_status="RUNNING" ) + def _wait(self): + """Handle wait request""" + # Set the status to wait + self._instance = self._ez_client.update_instance( + self._instance.id, new_status="AWAITING_SYSTEM" + ) + def _stop(self): """Handle stop Request""" # Because the run() method is on a 0.1s sleep there's a race regarding if the From 9e5a394dcf5fd7c085b583303b9505be0474196a Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Mon, 19 Aug 2024 14:10:26 +0000 Subject: [PATCH 06/15] Fix shutdown plugin on timeout --- brewtils/plugin.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/brewtils/plugin.py b/brewtils/plugin.py index be2469fc..4813040e 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -264,20 +264,21 @@ def run(self): try: self._startup() self._logger.info("Plugin %s has started", self.unique_name) + + try: + # Need the timeout param so this works correctly in Python 2 + while not self._shutdown_event.wait(timeout=0.1): + pass + except KeyboardInterrupt: + self._logger.debug("Received KeyboardInterrupt - shutting down") + except Exception as ex: + self._logger.exception("Exception during wait, shutting down: %s", ex) + + self._shutdown() except PluginValidationError: self._shutdown(status="ERROR") - - try: - # Need the timeout param so this works correctly in Python 2 - while not self._shutdown_event.wait(timeout=0.1): - pass - except KeyboardInterrupt: - self._logger.debug("Received KeyboardInterrupt - shutting down") - except Exception as ex: - self._logger.exception("Exception during wait, shutting down: %s", ex) - - self._shutdown() - self._logger.info("Plugin %s has terminated", self.unique_name) + finally: + self._logger.info("Plugin %s has terminated", self.unique_name) @property def client(self): From b84f0dd2ba5f8159fd211ef79e19ebea2f35ad91 Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:32:25 +0000 Subject: [PATCH 07/15] Add requires timeout --- brewtils/models.py | 2 ++ brewtils/plugin.py | 6 +++++- brewtils/schemas.py | 1 + brewtils/specification.py | 5 +++++ test/plugin_test.py | 2 ++ 5 files changed, 15 insertions(+), 1 deletion(-) diff --git a/brewtils/models.py b/brewtils/models.py index 3a067f44..d6578eea 100644 --- a/brewtils/models.py +++ b/brewtils/models.py @@ -763,6 +763,7 @@ def __init__( groups=None, prefix_topic=None, requires=None, + requires_timeout=None, ): self.name = name self.description = description @@ -780,6 +781,7 @@ def __init__( self.groups = groups or [] self.prefix_topic = prefix_topic self.requires = requires or [] + self.requires_timeout = requires_timeout def __str__(self): return "%s:%s-%s" % (self.namespace, self.name, self.version) diff --git a/brewtils/plugin.py b/brewtils/plugin.py index 4813040e..526c3ca2 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -87,6 +87,7 @@ class Plugin(object): - ``groups`` - ``require`` - ``requires`` + - ``requires_timeout`` Connection information tells the Plugin how to communicate with Beer-garden. The most important of these is the ``bg_host`` (to tell the plugin where to find the @@ -176,6 +177,7 @@ class Plugin(object): require (str): Required system dependency requires (list): Required systems dependencies + requires_timeout (int): Timeout to wait for dependencies group (str): Grouping label applied to plugin groups (list): Grouping labels applied to plugin @@ -401,7 +403,7 @@ def get_system_dependency(self, require, timeout=300): def await_dependencies(self, config): for req in config.requires: - system = self.get_system_dependency(req) + system = self.get_system_dependency(req, config.requires_timeout) self.logger.info( f"Resolved system {system} for {req}: {config.name} {config.instance_name}" ) @@ -446,6 +448,7 @@ def _startup(self): self._admin_processor.startup() try: + print(self._config) if self._config.requires: self.await_dependencies(self._config) except PluginValidationError: @@ -922,6 +925,7 @@ def _setup_system(self, system, plugin_kwargs): groups=self._config.groups, prefix_topic=self._config.prefix_topic, requires=self._config.requires, + requires_timeout=self._config.requires_timeout, ) return system diff --git a/brewtils/schemas.py b/brewtils/schemas.py index dbad8da1..085ae2bb 100644 --- a/brewtils/schemas.py +++ b/brewtils/schemas.py @@ -258,6 +258,7 @@ class SystemSchema(BaseSchema): groups = fields.List(fields.Str(), allow_none=True) prefix_topic = fields.Str(allow_none=True) requires = fields.List(fields.Str(), allow_none=True) + requires_timeout = fields.Integer(allow_none=True) class SystemDomainIdentifierSchema(BaseSchema): diff --git a/brewtils/specification.py b/brewtils/specification.py index 32e17497..2ae991c2 100644 --- a/brewtils/specification.py +++ b/brewtils/specification.py @@ -183,6 +183,11 @@ def _is_json_dict(s): "required": False, "default": [], }, + "requires_timeout": { + "type": "int", + "description": "The dependency timeout to use", + "default": 300, + }, } _PLUGIN_SPEC = { diff --git a/test/plugin_test.py b/test/plugin_test.py index 2cb251a7..5def7eba 100644 --- a/test/plugin_test.py +++ b/test/plugin_test.py @@ -171,6 +171,7 @@ def test_kwargs(self, client, bg_system): prefix_topic="custom.topic", require="SystemA", requires=["SystemB"], + requires_timeout=200, ) assert plugin._logger == logger @@ -186,6 +187,7 @@ def test_kwargs(self, client, bg_system): assert "SystemA" == plugin._config.require assert "SystemB" in plugin._config.requires assert "SystemA" not in plugin._config.requires + assert plugin._config.requires_timeout == 200 def test_env(self, client, bg_system): os.environ["BG_HOST"] = "remotehost" From ee6b19becde164af88a60d8f077c39955657c3f2 Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Tue, 20 Aug 2024 09:45:23 +0000 Subject: [PATCH 08/15] Fix tests by updating fixture --- brewtils/plugin.py | 1 - brewtils/test/fixtures.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/brewtils/plugin.py b/brewtils/plugin.py index 526c3ca2..b6bd0bd4 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -448,7 +448,6 @@ def _startup(self): self._admin_processor.startup() try: - print(self._config) if self._config.requires: self.await_dependencies(self._config) except PluginValidationError: diff --git a/brewtils/test/fixtures.py b/brewtils/test/fixtures.py index f27488a9..3013afd5 100644 --- a/brewtils/test/fixtures.py +++ b/brewtils/test/fixtures.py @@ -264,6 +264,7 @@ def system_dict(instance_dict, command_dict, command_dict_2, system_id): "groups": ["GroupB", "GroupA"], "prefix_topic": "custom_topic", "requires": ["SystemA"], + "requires_timeout": 300 } From b2496158772925f9b3fe65efb95b54fa73040532 Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Tue, 20 Aug 2024 09:47:07 +0000 Subject: [PATCH 09/15] Missing comma --- brewtils/test/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/brewtils/test/fixtures.py b/brewtils/test/fixtures.py index 3013afd5..f3561b05 100644 --- a/brewtils/test/fixtures.py +++ b/brewtils/test/fixtures.py @@ -264,7 +264,7 @@ def system_dict(instance_dict, command_dict, command_dict_2, system_id): "groups": ["GroupB", "GroupA"], "prefix_topic": "custom_topic", "requires": ["SystemA"], - "requires_timeout": 300 + "requires_timeout": 300, } From 24d65d20a2b94a3000d4131735c4a1d5194613b7 Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Tue, 20 Aug 2024 07:33:20 -0400 Subject: [PATCH 10/15] Update CHANGELOG.rst --- CHANGELOG.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e2ded8b3..2cc30d1c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,12 @@ Brewtils Changelog ================== +3.27.1 +------ +TBD + +- Updated Plugin class to accept requires and requires_timeout attributes to require plugin to wait for dependencies before starting. + 3.27.0 ------ 8/13/24 From 4cf67a4bb90d3342edc322775f9591b19eb05953 Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Tue, 20 Aug 2024 14:14:33 +0000 Subject: [PATCH 11/15] Add wait for plugin backoff --- brewtils/plugin.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/brewtils/plugin.py b/brewtils/plugin.py index b6bd0bd4..eb385c26 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -389,13 +389,13 @@ def get_system_dependency(self, require, timeout=300): and any("RUNNING" == instance.status for instance in system.instances) ): return system - self._wait() self.logger.error( f"Waiting {wait_time:.1f} seconds before next attempt for {self._system} " f"dependency for {require}" ) timeout = timeout - wait_time wait_time = min(wait_time * 2, 30) + self._wait(wait_time) raise PluginValidationError( f"Failed to resolve {self._system} dependency for {require}" @@ -688,12 +688,13 @@ def _start(self): self._instance.id, new_status="RUNNING" ) - def _wait(self): + def _wait(self, timeout): """Handle wait request""" # Set the status to wait self._instance = self._ez_client.update_instance( self._instance.id, new_status="AWAITING_SYSTEM" ) + self._shutdown_event.wait(timeout) def _stop(self): """Handle stop Request""" From 81f006f7143ea3917ea20b8fd954669dbd6bf6b4 Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Tue, 20 Aug 2024 14:34:43 +0000 Subject: [PATCH 12/15] Unique systems on local garden only for dependency check --- brewtils/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/brewtils/plugin.py b/brewtils/plugin.py index eb385c26..87e51a92 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -382,7 +382,7 @@ def _hook(exc_type, exc_value, traceback): def get_system_dependency(self, require, timeout=300): wait_time = 0.1 while timeout > 0: - system = self._ez_client.find_unique_system(name=require) + system = self._ez_client.find_unique_system(name=require, local=True) if ( system and system.instances From e1c10533127c1ca65b98d8d438047827e09fb9a2 Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Tue, 20 Aug 2024 17:45:44 +0000 Subject: [PATCH 13/15] Add requires to decorators --- brewtils/decorators.py | 10 ++++++++++ brewtils/plugin.py | 3 +++ test/decorators_test.py | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/brewtils/decorators.py b/brewtils/decorators.py index 7186aaa5..d6e332ba 100644 --- a/brewtils/decorators.py +++ b/brewtils/decorators.py @@ -41,6 +41,8 @@ def client( group=None, # type: str groups=[], # type: Optional[List[str]] prefix_topic=None, # type: Optional[str] + require=None, # type: str + requires=[], # type: Optional[List[str]] ): # type: (...) -> Type """Class decorator that marks a class as a beer-garden Client @@ -70,6 +72,8 @@ def client( group: Optional plugin group groups: Optional plugin groups prefix_topic: Optional prefix for Generated Command to Topic mappings + require: Optional system dependency + requires: Optional system dependencies Returns: The decorated class @@ -83,6 +87,8 @@ def client( groups=groups, group=group, prefix_topic=prefix_topic, + require=require, + requires=requires, ) # noqa # Assign these here so linters don't complain @@ -92,10 +98,14 @@ def client( _wrapped._current_request = None _wrapped._groups = groups _wrapped._prefix_topic = prefix_topic + _wrapped._requires = requires if group: _wrapped._groups.append(group) + if require: + _wrapped._requires.append(require) + return _wrapped diff --git a/brewtils/plugin.py b/brewtils/plugin.py index 87e51a92..b45c5fe6 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -310,6 +310,8 @@ def _set_client(self, new_client): self._system.prefix_topic = getattr( new_client, "_prefix_topic", None ) # noqa + if not self._system.requires: + self._system.requires = getattr(new_client, "_requires", []) # noqa # Now roll up / interpret all metadata to get the Commands self._system.commands = _parse_client(new_client) @@ -326,6 +328,7 @@ def _set_client(self, new_client): client_clazz._bg_commands = self._system.commands client_clazz._groups = self._system.groups client_clazz._prefix_topic = self._system.prefix_topic + client_clazz._requires = self._system.requires client_clazz._current_request = client_clazz.current_request except TypeError: if sys.version_info.major != 2: diff --git a/test/decorators_test.py b/test/decorators_test.py index c7e4ad78..ed01ca8a 100644 --- a/test/decorators_test.py +++ b/test/decorators_test.py @@ -512,6 +512,7 @@ def foo(self): assert hasattr(ClientClass, "_current_request") assert hasattr(ClientClass, "_groups") assert hasattr(ClientClass, "_prefix_topic") + assert hasattr(ClientClass, "_requires") def test_with_args(self): @client( @@ -519,6 +520,7 @@ def test_with_args(self): bg_version="1.0.0", groups=["GroupA"], prefix_topic="custom_topic", + requires=["SystemA"], ) class ClientClass(object): @command @@ -531,11 +533,13 @@ def foo(self): assert hasattr(ClientClass, "_current_request") assert hasattr(ClientClass, "_groups") assert hasattr(ClientClass, "_prefix_topic") + assert hasattr(ClientClass, "_requires") assert ClientClass._bg_name == "sys" assert ClientClass._bg_version == "1.0.0" assert ClientClass._groups == ["GroupA"] assert ClientClass._prefix_topic == "custom_topic" + assert ClientClass._requires == ["SystemA"] def test_group(self): @client(bg_name="sys", bg_version="1.0.0", group="GroupB") @@ -571,6 +575,42 @@ def foo(self): assert ClientClass._bg_version == "1.0.0" assert ClientClass._groups == ["GroupA", "GroupB"] + def test_require(self): + @client(bg_name="sys", bg_version="1.0.0", require="SystemB") + class ClientClass(object): + @command + def foo(self): + pass + + assert hasattr(ClientClass, "_bg_name") + assert hasattr(ClientClass, "_bg_version") + assert hasattr(ClientClass, "_bg_commands") + assert hasattr(ClientClass, "_current_request") + assert hasattr(ClientClass, "_requires") + + assert ClientClass._bg_name == "sys" + assert ClientClass._bg_version == "1.0.0" + assert ClientClass._requires == ["SystemB"] + + def test_requires_combine(self): + @client( + bg_name="sys", bg_version="1.0.0", requires=["SystemA"], require="SystemB" + ) + class ClientClass(object): + @command + def foo(self): + pass + + assert hasattr(ClientClass, "_bg_name") + assert hasattr(ClientClass, "_bg_version") + assert hasattr(ClientClass, "_bg_commands") + assert hasattr(ClientClass, "_current_request") + assert hasattr(ClientClass, "_requires") + + assert ClientClass._bg_name == "sys" + assert ClientClass._bg_version == "1.0.0" + assert ClientClass._requires == ["SystemA", "SystemB"] + class TestCommand(object): """Test command decorator""" From a2e31b628ed0b56f0019027605ff95832270a401 Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Wed, 21 Aug 2024 09:11:32 +0000 Subject: [PATCH 14/15] Use system requires instead of config --- brewtils/plugin.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/brewtils/plugin.py b/brewtils/plugin.py index b45c5fe6..b5b37e8b 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -404,8 +404,8 @@ def get_system_dependency(self, require, timeout=300): f"Failed to resolve {self._system} dependency for {require}" ) - def await_dependencies(self, config): - for req in config.requires: + def await_dependencies(self, requires, config): + for req in requires: system = self.get_system_dependency(req, config.requires_timeout) self.logger.info( f"Resolved system {system} for {req}: {config.name} {config.instance_name}" @@ -451,8 +451,8 @@ def _startup(self): self._admin_processor.startup() try: - if self._config.requires: - self.await_dependencies(self._config) + if self._system.requires: + self.await_dependencies(self._system.requires, self._config) except PluginValidationError: raise else: From 4cd486820f6788ba1fc3015bd8fd34ea50ecf1df Mon Sep 17 00:00:00 2001 From: 1maple1 <160027655+1maple1@users.noreply.github.com> Date: Wed, 21 Aug 2024 10:04:33 +0000 Subject: [PATCH 15/15] Attempting to fix plugin test --- test/plugin_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/plugin_test.py b/test/plugin_test.py index 5def7eba..51ec4688 100644 --- a/test/plugin_test.py +++ b/test/plugin_test.py @@ -368,6 +368,7 @@ def test_success( plugin._initialize_processors = Mock( return_value=(admin_processor, request_processor) ) + plugin._ez_client.find_unique_system = Mock(return_value=bg_system) plugin._startup() assert admin_processor.startup.called is True @@ -388,6 +389,7 @@ def test_success_no_ns( plugin._initialize_processors = Mock( return_value=(admin_processor, request_processor) ) + plugin._ez_client.find_unique_system = Mock(return_value=bg_system) plugin._startup() assert admin_processor.startup.called is True