From a1e00ac51c8cfffd7fcc84a61f0d31898c1ae4f6 Mon Sep 17 00:00:00 2001
From: Florent Poinsard <florent.poinsard@outlook.fr>
Date: Mon, 5 Feb 2024 16:30:26 -0600
Subject: [PATCH] Revert "Do not fetch and build vitess if it is the same
 commit as the previous execution"

This reverts commit d5473f2121fc875239c153a01a2aa5c060c1fc0e.
---
 ansible/roles/vitess_build/tasks/main.yml |  2 --
 go/exec/exec.go                           | 16 ++++++++--------
 go/infra/ansible/vars.go                  |  3 ---
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/ansible/roles/vitess_build/tasks/main.yml b/ansible/roles/vitess_build/tasks/main.yml
index 16ae07226..8e5bf5b9b 100644
--- a/ansible/roles/vitess_build/tasks/main.yml
+++ b/ansible/roles/vitess_build/tasks/main.yml
@@ -13,7 +13,6 @@
 - name: Update Vitess
   become: yes
   become_user: root
-  when: commit_previous_exec != vitess_git_version
   block:
     - name: Fetch Updated Vitess
       include_tasks: install_vitess.yml
@@ -53,7 +52,6 @@
       changed_when: false
 
 - name: Disbale AppArmor
-  when: not clean_previous_exec | bool
   block:
     - name: link apparmor
       file:
diff --git a/go/exec/exec.go b/go/exec/exec.go
index 0d1794a52..497950937 100644
--- a/go/exec/exec.go
+++ b/go/exec/exec.go
@@ -142,11 +142,10 @@ type Exec struct {
 
 	vitessSchemaPath string
 
-	// We always fetch the previous execution to let Ansible skip certain steps of the benchmark
-	// execution. Like cleaning up at the start of the execution, this is not required when the
-	// previous execution was clean and did not fail. Moreover, we can skip the fetch and build
-	// of Vitess if the previous execution share the same commit as the current execution.
-	previousExecution *Exec
+	// cleanPreviousExecution is set to true when the previous execution in the database is marked
+	// as "finished", meaning it has executed normally. In this case, Ansible doesn't need to clean up
+	// the execution server at the start of the execution to save time.
+	cleanPreviousExecution bool
 }
 
 const (
@@ -266,7 +265,9 @@ func (e *Exec) Prepare() error {
 	if err != nil {
 		return err
 	}
-	e.previousExecution = previousExec
+	if previousExec.Status == StatusFinished {
+		e.cleanPreviousExecution = true
+	}
 
 	// insert new exec in SQL
 	if _, err = e.clientDB.Insert(
@@ -394,8 +395,7 @@ func (e *Exec) prepareAnsibleForExecution() error {
 	e.AnsibleConfig.AddExtraVar(ansible.KeyGoVersion, e.GolangVersion)
 
 	// previous execution
-	e.AnsibleConfig.AddExtraVar(ansible.KeyCleanPreviousExec, e.previousExecution.Status == StatusFinished)
-	e.AnsibleConfig.AddExtraVar(ansible.KeyCommitPreviousExec, e.previousExecution.GitRef)
+	e.AnsibleConfig.AddExtraVar(ansible.KeyCleanPreviousExec, e.cleanPreviousExecution)
 
 	// stats database related values
 	e.statsRemoteDBConfig.AddToAnsible(&e.AnsibleConfig)
diff --git a/go/infra/ansible/vars.go b/go/infra/ansible/vars.go
index 3b4d91b92..79cb714b4 100644
--- a/go/infra/ansible/vars.go
+++ b/go/infra/ansible/vars.go
@@ -95,9 +95,6 @@ const (
 	// this is useful when doing optimization to skip certain steps of the setup
 	KeyCleanPreviousExec = "clean_previous_exec"
 
-	// KeyCommitPreviousExec is the commit SHA of the previous execution.
-	KeyCommitPreviousExec = "commit_previous_exec"
-
 	// Stats database related keys
 
 	// KeyStatsDBHost corresponding value in the map is the hostname for the stats