Merge "Fix implicit role repos missing in repo-state"

tests/fixtures/config/global-repo-state/git/common-config/playbooks/pre.yaml

@@ -1,3 +1,4 @@
 - hosts: localhost
   roles:
     - test-role
+    - implicit-role

tests/fixtures/config/global-repo-state/git/common-config/zuul.yaml

@@ -51,19 +51,20 @@
     precedence: high
 
 - job:
-    name: base
+    name: base-calling-implicit-role
     pre-run:
       - playbooks/pre.yaml
     roles:
       - zuul: org/roles
-    parent: null
 
 - job:
     name: test1
+    parent: base-calling-implicit-role
     run: playbooks/test1.yaml
 
 - job:
     name: test2
+    parent: base-calling-implicit-role
     run: playbooks/test2.yaml
 
 - job:

tests/fixtures/config/global-repo-state/git/org_implicit-role/.zuul.yaml

@@ -0,0 +1,3 @@
+- job:
+    name: base
+    parent: null

tests/fixtures/config/global-repo-state/git/org_implicit-role/roles/implicit-role/tasks/main.yaml

@@ -0,0 +1,3 @@
+- name: Test
+  debug:
+    msg: run implicit-role

tests/fixtures/config/global-repo-state/main.yaml

@@ -5,6 +5,7 @@
       gerrit:
         config-projects:
           - common-config
+          - org/implicit-role
         untrusted-projects:
           - org/dependentproject
           - org/project

tests/unit/test_v3.py

@@ -2476,6 +2476,53 @@ class TestGlobalRepoState(AnsibleZuulTestCase):
         # Reset repo to ensure the cached repo has the failing commit. This
         # is needed to ensure that the repo state has been restored.
         repo = self.executor_server.merger.getRepo('gerrit', 'common-config')
+        repo.update()
+        repo.reset()
+
+        self.executor_server.hold_jobs_in_start = False
+        self.waitUntilSettled()
+        self.assertHistory([
+            dict(name='test1', result='SUCCESS', changes='1,1'),
+            dict(name='test2', result='SUCCESS', changes='1,1'),
+        ])
+
+    def test_inherited_implicit_roles(self):
+        # Test that the repo state is restored globally for the whole buildset
+        # including inherited projects not in the dependency chain.
+        self.executor_server.hold_jobs_in_start = True
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
+        A.addApproval('Approved', 1)
+        self.fake_gerrit.addEvent(A.addApproval('Code-Review', 2))
+
+        for _ in iterate_timeout(30, 'Wait for build to be in starting phase'):
+            if self.executor_server.job_workers:
+                sleep(1)
+                break
+
+        # The build test1 is running while test2 is waiting for test1.
+        self.assertEqual(len(self.builds), 1)
+
+        # Now merge a change to the role out of band. This will break test2
+        # if it updates common-config to latest master. However due to the
+        # buildset-global repo state test2 must not be broken afterwards.
+        playbook = textwrap.dedent(
+            """
+            - name: fail
+              fail:
+                msg: foobar
+            """)
+
+        file_dict = {'roles/implicit-role/tasks/main.yaml': playbook}
+        B = self.fake_gerrit.addFakeChange('org/implicit-role', 'master', 'A',
+                                           files=file_dict)
+        self.log.info('Merge test change on org/implicit-role')
+        B.setMerged()
+
+        # Reset repo to ensure the cached repo has the failing commit. This
+        # is needed to ensure that the repo state has been restored.
+        repo = self.executor_server.merger.getRepo(
+            'gerrit', 'org/implicit-role')
+        repo.update()
         repo.reset()
 
         self.executor_server.hold_jobs_in_start = False
@@ -2551,6 +2598,7 @@ class TestGlobalRepoState(AnsibleZuulTestCase):
         # is needed to ensure that the repo state has been restored.
         repo = self.executor_server.merger.getRepo(
             'gerrit', 'org/requiredproject')
+        repo.update()
         repo.reset()
 
         self.executor_server.hold_jobs_in_start = False

zuul/model.py

@@ -1761,13 +1761,13 @@ class Job(ConfigObject):
 
         return True
 
-    def _projectsFromPlaybooks(self, playbooks):
+    def _projectsFromPlaybooks(self, playbooks, with_implicit=False):
         for playbook in playbooks:
             # noop job does not have source_context
             if playbook.source_context:
                 yield playbook.source_context.project.canonical_name
             for role in playbook.roles:
-                if role.implicit:
+                if role.implicit and not with_implicit:
                     continue
                 yield role.project_canonical_name
 
@@ -1780,7 +1780,7 @@ class Job(ConfigObject):
         project_canonical_names.update(self.required_projects.keys())
         project_canonical_names.update(self._projectsFromPlaybooks(
             chain(self.pre_run, [self.run[0]], self.post_run,
-                  self.cleanup_run)))
+                  self.cleanup_run), with_implicit=True))
 
         projects = list()
         for project_canonical_name in project_canonical_names: