Move tenant validation to separate method

In order to simplify and cleanup the reconfiguration event handling we
move to tenant validation to a separate method. This method will be
directly called from the scheduler cmd app, instead of handling the
validation via the reconfiguration event.

This should give us a clearer picture on the differences of smart and
full reconfigurations, so later on we might be able to have a single
method for handling the different types of reconfigs.

Change-Id: Ifb8715ea1436d1f7f3cc127a2be88d4f5f89e73d

5 files changed, 57 insertions, 50 deletions

diff --git a/tests/base.py b/tests/base.py
index 8e5316d4d..ca12c730a 100644
--- a/tests/base.py
+++ b/tests/base.py
@@ -4183,7 +4183,10 @@ class SchedulerTestApp:
 
     def start(self, validate_tenants: list):
         self.sched.start()
-        self.sched.prime(self.config, validate_tenants=validate_tenants)
+        if validate_tenants is None:
+            self.sched.prime(self.config)
+        else:
+            self.sched.validateTenants(self.config, validate_tenants)
 
     def fullReconfigure(self):
         try:
diff --git a/tests/unit/test_event_queues.py b/tests/unit/test_event_queues.py
index 0cd312986..61c7ba4bf 100644
--- a/tests/unit/test_event_queues.py
+++ b/tests/unit/test_event_queues.py
@@ -206,7 +206,7 @@ class TestManagementEventQueue(EventQueueBaseTestCase):
         self.assertEqual(len(queue), 0)
         self.assertFalse(queue.hasEvents())
 
-        event = model.ReconfigureEvent(None)
+        event = model.ReconfigureEvent()
         result_future = queue.put(event, needs_result=False)
         self.assertIsNone(result_future)
 
@@ -232,7 +232,7 @@ class TestManagementEventQueue(EventQueueBaseTestCase):
         # Test that management event errors are reported.
         queue = event_queues.TenantManagementEventQueue(
             self.zk_client, "tenant")
-        event = model.ReconfigureEvent(None)
+        event = model.ReconfigureEvent()
         result_future = queue.put(event)
 
         acked = 0
@@ -276,7 +276,7 @@ class TestManagementEventQueue(EventQueueBaseTestCase):
             self.zk_client
         )
 
-        event = model.ReconfigureEvent(None)
+        event = model.ReconfigureEvent()
         tenant_queue.put(event, needs_result=False)
         self.assertTrue(tenant_queue.hasEvents())
 
@@ -377,7 +377,7 @@ class TestManagementEventQueue(EventQueueBaseTestCase):
         external_queue = event_queues.TenantManagementEventQueue(
             external_client, "tenant")
 
-        event = model.ReconfigureEvent(None)
+        event = model.ReconfigureEvent()
         result_future = external_queue.put(event)
         self.assertIsNotNone(result_future)
 
@@ -419,7 +419,7 @@ class TestManagementEventQueue(EventQueueBaseTestCase):
             external_client, "tenant")
 
         # Submit the event
-        event = model.ReconfigureEvent(None)
+        event = model.ReconfigureEvent()
         result_future = external_queue.put(event)
         self.assertIsNotNone(result_future)
 
@@ -512,7 +512,7 @@ class TestEventWatchers(EventQueueBaseTestCase):
         )
         self.assertFalse(event.is_set())
 
-        management_queue["tenant"].put(model.ReconfigureEvent(None),
+        management_queue["tenant"].put(model.ReconfigureEvent(),
                                        needs_result=False)
         self._wait_for_event(event)
         event.clear()
@@ -538,7 +538,7 @@ class TestEventWatchers(EventQueueBaseTestCase):
         )
         self.assertFalse(event.is_set())
 
-        management_queues["tenant"]["check"].put(model.ReconfigureEvent(None))
+        management_queues["tenant"]["check"].put(model.ReconfigureEvent())
         self._wait_for_event(event)
         event.clear()
 
diff --git a/zuul/cmd/scheduler.py b/zuul/cmd/scheduler.py
index 603bf48c3..93c093ff4 100755
--- a/zuul/cmd/scheduler.py
+++ b/zuul/cmd/scheduler.py
@@ -145,7 +145,11 @@ class Scheduler(zuul.cmd.ZuulDaemonApp):
         self.log.info('Starting scheduler')
         try:
             self.sched.start()
-            self.sched.prime(self.config, self.args.validate_tenants)
+            if self.args.validate_tenants is not None:
+                self.sched.validateTenants(
+                    self.config, self.args.validate_tenants)
+            else:
+                self.sched.prime(self.config)
         except Exception:
             self.log.exception("Error starting Zuul:")
             # TODO(jeblair): If we had all threads marked as daemon,
diff --git a/zuul/model.py b/zuul/model.py
index 960f1e3f3..cfda0cc13 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -3760,21 +3760,6 @@ class ReconfigureEvent(ManagementEvent):
     """Reconfigure the scheduler.  The layout will be (re-)loaded from
     the path specified in the configuration."""
 
-    def __init__(self, validate_tenants=None):
-        super(ReconfigureEvent, self).__init__()
-        self.validate_tenants = validate_tenants
-
-    def toDict(self):
-        d = super().toDict()
-        d["validate_tenants"] = self.validate_tenants
-        return d
-
-    @classmethod
-    def fromDict(cls, data):
-        event = cls(data.get("validate_tenants"))
-        event.updateFromDict(data)
-        return event
-
 
 class SmartReconfigureEvent(ManagementEvent):
     """Reconfigure the scheduler.  The layout will be (re-)loaded from
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index 88a75e5e7..f7672c30b 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -620,10 +620,9 @@ class Scheduler(threading.Thread):
         self.repl.stop()
         self.repl = None
 
-    def prime(self, config, validate_tenants=None):
+    def prime(self, config):
         self.log.debug("Priming scheduler config")
-        event = ReconfigureEvent(validate_tenants=validate_tenants)
-        self._doReconfigureEvent(event)
+        self._doReconfigureEvent(ReconfigureEvent())
         self.log.debug("Config priming complete")
         self.last_reconfigured = int(time.time())
 
@@ -831,6 +830,44 @@ class Scheduler(threading.Thread):
                 "is missing from the configuration.")
         return tenant_config, script
 
+    def validateTenants(self, config, tenants_to_validate):
+        self.config = config
+        with self.layout_lock:
+            self.log.info("Config validation beginning")
+            start = time.monotonic()
+
+            loader = configloader.ConfigLoader(
+                self.connections, self, self.merger, self.keystore)
+            tenant_config, script = self._checkTenantSourceConf(self.config)
+            unparsed_abide = loader.readConfig(tenant_config,
+                                               from_script=script)
+
+            available_tenants = list(unparsed_abide.tenants)
+            tenants_to_load = tenants_to_validate or available_tenants
+            if not set(tenants_to_load).issubset(available_tenants):
+                invalid = tenants_to_load.difference(available_tenants)
+                raise RuntimeError(f"Invalid tenant(s) found: {invalid}")
+
+            abide = Abide()
+            loader.loadAdminRules(abide, unparsed_abide)
+            loader.loadTPCs(abide, unparsed_abide)
+            for tenant_name in tenants_to_load:
+                loader.loadTenant(abide, tenant_name, self.ansible_manager,
+                                  unparsed_abide, cache_ltime=None)
+
+            loading_errors = []
+            for tenant in abide.tenants.values():
+                for error in tenant.layout.loading_errors:
+                    loading_errors.append(repr(error))
+            if loading_errors:
+                summary = '\n\n\n'.join(loading_errors)
+                raise configloader.ConfigurationSyntaxError(
+                    f"Configuration errors: {summary}")
+
+        duration = round(time.monotonic() - start, 3)
+        self.log.info("Config validation complete (duration: %s seconds)",
+                      duration)
+
     def _doReconfigureEvent(self, event):
         # This is called in the scheduler loop after another thread submits
         # a request
@@ -857,26 +894,14 @@ class Scheduler(threading.Thread):
             self.unparsed_abide = loader.readConfig(
                 tenant_config, from_script=script)
 
-            tenants_to_load = list(self.unparsed_abide.tenants)
-            if event.validate_tenants is not None:
-                validate_tenants = set(event.validate_tenants)
-                if not validate_tenants.issubset(tenants_to_load):
-                    invalid = validate_tenants.difference(tenants_to_load)
-                    raise RuntimeError(f"Invalid tenant(s) found: {invalid}")
-                # In case we have an empty list, we validate all tenants.
-                tenants_to_load = event.validate_tenants or tenants_to_load
-
             abide = Abide()
             loader.loadAdminRules(abide, self.unparsed_abide)
             loader.loadTPCs(abide, self.unparsed_abide)
-            for tenant_name in tenants_to_load:
+            for tenant_name in self.unparsed_abide.tenants:
                 tenant = loader.loadTenant(abide, tenant_name,
                                            self.ansible_manager,
                                            self.unparsed_abide,
                                            cache_ltime=None)
-                if event.validate_tenants:
-                    # We are only validating the tenant config; skip reconfig
-                    continue
 
                 if tenant is not None:
                     old_tenant = self.abide.tenants.get(tenant_name)
@@ -887,16 +912,6 @@ class Scheduler(threading.Thread):
                     # We deleted a tenant
                     self._reconfigureDeleteTenant(old_tenant)
 
-            if event.validate_tenants is not None:
-                loading_errors = []
-                for tenant in abide.tenants.values():
-                    for error in tenant.layout.loading_errors:
-                        loading_errors.append(repr(error))
-                if loading_errors:
-                    summary = '\n\n\n'.join(loading_errors)
-                    raise configloader.ConfigurationSyntaxError(
-                        f"Configuration errors: {summary}")
-
             self.abide = abide
         finally:
             self.layout_lock.release()