Skip to content

Fix empty diff being returned when single *cached* step had to be executed

Warren Gifford requested to merge mrn+es/fix-off-by-one-step-caching into main

Created by: mrnugget

This PR fixes a bug in the step-wise caching logic that @eseliger and I ran into last week.

Problem

When no cached result for the complete task was found, but a cached result for a single step and that step was the only left to execute then an empty diff was returned.

How to reproduce

  1. Execute a batch spec with the following steps

    steps:
      - run: echo "this is step 1" >> README.txt
        container: alpine:3
      - run: echo "this is step 2" >> README.md
        container: alpine:3

    the complete results and the results for each step are now cached.

  2. Update the batch spec and re-execute:

    steps:
      - run: echo "this is step 1" >> README.txt
        container: alpine:3

This will produce an empty diff because the Coordinator did not find a cached result for the complete task/batch spec, but for the first step.

runSteps didn't handle this case though, when only a single step had to be executed but that was also cached.

The fix

Take a look at my comments in the diffs here in GitHub to see what the fix is. Spoiler: it's just a condition and an early exit to handle the case of "cached step result == the only step to execute".

In order to get to this fix though I wrote a lot of tests, found one other bug (Files was not cached), and cleaned up the 300 line long runSteps function.

runSteps is now much easier to understand and doesn't contain all levels of abstraction. The outer runSteps concerns itself with which steps need to be executed and how to handle their results and executeSingleStep does the lower-level work of rendering templates, starting containers, setting up the logger, etc.

Merge request reports

Loading