zuul/zuul
Code Issues Proposed changes

Avoid exception when no buildset files

Browse Source 
If a queue item has a change that modifies zuul.yaml files, then
the mergers send back the contents of the files and we store those
contents on the buildset as RepoFiles.

Likewise, if a queue item changes the zuul config (the same condition
as above) then we perform a dynamic reconfiguration for the queue item.

The way we determine whether a queue item updates the zuul config is
by consulting the updatesConfig method on the item's change objects.
If the branch is an always-dynamic-branch, then updatesConfig will
always return True because the purpose is to always trigger the
dynamic reconfig.

In the case of branch events, we skip the initial merge job,
which means we have no RepoFiles for the buildset.

In the case of branch event (ie, a post-merge job), that means that
the scheduler thinks it should create a dynamic configuration (because
updatesConfig is true) but it doesn't have the files to do it (because
RepoFiles is None).  This results in an exception when trying to
reconstitute the missing RepoFiles from ZooKeeper.  The scheduler
catches this exception and continues, but it is logged, and looks
alarming.  To avoid this, we will no longer attempt to create a
dynamic reconfiguration if we don't have a RepoFiles.

A second cause for the exception was observed with a newly created
branch (which is not an always-dynamic-branch).  In this case, the
event typcially does not arrive with a files list, so we perform a
fileschanges merger job to get the list from git.  That did not reset
the repo, which means it did not have a local copy of the newly
created branch from upstream.  This caused the fileschanges job to
fail.  This, in turn, caused the updatesConfig method to return True
(since it is conservative and says that the config is updated if it is
unsure).  This caused a dynamic reconfiguration which again failed
because we do not get the files contents on branch items.

To correct this, we now reset the repo before performing fileschanges
jobs.

The first condition is not practical to test because it fails safe
regardless, and is an expected behavior.  A test is added for the
second condition.

Change-Id: I1775ebdc989a38d597fe948ff9b6985afd2b5a81
This commit is contained in:
James E. Blair
2024-06-19 17:36:08 -07:00
parent 75e58e129e
commit 9bb4453b8f
4 changed files with 30 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
zuul/manager/__init__.py
View File

@@ -1435,8 +1435,15 @@ class PipelineManager(metaclass=ABCMeta):
return None
if build_set.unable_to_merge:
return self.getFallbackLayout(item)
if not build_set.hasFiles():
log.debug("No files on buildset for: %s", item)
# This is probably post update on an always dynamic
# branch; in this case the item says it updates the
# config, but we don't get repo files on branches, so
# there's nothing to check.
return self.getFallbackLayout(item)
log.debug("Preparing dynamic layout for: %s" % item)
log.debug("Preparing dynamic layout for: %s", item)
start = time.time()
layout = self._loadDynamicLayout(item)
self.reportPipelineTiming('layout_generation_time', start)
@@ -2059,11 +2066,13 @@ class PipelineManager(metaclass=ABCMeta):
for i, change in enumerate(item.changes):
source = self.sched.connections.getSource(
change.project.connection_name)
if event.files:
change_files = event.files[i]
if event.files is None:
self.log.warning(
"Did not receive expected merge files content")
files = None
else:
change_files = None
source.setChangeAttributes(change, files=change_files)
files = event.files[i]
source.setChangeAttributes(change, files=files)
build_set.updateAttributes(self.current_context,
files_state=build_set.COMPLETE)
if build_set.merge_state == build_set.COMPLETE: