From 23327242054ff2e4c8a0a192baa5bb8af946831d Mon Sep 17 00:00:00 2001
From: David Shrewsbury <shrewsbury.dave@gmail.com>
Date: Tue, 8 May 2018 14:07:19 -0400
Subject: [PATCH] Force driver provider configs to define pool attr

There is currently nothing forcing a driver's ProviderConfig
to define a 'pools' attribute, yet the code clearly expects
it to be defined (e.g., PoolWorker.getPoolConfig() method).
Force this by defining an abstract property, which helps define
the interface more clearly, too.

Change-Id: I2d6a0ee98076540cc722835ab6b5c3a077f83edc
---
 nodepool/driver/__init__.py                    | 8 ++++++++
 nodepool/driver/openstack/config.py            | 9 ++++++++-
 nodepool/driver/static/config.py               | 9 ++++++++-
 nodepool/tests/fixtures/drivers/test/config.py | 9 ++++++++-
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/nodepool/driver/__init__.py b/nodepool/driver/__init__.py
index d47ff78ed..7f110afb8 100644
--- a/nodepool/driver/__init__.py
+++ b/nodepool/driver/__init__.py
@@ -725,6 +725,14 @@ class ProviderConfig(ConfigValue):
     def __repr__(self):
         return "<Provider %s>" % self.name
 
+    @property
+    @abc.abstractmethod
+    def pools(self):
+        '''
+        Return a dict of ConfigPool-based objects, indexed by pool name.
+        '''
+        pass
+
     @abc.abstractmethod
     def __eq__(self, other):
         pass
diff --git a/nodepool/driver/openstack/config.py b/nodepool/driver/openstack/config.py
index cfdce8af6..e8c927f00 100644
--- a/nodepool/driver/openstack/config.py
+++ b/nodepool/driver/openstack/config.py
@@ -79,6 +79,10 @@ class ProviderPool(ConfigPool):
 class OpenStackProviderConfig(ProviderConfig):
     os_client_config = None
 
+    def __init__(self, *args, **kwargs):
+        self.__pools = {}
+        super().__init__(*args, **kwargs)
+
     def __eq__(self, other):
         if (other.cloud_config != self.cloud_config or
             other.pools != self.pools or
@@ -99,6 +103,10 @@ class OpenStackProviderConfig(ProviderConfig):
                 cloud_kwargs[arg] = self.provider[arg]
         return cloud_kwargs
 
+    @property
+    def pools(self):
+        return self.__pools
+
     @staticmethod
     def reset():
         OpenStackProviderConfig.os_client_config = None
@@ -174,7 +182,6 @@ class OpenStackProviderConfig(ProviderConfig):
                 default_port_mapping.get(i.connection_type, 22))
             self.cloud_images[i.name] = i
 
-        self.pools = {}
         for pool in self.provider.get('pools', []):
             pp = ProviderPool()
             pp.name = pool['name']
diff --git a/nodepool/driver/static/config.py b/nodepool/driver/static/config.py
index 5e52c98f8..f4e527a37 100644
--- a/nodepool/driver/static/config.py
+++ b/nodepool/driver/static/config.py
@@ -33,6 +33,10 @@ class StaticPool(ConfigPool):
 
 
 class StaticProviderConfig(ProviderConfig):
+    def __init__(self, *args, **kwargs):
+        self.__pools = {}
+        super().__init__(*args, **kwargs)
+
     def __eq__(self, other):
         if other.pools != self.pools:
             return False
@@ -42,8 +46,11 @@ class StaticProviderConfig(ProviderConfig):
     def reset():
         pass
 
+    @property
+    def pools(self):
+        return self.__pools
+
     def load(self, config):
-        self.pools = {}
         for pool in self.provider.get('pools', []):
             pp = StaticPool()
             pp.name = pool['name']
diff --git a/nodepool/tests/fixtures/drivers/test/config.py b/nodepool/tests/fixtures/drivers/test/config.py
index 6e0ba9dea..a67ca1b94 100644
--- a/nodepool/tests/fixtures/drivers/test/config.py
+++ b/nodepool/tests/fixtures/drivers/test/config.py
@@ -24,6 +24,10 @@ class TestPool(ConfigPool):
 
 
 class TestConfig(ProviderConfig):
+    def __init__(self, *args, **kwargs):
+        self.__pools = {}
+        super().__init__(*args, **kwargs)
+
     def __eq__(self, other):
         return self.name == other.name
 
@@ -31,8 +35,11 @@ class TestConfig(ProviderConfig):
     def reset():
         pass
 
+    @property
+    def pools(self):
+        return self.__pools
+
     def load(self, newconfig):
-        self.pools = {}
         self.labels = set()
         for pool in self.provider.get('pools', []):
             testpool = TestPool()