From ae0f54abeb9b95d8684193ec61135e8bfd275a16 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Thu, 31 May 2018 14:10:06 -0700 Subject: [PATCH] Directly link providers and request handlers Rather than relying on inspection and loading classes from specific filenames, ask each driver's Provider implementation to supply the NodeRequestHandler instance for requests. This is the first in a series of changes to remove all similar indirection from the drivers in order to make them easier to understand. The test driver is moved out of the fixtures subdir to be a "regular" driver because it now needs relative imports (provider code needs to import the handler code). Change-Id: Iad60d8bfe102d43b9fb1a1ff3ede13e4fa753572 --- nodepool/driver/__init__.py | 7 +++++- nodepool/driver/fake/provider.py | 4 ++++ nodepool/driver/openstack/handler.py | 22 ++++++++++--------- nodepool/driver/openstack/provider.py | 6 +++++ nodepool/driver/static/provider.py | 4 ++++ .../drivers => driver}/test/__init__.py | 0 .../drivers => driver}/test/config.py | 0 .../drivers => driver}/test/handler.py | 0 .../drivers => driver}/test/provider.py | 4 ++++ nodepool/launcher.py | 9 +++----- nodepool/tests/test_drivers.py | 4 ++-- 11 files changed, 41 insertions(+), 19 deletions(-) rename nodepool/{tests/fixtures/drivers => driver}/test/__init__.py (100%) rename nodepool/{tests/fixtures/drivers => driver}/test/config.py (100%) rename nodepool/{tests/fixtures/drivers => driver}/test/handler.py (100%) rename nodepool/{tests/fixtures/drivers => driver}/test/provider.py (87%) diff --git a/nodepool/driver/__init__.py b/nodepool/driver/__init__.py index c61d89130..71a1f333b 100644 --- a/nodepool/driver/__init__.py +++ b/nodepool/driver/__init__.py @@ -75,7 +75,6 @@ class Drivers: driver_obj = {} for name, parent_class in ( ("config", ProviderConfig), - ("handler", NodeRequestHandler), ("provider", Provider), ): driver_obj[name] = Drivers._load_class( @@ -202,6 +201,12 @@ class Provider(object, metaclass=abc.ABCMeta): """ pass + @abc.abstractmethod + def getRequestHandler(self, poolworker, request): + """Return a NodeRequestHandler for the supplied request + """ + pass + @abc.abstractmethod def listNodes(self): # TODO: This is used by the launcher to find leaked instances diff --git a/nodepool/driver/fake/provider.py b/nodepool/driver/fake/provider.py index dd6c6f376..45f6c2260 100644 --- a/nodepool/driver/fake/provider.py +++ b/nodepool/driver/fake/provider.py @@ -23,6 +23,7 @@ import shade from nodepool import exceptions from nodepool.driver.openstack.provider import OpenStackProvider +from nodepool.driver.fake.handler import FakeNodeRequestHandler class Dummy(object): @@ -303,3 +304,6 @@ class FakeProvider(OpenStackProvider): self.createServer_fails -= 1 raise Exception("Expected createServer exception") return super(FakeProvider, self).createServer(*args, **kwargs) + + def getRequestHandler(self, poolworker, request): + return FakeNodeRequestHandler(poolworker, request) diff --git a/nodepool/driver/openstack/handler.py b/nodepool/driver/openstack/handler.py index 9add86d72..09ab92ecd 100644 --- a/nodepool/driver/openstack/handler.py +++ b/nodepool/driver/openstack/handler.py @@ -24,7 +24,9 @@ from nodepool import nodeutils as utils from nodepool import zk from nodepool.driver.utils import NodeLauncher from nodepool.driver import NodeRequestHandler -from nodepool.driver.openstack.provider import QuotaInformation + +# Import entire module to avoid partial-loading, circular import +from nodepool.driver.openstack import provider class OpenStackNodeLauncher(NodeLauncher): @@ -300,10 +302,10 @@ class OpenStackNodeRequestHandler(NodeRequestHandler): # Now calculate pool specific quota. Values indicating no quota default # to math.inf representing infinity that can be calculated with. - pool_quota = QuotaInformation(cores=self.pool.max_cores, - instances=self.pool.max_servers, - ram=self.pool.max_ram, - default=math.inf) + pool_quota = provider.QuotaInformation(cores=self.pool.max_cores, + instances=self.pool.max_servers, + ram=self.pool.max_ram, + default=math.inf) pool_quota.subtract( self.manager.estimatedNodepoolQuotaUsed(self.zk, self.pool)) pool_quota.subtract(needed_quota) @@ -312,7 +314,7 @@ class OpenStackNodeRequestHandler(NodeRequestHandler): return pool_quota.non_negative() def hasProviderQuota(self, node_types): - needed_quota = QuotaInformation() + needed_quota = provider.QuotaInformation() for ntype in node_types: needed_quota.add( @@ -326,10 +328,10 @@ class OpenStackNodeRequestHandler(NodeRequestHandler): # Now calculate pool specific quota. Values indicating no quota default # to math.inf representing infinity that can be calculated with. - pool_quota = QuotaInformation(cores=self.pool.max_cores, - instances=self.pool.max_servers, - ram=self.pool.max_ram, - default=math.inf) + pool_quota = provider.QuotaInformation(cores=self.pool.max_cores, + instances=self.pool.max_servers, + ram=self.pool.max_ram, + default=math.inf) pool_quota.subtract(needed_quota) return pool_quota.non_negative() diff --git a/nodepool/driver/openstack/provider.py b/nodepool/driver/openstack/provider.py index 41fdf9370..b596f611e 100755 --- a/nodepool/driver/openstack/provider.py +++ b/nodepool/driver/openstack/provider.py @@ -29,6 +29,9 @@ from nodepool.task_manager import ManagerStoppedException from nodepool.task_manager import TaskManager from nodepool import version +# Import entire module to avoid partial-loading, circular import +from nodepool.driver.openstack import handler + IPS_LIST_AGE = 5 # How long to keep a cached copy of the ip list MAX_QUOTA_AGE = 5 * 60 # How long to keep the quota information cached @@ -130,6 +133,9 @@ class OpenStackProvider(Provider): if self._taskmanager: self._taskmanager.join() + def getRequestHandler(self, poolworker, request): + return handler.OpenStackNodeRequestHandler(poolworker, request) + @property def _flavors(self): if not self.__flavors: diff --git a/nodepool/driver/static/provider.py b/nodepool/driver/static/provider.py index 40294db72..92adc2866 100644 --- a/nodepool/driver/static/provider.py +++ b/nodepool/driver/static/provider.py @@ -17,6 +17,7 @@ import logging from nodepool import exceptions from nodepool.driver import Provider from nodepool.nodeutils import nodescan +from nodepool.driver.static.handler import StaticNodeRequestHandler class StaticNodeError(Exception): @@ -91,3 +92,6 @@ class StaticNodeProvider(Provider): def cleanupLeakedResources(self): pass + + def getRequestHandler(self, poolworker, request): + return StaticNodeRequestHandler(poolworker, request) diff --git a/nodepool/tests/fixtures/drivers/test/__init__.py b/nodepool/driver/test/__init__.py similarity index 100% rename from nodepool/tests/fixtures/drivers/test/__init__.py rename to nodepool/driver/test/__init__.py diff --git a/nodepool/tests/fixtures/drivers/test/config.py b/nodepool/driver/test/config.py similarity index 100% rename from nodepool/tests/fixtures/drivers/test/config.py rename to nodepool/driver/test/config.py diff --git a/nodepool/tests/fixtures/drivers/test/handler.py b/nodepool/driver/test/handler.py similarity index 100% rename from nodepool/tests/fixtures/drivers/test/handler.py rename to nodepool/driver/test/handler.py diff --git a/nodepool/tests/fixtures/drivers/test/provider.py b/nodepool/driver/test/provider.py similarity index 87% rename from nodepool/tests/fixtures/drivers/test/provider.py rename to nodepool/driver/test/provider.py index 182893d59..50761b030 100644 --- a/nodepool/tests/fixtures/drivers/test/provider.py +++ b/nodepool/driver/test/provider.py @@ -15,6 +15,7 @@ # limitations under the License. from nodepool.driver import Provider +from nodepool.driver.test import handler class TestProvider(Provider): @@ -44,3 +45,6 @@ class TestProvider(Provider): def listNodes(self): return [] + + def getRequestHandler(self, poolworker, request): + return handler.TestHandler(poolworker, request) diff --git a/nodepool/launcher.py b/nodepool/launcher.py index 048ffba41..57158bb05 100755 --- a/nodepool/launcher.py +++ b/nodepool/launcher.py @@ -31,7 +31,6 @@ from nodepool import provider_manager from nodepool import stats from nodepool import config as nodepool_config from nodepool import zk -from nodepool.driver import Drivers MINS = 60 @@ -148,10 +147,6 @@ class PoolWorker(threading.Thread): # Private methods # --------------------------------------------------------------- - def _get_node_request_handler(self, provider, request): - driver = Drivers.get(provider.driver.name) - return driver['handler'](self, request) - def _assignHandlers(self): ''' For each request we can grab, create a NodeRequestHandler for it. @@ -211,7 +206,9 @@ class PoolWorker(threading.Thread): # Got a lock, so assign it self.log.info("Assigning node request %s" % req) - rh = self._get_node_request_handler(provider, req) + + pm = self.getProviderManager() + rh = pm.getRequestHandler(self, req) rh.run() if rh.paused: self.paused_handler = rh diff --git a/nodepool/tests/test_drivers.py b/nodepool/tests/test_drivers.py index 5df9ce90d..65e373a92 100644 --- a/nodepool/tests/test_drivers.py +++ b/nodepool/tests/test_drivers.py @@ -21,8 +21,8 @@ from nodepool.driver import Drivers class TestDrivers(tests.DBTestCase): def setup_config(self, filename): - drivers_dir = os.path.join( - os.path.dirname(__file__), 'fixtures', 'drivers') + test_dir = os.path.dirname(__file__) + drivers_dir = os.path.join(os.path.dirname(test_dir), 'driver') Drivers.load([drivers_dir]) return super().setup_config(filename)