Can resumed CI reuse successful matrix configurations?

[panva]

Refs: https://openjs-foundation.slack.com/archives/C03BJP63CH0/p1787917001257859

I do not know Jenkins well enough to propose this confidently. I asked an LLM to inspect the relevant build metadata, current job configuration, and plugin sources. This is my understanding of what it found and the changes it suggested.

The existing resume behavior does seem to work between Multijob children. For example, node-test-commit #91372 reused the same builds for the twelve successful direct children from #91368.

The gap seems to be inside matrix jobs.

This is a worked example for node-test-commit-linux. Since the selector is also used by other matrix jobs, I think the same approach could likely be applied to the other matrix configurations too, but I have not checked them individually.

node-test-commit-linux #72543 had ten configurations. Eight succeeded, alpine-last-latest-x64 was unstable, and rhel8-x64 failed. Its resume, #72548, ran all ten configurations again.

My understanding is that Multijob sees node-test-commit-linux as one failed child. It starts a new matrix parent, whose selector then selects every eligible configuration without considering the individual results from the build being resumed.

There is a second issue in the same run. Both Linux builds used:

COMMIT_SHA_CHECK=76541f07d8615007c5f49205059eef3b4916fcc7

but they had different resolved bases:

#72543 REBASE_ONTO=9f04fcd70d28ac691f592d3c0c1e00a28998eb51
#72548 REBASE_ONTO=523e58319f2854071e245ff7dbe2a20ec5f5d5a6

That appears to have produced a resumed result containing successful jobs from the first base together with a new Linux job from the second base.

I assumed that Resume would keep the original resolved REBASE_ONTO, and that only a fresh request-ci would pick up a newer target. I may be wrong about the intended behavior there.

The LLM suggested two changes.

Reuse successful configurations from the matrix being resumed

The first suggestion was to extend VersionSelectorScript.groovy. Multijob already passes the previous failed child to the new child through MultiJobResumeControl, so the selector could follow that chain and exclude configurations whose newest exact result was SUCCESS.

Suggested VersionSelectorScript.groovy diff

diff --git a/jenkins/scripts/VersionSelectorScript.groovy b/jenkins/scripts/VersionSelectorScript.groovy
--- a/jenkins/scripts/VersionSelectorScript.groovy
+++ b/jenkins/scripts/VersionSelectorScript.groovy
@@ -97,26 +97,104 @@
 // NOTE: this assumes that the default "Agents"->"Name" in the Configuration
 // Matrix is left as "nodes", if it's changed then `it.nodes` below won't work
 // and returning a result with a "nodes" property won't work.
 result['nodes'] = []
 
 // Before running this script, `def buildType = 'release'` or some other value
 // to be able to use the appropriate `buildType` in the exclusions
 def _buildType
 try {
   _buildType = buildType
 } catch (groovy.lang.MissingPropertyException e) {
   _buildType = 'test'
 }
 
 combinations.each{
   def builderLabel = it.nodes
   // Default to running all builders if nodeMajorVersion is still -1
   // (i.e. the version check failed)
   if (nodeMajorVersion >= 4) {
     if (!canBuild(nodeMajorVersion, builderLabel, _buildType)) {
       println "Skipping $builderLabel for Node.js $nodeMajorVersion"
       return
     }
   }
   result['nodes'].add(it)
 }
+
+// A resumed Multijob child carries the MatrixBuild it is resuming from.
+// Avoid importing the action class because this script is evaluated by a
+// plugin whose class loader does not declare a Multijob dependency.
+def resumeControlClassName =
+  'com.tikal.jenkins.plugins.multijob.MultiJobResumeControl'
+def currentBuild = execution.build
+
+try {
+  def resumeControl = currentBuild.actions.find {
+    it.class.name == resumeControlClassName
+  }
+  def resumeSource = resumeControl?.run
+  def previousBuild = resumeSource
+  def latestResults = [:]
+  def seenBuilds = [] as Set
+
+  // Follow the whole resume lineage. getRuns() is deliberately not used: it
+  // may inherit a run from an unrelated matrix build that happened in between.
+  while (previousBuild != null) {
+    if (previousBuild.class.name != 'hudson.matrix.MatrixBuild' ||
+        previousBuild.parent != currentBuild.parent) {
+      throw new IllegalStateException(
+        "Unexpected resumed build ${previousBuild.externalizableId}")
+    }
+
+    if (!seenBuilds.add(previousBuild.externalizableId)) {
+      throw new IllegalStateException(
+        "Cycle in resume lineage at ${previousBuild.externalizableId}")
+    }
+
+    // The first exact run found for a combination is its newest result.
+    previousBuild.exactRuns.each { run ->
+      def combination = run.parent.combination
+      if (!latestResults.containsKey(combination))
+        latestResults[combination] = run.result
+    }
+
+    resumeControl = previousBuild.actions.find {
+      it.class.name == resumeControlClassName
+    }
+    previousBuild = resumeControl?.run
+  }
+
+  def successfulResumeCombinations = [] as Set
+  latestResults.each { combination, previousResult ->
+    if (previousResult?.toString() == 'SUCCESS')
+      successfulResumeCombinations.add(combination)
+  }
+
+  // Apply resume selection only after the normal version and MACHINES
+  // selection. If it would select no work, rerun the eligible set so that a
+  // non-cell matrix failure cannot turn into a no-op SUCCESS.
+  def eligibleCombinations = result['nodes']
+  def combinationsToRun = eligibleCombinations.findAll {
+    !successfulResumeCombinations.contains(it)
+  }
+  int skippedCount =
+    eligibleCombinations.size() - combinationsToRun.size()
+
+  if (skippedCount > 0) {
+    if (combinationsToRun.isEmpty()) {
+      println 'Resume would skip every eligible combination; running all'
+    } else {
+      println "Resume: reusing ${skippedCount} successful combination(s)"
+      // getBaseBuild() otherwise defaults to getPreviousBuild(), which may be
+      // an unrelated interleaved build. The base chain also preserves cells
+      // skipped by earlier resumes.
+      currentBuild.setBaseBuild(resumeSource)
+      result['nodes'] = combinationsToRun
+    }
+  }
+} catch (Exception e) {
+  // Leave the already-selected eligible combinations unchanged on any error.
+  println "Resume metadata could not be read; running all eligible " +
+    "combinations (${e.class.name}: ${e.message})"
+}

As I understand it:

• the existing version and MACHINES selection still runs first
• only exact SUCCESS results are reused
• unstable, failed, aborted, not-built, and missing configurations run again
• repeated resumes follow the explicit resume chain
• if the previous builds cannot be understood, the existing full selection is left unchanged

The setBaseBuild() call is apparently needed so Jenkins inherits results from the build actually being resumed, rather than whichever unrelated matrix build happened to run immediately before this one.

The live matrix job already loads this script from nodejs/build's main branch, so this part would only require a change in this repository.

Keep the resolved REBASE_ONTO on resume

I asked the LLM to work this part through as well. My understanding is that node-test-pull-request only passes a symbolic ref such as origin/main. node-test-commit is where that ref is first resolved to an exact commit, and it is also the last common parent before the platform jobs start. That seems like the right place to retain it.

The suggested behavior is:

• a fresh request resolves the requested ref as it does today
• node-test-commit records the pre-rebase source commit and exact resolved base as parent-only RESUMABLE_* variables
• a resume uses those values when the source commit still matches
• a pre-change build, or one whose saved inputs do not match, gets a full run
• a saved base which is no longer available fails instead of silently switching to the current target

The shell change would look roughly like this, shown without the surrounding XML escaping:

-rm -rf env.properties
+rm -f env.properties resumable.properties
 touch env.properties

@@
 git status
-git rev-parse HEAD
+SOURCE_SHA=$(git rev-parse HEAD)
+echo "${SOURCE_SHA}"

@@
-git rev-parse $REBASE_ONTO
-
-if [ -n "${REBASE_ONTO}" ]; then
-  REBASE_ONTO=`git rev-parse ${REBASE_ONTO}`
-  git rebase --committer-date-is-author-date $REBASE_ONTO
+if [ "${RESUMED_BUILD:-}" = "true" ]; then
+  if printf '%s\n' "${RESUMABLE_SOURCE_SHA:-}" |
+       grep -qE '^[0-9a-fA-F]{40}$' &&
+     printf '%s\n' "${RESUMABLE_REBASE_ONTO:-}" |
+       grep -qE '^(NONE|[0-9a-fA-F]{40})$' &&
+     [ "${RESUMABLE_SOURCE_SHA:-}" = "${SOURCE_SHA}" ]; then
+    if [ "${RESUMABLE_REBASE_ONTO}" = "NONE" ]; then
+      REBASE_ONTO=
+    else
+      REBASE_ONTO="${RESUMABLE_REBASE_ONTO}"
+    fi
+    echo "Using REBASE_ONTO from the build being resumed"
+  else
+    echo "Saved resume inputs are missing or do not match this source; resolving the requested base"
+  fi
+fi
+
+if [ -n "${REBASE_ONTO}" ]; then
+  REBASE_ONTO=$(git rev-parse --verify "${REBASE_ONTO}^{commit}")
+  git rebase --committer-date-is-author-date "${REBASE_ONTO}"
 fi

 POST_REBASE_SHA1_CHECK=`git rev-parse HEAD`

 # Temp workaround to git rebase issues:
 POST_REBASE_SHA1_CHECK=

-echo REBASE_ONTO=$REBASE_ONTO >> env.properties
+printf 'REBASE_ONTO=%s\n' "${REBASE_ONTO}" >> env.properties
 echo POST_REBASE_SHA1_CHECK=$POST_REBASE_SHA1_CHECK >> env.properties
 echo CONFIG_FLAGS="$CONFIG_FLAGS" >> env.properties
 echo NODE_DEBUG="$NODE_DEBUG" >> env.properties
 echo NODE_DEBUG_NATIVE="$NODE_DEBUG_NATIVE" >> env.properties
+
+if [ -n "${REBASE_ONTO}" ]; then
+  resumable_rebase_onto="${REBASE_ONTO}"
+else
+  resumable_rebase_onto=NONE
+fi
+
+printf 'RESUMABLE_SOURCE_SHA=%s\n' "${SOURCE_SHA}" > resumable.properties
+printf 'RESUMABLE_REBASE_ONTO=%s\n' "${resumable_rebase_onto}" >> resumable.properties

NONE is used so that “this build intentionally did not rebase” is distinguishable from missing resume metadata.

The RESUMABLE_* values could then be injected into the node-test-commit parent separately:

     </hudson.tasks.Shell>
+    <EnvInjectBuilder plugin="envinject@2.941.v351a_20c0a_3ca_">
+      <info>
+        <propertiesFilePath>resumable.properties</propertiesFilePath>
+      </info>
+    </EnvInjectBuilder>
     <hudson.tasks.Shell>
       <command>#!/bin/bash -x -e

Keeping them out of env.properties means they remain bookkeeping for the parent rather than being passed to every platform child.

Compare the inputs before reusing children

The existing Multijob environment comparison could then be enabled on node-test-commit:

diff --git a/jobs/node-test-commit/config.xml b/jobs/node-test-commit/config.xml
--- a/jobs/node-test-commit/config.xml
+++ b/jobs/node-test-commit/config.xml
@@
   </buildWrappers>
   <pollSubjobs>false</pollSubjobs>
+  <resumeEnvVars>GITHUB_ORG,REPO_NAME,GIT_REMOTE_REF,COMMIT_SHA_CHECK,REBASE_ONTO,RESUMABLE_SOURCE_SHA,RESUMABLE_REBASE_ONTO,CONFIG_FLAGS,NODE_DEBUG,NODE_DEBUG_NATIVE,IGNORE_FLAKY_TESTS</resumeEnvVars>
 </com.tikal.jenkins.plugins.multijob.MultiJobProject>

There are deliberately no spaces after the commas because the plugin does not trim the names. I left out POST_REBASE_SHA1_CHECK because the current job clears it before scheduling the child jobs.

My interpretation is that this gives us:

• fresh request-ci: resolve the current target and record its exact commit
• resume: retain the original exact target and reuse successful work
• changed source or other listed input: reuse nothing
• first resume of a build created before this change: reuse nothing
• repeated resumes: continue carrying the same source and base commits

Multijob result handling

There appears to be one inconsistency in the installed Multijob plugin. Its resume selection retries every child whose result is not SUCCESS, but its RESUMABLE_* handling is enabled only when it finds a child whose result is exactly FAILURE.

The minimum change to make those agree appears to be:

diff --git a/src/main/java/com/tikal/jenkins/plugins/multijob/MultiJobBuilder.java b/src/main/java/com/tikal/jenkins/plugins/multijob/MultiJobBuilder.java
--- a/src/main/java/com/tikal/jenkins/plugins/multijob/MultiJobBuilder.java
+++ b/src/main/java/com/tikal/jenkins/plugins/multijob/MultiJobBuilder.java
@@
-                        if (Result.FAILURE.equals(childBuild.getResult())) {
+                        if (!Result.SUCCESS.equals(childBuild.getResult())) {
                             resume = true;
                         }

That is in the plugin version currently installed.

It may be cleaner for the presence of MultiJobResumeControl itself to enable the persistent variables rather than inspecting child results at all. I do not know enough about the plugin to say which version would be preferable.

Does this match how Resume is intended to work? In particular:

• Should a resume retain the original resolved REBASE_ONTO?
• Is a separate parent-only properties injection the right way to retain it?
• Are these the right variables for resumeEnvVars?
• Should the plugin use any non-successful child, or the resume action itself, when deciding whether to restore RESUMABLE_* variables?