diff --git a/doc/source/devguide.rst b/doc/source/devguide.rst index bf6b01ac0..7ff70f4f5 100644 --- a/doc/source/devguide.rst +++ b/doc/source/devguide.rst @@ -54,6 +54,8 @@ its most important pieces:: Drivers ------- +.. autoclass:: nodepool.driver.Driver + :members: .. autoclass:: nodepool.driver.Provider :members: .. autoclass:: nodepool.driver.NodeRequestHandler diff --git a/nodepool/config.py b/nodepool/config.py index 3bcbad88b..fd2241816 100755 --- a/nodepool/config.py +++ b/nodepool/config.py @@ -204,8 +204,8 @@ def get_provider_config(provider): # Ensure legacy configuration still works when using fake cloud if provider.get('name', '').startswith('fake'): provider['driver'] = 'fake' - driver = Drivers.get(provider['driver']) - return driver['config'](provider) + driver = Drivers._get(provider['driver']) + return driver.getProviderConfig(provider) def openConfig(path): @@ -234,8 +234,8 @@ def loadConfig(config_path): config = openConfig(config_path) # Call driver config reset now to clean global hooks like os_client_config - for driver in Drivers.drivers.values(): - driver["config"].reset() + for driver in Drivers._drivers.values(): + driver.reset() newconfig = Config() diff --git a/nodepool/driver/__init__.py b/nodepool/driver/__init__.py index 71a1f333b..436104a26 100644 --- a/nodepool/driver/__init__.py +++ b/nodepool/driver/__init__.py @@ -32,6 +32,7 @@ class Drivers: log = logging.getLogger("nodepool.driver.Drivers") drivers = {} + _drivers = {} # TODO: replace drivers drivers_paths = None @staticmethod @@ -50,7 +51,7 @@ class Drivers: if not obj: error = "no %s implementation found" % parent_class if error: - Drivers.log.error("%s: %s" % (path, error)) + Drivers.log.error("%s: %s", path, error) return False return obj[0][1] @@ -66,15 +67,14 @@ class Drivers: for driver in drivers: driver_path = os.path.join(drivers_path, driver) if driver in Drivers.drivers: - Drivers.log.warning("%s: duplicate driver" % driver_path) + Drivers.log.warning("%s: duplicate driver", driver_path) continue if not os.path.isdir(driver_path) or \ "__init__.py" not in os.listdir(driver_path): continue - Drivers.log.debug("%s: loading driver" % driver_path) + Drivers.log.debug("%s: loading driver", driver_path) driver_obj = {} for name, parent_class in ( - ("config", ProviderConfig), ("provider", Provider), ): driver_obj[name] = Drivers._load_class( @@ -83,10 +83,21 @@ class Drivers: if not driver_obj[name]: break if not driver_obj[name]: - Drivers.log.error("%s: skipping incorrect driver" % - driver_path) + Drivers.log.error( + "%s: skipping incorrect driver from %s.py", + driver_path, name) continue Drivers.drivers[driver] = driver_obj + driver_obj = Drivers._load_class( + driver, os.path.join(driver_path, "__init__.py"), + Driver) + if not driver_obj: + Drivers.log.error( + "%s: skipping incorrect driver from __init__.py", + driver_path) + continue + Drivers._drivers[driver] = driver_obj() + Drivers.drivers_paths = drivers_paths @staticmethod @@ -98,6 +109,42 @@ class Drivers: except KeyError: raise RuntimeError("%s: unknown driver" % name) + # TODO: replace get + @staticmethod + def _get(name): + if not Drivers._drivers: + Drivers.load() + try: + return Drivers._drivers[name] + except KeyError: + raise RuntimeError("%s: unknown driver" % name) + + +class Driver(object, metaclass=abc.ABCMeta): + """The Driver interface + + This is the main entrypoint for a Driver. A single instance of + this will be created for each driver in the system and will + persist for the lifetime of the process. + + The class or instance attribute **name** must be provided as a string. + + """ + + def reset(self): + ''' + Called before loading configuration to reset any global state + ''' + pass + + @abc.abstractmethod + def getProviderConfig(self, provider): + """Return a ProviderConfig instance + + :arg dict provider: The parsed provider configuration + """ + pass + class Provider(object, metaclass=abc.ABCMeta): """The Provider interface @@ -723,12 +770,12 @@ class ConfigPool(ConfigValue): return False -class Driver(ConfigValue): +class DriverConfig(ConfigValue): def __init__(self): self.name = None def __eq__(self, other): - if isinstance(other, Driver): + if isinstance(other, DriverConfig): return self.name == other.name return False @@ -742,7 +789,7 @@ class ProviderConfig(ConfigValue, metaclass=abc.ABCMeta): def __init__(self, provider): self.name = provider['name'] self.provider = provider - self.driver = Driver() + self.driver = DriverConfig() self.driver.name = provider.get('driver', 'openstack') self.max_concurrency = provider.get('max-concurrency', -1) @@ -773,14 +820,6 @@ class ProviderConfig(ConfigValue, metaclass=abc.ABCMeta): ''' pass - # TODO: can we remove this? - @abc.abstractmethod - def reset(): - ''' - Called before loading configuration to reset any global state - ''' - pass - @abc.abstractmethod def load(self, newconfig): ''' diff --git a/nodepool/driver/fake/__init__.py b/nodepool/driver/fake/__init__.py index e69de29bb..f7cb7a4d4 100644 --- a/nodepool/driver/fake/__init__.py +++ b/nodepool/driver/fake/__init__.py @@ -0,0 +1,30 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# +# See the License for the specific language governing permissions and +# limitations under the License. + +import os_client_config + +from nodepool.driver import Driver +from nodepool.driver.fake.config import FakeProviderConfig + + +class FakeDriver(Driver): + def __init__(self): + super().__init__() + self.reset() + + def reset(self): + self.os_client_config = os_client_config.OpenStackConfig() + + def getProviderConfig(self, provider): + return FakeProviderConfig(self, provider) diff --git a/nodepool/driver/openstack/__init__.py b/nodepool/driver/openstack/__init__.py index e69de29bb..c7648edf9 100644 --- a/nodepool/driver/openstack/__init__.py +++ b/nodepool/driver/openstack/__init__.py @@ -0,0 +1,30 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# +# See the License for the specific language governing permissions and +# limitations under the License. + +import os_client_config + +from nodepool.driver import Driver +from nodepool.driver.openstack.config import OpenStackProviderConfig + + +class OpenStackDriver(Driver): + def __init__(self): + super().__init__() + self.reset() + + def reset(self): + self.os_client_config = os_client_config.OpenStackConfig() + + def getProviderConfig(self, provider): + return OpenStackProviderConfig(self, provider) diff --git a/nodepool/driver/openstack/config.py b/nodepool/driver/openstack/config.py index 3c7f811bc..bf6c9f6c6 100644 --- a/nodepool/driver/openstack/config.py +++ b/nodepool/driver/openstack/config.py @@ -15,7 +15,6 @@ # limitations under the License. import math -import os_client_config import voluptuous as v from nodepool.driver import ProviderConfig @@ -155,9 +154,8 @@ class ProviderPool(ConfigPool): class OpenStackProviderConfig(ProviderConfig): - os_client_config = None - - def __init__(self, *args, **kwargs): + def __init__(self, driver, provider): + self.driver_object = driver self.__pools = {} self.cloud_config = None self.image_type = None @@ -169,7 +167,7 @@ class OpenStackProviderConfig(ProviderConfig): self.cloud_images = {} self.hostname_format = None self.image_name_format = None - super().__init__(*args, **kwargs) + super().__init__(provider) def __eq__(self, other): if isinstance(other, OpenStackProviderConfig): @@ -200,16 +198,10 @@ class OpenStackProviderConfig(ProviderConfig): def manage_images(self): return True - @staticmethod - def reset(): - OpenStackProviderConfig.os_client_config = None - def load(self, config): - if OpenStackProviderConfig.os_client_config is None: - OpenStackProviderConfig.os_client_config = \ - os_client_config.OpenStackConfig() cloud_kwargs = self._cloudKwargs() - self.cloud_config = self.os_client_config.get_one_cloud(**cloud_kwargs) + occ = self.driver_object.os_client_config + self.cloud_config = occ.get_one_cloud(**cloud_kwargs) self.image_type = self.cloud_config.config['image_format'] self.region_name = self.provider.get('region-name') diff --git a/nodepool/driver/static/__init__.py b/nodepool/driver/static/__init__.py index e69de29bb..c29c0b85d 100644 --- a/nodepool/driver/static/__init__.py +++ b/nodepool/driver/static/__init__.py @@ -0,0 +1,21 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# +# See the License for the specific language governing permissions and +# limitations under the License. + +from nodepool.driver import Driver +from nodepool.driver.static.config import StaticProviderConfig + + +class StaticDriver(Driver): + def getProviderConfig(self, provider): + return StaticProviderConfig(provider) diff --git a/nodepool/driver/static/config.py b/nodepool/driver/static/config.py index 988a02ca4..e660391ab 100644 --- a/nodepool/driver/static/config.py +++ b/nodepool/driver/static/config.py @@ -54,10 +54,6 @@ class StaticProviderConfig(ProviderConfig): other.pools == self.pools) return False - @staticmethod - def reset(): - pass - @property def pools(self): return self.__pools diff --git a/nodepool/driver/test/__init__.py b/nodepool/driver/test/__init__.py index e69de29bb..25a4bf1ec 100644 --- a/nodepool/driver/test/__init__.py +++ b/nodepool/driver/test/__init__.py @@ -0,0 +1,21 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# +# See the License for the specific language governing permissions and +# limitations under the License. + +from nodepool.driver import Driver +from nodepool.driver.test.config import TestConfig + + +class TestDriver(Driver): + def getProviderConfig(self, provider): + return TestConfig(provider) diff --git a/nodepool/driver/test/config.py b/nodepool/driver/test/config.py index f9651a494..80f8a662f 100644 --- a/nodepool/driver/test/config.py +++ b/nodepool/driver/test/config.py @@ -31,10 +31,6 @@ class TestConfig(ProviderConfig): def __eq__(self, other): return self.name == other.name - @staticmethod - def reset(): - pass - @property def pools(self): return self.__pools diff --git a/nodepool/tests/test_config_comparisons.py b/nodepool/tests/test_config_comparisons.py index db55be5dc..d7724e818 100644 --- a/nodepool/tests/test_config_comparisons.py +++ b/nodepool/tests/test_config_comparisons.py @@ -18,7 +18,7 @@ from nodepool.config import Config from nodepool.config import DiskImage from nodepool.config import Label from nodepool.driver import ConfigPool -from nodepool.driver import Driver +from nodepool.driver import DriverConfig from nodepool.driver.openstack.config import OpenStackProviderConfig from nodepool.driver.openstack.config import ProviderDiskImage from nodepool.driver.openstack.config import ProviderCloudImage @@ -37,9 +37,9 @@ class TestConfigComparisons(tests.BaseTestCase): a.max_servers = 5 self.assertNotEqual(a, b) - def test_Driver(self): - a = Driver() - b = Driver() + def test_DriverConfig(self): + a = DriverConfig() + b = DriverConfig() self.assertEqual(a, b) a.name = "foo" self.assertNotEqual(a, b) @@ -100,8 +100,8 @@ class TestConfigComparisons(tests.BaseTestCase): def test_OpenStackProviderConfig(self): provider = {'name': 'foo'} - a = OpenStackProviderConfig(provider) - b = OpenStackProviderConfig(provider) + a = OpenStackProviderConfig(None, provider) + b = OpenStackProviderConfig(None, provider) self.assertEqual(a, b) # intentionally change an attribute of the base class a.name = 'bar' diff --git a/nodepool/tests/test_drivers.py b/nodepool/tests/test_drivers.py index 65e373a92..5f575c578 100644 --- a/nodepool/tests/test_drivers.py +++ b/nodepool/tests/test_drivers.py @@ -14,7 +14,6 @@ import os -from nodepool import config as nodepool_config from nodepool import tests from nodepool.driver import Drivers @@ -26,11 +25,6 @@ class TestDrivers(tests.DBTestCase): Drivers.load([drivers_dir]) return super().setup_config(filename) - def test_external_driver_config(self): - configfile = self.setup_config('external_driver.yaml') - nodepool_config.loadConfig(configfile) - self.assertIn("config", Drivers.get("test")) - def test_external_driver_handler(self): configfile = self.setup_config('external_driver.yaml') pool = self.useNodepool(configfile, watermark_sleep=1)