Clear step caches when -clear-cache is specified (#551)

Author: mrnugget

CHANGELOG.md

@@ -17,6 +17,8 @@ All notable changes to `src-cli` are documented in this file.
 
 ### Fixed
 
+- Cached step results produced by `src batch [apply|preview]` are now properly cleared when using the `-clear-cache` command line flag.
+
 ### Removed
 
 ## 3.28.2

internal/batches/executor/coordinator.go

@@ -139,6 +139,36 @@ func (c *Coordinator) checkCacheForTask(ctx context.Context, task *Task) (specs
 	return specs, true, nil
 }
 
+func (c *Coordinator) setCachedStepResults(ctx context.Context, task *Task) error {
+	// We start at the back so that we can find the _last_ cached step,
+	// then restart execution on the following step.
+	for i := len(task.Steps) - 1; i > -1; i-- {
+		key := StepsCacheKey{Task: task, StepIndex: i}
+
+		// If we need to clear the cache, we optimistically try this for every
+		// step.
+		if c.opts.ClearCache {
+			if err := c.cache.Clear(ctx, key); err != nil {
+				return errors.Wrapf(err, "clearing cache for step %d in %q", i, task.Repository.Name)
+			}
+		} else {
+			result, found, err := c.cache.GetStepResult(ctx, key)
+			if err != nil {
+				return errors.Wrapf(err, "checking for cached diff for step %d", i)
+			}
+
+			// Found a cached result, we're done
+			if found {
+				task.CachedResultFound = true
+				task.CachedResult = result
+				return nil
+			}
+		}
+	}
+
+	return nil
+}
+
 func (c *Coordinator) cacheAndBuildSpec(ctx context.Context, taskResult taskResult, status taskStatusHandler) (specs []*batches.ChangesetSpec, err error) {
 	defer func() {
 		// Set these two fields in any case
@@ -214,21 +244,8 @@ func (c *Coordinator) Execute(ctx context.Context, tasks []*Task, spec *batches.
 	// If we are here, that means we didn't find anything in the cache for the
 	// complete task. So, what if we have cached results for the steps?
 	for _, t := range tasks {
-		// We start at the back so that we can find the _last_ cached step,
-		// then restart execution on the following step.
-		for i := len(t.Steps) - 1; i > -1; i-- {
-			key := StepsCacheKey{Task: t, StepIndex: i}
-
-			result, found, err := c.cache.GetStepResult(ctx, key)
-			if err != nil {
-				return nil, nil, errors.Wrapf(err, "checking for cached diff for step %d", i)
-			}
-
-			if found {
-				t.CachedResultFound = true
-				t.CachedResult = result
-				break
-			}
+		if err := c.setCachedStepResults(ctx, t); err != nil {
+			return nil, nil, err
 		}
 	}
 

internal/batches/executor/coordinator_test.go

@@ -342,7 +342,9 @@ func TestCoordinator_Execute(t *testing.T) {
 }
 
 func TestCoordinator_Execute_StepCaching(t *testing.T) {
+	// Setup dependencies
 	cache := newInMemoryExecutionCache()
+	logManager := mock.LogNoOpManager{}
 
 	task := &Task{
 		Steps: []batches.Step{
@@ -370,46 +372,62 @@ func TestCoordinator_Execute_StepCaching(t *testing.T) {
 		},
 	}}
 
+	// Build Coordinator
+	coord := &Coordinator{cache: cache, exec: executor, logManager: logManager}
+
 	// First execution. Make sure that the Task executes all steps.
-	execAndEnsure(t, cache, executor, task, assertNoCachedResult(t))
+	execAndEnsure(t, coord, executor, task, assertNoCachedResult(t))
 	// We now expect the cache to have 1+N entries: 1 for the complete task, N
 	// for the steps.
 	wantCacheSize := len(task.Steps) + 1
 	assertCacheSize(t, cache, wantCacheSize)
 
+	// Reset task
+	task.CachedResultFound = false
+
 	// Change the 2nd step's definition:
 	task.Steps[1].Run = `echo "two modified"`
 	// Re-execution should start with the diff produced by steps[0] as the
 	// start state from which steps[1] is then re-executed.
-	execAndEnsure(t, cache, executor, task, assertCachedResultForStep(t, 0))
+	execAndEnsure(t, coord, executor, task, assertCachedResultForStep(t, 0))
 	// Cache now contains old entries, plus another "complete task" entry and
 	// two entries for newly executed steps.
 	wantCacheSize += 1 + 2
 	assertCacheSize(t, cache, wantCacheSize)
 
+	// Reset task
+	task.CachedResultFound = false
+
 	// Change the 3rd step's definition:
 	task.Steps[2].Run = `echo "three modified"`
 	// Re-execution should use the diff from steps[1] as start state
-	execAndEnsure(t, cache, executor, task, assertCachedResultForStep(t, 1))
+	execAndEnsure(t, coord, executor, task, assertCachedResultForStep(t, 1))
 	// Cache now contains old entries, plus another "complete task" entry and
 	// a single new step entry
 	wantCacheSize += 1 + 1
 	assertCacheSize(t, cache, wantCacheSize)
+
+	// Reset task
+	task.CachedResultFound = false
+
+	// Now we execute the spec with -clear-cache:
+	coord.opts.ClearCache = true
+	// We don't want any cached results set on the task:
+	execAndEnsure(t, coord, executor, task, assertNoCachedResult(t))
+	// Cache should have the same number of entries: the cached step results should
+	// have been cleared (the complete-task-result is cleared in another
+	// code path) and the same amount of cached entries has been added.
+	assertCacheSize(t, cache, wantCacheSize)
 }
 
 // execAndEnsure executes the given Task with the given cache and dummyExecutor
 // in a new Coordinator, setting cb as the startCallback on the executor.
-func execAndEnsure(t *testing.T, cache ExecutionCache, exec *dummyExecutor, task *Task, cb startCallback) {
+func execAndEnsure(t *testing.T, coord *Coordinator, exec *dummyExecutor, task *Task, cb startCallback) {
 	t.Helper()
 
-	// Setup dependencies
 	batchSpec := &batches.BatchSpec{ChangesetTemplate: testChangesetTemplate}
-	logManager := mock.LogNoOpManager{}
 	noopPrinter := func([]*TaskStatus) {}
 
-	// Build Coordinator
-	coord := &Coordinator{cache: cache, exec: exec, logManager: logManager}
-
 	// Set the ChangesetTemplate on Task
 	task.Template = batchSpec.ChangesetTemplate