From db0bf681d5c6553352f8d224e733a088c4fafe3c Mon Sep 17 00:00:00 2001 From: Simon Westphahl Date: Thu, 1 Jul 2021 13:31:52 +0200 Subject: 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 --- tests/base.py | 5 ++- tests/unit/test_event_queues.py | 14 ++++----- zuul/cmd/scheduler.py | 6 +++- zuul/model.py | 15 --------- zuul/scheduler.py | 67 +++++++++++++++++++++++++---------------- 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() -- cgit v1.2.1