Change core to literally load interface as a plugin

shepherd/agent/cli.py

@@ -178,6 +178,8 @@ def test(ctx, plugin_name, interface_function):
         sys.exit(1)
 
     log.info("Initialising plugins...")
+    # TODO Going to need to add 'shepherd' to this, so that its public plugin interface also gets
+    # init - functions and hooks
     plugin.init_plugins({plugin_name: plugin_configs[plugin_name]})
     log.info("Plugin initialisation done")
 
@@ -237,16 +239,15 @@ def template(ctx, plugin_name, include_all, config_path, plugin_dir):
         plugin_dir = Path.cwd()
 
     confspec = None
-    if (not plugin_name) or (plugin_name == "shepherd"):
+    if not plugin_name:
         plugin_name = "shepherd"
-        confspec = agent.core_interface.confspec
+
-    else:
+    try:
-        try:
+        plugin_interface = plugin.load_plugin(plugin_name, plugin_dir)
-            plugin_interface = plugin.load_plugin(plugin_name, plugin_dir)
+    except plugin.PluginLoadError as e:
-        except plugin.PluginLoadError as e:
+        log.error(e.args[0])
-            log.error(e.args[0])
+        sys.exit(1)
-            sys.exit(1)
+    confspec = plugin_interface.confspec
-        confspec = plugin_interface.confspec
 
     template_dict = confspec.get_template(include_all)
     template_toml = toml.dumps({plugin_name: template_dict}, encoder=BlankEncoder())

shepherd/agent/core.py

@@ -19,12 +19,54 @@ from . import tasks
 log = logging.getLogger("shepherd.agent")
 
 
+core_interface = plugin.PluginInterface()
+
+confspec = ConfigSpecification()
+# Relative pathnames here are all relative to "root_dir". `root_dir` itself is relative to
+# the directory the default config is loaded from
+confspec.add_specs({
+    "name": StringSpec(helptext="Identifying name for this device"),
+})
+
+confspec.add_specs(optional=True, spec_dict={
+    "root_dir":
+        (StringSpec(helptext="Operating directory for shepherd to place working files."
+                    " Relative to the directory containing the default config file."),
+            "./"),
+    "custom_config_path":
+        StringSpec(helptext="Path to custom config layer TOML file."),
+    "compiled_config_path":
+        (StringSpec(helptext="Path to custom file Shepherd will generate to show compiled"
+                    " config that was used and any errors in validation."),
+            "compiled-config.toml"),
+    "plugin_dir":
+        (StringSpec(helptext="Optional directory for Shepherd to look for plugins in."),
+            "./shepherd-plugins")
+})
+
+
+core_interface.register_confspec(confspec)
+
+# Allows plugins to add delay for system time to stabilise
+core_interface.register_hook("wait_for_stable_time")
+
+# Allow other modules to add to the core interface (confspec, hooks, interface functions)
+# Having modules modify a confspec after it's registered here is a bit of a hack.
+tasks.register_on(core_interface)
+control.register_on(core_interface)
+
+
+@ plugin.plugin_class
 class Agent():
     """
     Holds the main state required to run Shepherd Agent
     """
 
     def __init__(self):
+
+        # Make sure the plugin system uses this instance rather making its own
+        core_interface._plugin_obj = self
+
         # The config defined by the device (everything before the Control layer)
         self.local_config = None
         # The config actually being used
@@ -38,22 +80,11 @@ class Agent():
 
         self.restart_args = None
 
-        # Setup core interface
+    @ plugin.plugin_function
-        self.core_interface = plugin.PluginInterface()
-        self.core_interface.register_confspec(core_confspec())
-        self.core_interface.register_function(self.root_dir)
-        self.core_interface.register_function(self.device_name)
-        # Allows plugins to add delay for system time to stabilise
-        self.core_interface.register_hook("wait_for_stable_time")
-
-        # Allow other modules to add to the core interface (confspec, hooks, interface functions)
-        # Having modules modify a confspec after it's registered here is a bit of a hack.
-        tasks.register_on(self.core_interface)
-        control.register_on(self.core_interface)
-
     def root_dir(self):
         return self.core_config["root_dir"]
 
+    @ plugin.plugin_function
     def device_name(self):
         return self.core_config["name"]
 
@@ -74,9 +105,6 @@ class Agent():
                              use_custom_config, control_enabled, new_device_mode]
 
         self.control_enabled = control_enabled
-        # Because the plugin module caches interfaces, this will then get used when loading
-        # config layers and validating them
-        plugin.load_plugin_interface("shepherd", self.core_interface)
 
         # Compile the config layers
 
@@ -84,7 +112,7 @@ class Agent():
         # Pre-seed confman with core confspec to bootstrap 'plugin_dir'. This is required even
         # though 'load_config_layer_and_plugins()' will get the core confspec from the cached
         # interface, as it needs 'plugin_dir' to list the plugins to load first.
-        confman.add_confspec("shepherd", self.core_interface._confspec)
+        confman.add_confspec("shepherd", core_interface.confspec)
 
         compile_local_config(confman, default_config_path, use_custom_config)
         self.local_config = deepcopy(confman.get_config_bundles())
@@ -131,12 +159,13 @@ class Agent():
         self.plugin_interfaces = plugin.init_plugins(self.applied_config)
         # After this point, plugins may already have their own threads running if they created
         # them during init
-        self.interface_functions = self.core_interface.plugins
+        self.interface_functions = core_interface.plugins
 
         cmd_runner = control.CommandRunner(self.interface_functions)
         core_update_state = control.CoreUpdateState(cmd_runner.cmd_reader,
                                                     cmd_runner.cmd_result_writer)
-        core_update_state.set_static_state(self.local_config, self.applied_config, core_confspec())
+        core_update_state.set_static_state(
+            self.local_config, self.applied_config, core_interface.confspec)
 
         plugin_update_states = {name: iface._update_state
                                 for name, iface in self.plugin_interfaces.items()}
@@ -155,7 +184,7 @@ class Agent():
 
         # TODO Any time stabilisation or waiting for Control
 
-        tasks.start_tasks(self.core_interface, task_session)
+        tasks.start_tasks(core_interface, task_session)
 
         # tasks.init_tasks(self.core_config)  # seperate tasks.start?
 
@@ -270,37 +299,6 @@ def compile_remote_config(confman):
         log.warning("No cached Shepherd Control config layer available.")
 
 
-# Relative pathnames here are all relative to "root_dir". `root_dir` itself is relative to
-# the directory the default config is loaded from
-def core_confspec():
-    """
-    Returns the core config specification
-    """
-    confspec = ConfigSpecification()
-
-    confspec.add_specs({
-        "name": StringSpec(helptext="Identifying name for this device"),
-    })
-
-    confspec.add_specs(optional=True, spec_dict={
-        "root_dir":
-            (StringSpec(helptext="Operating directory for shepherd to place working files."
-                        " Relative to the directory containing the default config file."),
-             "./"),
-        "custom_config_path":
-            StringSpec(helptext="Path to custom config layer TOML file."),
-        "compiled_config_path":
-            (StringSpec(helptext="Path to custom file Shepherd will generate to show compiled"
-                        " config that was used and any errors in validation."),
-             "compiled-config.toml"),
-        "plugin_dir":
-            (StringSpec(helptext="Optional directory for Shepherd to look for plugins in."),
-                "./shepherd-plugins")
-    })
-
-    return confspec
-
-
 def resolve_core_conf_paths(core_conf, relative_dir):
     """
     Set the cwd to ``root_dir`` and resolve other core config paths relative to that.

shepherd/agent/plugin.py

@@ -385,6 +385,7 @@ class PluginInterface():
         self._attachments = []
         self._tasks = []
         self._plugin_class = None
+        self._plugin_obj = None
         self.config = None
         self.plugins = None
         self.hooks = None
@@ -615,7 +616,11 @@ def load_plugin(plugin_name, plugin_dir=None):
     module = None
     existing_modules = sys.modules.copy().values()
 
-    if plugin_name in discover_base_plugins():
+    if plugin_name == 'shepherd':
+        module = importlib.import_module("..core", __name__)
+        log.info("Loading core plugin interface")
+
+    elif plugin_name in discover_base_plugins():
         module = importlib.import_module(base_plugins.__name__+'.'+plugin_name)
 
         if module in existing_modules:
@@ -758,13 +763,20 @@ def init_plugins(plugin_configs, plugin_dir=None):
 
     # Run plugin init and init hooks
     for plugin_name, interface in plugin_interfaces.items():
+        # Though we set `plugins` to the proxy, it's empty until after init
         interface.plugins = interface_functions_proxy
         interface.config = plugin_configs[plugin_name]
+
+        # TODO This probably should be technically done after all plugins have finished init? Could
+        # then go in loop that also does the interface functions proxy
         interface.hooks = NamespaceProxy(interface._hooks)
 
         # If it has one, instantiate the plugin object and bind methods to it.
         if interface._plugin_class is not None:
-            interface._plugin_obj = interface._plugin_class()
+            # Special case with the 'shepherd' plugin where it is already instantiated
+            if interface._plugin_obj is None:
+                interface._plugin_obj = interface._plugin_class()
+
             for ifunc in interface._functions.values():
                 if isinstance(ifunc.func, UnboundMethod):
                     ifunc.func = ifunc.func.bind(interface._plugin_obj)
@@ -773,6 +785,11 @@ def init_plugins(plugin_configs, plugin_dir=None):
                 if isinstance(attachment.func, UnboundMethod):
                     attachment.func = attachment.func.bind(interface._plugin_obj)
 
+        # TODO Probably need special case for looking for `init` attachments, and run init here,
+        # before other attachments. Or can we rely on populating `.hooks` later, and that only we
+        # have access to attachments at this point? Probably. I mean _other_ plugins will have
+        # attached to this plugin's hooks at this point.
+
         # Find hooks attachments are referring to and attach them
         for attachment in interface._attachments:
             hook_plugin_name = attachment.plugin_name

tests/test_core.py

@@ -1,6 +1,7 @@
 # pylint: disable=redefined-outer-name
 from pathlib import Path
 import logging
+import importlib
 
 import pytest
 
@@ -11,6 +12,7 @@ from shepherd.agent import plugin
 @pytest.fixture
 def local_agent():
     plugin.unload_plugins()
+    importlib.reload(core)
     return core.Agent()
 
 

tests/test_plugins.py

@@ -9,12 +9,12 @@ from shepherd.agent import plugin
 
 @pytest.fixture
 def simple_plugin(request):
+    # Load a simple plugin as a custom plugin, using `./assets` as the plugin dir
     interface = plugin.load_plugin("simpletestplugin", Path(request.fspath.dirname)/'assets')
     return interface
 
 
 def test_simple_plugin_load(simple_plugin: plugin.PluginInterface):
-    # If successful, will load as if it's a custom plugin
     assert simple_plugin._plugin_name == "simpletestplugin"
 
 
@@ -28,6 +28,12 @@ def test_simple_interface_function_load(simple_plugin: plugin.PluginInterface):
 def simple_running_plugin(request):
     plugin.unload_plugin("simpletestplugin")
     interface = plugin.load_plugin("simpletestplugin", Path(request.fspath.dirname)/'assets')
+    # The plugin system is _not_ responsible for making sure the config passed to it is valid. It
+    # stores and provides the conf-spec, but it's up to Core.Agent to actually validate the
+    # config - otherwise we wouldn't be able to do the whole multiple-layer-fallback thing before
+    # actually initialising the plugin.
+    # Therefore, part of the promise we make with the plugin interface is that the config we pass
+    # in _will_ fit the config-spec
     template_config = interface.confspec.get_template()
     plugin.init_plugins({"simpletestplugin": template_config})
     return interface