zuul/nodepool
Code Issues Proposed changes

Create a base Driver class

Browse Source 
This is, like the drivers in zuul, designed to be a single instance
per driver that survives for the life of the process.  It is used
to further instantiate driver-specific interfaces.

Here we have it return the config object for the driver (replacing
the previous system which loaded it from specific config files).

We also move the reset method from the ProviderConfig to the Driver
object.  It's currently only used to clear a global os client config
object, so this better matches its lifecycle.

Change-Id: I1f5a7be9c597be842bfe4dbea8f153d7a96d7b9a
This commit is contained in:
James E. Blair 2018-05-31 14:53:39 -07:00 committed by David Shrewsbury
parent 674c9516dc
commit 82d8c51250
12 changed files with 175 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
doc/source/devguide.rst
View File

@ -54,6 +54,8 @@ its most important pieces::
Drivers
-------
.. autoclass:: nodepool.driver.Driver
:members:
.. autoclass:: nodepool.driver.Provider
:members:
.. autoclass:: nodepool.driver.NodeRequestHandler

8
nodepool/config.py
View File

@ -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()

73
nodepool/driver/__init__.py
View File

@ -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):
'''

30
nodepool/driver/fake/__init__.py
View File

@ -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)

30
nodepool/driver/openstack/__init__.py
View File

@ -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)

18
nodepool/driver/openstack/config.py
View File

@ -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')

21
nodepool/driver/static/__init__.py
View File

@ -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)

4
nodepool/driver/static/config.py
View File

@ -54,10 +54,6 @@ class StaticProviderConfig(ProviderConfig):
other.pools == self.pools)
return False
@staticmethod
def reset():
pass
@property
def pools(self):
return self.__pools

21
nodepool/driver/test/__init__.py
View File

@ -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)

4
nodepool/driver/test/config.py
View File

@ -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

12
nodepool/tests/test_config_comparisons.py
View File

@ -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'

6
nodepool/tests/test_drivers.py
View File

@ -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)