diff options
22 files changed, 355 insertions, 85 deletions
diff --git a/doc/source/examples/etc_nodepool/nodepool.yaml b/doc/source/examples/etc_nodepool/nodepool.yaml index 1c1830635..105b0ef54 100644 --- a/doc/source/examples/etc_nodepool/nodepool.yaml +++ b/doc/source/examples/etc_nodepool/nodepool.yaml @@ -7,7 +7,7 @@ zookeeper-tls: ca: /var/certs/certs/cacert.pem labels: - - name: ubuntu-focal + - name: ubuntu-jammy providers: - name: static-vms @@ -16,7 +16,7 @@ providers: - name: main nodes: - name: node - labels: ubuntu-focal + labels: ubuntu-jammy host-key: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOgHJYejINIKzUiuSJ2MN8uPc+dfFrZ9JH1hLWS8gI+g" python-path: /usr/bin/python3 username: root diff --git a/doc/source/examples/node-Dockerfile b/doc/source/examples/node-Dockerfile index ff74aa592..b588bcf2c 100644 --- a/doc/source/examples/node-Dockerfile +++ b/doc/source/examples/node-Dockerfile @@ -1,4 +1,4 @@ -FROM docker.io/ubuntu:20.04 +FROM docker.io/ubuntu:22.04 RUN apt-get update \ && DEBIAN_FRONTEND="noninteractive" apt-get -y install \ diff --git a/doc/source/examples/zuul-config/zuul.d/jobs.yaml b/doc/source/examples/zuul-config/zuul.d/jobs.yaml index 8ad979e46..bb9822f48 100644 --- a/doc/source/examples/zuul-config/zuul.d/jobs.yaml +++ b/doc/source/examples/zuul-config/zuul.d/jobs.yaml @@ -3,5 +3,5 @@ parent: null nodeset: nodes: - - name: ubuntu-focal - label: ubuntu-focal + - name: ubuntu-jammy + label: ubuntu-jammy diff --git a/doc/source/examples/zuul-config/zuul.d/jobs2.yaml b/doc/source/examples/zuul-config/zuul.d/jobs2.yaml index a6ed1a633..c7b4a6878 100644 --- a/doc/source/examples/zuul-config/zuul.d/jobs2.yaml +++ b/doc/source/examples/zuul-config/zuul.d/jobs2.yaml @@ -18,5 +18,5 @@ timeout: 1800 nodeset: nodes: - - name: ubuntu-focal - label: ubuntu-focal + - name: ubuntu-jammy + label: ubuntu-jammy diff --git a/doc/source/job-content.rst b/doc/source/job-content.rst index d6bb07683..643632d5b 100644 --- a/doc/source/job-content.rst +++ b/doc/source/job-content.rst @@ -669,6 +669,68 @@ of item. - shell: echo example when: zuul_success | bool +.. var:: nodepool + + Information about each host from Nodepool is supplied in the + `nodepool` host variable. Availability of values varies based on + the node and the driver that supplied it. Values may be ``null`` + if they are not applicable. + + .. var:: label + + The nodepool label of this node. + + .. var:: az + + The availability zone in which this node was placed. + + .. var:: cloud + + The name of the cloud in which this node was created. + + .. var:: provider + + The name of the nodepool provider of this node. + + .. var:: region + + The name of the nodepool provider's region. + + .. var:: host_id + + The cloud's host identification for this node's hypervisor. + + .. var:: external_id + + The cloud's identifier for this node. + + .. var:: slot + + If the node supports running multiple jobs on the node, a unique + numeric ID for the subdivision of the node assigned to this job. + This may be used to avoid build directory collisions. + + .. var:: interface_ip + + The best IP address to use to contact the node as determined by + the cloud provider and nodepool. + + .. var:: public_ipv4 + + A public IPv4 address of the node. + + .. var:: private_ipv4 + + A private IPv4 address of the node. + + .. var:: public_ipv6 + + A public IPv6 address of the node. + + .. var:: private_ipv6 + + A private IPv6 address of the node. + Change Items ~~~~~~~~~~~~ diff --git a/releasenotes/notes/handle-existing-commits-with-cherry-pick-e1a979c2e7ed1a78.yaml b/releasenotes/notes/handle-existing-commits-with-cherry-pick-e1a979c2e7ed1a78.yaml new file mode 100644 index 000000000..dd5c502d2 --- /dev/null +++ b/releasenotes/notes/handle-existing-commits-with-cherry-pick-e1a979c2e7ed1a78.yaml @@ -0,0 +1,14 @@ +--- +fixes: + - | + The `cherry-pick` merge mode will now silently skip commits that have + already been applied to the tree when cherry-picking, instead of failing + with an error. + + The exception to this is if the source of the cherry-pick is an empty + commit, in which case it is always kept. + + Skipping commits that have already been applied is important in a pipeline + triggered by the Gerrit `change-merged` event (like the `deploy` pipeline), + since the scheduler would previously try to cherry-pick the change on top + of the commit that just merged and fail. diff --git a/releasenotes/notes/nodepool-slot-2061128253e50580.yaml b/releasenotes/notes/nodepool-slot-2061128253e50580.yaml new file mode 100644 index 000000000..c7ba3e1dc --- /dev/null +++ b/releasenotes/notes/nodepool-slot-2061128253e50580.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + The :var:`nodepool.slot` variable has been added to host vars. + This is supplied by the nodepool static and metastatic drivers + starting with version 8.0.0. It may be used to avoid build + directory collisions on nodes that run more than one job. diff --git a/tests/base.py b/tests/base.py index fd927a92c..290d51934 100644 --- a/tests/base.py +++ b/tests/base.py @@ -384,7 +384,7 @@ class FakeGerritChange(object): def __init__(self, gerrit, number, project, branch, subject, status='NEW', upstream_root=None, files={}, parent=None, merge_parents=None, merge_files=None, - topic=None): + topic=None, empty=False): self.gerrit = gerrit self.source = gerrit self.reported = 0 @@ -429,7 +429,7 @@ class FakeGerritChange(object): self.addMergePatchset(parents=merge_parents, merge_files=merge_files) else: - self.addPatchset(files=files, parent=parent) + self.addPatchset(files=files, parent=parent, empty=empty) if merge_parents: self.data['parents'] = merge_parents elif parent: @@ -503,9 +503,11 @@ class FakeGerritChange(object): repo.heads['master'].checkout() return r - def addPatchset(self, files=None, large=False, parent=None): + def addPatchset(self, files=None, large=False, parent=None, empty=False): self.latest_patchset += 1 - if not files: + if empty: + files = {} + elif not files: fn = '%s-%s' % (self.branch.replace('/', '_'), self.number) data = ("test %s %s %s\n" % (self.branch, self.number, self.latest_patchset)) @@ -1330,7 +1332,7 @@ class FakeGerritConnection(gerritconnection.GerritConnection): def addFakeChange(self, project, branch, subject, status='NEW', files=None, parent=None, merge_parents=None, - merge_files=None, topic=None): + merge_files=None, topic=None, empty=False): """Add a change to the fake Gerrit.""" self.change_number += 1 c = FakeGerritChange(self, self.change_number, project, branch, @@ -1338,7 +1340,7 @@ class FakeGerritConnection(gerritconnection.GerritConnection): status=status, files=files, parent=parent, merge_parents=merge_parents, merge_files=merge_files, - topic=topic) + topic=topic, empty=empty) self.changes[self.change_number] = c return c diff --git a/tests/fakegithub.py b/tests/fakegithub.py index 725c083e2..25dcb15da 100644 --- a/tests/fakegithub.py +++ b/tests/fakegithub.py @@ -730,7 +730,7 @@ class FakeGithubSession(object): 'message': 'Merge not allowed because of fake reason', } return FakeResponse(data, 405, 'Method not allowed') - pr.setMerged(json["commit_message"]) + pr.setMerged(json.get("commit_message", "")) return FakeResponse({"merged": True}, 200) return FakeResponse(None, 404) diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 47e84ca7f..3060f5673 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -1430,7 +1430,9 @@ class TestGithubDriver(ZuulTestCase): repo._set_branch_protection( 'master', contexts=['tenant-one/check', 'tenant-one/gate']) - A = self.fake_github.openFakePullRequest('org/project', 'master', 'A') + pr_description = "PR description" + A = self.fake_github.openFakePullRequest('org/project', 'master', 'A', + body_text=pr_description) self.fake_github.emitEvent(A.getPullRequestOpenedEvent()) self.waitUntilSettled() @@ -1448,6 +1450,9 @@ class TestGithubDriver(ZuulTestCase): merges = [report for report in self.fake_github.github_data.reports if report[2] == 'merge'] assert (len(merges) == 1 and merges[0][3] == 'squash') + # Assert that we won't duplicate the PR title in the merge + # message description. + self.assertEqual(A.merge_message, pr_description) @simple_layout('layouts/basic-github.yaml', driver='github') def test_invalid_event(self): diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 172ed34dc..131034f17 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -7440,6 +7440,85 @@ class TestSchedulerMerges(ZuulTestCase): result = self._test_project_merge_mode('cherry-pick') self.assertEqual(result, expected_messages) + def test_project_merge_mode_cherrypick_redundant(self): + # A redundant commit (that is, one that has already been applied to the + # working tree) should be skipped + self.executor_server.keep_jobdir = False + project = 'org/project-cherry-pick' + files = { + "foo.txt": "ABC", + } + A = self.fake_gerrit.addFakeChange(project, 'master', 'A', files=files) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.executor_server.hold_jobs_in_build = True + B = self.fake_gerrit.addFakeChange(project, 'master', 'B', files=files) + B.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(B.addApproval('Approved', 1)) + self.waitUntilSettled() + + build = self.builds[-1] + path = os.path.join(build.jobdir.src_root, 'review.example.com', + project) + repo = git.Repo(path) + repo_messages = [c.message.strip() for c in repo.iter_commits()] + repo_messages.reverse() + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + expected_messages = [ + 'initial commit', + 'add content from fixture', + 'A-1', + ] + self.assertHistory([ + dict(name='project-test1', result='SUCCESS', changes='1,1'), + dict(name='project-test1', result='SUCCESS', changes='2,1'), + ]) + self.assertEqual(A.data['status'], 'MERGED') + self.assertEqual(B.data['status'], 'MERGED') + self.assertEqual(repo_messages, expected_messages) + + def test_project_merge_mode_cherrypick_empty(self): + # An empty commit (that is, one that doesn't modify any files) should + # be preserved + self.executor_server.keep_jobdir = False + project = 'org/project-cherry-pick' + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange(project, 'master', 'A', empty=True) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + build = self.builds[-1] + path = os.path.join(build.jobdir.src_root, 'review.example.com', + project) + repo = git.Repo(path) + repo_messages = [c.message.strip() for c in repo.iter_commits()] + repo_messages.reverse() + + changed_files = list(repo.commit("HEAD").diff(repo.commit("HEAD~1"))) + self.assertEqual(changed_files, []) + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + expected_messages = [ + 'initial commit', + 'add content from fixture', + 'A-1', + ] + self.assertHistory([ + dict(name='project-test1', result='SUCCESS', changes='1,1'), + ]) + self.assertEqual(A.data['status'], 'MERGED') + self.assertEqual(repo_messages, expected_messages) + def test_project_merge_mode_cherrypick_branch_merge(self): "Test that branches can be merged together in cherry-pick mode" self.create_branch('org/project-merge-branches', 'mp') diff --git a/web/src/containers/FilterToolbar.jsx b/web/src/containers/FilterToolbar.jsx index 328d60d85..bcf74c062 100644 --- a/web/src/containers/FilterToolbar.jsx +++ b/web/src/containers/FilterToolbar.jsx @@ -13,6 +13,7 @@ // under the License. import React, { useState } from 'react' +import { useDispatch } from 'react-redux' import PropTypes from 'prop-types' import { Button, @@ -32,12 +33,14 @@ import { } from '@patternfly/react-core' import { FilterIcon, SearchIcon } from '@patternfly/react-icons' +import { addNotification } from '../actions/notifications' import { FilterSelect } from './filters/Select' import { FilterTernarySelect } from './filters/TernarySelect' import { FilterCheckbox } from './filters/Checkbox' function FilterToolbar(props) { + const dispatch = useDispatch() const [isCategoryDropdownOpen, setIsCategoryDropdownOpen] = useState(false) const [currentCategory, setCurrentCategory] = useState( props.filterCategories[0].title @@ -58,15 +61,22 @@ function FilterToolbar(props) { } function handleInputSend(event, category) { - const { onFilterChange, filters } = props + const { onFilterChange, filters, filterInputValidation } = props // In case the event comes from a key press, only accept "Enter" if (event.key && event.key !== 'Enter') { return } - // Ignore empty values - if (!inputValue) { + const validationResult = filterInputValidation(category.key, inputValue) + if (!validationResult.success) { + dispatch(addNotification( + { + text: validationResult.message, + type: 'error', + status: '', + url: '', + })) return } @@ -250,6 +260,7 @@ FilterToolbar.propTypes = { onFilterChange: PropTypes.func.isRequired, filters: PropTypes.object.isRequired, filterCategories: PropTypes.array.isRequired, + filterInputValidation: PropTypes.func.isRequired, } function getChipsFromFilters(filters, category) { diff --git a/web/src/pages/Builds.jsx b/web/src/pages/Builds.jsx index b89bcd678..f1c449ec0 100644 --- a/web/src/pages/Builds.jsx +++ b/web/src/pages/Builds.jsx @@ -195,6 +195,28 @@ class BuildsPage extends React.Component { this.updateData(filters) } } + + filterInputValidation = (filterKey, filterValue) => { + // Input value should not be empty for all cases + if (!filterValue) { + return { + success: false, + message: 'Input should not be empty' + } + } + + // For change filter, it must be an integer + if (filterKey === 'change' && isNaN(filterValue)) { + return { + success: false, + message: 'Change must be an integer (do not include revision)' + } + } + + return { + success: true + } + } handleFilterChange = (newFilters) => { const { location, history } = this.props @@ -261,6 +283,7 @@ class BuildsPage extends React.Component { filterCategories={this.filterCategories} onFilterChange={this.handleFilterChange} filters={filters} + filterInputValidation={this.filterInputValidation} /> <Pagination toggleTemplate={({ firstIndex, lastIndex, itemCount }) => ( diff --git a/web/src/pages/Buildsets.jsx b/web/src/pages/Buildsets.jsx index 98d86d640..938309034 100644 --- a/web/src/pages/Buildsets.jsx +++ b/web/src/pages/Buildsets.jsx @@ -148,6 +148,28 @@ class BuildsetsPage extends React.Component { } } + filterInputValidation = (filterKey, filterValue) => { + // Input value should not be empty for all cases + if (!filterValue) { + return { + success: false, + message: 'Input should not be empty' + } + } + + // For change filter, it must be an integer + if (filterKey === 'change' && isNaN(filterValue)) { + return { + success: false, + message: 'Change must be an integer (do not include revision)' + } + } + + return { + success: true + } + } + handleFilterChange = (newFilters) => { const { location, history } = this.props const { filters, itemCount } = this.state @@ -213,6 +235,7 @@ class BuildsetsPage extends React.Component { filterCategories={this.filterCategories} onFilterChange={this.handleFilterChange} filters={filters} + filterInputValidation={this.filterInputValidation} /> <Pagination toggleTemplate={({ firstIndex, lastIndex, itemCount }) => ( diff --git a/zuul/configloader.py b/zuul/configloader.py index fe22fe0f8..4f472cb4e 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -2151,21 +2151,26 @@ class TenantParser(object): for future in futures: future.result() - try: - self._processCatJobs(abide, tenant, loading_errors, jobs, - min_ltimes) - except Exception: - self.log.exception("Error processing cat jobs, canceling") - for job in jobs: + for i, job in enumerate(jobs, start=1): + try: try: - self.log.debug("Canceling cat job %s", job) + self._processCatJob(abide, tenant, loading_errors, job, + min_ltimes) + except TimeoutError: self.merger.cancel(job) - except Exception: - self.log.exception("Unable to cancel job %s", job) - if not ignore_cat_exception: - raise - if not ignore_cat_exception: - raise + raise + except Exception: + self.log.exception("Error processing cat job") + if not ignore_cat_exception: + # Cancel remaining jobs + for cancel_job in jobs[i:]: + self.log.debug("Canceling cat job %s", cancel_job) + try: + self.merger.cancel(cancel_job) + except Exception: + self.log.exception( + "Unable to cancel job %s", cancel_job) + raise def _cacheTenantYAMLBranch(self, abide, tenant, loading_errors, min_ltimes, tpc, project, branch, jobs): @@ -2234,49 +2239,48 @@ class TenantParser(object): job.source_context = source_context jobs.append(job) - def _processCatJobs(self, abide, tenant, loading_errors, jobs, min_ltimes): + def _processCatJob(self, abide, tenant, loading_errors, job, min_ltimes): # Called at the end of _cacheTenantYAML after all cat jobs # have been submitted - for job in jobs: - self.log.debug("Waiting for cat job %s" % (job,)) - res = job.wait(self.merger.git_timeout) - if not res: - # We timed out - raise Exception("Cat job %s timed out; consider setting " - "merger.git_timeout in zuul.conf" % (job,)) - if not job.updated: - raise Exception("Cat job %s failed" % (job,)) - self.log.debug("Cat job %s got files %s" % - (job, job.files.keys())) - - self._updateUnparsedBranchCache(abide, tenant, job.source_context, - job.files, loading_errors, - job.ltime, min_ltimes) - - # Save all config files in Zookeeper (not just for the current tpc) - files_cache = self.unparsed_config_cache.getFilesCache( - job.source_context.project_canonical_name, - job.source_context.branch) - with self.unparsed_config_cache.writeLock( - job.source_context.project_canonical_name): - # Prevent files cache ltime from going backward - if files_cache.ltime >= job.ltime: - self.log.info( - "Discarding job %s result since the files cache was " - "updated in the meantime", job) - continue - # Since the cat job returns all required config files - # for ALL tenants the project is a part of, we can - # clear the whole cache and then populate it with the - # updated content. - files_cache.clear() - for fn, content in job.files.items(): - # Cache file in Zookeeper - if content is not None: - files_cache[fn] = content - files_cache.setValidFor(job.extra_config_files, - job.extra_config_dirs, - job.ltime) + self.log.debug("Waiting for cat job %s" % (job,)) + res = job.wait(self.merger.git_timeout) + if not res: + # We timed out + raise TimeoutError(f"Cat job {job} timed out; consider setting " + "merger.git_timeout in zuul.conf") + if not job.updated: + raise Exception("Cat job %s failed" % (job,)) + self.log.debug("Cat job %s got files %s" % + (job, job.files.keys())) + + self._updateUnparsedBranchCache(abide, tenant, job.source_context, + job.files, loading_errors, + job.ltime, min_ltimes) + + # Save all config files in Zookeeper (not just for the current tpc) + files_cache = self.unparsed_config_cache.getFilesCache( + job.source_context.project_canonical_name, + job.source_context.branch) + with self.unparsed_config_cache.writeLock( + job.source_context.project_canonical_name): + # Prevent files cache ltime from going backward + if files_cache.ltime >= job.ltime: + self.log.info( + "Discarding job %s result since the files cache was " + "updated in the meantime", job) + return + # Since the cat job returns all required config files + # for ALL tenants the project is a part of, we can + # clear the whole cache and then populate it with the + # updated content. + files_cache.clear() + for fn, content in job.files.items(): + # Cache file in Zookeeper + if content is not None: + files_cache[fn] = content + files_cache.setValidFor(job.extra_config_files, + job.extra_config_dirs, + job.ltime) def _updateUnparsedBranchCache(self, abide, tenant, source_context, files, loading_errors, ltime, min_ltimes): diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index dcc2b4efd..0155e6963 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -81,6 +81,10 @@ ANNOTATION_LEVELS = { "warning": "warning", "error": "failure", } +# The maximum size for the 'message' field is 64 KB. Since it's unclear +# from the Github docs if the unit is KiB or KB we'll use KB to be on +# the safe side. +ANNOTATION_MAX_MESSAGE_SIZE = 64 * 1000 EventTuple = collections.namedtuple( "EventTuple", [ @@ -2433,7 +2437,9 @@ class GithubConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): raw_annotation = { "path": fn, "annotation_level": annotation_level, - "message": comment["message"], + "message": comment["message"].encode( + "utf8")[:ANNOTATION_MAX_MESSAGE_SIZE].decode( + "utf8", "ignore"), "start_line": start_line, "end_line": end_line, "start_column": start_column, diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py index 1f44303bd..396516038 100644 --- a/zuul/driver/github/githubreporter.py +++ b/zuul/driver/github/githubreporter.py @@ -193,13 +193,13 @@ class GithubReporter(BaseReporter): self.log.warning('Merge mode %s not supported by Github', mode) raise MergeFailure('Merge mode %s not supported by Github' % mode) - merge_mode = self.merge_modes[merge_mode] project = item.change.project.name pr_number = item.change.number sha = item.change.patchset log.debug('Reporting change %s, params %s, merging via API', item.change, self.config) - message = self._formatMergeMessage(item.change) + message = self._formatMergeMessage(item.change, merge_mode) + merge_mode = self.merge_modes[merge_mode] for i in [1, 2]: try: @@ -319,10 +319,13 @@ class GithubReporter(BaseReporter): self.connection.unlabelPull(project, pr_number, label, zuul_event_id=item.event) - def _formatMergeMessage(self, change): + def _formatMergeMessage(self, change, merge_mode): message = [] - if change.title: - message.append(change.title) + # For squash merges we don't need to add the title to the body + # as it will already be set as the commit subject. + if merge_mode != model.MERGER_SQUASH_MERGE: + if change.title: + message.append(change.title) if change.body_text: message.append(change.body_text) merge_message = "\n\n".join(message) diff --git a/zuul/driver/mqtt/mqttconnection.py b/zuul/driver/mqtt/mqttconnection.py index 7f221282f..4a028ba23 100644 --- a/zuul/driver/mqtt/mqttconnection.py +++ b/zuul/driver/mqtt/mqttconnection.py @@ -64,6 +64,12 @@ class MQTTConnection(BaseConnection): def onLoad(self, zk_client, component_registry): self.log.debug("Starting MQTT Connection") + + # If the connection was not loaded by a scheduler, but by e.g. + # zuul-web, we want to stop here. + if not self.sched: + return + try: self.client.connect( self.connection_config.get('server', 'localhost'), @@ -76,10 +82,11 @@ class MQTTConnection(BaseConnection): self.client.loop_start() def onStop(self): - self.log.debug("Stopping MQTT Connection") - self.client.loop_stop() - self.client.disconnect() - self.connected = False + if self.connected: + self.log.debug("Stopping MQTT Connection") + self.client.loop_stop() + self.client.disconnect() + self.connected = False def publish(self, topic, message, qos, zuul_event_id): log = get_annotated_logger(self.log, zuul_event_id) diff --git a/zuul/executor/server.py b/zuul/executor/server.py index a49bbbbbf..6dbf62de0 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -1931,6 +1931,7 @@ class AnsibleJob(object): region=node.region, host_id=node.host_id, external_id=getattr(node, 'external_id', None), + slot=node.slot, interface_ip=node.interface_ip, public_ipv4=node.public_ipv4, private_ipv4=node.private_ipv4, @@ -3632,6 +3633,10 @@ class ExecutorServer(BaseMergeServer): log.exception('Process pool got broken') self.resetProcessPool() task.transient_error = True + except IOError: + log.exception('Got I/O error while updating repo %s/%s', + task.connection_name, task.project_name) + task.transient_error = True except Exception: log.exception('Got exception while updating repo %s/%s', task.connection_name, task.project_name) diff --git a/zuul/merger/merger.py b/zuul/merger/merger.py index e4688a1b7..1df833bc5 100644 --- a/zuul/merger/merger.py +++ b/zuul/merger/merger.py @@ -595,14 +595,32 @@ class Repo(object): log = get_annotated_logger(self.log, zuul_event_id) repo = self.createRepoObject(zuul_event_id) self.fetch(ref, zuul_event_id=zuul_event_id) - if len(repo.commit("FETCH_HEAD").parents) > 1: + fetch_head = repo.commit("FETCH_HEAD") + if len(fetch_head.parents) > 1: args = ["-s", "resolve", "FETCH_HEAD"] log.debug("Merging %s with args %s instead of cherry-picking", ref, args) repo.git.merge(*args) else: log.debug("Cherry-picking %s", ref) - repo.git.cherry_pick("FETCH_HEAD") + # Git doesn't have an option to ignore commits that are already + # applied to the working tree when cherry-picking, so pass the + # --keep-redundant-commits option, which will cause it to make an + # empty commit + repo.git.cherry_pick("FETCH_HEAD", keep_redundant_commits=True) + + # If the newly applied commit is empty, it means either: + # 1) The commit being cherry-picked was empty, in which the empty + # commit should be kept + # 2) The commit being cherry-picked was already applied to the + # tree, in which case the empty commit should be backed out + head = repo.commit("HEAD") + parent = head.parents[0] + if not any(head.diff(parent)) and \ + any(fetch_head.diff(fetch_head.parents[0])): + log.debug("%s was already applied. Removing it", ref) + self._checkout(repo, parent) + return repo.head.commit def merge(self, ref, strategy=None, zuul_event_id=None): diff --git a/zuul/model.py b/zuul/model.py index e526b749c..5be5923a5 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -1408,6 +1408,7 @@ class Node(ConfigObject): self.private_ipv6 = None self.connection_port = 22 self.connection_type = None + self.slot = None self._keys = [] self.az = None self.provider = None diff --git a/zuul/zk/job_request_queue.py b/zuul/zk/job_request_queue.py index 175c57b90..7c85ae95e 100644 --- a/zuul/zk/job_request_queue.py +++ b/zuul/zk/job_request_queue.py @@ -609,7 +609,7 @@ class JobRequestQueue(ZooKeeperSimpleBase): self.kazoo_client.delete(lock_path, recursive=True) except Exception: self.log.exception( - "Unable to delete lock %s", path) + "Unable to delete lock %s", lock_path) except Exception: self.log.exception("Error cleaning up locks %s", self) |