zuul
/
zuul
Code Issues Proposed changes

Fix config error detection on same project branch 

The following commits (changes):

050c640b0b (I885a2d3ab6d60402425b879e639c1e597717622a)
0fc385ebed (I8a72e57e068b073e37274cdabeacffc9daee2e94)

established the idea that we should not report configuration errors
that already exist unless they are present on the project-branch of
the change under test.  That is to say, we should report:

* Errors in other projects or branches of this project which did not
  exist before this change.  I.e., removing a job from this repo which
  breaks another repo should be reported.
* Errors in this project-branch.

But both of these only under the condition that the configuration
is being changed in this patch.

We could almost get away with not reporting the second condition,
and in fact, because of the bug which is fixed in this commit, we
are not currently reporting the second condition.  But we should
because we can't tell with perfect fidelity whether an error in this
project-branch which existed before this change and also exists in
this change is really the same error.  Or if there are two errors,
they may fix one but not realize that the second remains.

This patch restores the original intent of the code and corrects a
bug where we compared a project object to a project name (thereby
rendering the condition always false).

This will require users to, if they change Zuul's configuration,
ensure that all errors on the project-branch of that change are
resolved in order for Zuul to validate the change.  This does not
affect changes which do not change the Zuul config.

Change-Id: I50c37a621be6c2f2929284b9af994b36eca2ba8f
This commit is contained in:
James E. Blair 2021-09-30 15:36:18 -07:00
parent 5858510e22
commit 41e00215ba
2 changed files with 9 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
tests/unit/test_scheduler.py
View File

@ -6880,7 +6880,7 @@ class TestJobUpdateBrokenConfig(ZuulTestCase):
# When the config is broken, we don't override any files
# matchers since we don't have a valid basis. Since this
# does update README.txt, the job should run.
file_dict = {'zuul.d/template.yaml': in_repo_conf,
file_dict = {'zuul.d/existing.yaml': in_repo_conf,
'README.txt': ''}
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
files=file_dict)

10
zuul/manager/__init__.py
View File

@ -829,12 +829,18 @@ class PipelineManager(metaclass=ABCMeta):
parent_error_keys.extend(
e.key for e in item.item_ahead.current_build_set.config_errors)
# Then find config errors which aren't in the parent.
# Then find config errors which aren't in the parent. But
# include errors in this project-branch because the error
# detection hash is imperfect and someone attempting to fix an
# error may create a near duplicate error and it would go
# undetected. Or if there are two errors and the user only
# fixes one, then they may not realize their work is
# incomplete.
relevant_errors = []
for err in layout.loading_errors.errors:
econtext = err.key.context
if ((err.key not in parent_error_keys) or
(econtext.project == item.change.project.name and
(econtext.project.name == item.change.project.name and
econtext.branch == item.change.branch)):
relevant_errors.append(err)
return relevant_errors