From e77bd9b84156cf0e3b43c73d51e1ec3a482942b1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Thu, 20 Oct 2022 12:30:37 +0200
Subject: [PATCH] Fix workers immediately switching state on a lazy request

Fix an issue where workers would switch immediately on a state change
request, even if it was of the "after task is finished" kind.

The "may I keep running" endpoint wasn't checking the lazyness flag, and
thus any state change, lazy or otherwise, would interrupt the worker's
current task.
---
 CHANGELOG.md                              |  1 +
 internal/manager/api_impl/workers.go      |  2 +-
 internal/manager/api_impl/workers_test.go | 18 +++++++++++++++++-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 41f9a1dc..54c91f8f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@ bugs in actually-released versions.
 ## 3.2 - in development
 
 - When rendering EXR files, use Blender's preview JPEG files to generate the preview video ([43bc22f10fae](https://developer.blender.org/rF43bc22f10fae0fcaed6a4a3b3ace1be617193e21)).
+- Fix issue where workers would switch immediately on a state change request, even if it was of the "after task is finished" kind.
 
 
 ## 3.1 - released 2022-10-18
diff --git a/internal/manager/api_impl/workers.go b/internal/manager/api_impl/workers.go
index 3bcf5062..0f599824 100644
--- a/internal/manager/api_impl/workers.go
+++ b/internal/manager/api_impl/workers.go
@@ -550,7 +550,7 @@ func (f *Flamenco) MayWorkerRun(e echo.Context, taskID string) error {
 
 // mayWorkerRun checks the worker and the task, to see if this worker may keep running this task.
 func mayWorkerRun(worker *persistence.Worker, dbTask *persistence.Task) api.MayKeepRunning {
-	if worker.StatusRequested != "" {
+	if worker.StatusRequested != "" && !worker.LazyStatusRequest {
 		return api.MayKeepRunning{
 			Reason:                "worker status change requested",
 			StatusChangeRequested: true,
diff --git a/internal/manager/api_impl/workers_test.go b/internal/manager/api_impl/workers_test.go
index e24d9832..d4d54db9 100644
--- a/internal/manager/api_impl/workers_test.go
+++ b/internal/manager/api_impl/workers_test.go
@@ -465,7 +465,7 @@ func TestMayWorkerRun(t *testing.T) {
 	// Expect the worker to be marked as 'seen' regardless of whether it may run
 	// its current task or not, so equal to the number of calls to
 	// `MayWorkerRun()` below.
-	mf.persistence.EXPECT().WorkerSeen(gomock.Any(), &worker).Times(4)
+	mf.persistence.EXPECT().WorkerSeen(gomock.Any(), &worker).Times(5)
 
 	// Test: unhappy, task unassigned
 	{
@@ -519,6 +519,22 @@ func TestMayWorkerRun(t *testing.T) {
 			StatusChangeRequested: true,
 		})
 	}
+
+	// Test: happy, assigned and runnable; worker should go to bed after task is finished.
+	{
+		// Expect a 'touch' of the task.
+		mf.persistence.EXPECT().TaskTouchedByWorker(gomock.Any(), &task).Return(nil)
+
+		worker.StatusChangeRequest(api.WorkerStatusAsleep, true)
+		echo := prepareRequest()
+		task.WorkerID = &worker.ID
+		task.Status = api.TaskStatusActive
+		err := mf.flamenco.MayWorkerRun(echo, task.UUID)
+		assert.NoError(t, err)
+		assertResponseJSON(t, echo, http.StatusOK, api.MayKeepRunning{
+			MayKeepRunning: true,
+		})
+	}
 }
 
 func TestTaskOutputProduced(t *testing.T) {