From 033470e8b3f9b9fe7d39c267bbe9f1b2e8bb1eba Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Thu, 30 Nov 2023 10:01:26 -0800 Subject: [PATCH] Fix repo state restore for zuul role tag override When a repo that is being used for a zuul role has override-checkout set to a tag, the job would fail because we did not reconstruct the tag in our zuul-role checkout; we only did that for branches. This fixes the repo state restore for any type of ref. There is a an untested code path where a zuul role repo is checked out to a tag using override-checkout. Add a test for that (and also the same for a branch, for good measure). Change-Id: I36f142cd3c4e7d0b930318dddd7276f3635cc3a2 --- .../git/common-config/playbooks/run.yaml | 1 + .../git/common-config/zuul.yaml | 56 ++++++++++++++ .../executor-repos/git/org_project/README | 1 + .../git/org_project/playbooks/run.yaml | 1 + .../git/org_project/roles/README | 0 .../executor-repos/git/org_project/zuul.yaml | 31 ++++++++ .../executor-repos/git/org_project1/README | 1 + .../git/org_project1/roles/README | 0 .../executor-repos/git/org_project2/README | 1 + .../git/org_project2/roles/README | 0 .../executor-repos/git/org_project3/README | 1 + .../executor-repos/git/org_project3/zuul.yaml | 25 ++++++ .../fixtures/config/executor-repos/main.yaml | 10 +++ tests/unit/test_executor.py | 77 +++++++++++++++++-- zuul/executor/server.py | 12 ++- zuul/merger/merger.py | 10 ++- 16 files changed, 214 insertions(+), 13 deletions(-) create mode 100644 tests/fixtures/config/executor-repos/git/common-config/playbooks/run.yaml create mode 100644 tests/fixtures/config/executor-repos/git/common-config/zuul.yaml create mode 100644 tests/fixtures/config/executor-repos/git/org_project/README create mode 100644 tests/fixtures/config/executor-repos/git/org_project/playbooks/run.yaml create mode 100644 tests/fixtures/config/executor-repos/git/org_project/roles/README create mode 100644 tests/fixtures/config/executor-repos/git/org_project/zuul.yaml create mode 100644 tests/fixtures/config/executor-repos/git/org_project1/README create mode 100644 tests/fixtures/config/executor-repos/git/org_project1/roles/README create mode 100644 tests/fixtures/config/executor-repos/git/org_project2/README create mode 100644 tests/fixtures/config/executor-repos/git/org_project2/roles/README create mode 100644 tests/fixtures/config/executor-repos/git/org_project3/README create mode 100644 tests/fixtures/config/executor-repos/git/org_project3/zuul.yaml create mode 100644 tests/fixtures/config/executor-repos/main.yaml diff --git a/tests/fixtures/config/executor-repos/git/common-config/playbooks/run.yaml b/tests/fixtures/config/executor-repos/git/common-config/playbooks/run.yaml new file mode 100644 index 0000000000..ed97d539c0 --- /dev/null +++ b/tests/fixtures/config/executor-repos/git/common-config/playbooks/run.yaml @@ -0,0 +1 @@ +--- diff --git a/tests/fixtures/config/executor-repos/git/common-config/zuul.yaml b/tests/fixtures/config/executor-repos/git/common-config/zuul.yaml new file mode 100644 index 0000000000..dc6a3cf489 --- /dev/null +++ b/tests/fixtures/config/executor-repos/git/common-config/zuul.yaml @@ -0,0 +1,56 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + - event: comment-added + comment: '^(Patch Set [0-9]+:\n\n)?(?i:recheck)$' + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- pipeline: + name: gate + manager: dependent + success-message: Build succeeded (gate). + trigger: + gerrit: + - event: comment-added + approval: + - Approved: 1 + success: + gerrit: + Verified: 2 + submit: true + failure: + gerrit: + Verified: -2 + start: + gerrit: + Verified: 0 + precedence: high + +- pipeline: + name: post + manager: independent + trigger: + gerrit: + - event: ref-updated + ref: ^(?!refs/).*$ + precedence: low + +- pipeline: + name: periodic + manager: independent + trigger: + timer: + - time: '0 0 * * *' + +- job: + name: base + run: playbooks/run.yaml + parent: null diff --git a/tests/fixtures/config/executor-repos/git/org_project/README b/tests/fixtures/config/executor-repos/git/org_project/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/executor-repos/git/org_project/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/executor-repos/git/org_project/playbooks/run.yaml b/tests/fixtures/config/executor-repos/git/org_project/playbooks/run.yaml new file mode 100644 index 0000000000..ed97d539c0 --- /dev/null +++ b/tests/fixtures/config/executor-repos/git/org_project/playbooks/run.yaml @@ -0,0 +1 @@ +--- diff --git a/tests/fixtures/config/executor-repos/git/org_project/roles/README b/tests/fixtures/config/executor-repos/git/org_project/roles/README new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/config/executor-repos/git/org_project/zuul.yaml b/tests/fixtures/config/executor-repos/git/org_project/zuul.yaml new file mode 100644 index 0000000000..a445592ec4 --- /dev/null +++ b/tests/fixtures/config/executor-repos/git/org_project/zuul.yaml @@ -0,0 +1,31 @@ +- job: + name: branch-job + run: playbooks/run.yaml + override-checkout: test-branch + required-projects: + - name: org/project1 + - name: org/project2 + roles: + - zuul: org/project1 + - zuul: org/project2 + files: + - branch + +- job: + name: tag-job + run: playbooks/run.yaml + override-checkout: test-tag + required-projects: + - name: org/project1 + - name: org/project2 + roles: + - zuul: org/project1 + - zuul: org/project2 + files: + - tag + +- project: + check: + jobs: + - branch-job + - tag-job diff --git a/tests/fixtures/config/executor-repos/git/org_project1/README b/tests/fixtures/config/executor-repos/git/org_project1/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/executor-repos/git/org_project1/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/executor-repos/git/org_project1/roles/README b/tests/fixtures/config/executor-repos/git/org_project1/roles/README new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/config/executor-repos/git/org_project2/README b/tests/fixtures/config/executor-repos/git/org_project2/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/executor-repos/git/org_project2/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/executor-repos/git/org_project2/roles/README b/tests/fixtures/config/executor-repos/git/org_project2/roles/README new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/config/executor-repos/git/org_project3/README b/tests/fixtures/config/executor-repos/git/org_project3/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/executor-repos/git/org_project3/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/executor-repos/git/org_project3/zuul.yaml b/tests/fixtures/config/executor-repos/git/org_project3/zuul.yaml new file mode 100644 index 0000000000..a588b7aa26 --- /dev/null +++ b/tests/fixtures/config/executor-repos/git/org_project3/zuul.yaml @@ -0,0 +1,25 @@ +- project: + queue: integrated + check: + jobs: + - project-merge + - project-test1: + dependencies: project-merge + - project-test2: + dependencies: project-merge + - project1-project2-integration: + dependencies: + - project-test1 + - project-test2 + gate: + jobs: + - project-merge + - project-test1: + dependencies: project-merge + - project-test2: + dependencies: project-merge + - project1-project2-integration: + dependencies: + - project-merge + - project-test1 + - project-test2 diff --git a/tests/fixtures/config/executor-repos/main.yaml b/tests/fixtures/config/executor-repos/main.yaml new file mode 100644 index 0000000000..83ed0925a9 --- /dev/null +++ b/tests/fixtures/config/executor-repos/main.yaml @@ -0,0 +1,10 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - org/project + - org/project1 + - org/project2 diff --git a/tests/unit/test_executor.py b/tests/unit/test_executor.py index f7a6e2a497..c941e23d99 100644 --- a/tests/unit/test_executor.py +++ b/tests/unit/test_executor.py @@ -38,11 +38,7 @@ from zuul.executor.server import squash_variables from zuul.model import NodeSet, Group -class TestExecutorRepos(ZuulTestCase): - tenant_config_file = 'config/single-tenant/main.yaml' - - log = logging.getLogger("zuul.test.executor") - +class ExecutorReposMixin: def assertRepoState(self, repo, state, project, build, number): if 'branch' in state: self.assertFalse(repo.head.is_detached, @@ -55,7 +51,7 @@ class TestExecutorRepos(ZuulTestCase): 'be on the correct branch' % ( project, build, number)) # Remote 'origin' needs to be kept intact with a bogus URL - self.assertEqual(repo.remotes.origin.url, 'file:///dev/null') + self.assertEqual('file:///dev/null', repo.remotes.origin.url) self.assertIn(state['branch'], repo.remotes.origin.refs) if 'commit' in state: self.assertEqual(state['commit'], @@ -94,6 +90,12 @@ class TestExecutorRepos(ZuulTestCase): self.executor_server.release() self.waitUntilSettled() + +class TestExecutorRepos(ZuulTestCase, ExecutorReposMixin): + tenant_config_file = 'config/single-tenant/main.yaml' + + log = logging.getLogger("zuul.test.executor") + @simple_layout('layouts/repo-checkout-two-project.yaml') def test_one_branch(self): self.executor_server.hold_jobs_in_build = True @@ -430,6 +432,69 @@ class TestExecutorRepos(ZuulTestCase): self.assertBuildStates(states, projects) +class TestExecutorRepoRoles(ZuulTestCase, ExecutorReposMixin): + tenant_config_file = 'config/executor-repos/main.yaml' + + def test_repo_checkout_roles_branch(self): + self.executor_server.hold_jobs_in_build = True + p0 = "review.example.com/org/project" + p1 = "review.example.com/org/project1" + p2 = "review.example.com/org/project2" + projects = [p0, p1, p2] + + self.create_branch('org/project1', 'test-branch') + self.create_branch('org/project2', 'test-branch') + + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files={'branch': 'run branch'}) + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + states = [ + { + p0: dict(present=[A], branch='master'), + p1: dict(absent=[A], branch='test-branch'), + p2: dict(absent=[A], branch='test-branch'), + }, + ] + + self.assertBuildStates(states, projects) + + def test_repo_checkout_roles_tag(self): + self.executor_server.hold_jobs_in_build = True + p0 = "review.example.com/org/project" + p1 = "review.example.com/org/project1" + p2 = "review.example.com/org/project2" + projects = [p0, p1, p2] + upstream = self.getUpstreamRepos(projects) + + self.create_branch('org/project1', 'unused-branch') + files = {'README': 'tagged readme'} + self.addCommitToRepo('org/project1', 'tagged commit', + files, branch='unused-branch', tag='test-tag') + self.create_branch('org/project2', 'unused-branch') + files = {'README': 'tagged readme'} + self.addCommitToRepo('org/project2', 'tagged commit', + files, branch='unused-branch', tag='test-tag') + + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files={'tag': 'run tag'}) + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + states = [ + { + p0: dict(present=[A], branch='master'), + p1: dict(commit=str(upstream[p1].commit('test-tag')), + absent=[A]), + p2: dict(commit=str(upstream[p2].commit('test-tag')), + absent=[A]), + }, + ] + + self.assertBuildStates(states, projects) + + class TestAnsibleJob(ZuulTestCase): tenant_config_file = 'config/single-tenant/main.yaml' diff --git a/zuul/executor/server.py b/zuul/executor/server.py index 489984df9e..b2ce7aa37c 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -2287,12 +2287,18 @@ class AnsibleJob(object): # do need to at least ensure the branch we're # going to check out exists. repo = self.workspace_merger.getRepo(p['connection'], - p['name']) + p['name'], + keep_remote_url=True) + # We call it a branch, but it can actually be any + # ref including a tag. Get the ref object so we + # can duplicate the full path. + ref_obj = repo.getRef(branch) + ref_path = ref_obj.path + ref_sha = ref_obj.commit.hexsha repo_state = { p['connection']: { p['name']: { - f'refs/heads/{branch}': - repo.getBranchHead(branch).hexsha + ref_path: ref_sha, } } } diff --git a/zuul/merger/merger.py b/zuul/merger/merger.py index ff7d7dcb0b..3000a87ab6 100644 --- a/zuul/merger/merger.py +++ b/zuul/merger/merger.py @@ -481,12 +481,12 @@ class Repo(object): repo = self.createRepoObject(zuul_event_id) return [x.name for x in repo.heads] - def getCommitFromRef(self, refname, zuul_event_id=None): + def getRef(self, refname, zuul_event_id=None): repo = self.createRepoObject(zuul_event_id) if refname not in repo.refs: return None ref = repo.refs[refname] - return ref.commit + return ref def getRefs(self, zuul_event_id=None): repo = self.createRepoObject(zuul_event_id) @@ -998,7 +998,8 @@ class Merger(object): return repo def getRepo(self, connection_name, project_name, - zuul_event_id=None, process_worker=None): + zuul_event_id=None, process_worker=None, + keep_remote_url=False): source = self.connections.getSource(connection_name) project = source.getProject(project_name) hostname = project.canonical_hostname @@ -1007,7 +1008,8 @@ class Merger(object): key = '/'.join([hostname, project_name]) if key in self.repos: repo = self.repos[key] - repo.setRemoteUrl(url) + if not keep_remote_url: + repo.setRemoteUrl(url) return repo sshkey = self.connections.connections.get(connection_name).\ connection_config.get('sshkey')