From 2da18ca572ae6c9b08b4f212fd8558327b9b9949 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Tue, 23 Feb 2016 14:36:31 -0700 Subject: [PATCH 01/13] Factor out plugin loading. --- flocker/common/plugin.py | 36 ++++++++++++++++++++++++++++++++++++ flocker/node/script.py | 9 +++------ 2 files changed, 39 insertions(+), 6 deletions(-) create mode 100644 flocker/common/plugin.py diff --git a/flocker/common/plugin.py b/flocker/common/plugin.py new file mode 100644 index 0000000000..c1148878e8 --- /dev/null +++ b/flocker/common/plugin.py @@ -0,0 +1,36 @@ +from twisted.python.reflect import namedAny +from characteristic import attributes + + +@attributes(["plugin_name"]) +class PluginNotFound(Exception): + """ + A plugin with the given name was not found. + + :attr str plugin_name: Name of the plugin looked for. + """ + + +def get_plugin(plugin_name, builtins, module_attribute): + """ + Find the backend in ``backends`` that matches the one named by + ``backend_name``. If not found then attempt is made to load it as + plugin. + + :param backend_name: The name of the backend. + :param backends: Collection of `BackendDescription`` instances. + + :raise ValueError: If ``backend_name`` doesn't match any known backend. + :return: The matching ``BackendDescription``. + """ + for builtin in builtins: + if builtin.name == plugin_name: + return builtin + try: + return getattr(namedAny(plugin_name), module_attribute) + except (AttributeError, ValueError): + raise PluginNotFound(plugin_name=plugin_name) + raise ValueError( + "'{!s}' is neither a built-in backend nor a 3rd party " + "module.".format(plugin_name), + ) diff --git a/flocker/node/script.py b/flocker/node/script.py index 1fe918a657..6b4406a92e 100644 --- a/flocker/node/script.py +++ b/flocker/node/script.py @@ -26,7 +26,6 @@ from twisted.internet import reactor # pylint: disable=unused-import from twisted.internet.defer import succeed from twisted.python.constants import Names, NamedConstant -from twisted.python.reflect import namedAny from ..volume.filesystems import zfs from ..volume.service import ( @@ -504,12 +503,10 @@ def get_backend(backend_name, backends=_DEFAULT_BACKENDS): :raise ValueError: If ``backend_name`` doesn't match any known backend. :return: The matching ``BackendDescription``. """ - for backend in backends: - if backend.name == backend_name: - return backend + from flocker.common.plugin import get_plugin, PluginNotFound try: - return namedAny(backend_name + ".FLOCKER_BACKEND") - except (AttributeError, ValueError): + return get_plugin(backend_name, backends, "FLOCKER_BACKEND") + except PluginNotFound: raise ValueError( "'{!s}' is neither a built-in backend nor a 3rd party " "module.".format(backend_name), From 548277fd3af82e2338346e56735c6953fe040732 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Fri, 26 Feb 2016 11:23:42 -0700 Subject: [PATCH 02/13] Factor out backend description code from `node.script`. --- flocker/node/__init__.py | 3 +- flocker/node/backends.py | 150 +++++++++++++++++++++++++++++++++++ flocker/node/script.py | 166 +++------------------------------------ 3 files changed, 163 insertions(+), 156 deletions(-) create mode 100644 flocker/node/backends.py diff --git a/flocker/node/__init__.py b/flocker/node/__init__.py index 3ffa57038a..4e3b219abf 100644 --- a/flocker/node/__init__.py +++ b/flocker/node/__init__.py @@ -16,7 +16,8 @@ from ._container import ApplicationNodeDeployer from ._p2p import P2PManifestationDeployer -from .script import BackendDescription, DeployerType +from .backends import BackendDescription +from .script import DeployerType from ._docker import dockerpy_client diff --git a/flocker/node/backends.py b/flocker/node/backends.py new file mode 100644 index 0000000000..be9a66ea0d --- /dev/null +++ b/flocker/node/backends.py @@ -0,0 +1,150 @@ +from pyrsistent import PClass, field, pset_field + +from twisted.python.filepath import FilePath +from twisted.python.constants import Names, NamedConstant + + +from ..common.plugin import get_plugin + +from ..volume.filesystems import zfs +from ..volume.service import ( + VolumeService, DEFAULT_CONFIG_PATH, FLOCKER_MOUNTPOINT, FLOCKER_POOL) + +from .agents.loopback import ( + LoopbackBlockDeviceAPI, +) +from .agents.cinder import cinder_from_configuration +from .agents.ebs import aws_from_configuration + + +def _zfs_storagepool( + reactor, pool=FLOCKER_POOL, mount_root=None, volume_config_path=None): + """ + Create a ``VolumeService`` with a ``zfs.StoragePool``. + + :param pool: The name of the ZFS storage pool to use. + :param bytes mount_root: The path to the directory where ZFS filesystems + will be mounted. + :param bytes volume_config_path: The path to the volume service's + configuration file. + + :return: The ``VolumeService``, started. + """ + if mount_root is None: + mount_root = FLOCKER_MOUNTPOINT + else: + mount_root = FilePath(mount_root) + if volume_config_path is None: + config_path = DEFAULT_CONFIG_PATH + else: + config_path = FilePath(volume_config_path) + + pool = zfs.StoragePool( + reactor=reactor, name=pool, mount_root=mount_root, + ) + api = VolumeService( + config_path=config_path, + pool=pool, + reactor=reactor, + ) + api.startService() + return api + + +class DeployerType(Names): + """ + References to the different ``IDeployer`` implementations that are + available. + + :ivar p2p: The "peer-to-peer" deployer - suitable for use with system like + ZFS where nodes interact directly with each other for data movement. + :ivar block: The Infrastructure-as-a-Service deployer - suitable for use + with system like EBS where volumes can be attached to nodes as block + devices and then detached (and then re-attached to other nodes). + """ + p2p = NamedConstant() + block = NamedConstant() + + +class BackendDescription(PClass): + """ + Represent one kind of storage backend we might be able to use. + + :ivar name: The human-meaningful name of this storage backend. + :ivar needs_reactor: A flag which indicates whether this backend's API + factory needs to have a reactor passed to it. + :ivar needs_cluster_id: A flag which indicates whether this backend's API + factory needs to have the cluster's unique identifier passed to it. + :ivar api_factory: An object which can be called with some simple + configuration data and which returns the API object implementing this + storage backend. + :ivar required_config: A set of the dataset configuration keys + required to initialize this backend. + :type required_config: ``PSet`` of ``unicode`` + :ivar deployer_type: A constant from ``DeployerType`` indicating which kind + of ``IDeployer`` the API object returned by ``api_factory`` is usable + with. + """ + name = field(type=unicode, mandatory=True) + needs_reactor = field(type=bool, mandatory=True) + # XXX Eventually everyone will take cluster_id so we will throw this flag + # out. + needs_cluster_id = field(type=bool, mandatory=True) + # Config "dataset" keys required to initialize this backend. + required_config = pset_field(unicode) + api_factory = field(mandatory=True) + deployer_type = field( + mandatory=True, + invariant=lambda value: ( + value in DeployerType.iterconstants(), "Unknown deployer_type" + ), + ) + +# These structures should be created dynamically to handle plug-ins +_DEFAULT_BACKENDS = [ + # P2PManifestationDeployer doesn't currently know anything about + # cluster_uuid. It probably should so that it can make sure it + # only talks to other nodes in the same cluster (maybe the + # authentication layer would mostly handle this but maybe not if + # you're slightly careless with credentials - also ZFS backend + # doesn't use TLS yet). + BackendDescription( + name=u"zfs", needs_reactor=True, needs_cluster_id=False, + api_factory=_zfs_storagepool, deployer_type=DeployerType.p2p, + ), + BackendDescription( + name=u"loopback", needs_reactor=False, needs_cluster_id=False, + # XXX compute_instance_id is the wrong type + api_factory=LoopbackBlockDeviceAPI.from_path, + deployer_type=DeployerType.block, + ), + BackendDescription( + name=u"openstack", needs_reactor=False, needs_cluster_id=True, + api_factory=cinder_from_configuration, + deployer_type=DeployerType.block, + required_config={u"region"}, + ), + BackendDescription( + name=u"aws", needs_reactor=False, needs_cluster_id=True, + api_factory=aws_from_configuration, + deployer_type=DeployerType.block, + required_config={ + u"region", u"zone", u"access_key_id", u"secret_access_key", + }, + ), +] + + +def get_backend(backend_name, backends=_DEFAULT_BACKENDS): + """ + Find the backend in ``backends`` that matches the one named by + ``backend_name``. If not found then attempt is made to load it as + plugin. + + :param backend_name: The name of the backend. + :param backends: Collection of `BackendDescription`` instances. + + :raise ValueError: If ``backend_name`` doesn't match any known backend. + :return: The matching ``BackendDescription``. + """ + return get_plugin(backend_name, backends, "FLOCKER_BACKEND") diff --git a/flocker/node/script.py b/flocker/node/script.py index 6b4406a92e..b74e4059a5 100644 --- a/flocker/node/script.py +++ b/flocker/node/script.py @@ -14,7 +14,7 @@ from jsonschema import FormatChecker, Draft4Validator -from pyrsistent import PClass, field, PMap, pmap, pvector, pset_field +from pyrsistent import PClass, field, PMap, pmap from eliot import ActionType, fields @@ -25,15 +25,12 @@ from twisted.internet.ssl import Certificate from twisted.internet import reactor # pylint: disable=unused-import from twisted.internet.defer import succeed -from twisted.python.constants import Names, NamedConstant -from ..volume.filesystems import zfs -from ..volume.service import ( - VolumeService, DEFAULT_CONFIG_PATH, FLOCKER_MOUNTPOINT, FLOCKER_POOL) from ..common.script import ( ICommandLineScript, flocker_standard_options, FlockerScriptRunner, main_for_service) +from flocker.common.plugin import PluginNotFound from . import P2PManifestationDeployer, ApplicationNodeDeployer from ._loop import AgentLoopService from .exceptions import StorageInitializationError @@ -44,14 +41,11 @@ from .agents.blockdevice import ( BlockDeviceDeployer, ProcessLifetimeCache, ) -from .agents.loopback import ( - LoopbackBlockDeviceAPI, -) -from .agents.cinder import cinder_from_configuration -from .agents.ebs import aws_from_configuration from ..ca import ControlServicePolicy, NodeCredential from ..common._era import get_era +from .backends import DeployerType, get_backend + __all__ = [ "flocker_dataset_agent_main", "flocker_container_agent_main", @@ -364,123 +358,6 @@ def get_configuration(options): return configuration -def _zfs_storagepool( - reactor, pool=FLOCKER_POOL, mount_root=None, volume_config_path=None): - """ - Create a ``VolumeService`` with a ``zfs.StoragePool``. - - :param pool: The name of the ZFS storage pool to use. - :param bytes mount_root: The path to the directory where ZFS filesystems - will be mounted. - :param bytes volume_config_path: The path to the volume service's - configuration file. - - :return: The ``VolumeService``, started. - """ - if mount_root is None: - mount_root = FLOCKER_MOUNTPOINT - else: - mount_root = FilePath(mount_root) - if volume_config_path is None: - config_path = DEFAULT_CONFIG_PATH - else: - config_path = FilePath(volume_config_path) - - pool = zfs.StoragePool( - reactor=reactor, name=pool, mount_root=mount_root, - ) - api = VolumeService( - config_path=config_path, - pool=pool, - reactor=reactor, - ) - api.startService() - return api - - -class DeployerType(Names): - """ - References to the different ``IDeployer`` implementations that are - available. - - :ivar p2p: The "peer-to-peer" deployer - suitable for use with system like - ZFS where nodes interact directly with each other for data movement. - :ivar block: The Infrastructure-as-a-Service deployer - suitable for use - with system like EBS where volumes can be attached to nodes as block - devices and then detached (and then re-attached to other nodes). - """ - p2p = NamedConstant() - block = NamedConstant() - - -class BackendDescription(PClass): - """ - Represent one kind of storage backend we might be able to use. - - :ivar name: The human-meaningful name of this storage backend. - :ivar needs_reactor: A flag which indicates whether this backend's API - factory needs to have a reactor passed to it. - :ivar needs_cluster_id: A flag which indicates whether this backend's API - factory needs to have the cluster's unique identifier passed to it. - :ivar api_factory: An object which can be called with some simple - configuration data and which returns the API object implementing this - storage backend. - :ivar required_config: A set of the dataset configuration keys - required to initialize this backend. - :type required_config: ``PSet`` of ``unicode`` - :ivar deployer_type: A constant from ``DeployerType`` indicating which kind - of ``IDeployer`` the API object returned by ``api_factory`` is usable - with. - """ - name = field(type=unicode, mandatory=True) - needs_reactor = field(type=bool, mandatory=True) - # XXX Eventually everyone will take cluster_id so we will throw this flag - # out. - needs_cluster_id = field(type=bool, mandatory=True) - # Config "dataset" keys required to initialize this backend. - required_config = pset_field(unicode) - api_factory = field(mandatory=True) - deployer_type = field( - mandatory=True, - invariant=lambda value: ( - value in DeployerType.iterconstants(), "Unknown deployer_type" - ), - ) - -# These structures should be created dynamically to handle plug-ins -_DEFAULT_BACKENDS = [ - # P2PManifestationDeployer doesn't currently know anything about - # cluster_uuid. It probably should so that it can make sure it - # only talks to other nodes in the same cluster (maybe the - # authentication layer would mostly handle this but maybe not if - # you're slightly careless with credentials - also ZFS backend - # doesn't use TLS yet). - BackendDescription( - name=u"zfs", needs_reactor=True, needs_cluster_id=False, - api_factory=_zfs_storagepool, deployer_type=DeployerType.p2p, - ), - BackendDescription( - name=u"loopback", needs_reactor=False, needs_cluster_id=False, - # XXX compute_instance_id is the wrong type - api_factory=LoopbackBlockDeviceAPI.from_path, - deployer_type=DeployerType.block, - ), - BackendDescription( - name=u"openstack", needs_reactor=False, needs_cluster_id=True, - api_factory=cinder_from_configuration, - deployer_type=DeployerType.block, - required_config={u"region"}, - ), - BackendDescription( - name=u"aws", needs_reactor=False, needs_cluster_id=True, - api_factory=aws_from_configuration, - deployer_type=DeployerType.block, - required_config={ - u"region", u"zone", u"access_key_id", u"secret_access_key", - }, - ), -] - _DEFAULT_DEPLOYERS = { DeployerType.p2p: lambda api, **kw: P2PManifestationDeployer(volume_service=api, **kw), @@ -491,28 +368,6 @@ class BackendDescription(PClass): } -def get_backend(backend_name, backends=_DEFAULT_BACKENDS): - """ - Find the backend in ``backends`` that matches the one named by - ``backend_name``. If not found then attempt is made to load it as - plugin. - - :param backend_name: The name of the backend. - :param backends: Collection of `BackendDescription`` instances. - - :raise ValueError: If ``backend_name`` doesn't match any known backend. - :return: The matching ``BackendDescription``. - """ - from flocker.common.plugin import get_plugin, PluginNotFound - try: - return get_plugin(backend_name, backends, "FLOCKER_BACKEND") - except PluginNotFound: - raise ValueError( - "'{!s}' is neither a built-in backend nor a 3rd party " - "module.".format(backend_name), - ) - - def get_api(backend, api_args, reactor, cluster_id): """ Get an storage driver which can be used to create an ``IDeployer``. @@ -549,8 +404,6 @@ def get_api(backend, api_args, reactor, cluster_id): class AgentService(PClass): """ - :ivar backends: ``BackendDescription`` instances describing how to use each - available storage backend. :ivar deployers: Factories to create ``IDeployer`` providers given an API object and some extra keyword arguments. Keyed on a value from ``DeployerType``. @@ -563,9 +416,6 @@ class AgentService(PClass): :ivar get_external_ip: Typically ``_get_external_ip``, but overrideable for tests. """ - backends = field( - factory=pvector, initial=_DEFAULT_BACKENDS, mandatory=True, - ) deployers = field(factory=pmap, initial=_DEFAULT_DEPLOYERS, mandatory=True) reactor = field(initial=reactor, mandatory=True) @@ -627,7 +477,13 @@ def get_backend(self): :raise ValueError: If ``backend_name`` doesn't match any known backend. :return: The matching ``BackendDescription``. """ - return get_backend(self.backend_name, self.backends) + try: + return get_backend(self.backend_name) + except PluginNotFound: + raise ValueError( + "'{!s}' is neither a built-in backend nor a 3rd party " + "module.".format(self.backend_name), + ) # Needs tests: FLOC-1964. def get_tls_context(self): From 37e81456453ec4d275aa4bf7efe43b2bca0a335d Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Fri, 26 Feb 2016 13:58:03 -0700 Subject: [PATCH 03/13] Testable. --- flocker/common/plugin.py | 95 +++++++++++++++++++++++++++++++--------- flocker/node/backends.py | 21 +++------ flocker/node/script.py | 17 ++++--- 3 files changed, 89 insertions(+), 44 deletions(-) diff --git a/flocker/common/plugin.py b/flocker/common/plugin.py index c1148878e8..91c389f9e6 100644 --- a/flocker/common/plugin.py +++ b/flocker/common/plugin.py @@ -1,5 +1,6 @@ -from twisted.python.reflect import namedAny from characteristic import attributes +from pyrsistent import PClass, field, PVector, pvector +from twisted.python.reflect import namedAny @attributes(["plugin_name"]) @@ -9,28 +10,82 @@ class PluginNotFound(Exception): :attr str plugin_name: Name of the plugin looked for. """ + def __str__(self): + return ( + "'{!s}' is neither a built-in plugin nor a 3rd party " + "module.".format(self.plugin_name) + ) -def get_plugin(plugin_name, builtins, module_attribute): +@attributes(["plugin_name", "plugin_type", "actual_type", "module_attribute"]) +class InvalidPlugin(Exception): """ - Find the backend in ``backends`` that matches the one named by - ``backend_name``. If not found then attempt is made to load it as - plugin. - - :param backend_name: The name of the backend. - :param backends: Collection of `BackendDescription`` instances. + A plugin with the given name was not found. - :raise ValueError: If ``backend_name`` doesn't match any known backend. - :return: The matching ``BackendDescription``. + :attr str plugin_name: Name of the plugin looked for. """ - for builtin in builtins: - if builtin.name == plugin_name: - return builtin - try: - return getattr(namedAny(plugin_name), module_attribute) - except (AttributeError, ValueError): - raise PluginNotFound(plugin_name=plugin_name) - raise ValueError( - "'{!s}' is neither a built-in backend nor a 3rd party " - "module.".format(plugin_name), + def __str__(self): + return ( + "The 3rd party plugin '{plugin_name!s}' does not " + "correspond to the expected interface.\n" + "`{plugin_name!s}.{module_attribute!s}` is of " + "type `{actual_type.__name__}`, not `{plugin_type.__name__}`." + .format( + plugin_name=self.plugin_name, + actual_type=self.actual_type, + plugin_type=self.plugin_type, + module_attribute=self.module_attribute, + ) ) + + +class PluginLoader(PClass): + builtin_plugins = field(PVector, mandatory=True, factory=pvector) + module_attribute = field(str, mandatory=True) + plugin_type = field(type, mandatory=True) + + def __invariant__(self): + for builtin in self.builtin_plugins: + if not isinstance(builtin, self.plugin_type): + return ( + False, + "Builtin plugins must be of `{plugin_type.__name__}`, not " + "`{actual_type.__name__}`.".format( + plugin_type=self.plugin_type, + actual_type=type(builtin), + ) + ) + + return (True, "") + + def get_plugin(self, plugin_name): + """ + Find the backend in ``backends`` that matches the one named by + ``backend_name``. If not found then attempt is made to load it as + plugin. + + :param backend_name: The name of the backend. + :param backends: Collection of `BackendDescription`` instances. + + :raise PluginNotFound: If ``backend_name`` doesn't match any + known backend. + :return: The matching ``BackendDescription``. + """ + for builtin in self.builtin_plugins: + if builtin.name == plugin_name: + return builtin + + try: + plugin = getattr(namedAny(plugin_name), self.module_attribute) + except (AttributeError, ValueError): + raise PluginNotFound(plugin_name=plugin_name) + + if not isinstance(plugin, self.plugin_type): + raise InvalidPlugin( + plugin_name=plugin_name, + plugin_type=self.plugin_type, + actual_type=type(plugin), + module_attribute=self.module_attribute, + ) + + return plugin diff --git a/flocker/node/backends.py b/flocker/node/backends.py index be9a66ea0d..35cde22fe2 100644 --- a/flocker/node/backends.py +++ b/flocker/node/backends.py @@ -4,7 +4,7 @@ from twisted.python.constants import Names, NamedConstant -from ..common.plugin import get_plugin +from ..common.plugin import PluginLoader from ..volume.filesystems import zfs from ..volume.service import ( @@ -134,17 +134,8 @@ class BackendDescription(PClass): ), ] - -def get_backend(backend_name, backends=_DEFAULT_BACKENDS): - """ - Find the backend in ``backends`` that matches the one named by - ``backend_name``. If not found then attempt is made to load it as - plugin. - - :param backend_name: The name of the backend. - :param backends: Collection of `BackendDescription`` instances. - - :raise ValueError: If ``backend_name`` doesn't match any known backend. - :return: The matching ``BackendDescription``. - """ - return get_plugin(backend_name, backends, "FLOCKER_BACKEND") +backend_loader = PluginLoader( + builtin_plugins=_DEFAULT_BACKENDS, + module_attribute="FLOCKER_BACKEND", + plugin_type=BackendDescription, +) diff --git a/flocker/node/script.py b/flocker/node/script.py index b74e4059a5..6565df8a56 100644 --- a/flocker/node/script.py +++ b/flocker/node/script.py @@ -30,7 +30,7 @@ from ..common.script import ( ICommandLineScript, flocker_standard_options, FlockerScriptRunner, main_for_service) -from flocker.common.plugin import PluginNotFound +from ..common.plugin import PluginLoader from . import P2PManifestationDeployer, ApplicationNodeDeployer from ._loop import AgentLoopService from .exceptions import StorageInitializationError @@ -44,7 +44,7 @@ from ..ca import ControlServicePolicy, NodeCredential from ..common._era import get_era -from .backends import DeployerType, get_backend +from .backends import DeployerType, backend_loader __all__ = [ "flocker_dataset_agent_main", @@ -416,6 +416,11 @@ class AgentService(PClass): :ivar get_external_ip: Typically ``_get_external_ip``, but overrideable for tests. """ + backends = field( + PluginLoader, + mandatory=True, + initial=backend_loader, + ) deployers = field(factory=pmap, initial=_DEFAULT_DEPLOYERS, mandatory=True) reactor = field(initial=reactor, mandatory=True) @@ -477,13 +482,7 @@ def get_backend(self): :raise ValueError: If ``backend_name`` doesn't match any known backend. :return: The matching ``BackendDescription``. """ - try: - return get_backend(self.backend_name) - except PluginNotFound: - raise ValueError( - "'{!s}' is neither a built-in backend nor a 3rd party " - "module.".format(self.backend_name), - ) + return self.backends.get_plugin(self.backend_name) # Needs tests: FLOC-1964. def get_tls_context(self): From f332d763e6b481ec3b2121f39cf3a45250194d83 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Fri, 26 Feb 2016 14:03:37 -0700 Subject: [PATCH 04/13] Update tests. --- flocker/node/test/test_script.py | 34 ++++++++++++++------------------ 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/flocker/node/test/test_script.py b/flocker/node/test/test_script.py index d731ee4e94..b31ef4cc11 100644 --- a/flocker/node/test/test_script.py +++ b/flocker/node/test/test_script.py @@ -27,6 +27,7 @@ from twisted.python.usage import UsageError from ...common.script import ICommandLineScript +from ...common.plugin import PluginNotFound from ...common import get_all_ips from ...common._era import get_era @@ -34,9 +35,10 @@ AgentScript, ContainerAgentOptions, AgentServiceFactory, DatasetAgentOptions, validate_configuration, _context_factory_and_credential, DatasetServiceFactory, - AgentService, BackendDescription, get_configuration, + AgentService, get_configuration, DeployerType, _get_external_ip, LOG_GET_EXTERNAL_IP ) +from ..backends import BackendDescription from ..agents.cinder import CinderBlockDeviceAPI from ..agents.ebs import EBSBlockDeviceAPI @@ -322,8 +324,8 @@ class API(PClass): class WrongAPI(object): pass - agent_service = self.agent_service.set( - "backends", [ + agent_service = self.agent_service.transform( + ["backends", "builtin_plugins"], [ BackendDescription( name=u"foo", needs_reactor=False, needs_cluster_id=False, api_factory=API, deployer_type=DeployerType.p2p, @@ -356,8 +358,8 @@ def test_needs_reactor(self): class API(PClass): reactor = field() - agent_service = self.agent_service.set( - "backends", [ + agent_service = self.agent_service.transform( + ["backends", "builtin_plugins"], [ BackendDescription( name=self.agent_service.backend_name, needs_reactor=True, needs_cluster_id=False, @@ -383,8 +385,8 @@ def test_needs_cluster_id(self): class API(PClass): cluster_id = field() - agent_service = self.agent_service.set( - "backends", [ + agent_service = self.agent_service.transform( + ["backends", "builtin_plugins"], [ BackendDescription( name=self.agent_service.backend_name, needs_reactor=False, needs_cluster_id=True, @@ -408,8 +410,8 @@ class API(PClass): region = field() api_key = field() - agent_service = self.agent_service.set( - "backends", [ + agent_service = self.agent_service.transform( + ["backends", "builtin_plugins"], [ BackendDescription( name=self.agent_service.backend_name, needs_reactor=False, needs_cluster_id=False, @@ -489,10 +491,7 @@ def test_wrong_attribute_3rd_party_backend(self): agent_service = self.agent_service.set( "backend_name", u"flocker.not.a.real.module", ) - exc = self.assertRaises(ValueError, agent_service.get_api) - self.assertEqual(str(exc), - "'flocker.not.a.real.module' is neither a " - "built-in backend nor a 3rd party module.") + self.assertRaises(PluginNotFound, agent_service.get_api) def test_wrong_package_3rd_party_backend(self): """ @@ -502,10 +501,7 @@ def test_wrong_package_3rd_party_backend(self): agent_service = self.agent_service.set( "backend_name", u"notarealmoduleireallyhope", ) - exc = self.assertRaises(ValueError, agent_service.get_api) - self.assertEqual(str(exc), - "'notarealmoduleireallyhope' is neither a " - "built-in backend nor a 3rd party module.") + self.assertRaises(PluginNotFound, agent_service.get_api) class AgentServiceDeployerTests(TestCase): @@ -542,8 +538,8 @@ def get_external_ip(host, port): agent_service = self.agent_service.set( "get_external_ip", get_external_ip, - ).set( - "backends", [ + ).transform( + ["backends", "builtin_plugins"], [ BackendDescription( name=self.agent_service.backend_name, needs_reactor=False, needs_cluster_id=False, From 0f122955aeba98f30dd1373a7c014fb4b896065f Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Mon, 29 Feb 2016 11:56:51 -0700 Subject: [PATCH 05/13] Copyright headers. --- flocker/common/plugin.py | 6 ++++++ flocker/node/backends.py | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/flocker/common/plugin.py b/flocker/common/plugin.py index 91c389f9e6..9f7f5695a7 100644 --- a/flocker/common/plugin.py +++ b/flocker/common/plugin.py @@ -1,3 +1,9 @@ +# Copyright ClusterHQ Inc. See LICENSE file for details. + +""" +Tools for loading third-party plugins. +""" + from characteristic import attributes from pyrsistent import PClass, field, PVector, pvector from twisted.python.reflect import namedAny diff --git a/flocker/node/backends.py b/flocker/node/backends.py index 35cde22fe2..dbef8e38e3 100644 --- a/flocker/node/backends.py +++ b/flocker/node/backends.py @@ -1,3 +1,9 @@ +# Copyright ClusterHQ Inc. See LICENSE file for details. + +""" +Dataset backend descriptions. +""" + from pyrsistent import PClass, field, pset_field from twisted.python.filepath import FilePath From 45b9fc58d57432504f7e2f002c6c6a953c165b6f Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Mon, 29 Feb 2016 11:57:04 -0700 Subject: [PATCH 06/13] Docstring and errors. --- flocker/common/plugin.py | 61 ++++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 9 deletions(-) diff --git a/flocker/common/plugin.py b/flocker/common/plugin.py index 9f7f5695a7..726eb3c0f0 100644 --- a/flocker/common/plugin.py +++ b/flocker/common/plugin.py @@ -23,17 +23,44 @@ def __str__(self): ) -@attributes(["plugin_name", "plugin_type", "actual_type", "module_attribute"]) +@attributes(["plugin_name"]) class InvalidPlugin(Exception): """ - A plugin with the given name was not found. + A module with the given plugin name was found, but doesn't + provide a valid flocker plugin. :attr str plugin_name: Name of the plugin looked for. """ + + +@attributes(["module_attribute"]) +class MissingPluginAttribute(InvalidPlugin): + """ + The named module doesn't have the attribute expected of plugins. + """ + def __str__(self): + return ( + "The 3rd party plugin '{plugin_name!s}' does not " + "correspond to the expected interface. " + "`{plugin_name!s}.{module_attribute!s}` is not defined." + .format( + plugin_name=self.plugin_name, + actual_type=self.actual_type, + plugin_type=self.plugin_type, + module_attribute=self.module_attribute, + ) + ) + + +@attributes(["plugin_type", "actual_type", "module_attribute"]) +class InvalidPluginType(InvalidPlugin): + """ + A plugin with the given name was not found. + """ def __str__(self): return ( "The 3rd party plugin '{plugin_name!s}' does not " - "correspond to the expected interface.\n" + "correspond to the expected interface. " "`{plugin_name!s}.{module_attribute!s}` is of " "type `{actual_type.__name__}`, not `{plugin_type.__name__}`." .format( @@ -46,6 +73,12 @@ def __str__(self): class PluginLoader(PClass): + """ + :ivar PVector builtin_plugins: The plugins shipped with flocker. + :ivar str module_attribute: The module attribute that third-party plugins + should declare. + :ivar type plugin_type: The type describing a plugin. + """ builtin_plugins = field(PVector, mandatory=True, factory=pvector) module_attribute = field(str, mandatory=True) plugin_type = field(type, mandatory=True) @@ -70,24 +103,34 @@ def get_plugin(self, plugin_name): ``backend_name``. If not found then attempt is made to load it as plugin. - :param backend_name: The name of the backend. + :param plugin_name: The name of the backend. :param backends: Collection of `BackendDescription`` instances. - :raise PluginNotFound: If ``backend_name`` doesn't match any - known backend. - :return: The matching ``BackendDescription``. + :raise PluginNotFound: If ``plugin_name`` doesn't match any + known plugin. + :raise InvalidPlugin: If ``plugin_name`` names a module that + doesn't satisfy the plugin interface. + :return: The matching ``plugin_type`` instance. """ for builtin in self.builtin_plugins: if builtin.name == plugin_name: return builtin try: - plugin = getattr(namedAny(plugin_name), self.module_attribute) + plugin_module = namedAny(plugin_name) except (AttributeError, ValueError): raise PluginNotFound(plugin_name=plugin_name) + try: + plugin = getattr(plugin_module, self.module_attribute) + except AttributeError: + raise MissingPluginAttribute( + plugin_name=plugin_name, + module_attribute=self.module_attribute, + ) + if not isinstance(plugin, self.plugin_type): - raise InvalidPlugin( + raise InvalidPluginType( plugin_name=plugin_name, plugin_type=self.plugin_type, actual_type=type(plugin), From c5f0fef65b3eaa1781b8a9f7949fd6f9b3aba3bd Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Mon, 29 Feb 2016 12:55:11 -0700 Subject: [PATCH 07/13] Simplify naming. --- flocker/common/plugin.py | 2 +- flocker/node/script.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/flocker/common/plugin.py b/flocker/common/plugin.py index 726eb3c0f0..5808e34a8c 100644 --- a/flocker/common/plugin.py +++ b/flocker/common/plugin.py @@ -97,7 +97,7 @@ def __invariant__(self): return (True, "") - def get_plugin(self, plugin_name): + def get(self, plugin_name): """ Find the backend in ``backends`` that matches the one named by ``backend_name``. If not found then attempt is made to load it as diff --git a/flocker/node/script.py b/flocker/node/script.py index 6565df8a56..9eb6936e73 100644 --- a/flocker/node/script.py +++ b/flocker/node/script.py @@ -482,7 +482,7 @@ def get_backend(self): :raise ValueError: If ``backend_name`` doesn't match any known backend. :return: The matching ``BackendDescription``. """ - return self.backends.get_plugin(self.backend_name) + return self.backends.get(self.backend_name) # Needs tests: FLOC-1964. def get_tls_context(self): From 061b2596988a08b2658a24666311e3b626b0346d Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Mon, 29 Feb 2016 12:55:31 -0700 Subject: [PATCH 08/13] Plugin tests. --- flocker/common/plugin.py | 11 +-- flocker/common/test/test_plugin.py | 113 +++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 8 deletions(-) create mode 100644 flocker/common/test/test_plugin.py diff --git a/flocker/common/plugin.py b/flocker/common/plugin.py index 5808e34a8c..de2148b351 100644 --- a/flocker/common/plugin.py +++ b/flocker/common/plugin.py @@ -23,17 +23,14 @@ def __str__(self): ) -@attributes(["plugin_name"]) -class InvalidPlugin(Exception): +class InvalidPlugin(Exception, object): """ A module with the given plugin name was found, but doesn't provide a valid flocker plugin. - - :attr str plugin_name: Name of the plugin looked for. """ -@attributes(["module_attribute"]) +@attributes(["plugin_name", "module_attribute"]) class MissingPluginAttribute(InvalidPlugin): """ The named module doesn't have the attribute expected of plugins. @@ -45,14 +42,12 @@ def __str__(self): "`{plugin_name!s}.{module_attribute!s}` is not defined." .format( plugin_name=self.plugin_name, - actual_type=self.actual_type, - plugin_type=self.plugin_type, module_attribute=self.module_attribute, ) ) -@attributes(["plugin_type", "actual_type", "module_attribute"]) +@attributes(["plugin_name", "plugin_type", "actual_type", "module_attribute"]) class InvalidPluginType(InvalidPlugin): """ A plugin with the given name was not found. diff --git a/flocker/common/test/test_plugin.py b/flocker/common/test/test_plugin.py new file mode 100644 index 0000000000..e0eefdaa28 --- /dev/null +++ b/flocker/common/test/test_plugin.py @@ -0,0 +1,113 @@ +# Copyright ClusterHQ Inc. See LICENSE file for details. + +""" +Tests for ``flocker.common.plugin`` +""" + +from pyrsistent import PClass, field + +from flocker.testtools import TestCase + +from ..plugin import ( + PluginLoader, + PluginNotFound, + MissingPluginAttribute, + InvalidPluginType, +) + + +class DummyDescription(PClass): + name = field(unicode, mandatory=True) + + +# The following test examples use classes instead of modules for +# namespacing. Real plugins should use modules. +class DummyPlugin(object): + """ + A plugin." + """ + FLOCKER_PLUGIN = DummyDescription( + name=u"dummyplugin", + ) + + +class DummyPluginMissingAttribute(object): + """ + A purported plugin that is missing the expected attribute. + """ + + +class DummyPluginWrongType(object): + """ + A purported plugin that has the wrong type of description. + """ + FLOCKER_PLUGIN = object() + + +DUMMY_LOADER = PluginLoader( + builtin_plugins=[], + module_attribute="FLOCKER_PLUGIN", + plugin_type=DummyDescription, +) + + +class PluginLoaderTests(TestCase): + + def test_builtin_backend(self): + """ + If the plugin name is that of a pre-configured plugin, the + corresponding builtin plugin is returned. + """ + loader = DUMMY_LOADER.set( + "builtin_plugins", [ + DummyDescription(name=u"other-builtin"), + DummyDescription(name=u"builtin"), + ] + ) + plugin = loader.get("builtin") + self.assertEqual(plugin, DummyDescription(name=u"builtin")) + + def test_3rd_party_plugin(self): + """ + If the plugin name is not that of a pre-configured plugin, the + plugin name is treated as a Python import path, and the + specified attribute of that is used as the plugin. + """ + plugin = DUMMY_LOADER.get( + "flocker.common.test.test_plugin.DummyPlugin" + ) + self.assertEqual(plugin, DummyDescription(name=u"dummyplugin")) + + def test_wrong_package_3rd_party_backend(self): + """ + If the plugin name refers to an unimportable package, + ``PluginNotFound`` is raised. + """ + self.assertRaises( + PluginNotFound, + DUMMY_LOADER.get, + "notarealmoduleireallyhope", + ) + + def test_missing_attribute_3rd_party_backend(self): + """ + If the plugin name refers to an object that doesn't have the + specified attribute, ``MissingPluginAttribute`` is raised. + """ + self.assertRaises( + MissingPluginAttribute, + DUMMY_LOADER.get, + "flocker.common.test.test_plugin.DummyPluginMissingAttribute" + ) + + def test_wrong_attribute_type_3rd_party_backend(self): + """ + If the plugin name refers to an object whose specified + attribute isn't of the right type, ``InvalidPluginType`` is + raised. + """ + self.assertRaises( + InvalidPluginType, + DUMMY_LOADER.get, + "flocker.common.test.test_plugin.DummyPluginWrongType" + ) From c2f1477f3ae688d473f53fbd517495712806b391 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Mon, 29 Feb 2016 12:59:53 -0700 Subject: [PATCH 09/13] Docstring. --- flocker/common/test/test_plugin.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/flocker/common/test/test_plugin.py b/flocker/common/test/test_plugin.py index e0eefdaa28..ff04896132 100644 --- a/flocker/common/test/test_plugin.py +++ b/flocker/common/test/test_plugin.py @@ -17,6 +17,9 @@ class DummyDescription(PClass): + """ + Dummy plugin type." + """ name = field(unicode, mandatory=True) @@ -52,6 +55,9 @@ class DummyPluginWrongType(object): class PluginLoaderTests(TestCase): + """ + Tests for ``PluginLoader``. + """ def test_builtin_backend(self): """ From 4ea697725d0935aecae5e24bc263d7d0e754adda Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Mon, 29 Feb 2016 18:04:27 -0700 Subject: [PATCH 10/13] Get rid of unneeded subclassing of object. --- flocker/common/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flocker/common/plugin.py b/flocker/common/plugin.py index de2148b351..4be4140999 100644 --- a/flocker/common/plugin.py +++ b/flocker/common/plugin.py @@ -23,7 +23,7 @@ def __str__(self): ) -class InvalidPlugin(Exception, object): +class InvalidPlugin(Exception): """ A module with the given plugin name was found, but doesn't provide a valid flocker plugin. From b1ac004a550d86cf1a736e6cbb0236955179dd63 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Mon, 29 Feb 2016 18:05:49 -0700 Subject: [PATCH 11/13] Docstring. --- flocker/common/plugin.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flocker/common/plugin.py b/flocker/common/plugin.py index 4be4140999..925fc5d020 100644 --- a/flocker/common/plugin.py +++ b/flocker/common/plugin.py @@ -94,9 +94,9 @@ def __invariant__(self): def get(self, plugin_name): """ - Find the backend in ``backends`` that matches the one named by - ``backend_name``. If not found then attempt is made to load it as - plugin. + Find the plugin in ``builtin_plugins`` that matches the one named by + ``plugin_name``. If not found then an attempt is made to load it as + module describing a plugin. :param plugin_name: The name of the backend. :param backends: Collection of `BackendDescription`` instances. From 2a7d300227bc9a510ce5afe99c4b240cd832bd59 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Mon, 29 Feb 2016 18:08:14 -0700 Subject: [PATCH 12/13] Docstring. --- flocker/node/script.py | 1 + 1 file changed, 1 insertion(+) diff --git a/flocker/node/script.py b/flocker/node/script.py index 9eb6936e73..0f2ca03bf0 100644 --- a/flocker/node/script.py +++ b/flocker/node/script.py @@ -404,6 +404,7 @@ def get_api(backend, api_args, reactor, cluster_id): class AgentService(PClass): """ + :cvar PluginLoader backends: Plugin loader to get dataset backend from. :ivar deployers: Factories to create ``IDeployer`` providers given an API object and some extra keyword arguments. Keyed on a value from ``DeployerType``. From 0ff2c9149650ccc707af1b41b36ac13cf42e6a26 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Mon, 29 Feb 2016 19:00:14 -0700 Subject: [PATCH 13/13] Adjust acceptance tests to use plugin loader for finding backend. --- flocker/acceptance/testtools.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/flocker/acceptance/testtools.py b/flocker/acceptance/testtools.py index ba236dde5c..ee89d732b0 100644 --- a/flocker/acceptance/testtools.py +++ b/flocker/acceptance/testtools.py @@ -48,7 +48,8 @@ from ..ca import treq_with_authentication, UserCredential from ..testtools import random_name from ..apiclient import FlockerClient, DatasetState -from ..node.script import get_backend, get_api +from ..node.backends import backend_loader +from ..node.script import get_api from ..node import dockerpy_client from ..node.agents.blockdevice import _SyncToThreadedAsyncAPIAdapter from ..provision import reinstall_flocker_from_package_source @@ -247,7 +248,7 @@ def get_backend_api(cluster_id): backend_config = full_backend_config.get(backend_name) if 'backend' in backend_config: backend_config.pop('backend') - backend = get_backend(backend_name) + backend = backend_loader.get(backend_name) return get_api(backend, pmap(backend_config), reactor, cluster_id)