From d6a49540fb8e84d9fc506f2d23abe37061b47543 Mon Sep 17 00:00:00 2001
From: Chris Pine <cpine@newrelic.com>
Date: Wed, 26 Aug 2020 14:41:48 -0700
Subject: [PATCH 1/2] Do not re-execute campaign spec steps when the diff is
 still valid #13172

---
 internal/campaigns/executor.go | 53 ++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/internal/campaigns/executor.go b/internal/campaigns/executor.go
index de39b76aee..f971726a26 100644
--- a/internal/campaigns/executor.go
+++ b/internal/campaigns/executor.go
@@ -23,7 +23,7 @@ type Executor interface {
 type Task struct {
 	Repository *graphql.Repository
 	Steps      []Step
-	Template   *ChangesetTemplate
+	Template   *ChangesetTemplate `json:"-"`
 }
 
 func (t *Task) cacheKey() ExecutionCacheKey {
@@ -144,15 +144,19 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) {
 		if result, err = x.cache.Get(ctx, cacheKey); err != nil {
 			err = errors.Wrapf(err, "checking cache for %q", task.Repository.Name)
 			return
-		} else if result != nil {
+		} else if result != nil && len(result.Commits) == 1 {
+			// Build the changeset spec.
+			diff := result.Commits[0].Diff
+			spec := createChangesetSpec(task, diff)
+
 			status.Cached = true
-			status.ChangesetSpec = result
+			status.ChangesetSpec = spec
 			status.FinishedAt = time.Now()
 			x.updateTaskStatus(task, status)
 
 			// Add the spec to the executor's list of completed specs.
 			x.specsMu.Lock()
-			x.specs = append(x.specs, result)
+			x.specs = append(x.specs, spec)
 			x.specsMu.Unlock()
 
 			return
@@ -188,24 +192,8 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) {
 	}
 
 	// Build the changeset spec.
-	spec := &ChangesetSpec{
-		BaseRepository: task.Repository.ID,
-		CreatedChangeset: &CreatedChangeset{
-			BaseRef:        task.Repository.BaseRef(),
-			BaseRev:        task.Repository.Rev(),
-			HeadRepository: task.Repository.ID,
-			HeadRef:        "refs/heads/" + task.Template.Branch,
-			Title:          task.Template.Title,
-			Body:           task.Template.Body,
-			Commits: []GitCommitDescription{
-				{
-					Message: task.Template.Commit.Message,
-					Diff:    string(diff),
-				},
-			},
-			Published: task.Template.Published,
-		},
-	}
+	spec := createChangesetSpec(task, string(diff))
+
 	status.ChangesetSpec = spec
 	x.updateTaskStatus(task, status)
 
@@ -245,3 +233,24 @@ func reachedTimeout(cmdCtx context.Context, err error) bool {
 
 	return errors.Is(err, context.DeadlineExceeded)
 }
+
+func createChangesetSpec(task *Task, diff string) *ChangesetSpec {
+	return &ChangesetSpec{
+		BaseRepository: task.Repository.ID,
+		CreatedChangeset: &CreatedChangeset{
+			BaseRef:        task.Repository.BaseRef(),
+			BaseRev:        task.Repository.Rev(),
+			HeadRepository: task.Repository.ID,
+			HeadRef:        "refs/heads/" + task.Template.Branch,
+			Title:          task.Template.Title,
+			Body:           task.Template.Body,
+			Commits: []GitCommitDescription{
+				{
+					Message: task.Template.Commit.Message,
+					Diff:    string(diff),
+				},
+			},
+			Published: task.Template.Published,
+		},
+	}
+}

From cbd63cb08904c588b2fefeca8259bd9f735e9699 Mon Sep 17 00:00:00 2001
From: Chris Pine <cpine@newrelic.com>
Date: Thu, 27 Aug 2020 13:52:41 -0700
Subject: [PATCH 2/2] improved comments

---
 internal/campaigns/executor.go | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/internal/campaigns/executor.go b/internal/campaigns/executor.go
index f971726a26..05450024ef 100644
--- a/internal/campaigns/executor.go
+++ b/internal/campaigns/executor.go
@@ -145,7 +145,10 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) {
 			err = errors.Wrapf(err, "checking cache for %q", task.Repository.Name)
 			return
 		} else if result != nil && len(result.Commits) == 1 {
-			// Build the changeset spec.
+			// Build a new changeset spec. We don't want to use `result` as is,
+			// because the changesetTemplate may have changed. In that case
+			// the diff would still be valid, so we take it from the cache,
+			// but we still build a new ChangesetSpec from the task.
 			diff := result.Commits[0].Diff
 			spec := createChangesetSpec(task, diff)