zuul/zuul
Code Issues Proposed changes

Merge "Add max-changes-per-pipeline tenant limit"

Browse Source
This commit is contained in:
Zuul 2024-07-24 02:09:12 +00:00 committed by Gerrit Code Review
commit e5a49c6b7d
7 changed files with 182 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
doc/source/tenants.rst
View File

@ -360,6 +360,34 @@ configuration. Some examples of tenant definitions are:
this option; instead it limits Zuul to zero dependencies. This
is distinct from :attr:`<gerrit connection>.max_dependencies`.
.. attr:: max-changes-per-pipeline
The number of changes (not queue items) allowed in any
individual pipeline in this tenant. Live changes, non-live
changes used for dependencies, and changes that are part of a
dependency cycle are all counted. If a change appears in more
than one queue item, it is counted multiple times.
For example, if this value was set to 100, then Zuul would allow
any of the following (but no more):
* 100 changes in individual queue items
* 1 queue item of 100 changes in a dependency cycle
* 1 queue item with 99 changes in a cyle plus one item depending
on that cycle
This counts changes across all queues in the pipeline; it is
therefore possible for a set of projects in one queue to affect
others in the same tenant.
This value is not set by default, which means there is no limit.
It is generally expected that the pipeline window configuration
should be sufficient to protect against excessive resource
usage. However in some circumstances with large dependency
cycles, setting this value may be useful. Note that the value
``0`` does not disable this option; instead it limits Zuul to
zero changes.
.. attr:: max-nodes-per-job
:default: 5

7
releasenotes/notes/pipeline-max-changes-c93525d199a951a0.yaml Normal file
View File

@ -0,0 +1,7 @@
---
features:
- |
The tenant configuration may now specify the maximum number of
changes that should be enqueued in a pipeline in order to protect
Zuul from resource exhaustion. This is not necessary in most
circumstances, so the default remains no limit.

12
tests/fixtures/config/single-tenant/main-max-changes.yaml vendored Normal file
View File

@ -0,0 +1,12 @@
- tenant:
name: tenant-one
max-changes-per-pipeline: 1
source:
gerrit:
config-projects:
- common-config
untrusted-projects:
- org/project
- org/project1
- org/project2
- org/project4

95
tests/unit/test_v3.py
View File

@ -10312,3 +10312,98 @@ class TestMaxDeps(ZuulTestCase):
self.assertHistory([
dict(name='project-merge', result='SUCCESS', changes='1,1 2,1'),
], ordered=False)
class TestPipelineLimits(ZuulTestCase):
tenant_config_file = 'config/single-tenant/main-max-changes.yaml'
def test_pipeline_max_changes_check(self):
# Max-changes is 1, so this is allowed
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertHistory([
dict(name='project-merge', result='SUCCESS', changes='1,1'),
dict(name='project-test1', result='SUCCESS', changes='1,1'),
dict(name='project-test2', result='SUCCESS', changes='1,1'),
], ordered=False)
# And this is not
B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
B.setDependsOn(A, 1)
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertHistory([
dict(name='project-merge', result='SUCCESS', changes='1,1'),
dict(name='project-test1', result='SUCCESS', changes='1,1'),
dict(name='project-test2', result='SUCCESS', changes='1,1'),
], ordered=False)
self.assertEqual(A.reported, 1)
self.assertEqual(B.reported, 1)
self.assertIn('Unable to enqueue change: 2 changes to enqueue '
'greater than pipeline max of 1', B.messages[0])
def test_pipeline_max_changes_current_check(self):
# Max-changes is 1, so this is allowed
self.executor_server.hold_jobs_in_build = True
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
# And this is not
B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
self.assertHistory([
dict(name='project-merge', result='SUCCESS', changes='1,1'),
dict(name='project-test1', result='SUCCESS', changes='1,1'),
dict(name='project-test2', result='SUCCESS', changes='1,1'),
], ordered=False)
self.assertEqual(A.reported, 1)
self.assertEqual(B.reported, 1)
self.assertIn('Unable to enqueue change: 1 additional changes would '
'exceed pipeline max of 1 under current conditions',
B.messages[0])
def test_pipeline_max_changes_gate(self):
# Max-changes is 1, so this is allowed
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
A.addApproval('Code-Review', 2)
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
self.waitUntilSettled()
self.assertHistory([
dict(name='project-merge', result='SUCCESS', changes='1,1'),
dict(name='project-test1', result='SUCCESS', changes='1,1'),
dict(name='project-test2', result='SUCCESS', changes='1,1'),
], ordered=False)
# And this is not
B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C')
C.setDependsOn(B, 1)
B.addApproval('Code-Review', 2)
C.addApproval('Code-Review', 2)
B.addApproval('Approved', 1)
self.fake_gerrit.addEvent(C.addApproval('Approved', 1))
self.waitUntilSettled()
self.assertHistory([
dict(name='project-merge', result='SUCCESS', changes='1,1'),
dict(name='project-test1', result='SUCCESS', changes='1,1'),
dict(name='project-test2', result='SUCCESS', changes='1,1'),
], ordered=False)
self.assertEqual(A.reported, 2)
self.assertEqual(B.reported, 0)
self.assertEqual(C.reported, 1)
self.assertIn('Unable to enqueue change: 2 changes to enqueue '
'greater than pipeline max of 1', C.messages[0])

3
zuul/configloader.py
View File

@ -1874,6 +1874,7 @@ class TenantParser(object):
def getSchema(self):
tenant = {vs.Required('name'): str,
'max-changes-per-pipeline': int,
'max-dependencies': int,
'max-nodes-per-job': int,
'max-job-timeout': int,
@ -1911,6 +1912,8 @@ class TenantParser(object):
tenant = model.Tenant(conf['name'])
pcontext = ParseContext(self.connections, self.scheduler,
tenant, ansible_manager)
if conf.get('max-changes-per-pipeline') is not None:
tenant.max_changes_per_pipeline = conf['max-changes-per-pipeline']
if conf.get('max-dependencies') is not None:
tenant.max_dependencies = conf['max-dependencies']
if conf.get('max-nodes-per-job') is not None:

37
zuul/manager/__init__.py
View File

@ -28,7 +28,7 @@ from zuul.lib.tarjan import strongly_connected_components
import zuul.lib.tracing as tracing
from zuul.model import (
Change, PipelineState, PipelineChangeList,
filter_severity, EnqueueEvent
filter_severity, EnqueueEvent, FalseWithReason,
)
from zuul.zk.change_cache import ChangeKey
from zuul.zk.exceptions import LockException
@ -722,6 +722,21 @@ class PipelineManager(metaclass=ABCMeta):
change_queue, change, event, warnings)
return False
if not (reason := self.checkPipelineWithinLimits(cycle, history)):
log.info("Not enqueueing change %s since "
"pipeline not within limits: %s",
change, reason)
warnings.append(
f"Unable to enqueue change: {reason}"
)
if not history:
# Only report if we are the originating change;
# otherwise we're being called from
# enqueueChangesAhead.
self._reportNonEnqueuedItem(
change_queue, change, event, warnings[-1:])
return False
warnings = []
if not self.enqueueChangesAhead(
cycle, event, quiet,
@ -933,6 +948,26 @@ class PipelineManager(metaclass=ABCMeta):
return queue_config.dependencies_by_topic
def checkPipelineWithinLimits(self, cycle, history):
pipeline_max = self.pipeline.tenant.max_changes_per_pipeline
if pipeline_max is None:
return True
additional = len(cycle) + len(history)
if additional > pipeline_max:
return FalseWithReason(
f"{additional} changes to enqueue greater than "
f"pipeline max of {pipeline_max}")
count = additional
for item in self.pipeline.getAllItems():
count += len(item.changes)
if count > pipeline_max:
return FalseWithReason(
f"{additional} additional changes would exceed "
f"pipeline max of {pipeline_max} under current "
"conditions")
return True
def getNonMergeableCycleChanges(self, item):
"""Return changes in the cycle that do not fulfill

1
zuul/model.py
View File

@ -9072,6 +9072,7 @@ class Tenant(object):
self.name = name
self.max_nodes_per_job = 5
self.max_job_timeout = 10800
self.max_changes_per_pipeline = None
self.max_dependencies = None
self.exclude_unprotected_branches = False
self.exclude_locked_branches = False