Add forcePull option

* Add forcePull option
for pulling the image when the execution environment gets updated

* Apply suggestions from code review

Co-authored-by: Sebastian Serth <MrSerth@users.noreply.github.com>

* Add unit tests

* Clean up and implement option two

Co-authored-by: Sebastian Serth <MrSerth@users.noreply.github.com>

docs/resources/client.example.hcl

@@ -6,10 +6,14 @@ client {
     ]
 }
 
-# plugin "docker" {
+plugin "docker" {
-#   config {
+    config {
+        gc {
+            image_delay = "0s"
+        }
+
         # auth {
             # config = "/root/.docker/config.json"
         # }
-#   }
+    }
-# }
+}

internal/environment/environment.go

@@ -173,6 +173,7 @@ func (n *NomadEnvironment) SetNetworkAccess(allow bool, exposedPorts []uint16) {
 // Register creates a Nomad job based on the default job configuration and the given parameters.
 // It registers the job with Nomad and waits until the registration completes.
 func (n *NomadEnvironment) Register(apiClient nomad.ExecutorAPI) error {
+	nomad.SetForcePullFlag(n.job, true) // This must be the default as otherwise new runners could have different images.
 	evalID, err := apiClient.RegisterNomadJob(n.job)
 	if err != nil {
 		return fmt.Errorf("couldn't register job: %w", err)
@@ -202,7 +203,7 @@ func (n *NomadEnvironment) Scale(apiClient nomad.ExecutorAPI) error {
 	required := int(n.PrewarmingPoolSize()) - n.idleRunners.Length()
 
 	if required > 0 {
-		return n.createRunners(apiClient, uint(required))
+		return n.createRunners(apiClient, uint(required), true)
 	} else {
 		return n.removeRunners(apiClient, uint(-required))
 	}
@@ -221,6 +222,7 @@ func (n *NomadEnvironment) UpdateRunnerSpecs(apiClient nomad.ExecutorAPI) error
 		updatedRunnerJob := n.DeepCopyJob()
 		updatedRunnerJob.ID = &runnerID
 		updatedRunnerJob.Name = &runnerID
+		nomad.SetForcePullFlag(updatedRunnerJob, true)
 
 		err := apiClient.RegisterRunnerJob(updatedRunnerJob)
 		if err != nil {
@@ -237,7 +239,7 @@ func (n *NomadEnvironment) Sample(apiClient nomad.ExecutorAPI) (runner.Runner, b
 	r, ok := n.idleRunners.Sample()
 	if ok {
 		go func() {
-			err := n.createRunner(apiClient)
+			err := n.createRunner(apiClient, false)
 			if err != nil {
 				log.WithError(err).WithField("environmentID", n.ID()).Error("Couldn't create new runner for claimed one")
 			}
@@ -315,10 +317,10 @@ func parseJob(jobHCL string) (*nomadApi.Job, error) {
 	return job, nil
 }
 
-func (n *NomadEnvironment) createRunners(apiClient nomad.ExecutorAPI, count uint) error {
+func (n *NomadEnvironment) createRunners(apiClient nomad.ExecutorAPI, count uint, forcePull bool) error {
 	log.WithField("runnersRequired", count).WithField("id", n.ID()).Debug("Creating new runners")
 	for i := 0; i < int(count); i++ {
-		err := n.createRunner(apiClient)
+		err := n.createRunner(apiClient, forcePull)
 		if err != nil {
 			return fmt.Errorf("couldn't create new runner: %w", err)
 		}
@@ -326,7 +328,7 @@ func (n *NomadEnvironment) createRunners(apiClient nomad.ExecutorAPI, count uint
 	return nil
 }
 
-func (n *NomadEnvironment) createRunner(apiClient nomad.ExecutorAPI) error {
+func (n *NomadEnvironment) createRunner(apiClient nomad.ExecutorAPI, forcePull bool) error {
 	newUUID, err := uuid.NewUUID()
 	if err != nil {
 		return fmt.Errorf("failed generating runner id: %w", err)
@@ -336,6 +338,7 @@ func (n *NomadEnvironment) createRunner(apiClient nomad.ExecutorAPI) error {
 	template := n.DeepCopyJob()
 	template.ID = &newRunnerID
 	template.Name = &newRunnerID
+	nomad.SetForcePullFlag(template, forcePull)
 
 	err = apiClient.RegisterRunnerJob(template)
 	if err != nil {

internal/environment/environment_test.go

@@ -183,3 +183,28 @@ func TestTwoSampleAddExactlyTwoRunners(t *testing.T) {
 	<-time.After(tests.ShortTimeout) // New Runners are requested asynchronously
 	apiMock.AssertNumberOfCalls(t, "RegisterRunnerJob", 2)
 }
+
+func TestSampleDoesNotSetForcePullFlag(t *testing.T) {
+	apiMock := &nomad.ExecutorAPIMock{}
+	call := apiMock.On("RegisterRunnerJob", mock.AnythingOfType("*api.Job"))
+	call.Run(func(args mock.Arguments) {
+		job, ok := args.Get(0).(*nomadApi.Job)
+		assert.True(t, ok)
+
+		taskGroup := nomad.FindOrCreateDefaultTaskGroup(job)
+		task := nomad.FindOrCreateDefaultTask(taskGroup)
+		assert.False(t, task.Config["force_pull"].(bool))
+
+		call.ReturnArguments = mock.Arguments{nil}
+	})
+
+	_, job := helpers.CreateTemplateJob()
+	environment := &NomadEnvironment{templateEnvironmentJobHCL, job, runner.NewLocalRunnerStorage()}
+	runner1 := &runner.RunnerMock{}
+	runner1.On("ID").Return(tests.DefaultRunnerID)
+	environment.AddRunner(runner1)
+
+	_, ok := environment.Sample(apiMock)
+	require.True(t, ok)
+	<-time.After(tests.ShortTimeout) // New Runners are requested asynchronously
+}

internal/environment/manager_test.go

@@ -60,6 +60,33 @@ func (s *CreateOrUpdateTestSuite) TestReturnsErrorIfCreatesOrUpdateEnvironmentRe
 	s.ErrorIs(err, tests.ErrDefault)
 }
 
+func (s *CreateOrUpdateTestSuite) TestCreateOrUpdatesSetsForcePullFlag() {
+	s.apiMock.On("RegisterNomadJob", mock.AnythingOfType("*api.Job")).Return("", nil)
+	s.runnerManagerMock.On("GetEnvironment", mock.AnythingOfType("dto.EnvironmentID")).Return(nil, false)
+	s.runnerManagerMock.On("SetEnvironment", mock.AnythingOfType("*environment.NomadEnvironment")).Return(true)
+	s.apiMock.On("MonitorEvaluation", mock.AnythingOfType("string"), mock.Anything).Return(nil)
+	s.apiMock.On("LoadRunnerIDs", mock.AnythingOfType("string")).Return([]string{}, nil)
+	call := s.apiMock.On("RegisterRunnerJob", mock.AnythingOfType("*api.Job"))
+	count := 0
+	call.Run(func(args mock.Arguments) {
+		count++
+		job, ok := args.Get(0).(*nomadApi.Job)
+		s.True(ok)
+
+		// The environment job itself has not the force_pull flag
+		if count > 1 {
+			taskGroup := nomad.FindOrCreateDefaultTaskGroup(job)
+			task := nomad.FindOrCreateDefaultTask(taskGroup)
+			s.True(task.Config["force_pull"].(bool))
+		}
+
+		call.ReturnArguments = mock.Arguments{nil}
+	})
+	_, err := s.manager.CreateOrUpdate(dto.EnvironmentID(tests.DefaultEnvironmentIDAsInteger), s.request)
+	s.NoError(err)
+	s.True(count > 1)
+}
+
 func TestNewNomadEnvironmentManager(t *testing.T) {
 	executorAPIMock := &nomad.ExecutorAPIMock{}
 	executorAPIMock.On("LoadEnvironmentJobs").Return([]*nomadApi.Job{}, nil)

internal/nomad/job.go

@@ -130,6 +130,13 @@ func FindOrCreateDefaultTask(taskGroup *nomadApi.TaskGroup) *nomadApi.Task {
 	return task
 }
 
+// SetForcePullFlag sets the flag of a job if the image should be pulled again.
+func SetForcePullFlag(job *nomadApi.Job, value bool) {
+	taskGroup := FindOrCreateDefaultTaskGroup(job)
+	task := FindOrCreateDefaultTask(taskGroup)
+	task.Config["force_pull"] = value
+}
+
 // IsEnvironmentTemplateID checks if the passed job id belongs to a template job.
 func IsEnvironmentTemplateID(jobID string) bool {
 	parts := strings.Split(jobID, "-")

tests/constants.go

@@ -20,6 +20,7 @@ const (
 	AnotherEnvironmentIDAsString  = "42"
 	DefaultUUID                   = "MY-DEFAULT-RANDOM-UUID"
 	AnotherUUID                   = "another-uuid-43"
+	DefaultTemplateJobID          = "template-" + DefaultEnvironmentIDAsString
 	DefaultRunnerID               = DefaultEnvironmentIDAsString + "-" + DefaultUUID
 	AnotherRunnerID               = AnotherEnvironmentIDAsString + "-" + AnotherUUID
 	DefaultExecutionID            = "s0m3-3x3cu710n-1d"

tests/helpers/test_helpers.go

@@ -174,8 +174,8 @@ func HTTPPutJSON(url string, body interface{}) (response *http.Response, err err
 const templateJobPriority = 100
 
 func CreateTemplateJob() (base, job *nomadApi.Job) {
-	base = nomadApi.NewBatchJob(tests.DefaultRunnerID, tests.DefaultRunnerID, "global", templateJobPriority)
+	base = nomadApi.NewBatchJob(tests.DefaultTemplateJobID, tests.DefaultTemplateJobID, "global", templateJobPriority)
-	job = nomadApi.NewBatchJob(tests.DefaultRunnerID, tests.DefaultRunnerID, "global", templateJobPriority)
+	job = nomadApi.NewBatchJob(tests.DefaultTemplateJobID, tests.DefaultTemplateJobID, "global", templateJobPriority)
 	job.Datacenters = []string{"dc1"}
 	configTaskGroup := nomadApi.NewTaskGroup("config", 0)
 	configTaskGroup.Meta = make(map[string]string)