Merge "Refactor config loading in builder and launcher"

nodepool/builder.py

@@ -132,6 +132,10 @@ class BaseWorker(threading.Thread):
             self.log.debug("Detected ZooKeeper server changes")
             self._zk.resetHosts(list(new_config.zookeeper_servers.values()))
 
+    def _checkConfigRecent(self):
+        return self._config and nodepool_config.checkRecentConfig(
+            self._config, self._config_path, self._secure_path)
+
     def _readConfig(self):
         new_config = nodepool_config.loadConfig(self._config_path)
         if self._secure_path:
@@ -585,16 +589,15 @@ class CleanupWorker(BaseWorker):
         '''
         Body of run method for exception handling purposes.
         '''
-        new_config = self._readConfig()
-        if not self._config:
+        if not self._checkConfigRecent():
+            new_config = self._readConfig()
+            if not self._config:
+                self._config = new_config
+            self._checkForZooKeeperChanges(new_config)
+            provider_manager.ProviderManager.reconfigure(
+                self._config, new_config, self._zk, only_image_manager=True)
             self._config = new_config
 
-        self._checkForZooKeeperChanges(new_config)
-        provider_manager.ProviderManager.reconfigure(self._config, new_config,
-                                                     self._zk,
-                                                     only_image_manager=True)
-        self._config = new_config
-
         self._cleanup()
         self._emitBuildRequestStats()
 
@@ -648,8 +651,7 @@ class BuildWorker(BaseWorker):
             if not self._running or self._zk.suspended or self._zk.lost:
                 return
             try:
-                new_config = self._readConfig()
-                if new_config != self._config:
+                if not self._checkConfigRecent():
                     # If our config isn't up to date then return and start
                     # over with a new config load.
                     return
@@ -761,8 +763,7 @@ class BuildWorker(BaseWorker):
             if not self._running or self._zk.suspended or self._zk.lost:
                 return
             try:
-                new_config = self._readConfig()
-                if new_config != self._config:
+                if not self._checkConfigRecent():
                     # If our config isn't up to date then return and start
                     # over with a new config load.
                     return
@@ -1036,13 +1037,13 @@ class BuildWorker(BaseWorker):
         '''
         Body of run method for exception handling purposes.
         '''
-        # NOTE: For the first iteration, we expect self._config to be None
-        new_config = self._readConfig()
-        if not self._config:
+        if not self._checkConfigRecent():
+            new_config = self._readConfig()
+            if not self._config:
+                self._config = new_config
+            self._checkForZooKeeperChanges(new_config)
             self._config = new_config
 
-        self._checkForZooKeeperChanges(new_config)
-        self._config = new_config
         self._checkForScheduledImageUpdates()
         self._checkForManualBuildRequest()
 
@@ -1059,16 +1060,15 @@ class UploadWorker(BaseWorker):
         '''
         Reload the nodepool configuration file.
         '''
-        new_config = self._readConfig()
-        if not self._config:
+        if not self._checkConfigRecent():
+            new_config = self._readConfig()
+            if not self._config:
+                self._config = new_config
+            self._checkForZooKeeperChanges(new_config)
+            provider_manager.ProviderManager.reconfigure(
+                self._config, new_config, self._zk, only_image_manager=True)
             self._config = new_config
 
-        self._checkForZooKeeperChanges(new_config)
-        provider_manager.ProviderManager.reconfigure(self._config, new_config,
-                                                     self._zk,
-                                                     only_image_manager=True)
-        self._config = new_config
-
     def _uploadImage(self, build_id, upload_id, image_name, images, provider,
                      username, python_path, shell_type):
         '''

nodepool/config.py

@@ -114,6 +114,8 @@ class Config(ConfigValue):
         self.max_hold_age = None
         self.webapp = None
         self.tenant_resource_limits = {}
+        # Last modified timestamps of loaded config files
+        self.config_mtimes = {}
 
     def __eq__(self, other):
         if isinstance(other, Config):
@@ -133,6 +135,9 @@ class Config(ConfigValue):
                     )
         return False
 
+    def setConfigPathMtime(self, path, mtime):
+        self.config_mtimes[path] = mtime
+
     def setElementsDir(self, value):
         self.elements_dir = value
 
@@ -428,6 +433,7 @@ def openConfig(path, env):
 
 
 def loadConfig(config_path, env=os.environ):
+    config_mtime = os.stat(config_path).st_mtime_ns
     config = openConfig(config_path, env)
 
     # Call driver config reset now to clean global hooks like openstacksdk
@@ -449,11 +455,13 @@ def loadConfig(config_path, env=os.environ):
     newconfig.setProviders(config.get('providers'))
     newconfig.setZooKeeperTLS(config.get('zookeeper-tls'))
     newconfig.setTenantResourceLimits(config.get('tenant-resource-limits'))
+    newconfig.setConfigPathMtime(config_path, config_mtime)
 
     return newconfig
 
 
 def loadSecureConfig(config, secure_config_path, env=os.environ):
+    secure_mtime = os.stat(secure_config_path).st_mtime_ns
     secure = openConfig(secure_config_path, env)
     if not secure:   # empty file
         return
@@ -466,3 +474,19 @@ def loadSecureConfig(config, secure_config_path, env=os.environ):
     config.setSecureDiskimageEnv(
         secure.get('diskimages', []), secure_config_path)
     config.setZooKeeperTLS(secure.get('zookeeper-tls'))
+    config.setConfigPathMtime(secure_config_path, secure_mtime)
+
+
+def checkRecentConfig(config, config_path, secure_path=None):
+    current_config_mtime = config.config_mtimes.get(config_path, 0)
+    new_config_mtime = os.stat(config_path).st_mtime_ns
+    if current_config_mtime != new_config_mtime:
+        return False
+
+    if secure_path:
+        current_secure_mtime = config.config_mtimes.get(secure_path, 0)
+        new_secure_mtime = os.stat(secure_path).st_mtime_ns
+        if current_secure_mtime != new_secure_mtime:
+            return False
+
+    return True

nodepool/launcher.py

@@ -1034,9 +1034,7 @@ class NodePool(threading.Thread):
                  watermark_sleep=WATERMARK_SLEEP):
         threading.Thread.__init__(self, name='NodePool')
         self.securefile = securefile
-        self._secure_mtime = 0
         self.configfile = configfile
-        self._config_mtime = 0
         self.watermark_sleep = watermark_sleep
         self.cleanup_interval = 60
         self.delete_interval = 5
@@ -1145,15 +1143,9 @@ class NodePool(threading.Thread):
                 t.provider_name == provider_name]
 
     def updateConfig(self):
-        secure_mtime = 0
-        config_mtime = os.stat(self.configfile).st_mtime_ns
-        if self._config_mtime == config_mtime:
-            if self.securefile:
-                secure_mtime = os.stat(self.securefile).st_mtime_ns
-                if self._secure_mtime == secure_mtime:
-                    return
-            else:
-                return
+        if self.config and nodepool_config.checkRecentConfig(
+                self.config, self.configfile, self.securefile):
+            return
 
         config = self.loadConfig()
         self.reconfigureZooKeeper(config)
@@ -1164,8 +1156,6 @@ class NodePool(threading.Thread):
                 del config.providers[provider_name]
 
         self.setConfig(config)
-        self._config_mtime = config_mtime
-        self._secure_mtime = secure_mtime
 
     def removeCompletedRequests(self):
         '''