Reconfigure CI pipelines for PR and merge queue (backport #11526) (#1…

…1586) * Reconfigure CI pipelines for PR and merge queue (#11526) * Reconfigure CI pipelines for PR and merge queue This commit refactors the pipeline organisation of our CI to reduce the total number of jobs run on any given PR merge, and to reduce the critical path length of merging. In particular: - the lint, docs and QPY tests are combined into one job for a single CI worker. These three jobs individually add up to less than a test run, and moving QPY from the first-stage PR test run to this rebalances the (now) two jobs in the stage to be more equal in runtime. - a new pipeline is added specifically for the merge queue. Previously this reused the same two-stage PR pipeline. The two-stage system unnecessarily lengthened the critical path, as when a PR enters the merge queue, it as already passed PR CI, and so is highly likely to pass all jobs. In addition to flattening to a single stage, one each of the macOS and Windows jobs are removed to lower the amount of VMs needed and reduce the chances of a timeout (these OSes are more likely to get a dodgy VM and time out than Linux). The only way a new test failure should appear (other than a flaky test) is by a logical merge conflict, which would be quite unlikely to only affect a particular Python version. To make it easier to run the lint and docs jobs together, and ensure that both run even if there's a failure in one, the full lint configuration is merged back to `tox.ini`, and that's used to do the linting. This makes it more consistent for developers, as well. * Allow cargo as an external in 'tox' * Add display name to lint job * Use editable install for tox lint job The lint job will fail unless the Rust components are installed into the source directory, since that's where `pylint` will look for them to resolve the imports. * Fix typo in merge-queue stage condition (cherry picked from commit ed79d42) # Conflicts: # .azure/lint-linux.yml # tox.ini * Fix conflict --------- Co-authored-by: Jake Lishman <[email protected]>
Qiskit · Jan 31, 2024 · 39caa8d · 39caa8d
1 parent d3dd6fe
commit 39caa8d
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 48 deletions.
diff --git a/.azure/docs-linux.yml → .azure/lint_docs_qpy-linux.yml b/.azure/docs-linux.yml → .azure/lint_docs_qpy-linux.yml
@@ -4,7 +4,8 @@ parameters:
     displayName: "Version of Python to use"
 
 jobs:
-  - job: 'Docs'
+  - job: 'Lint_Docs_QPY'
+    displayName: 'Lint, documentation and QPY'
     pool: {vmImage: 'ubuntu-latest'}
 
     variables:
@@ -20,14 +21,13 @@ jobs:
         displayName: 'Use Python ${{ parameters.pythonVersion }}'
 
       - bash: tools/install_ubuntu_docs_dependencies.sh
-        displayName: 'Install dependencies'
+        displayName: 'Install docs dependencies'
 
-      - bash: |
-          set -e
-          tox -e docs
-          # Clean up Sphinx detritus.
-          rm -rf docs/_build/html/{.doctrees,.buildinfo}
-        displayName: 'Run Docs build'
+      - bash: tox run -e docs,lint
+        displayName: 'Lint and docs'
+
+      - bash: rm -rf docs/_build/html/{.doctrees,.buildinfo}
+        displayName: 'Clean up docs detritus'
 
       - task: ArchiveFiles@2
         inputs:
@@ -43,3 +43,24 @@ jobs:
           artifactName: 'html_docs'
           Parallel: true
           ParallelCount: 8
+
+      - task: Cache@2
+        inputs:
+          key: 'qpy | test/qpy_compat/test_qpy.py | "$(Build.BuildNumber)"'
+          restoreKeys: |
+            qpy | test/qpy_compat/test_qpy.py
+          path: qpy_files
+        displayName: Cache old QPY files
+
+      - bash: |
+          set -e
+          # Reuse the docs environment to avoid needing to rebuild another
+          # version of Qiskit.
+          source .tox/docs/bin/activate
+          mv qpy_files/* test/qpy_compat || :
+          pushd test/qpy_compat
+          ./run_tests.sh
+          popd
+          mkdir qpy_files || :
+          mv test/qpy_compat/qpy_* qpy_files/.
+        displayName: 'Run QPY backwards compat tests'
diff --git a/.azure/test-linux.yml b/.azure/test-linux.yml
@@ -6,9 +6,6 @@ parameters:
   - name: "testRust"
     type: boolean
 
-  - name: "testQPY"
-    type: boolean
-
   - name: "testImages"
     type: boolean
 
@@ -168,24 +165,6 @@ jobs:
         displayName: 'Publish images on test failure'
         condition: failed()
 
-      - ${{ if eq(parameters.testQPY, true) }}:
-        - task: Cache@2
-          inputs:
-            key: 'qpy | test/qpy_compat/test_qpy.py | "$(Build.BuildNumber)"'
-            restoreKeys: |
-              qpy | test/qpy_compat/test_qpy.py
-            path: qpy_files
-          displayName: cache old qpy
-        - bash: |
-            set -e
-            mv qpy_files/* test/qpy_compat || :
-            pushd test/qpy_compat
-            ./run_tests.sh
-            popd
-            mkdir qpy_files || :
-            mv test/qpy_compat/qpy_* qpy_files/.
-          displayName: 'Run QPY backwards compat tests'
-
       - ${{ if eq(parameters.testImages, true) }}:
         - bash: |
             set -e

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
@@ -16,6 +16,12 @@ pr:
     include:
       - '*'
 
+# A schedule only runs on branches that match include rules from _both_ `main`
+# and the branch itself.  On `main`, we blanket include all branches that might
+# want to be enabled, then particular branches can override it to exclude
+# themselves by removing themselves from the trigger list.  For example, old
+# stable branches can remove `stable/*` from their copy of this file once they
+# reach their end-of-life.
 schedules:
   - cron: "20 6 * * *"
     displayName: "Complete matrix test"
@@ -82,9 +88,13 @@ parameters:
 #   - push to a pull request (`PullRequest`)
 # For `IndividualCI` (push/merge to a branch/tag on Qiskit/qiskit-terra), you
 # need to examine `variables['Build.SourceBranch']` to determine whether it's a
-# branch or a tag.
+# branch or a tag, and if a branch, then whether it's in the merge queue or a
+# push to a "real" branch.
 stages:
   # Nightly cron job.
+  #
+  # For this to run on a branch, the `schedules` trigger  up at the top of this
+  # file needs to match on _both_ `main` and the branch itself.
   - ${{ if eq(variables['Build.Reason'], 'Schedule') }}:
     - stage: "Nightly"
       displayName: "Nightly complete matrix tests"
@@ -93,7 +103,6 @@ stages:
           - template: ".azure/test-linux.yml"
             parameters:
               pythonVersion: ${{ version }}
-              testQPY: false
               testImages: false
               testRust: false
               installOptionals: true
@@ -123,10 +132,10 @@ stages:
                 id: 7864
                 comment: Nightly test job failed at commit $(Build.SourceVersion).  View the logs at $(System.TeamFoundationCollectionUri)$(System.TeamProject)/_build/results?buildId=$(Build.BuildId).
 
-  # Full PR suite.  This also needs to apply to the merge queue, which appears
-  # as a push event.  The queue won't get cron triggered because the schedule
-  # trigger doesn't have it as a target.
-  - ${{ if or(eq(variables['Build.Reason'], 'PullRequest'), contains(variables['Build.SourceBranch'], 'gh-readonly-queue')) }}:
+  # Full PR suite.  PRs need to pass this pipeline in order to be moved to the
+  # merge queue, where they'll use the next rule as the branch-protection rule
+  # for the final merge to the base branch.
+  - ${{ if eq(variables['Build.Reason'], 'PullRequest') }}:
     # The preliminary stage should be small in both total runtime (including
     # provisioning) and resources required.  About half of PR commits result in
     # a CI failure, and over 90% of these are in linting, documention or a test
@@ -135,14 +144,10 @@ stages:
     - stage: "Lint_Docs_Prelim_Tests"
       displayName: "Preliminary tests"
       jobs:
-        - template: ".azure/lint-linux.yml"
+        - template: ".azure/lint_docs_qpy-linux.yml"
           parameters:
             pythonVersion: ${{ parameters.minimumPythonVersion }}
 
-        - template: ".azure/docs-linux.yml"
-          parameters:
-            pythonVersion: ${{ parameters.documentationPythonVersion }}
-
         - template: ".azure/test-linux.yml"
           parameters:
             pythonVersion: ${{ parameters.minimumPythonVersion }}
@@ -153,13 +158,12 @@ stages:
             # to fail fast in the first CI stage.
             installOptionals: true
             testRust: true
-            testQPY: true
             testImages: true
 
     # The rest of the PR pipeline is to test the oldest and newest supported
-    # versions of Python. It's very rare for a failure to be specific to an intermediate version of
-    # Python, so we just catch those in the cron-job pipeline to reduce the
-    # amount of resources used.
+    # versions of Python. It's very rare for a failure to be specific to an
+    # intermediate version of Python, so we just catch those in the cron-job
+    # pipeline to reduce the amount of resources used.
     - stage: "Tests"
       displayName: "Main tests"
       dependsOn: "Lint_Docs_Prelim_Tests"
@@ -168,7 +172,6 @@ stages:
           parameters:
             pythonVersion: ${{ parameters.maximumPythonVersion }}
             testRust: false
-            testQPY: false
             testImages: false
             installFromSdist: true
             installOptionals: false
@@ -193,6 +196,46 @@ stages:
             pythonVersion: ${{ parameters.maximumPythonVersion }}
             installOptionals: false
 
+  # Merge queue.  A PR that reaches here has already passed the more rigorous PR
+  # suite, so is very likely to pass.  The main reasons for failures here are
+  # flaky VMs timing out (which we can't do much about), or a merge conflict
+  # with another PR that is also in the merge queue.
+  #
+  # There's no reason to have multiple stages in this case, because we're
+  # expecting it to pass.  Having more than one stage frustrates parallel
+  # throughput in low-contention cases, and guarantees a longer critical path.
+  - ${{ if and(eq(variables['Build.Reason'], 'IndividualCI'), contains(variables['Build.SourceBranch'], 'gh-readonly-queue')) }}:
+    - stage: "Merge_Queue"
+      displayName: "Merge queue"
+      jobs:
+        - template: ".azure/lint_docs_qpy-linux.yml"
+          parameters:
+            pythonVersion: ${{ parameters.minimumPythonVersion }}
+
+        - template: ".azure/test-linux.yml"
+          parameters:
+            pythonVersion: ${{ parameters.minimumPythonVersion }}
+            installOptionals: true
+            testRust: true
+            testImages: true
+
+        - template: ".azure/test-linux.yml"
+          parameters:
+            pythonVersion: ${{ parameters.maximumPythonVersion }}
+            installOptionals: false
+            testRust: false
+            testImages: false
+
+        - template: ".azure/test-macos.yml"
+          parameters:
+            pythonVersion: ${{ parameters.maximumPythonVersion }}
+            installOptionals: false
+
+        - template: ".azure/test-windows.yml"
+          parameters:
+            pythonVersion: ${{ parameters.maximumPythonVersion }}
+            installOptionals: false
+
   # Push to main or the stable branches.  The triggering branches also need to
   # be in the triggers at the top of this file.
   - ${{ if and(eq(variables['Build.Reason'], 'IndividualCI'), or(startsWith(variables['Build.SourceBranch'], 'refs/heads/main'), startsWith(variables['Build.SourceBranch'], 'refs/heads/stable/'))) }}:
@@ -202,6 +245,5 @@ stages:
           parameters:
             pythonVersion: ${{ parameters.branchPushPythonVersion }}
             testRust: true
-            testQPY: true
             testImages: true
             installOptionals: false
diff --git a/tox.ini b/tox.ini
@@ -1,5 +1,5 @@
 [tox]
-minversion = 3.3.0
+minversion = 4.0
 envlist = py38, py39, py310, py311, py312, lint-incr
 isolated_build = true
 
@@ -29,9 +29,16 @@ commands =
 
 [testenv:lint]
 basepython = python3
+# `pylint` will examine the source code, not the version that would otherwise be
+# installed in `site-packages`, so we use an editable install to make sure the
+# compiled modules are built into a valid place for it to find them.
+package = editable
+allowlist_externals = cargo
 commands =
-  ruff check qiskit test tools examples setup.py qiskit_pkg
   black --check {posargs} qiskit test tools examples setup.py qiskit_pkg
+  cargo fmt --check
+  ruff check qiskit test tools examples setup.py qiskit_pkg
+  cargo clippy -- -D warnings
   pylint -rn qiskit test tools
   # This line is commented out until #6649 merges. We can't run this currently
   # via tox because tox doesn't support globbing