From 678308fb6d5a1d9c95ef9492ea2c37732af2c222 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Thu, 2 Jun 2022 12:43:16 +0200
Subject: [PATCH] Manager: allow cancelling worker state change requests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A worker state change request can now be cancelled by requesting the worker
to go to its current state. In other words, a previously requested change
`A → B` can be cancelled by requesting the worker goes to state `A`.

Previously this would simply overwrite the last request, resulting in a
requested state change `A → A`. Having this non-lazy would even interrupt
the currently running task.
---
 internal/manager/api_impl/worker_mgt.go      | 16 +++++--
 internal/manager/api_impl/worker_mgt_test.go | 46 ++++++++++++++++++++
 2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/internal/manager/api_impl/worker_mgt.go b/internal/manager/api_impl/worker_mgt.go
index 87612e4e..64edf2a2 100644
--- a/internal/manager/api_impl/worker_mgt.go
+++ b/internal/manager/api_impl/worker_mgt.go
@@ -81,16 +81,24 @@ func (f *Flamenco) RequestWorkerStatusChange(e echo.Context, workerUUID string)
 		return sendAPIError(e, http.StatusInternalServerError, "error fetching worker: %v", err)
 	}
 
-	// Store the status change.
 	logger = logger.With().
 		Str("status", string(dbWorker.Status)).
 		Str("requested", string(change.Status)).
 		Bool("lazy", change.IsLazy).
 		Logger()
-
 	logger.Info().Msg("worker status change requested")
-	dbWorker.StatusRequested = change.Status
-	dbWorker.LazyStatusRequest = change.IsLazy
+
+	if dbWorker.Status == change.Status {
+		// Requesting that the worker should go to its current status basically
+		// means cancelling any previous status change request.
+		dbWorker.StatusRequested = ""
+		dbWorker.LazyStatusRequest = false
+	} else {
+		dbWorker.StatusRequested = change.Status
+		dbWorker.LazyStatusRequest = change.IsLazy
+	}
+
+	// Store the status change.
 	if err := f.persist.SaveWorker(e.Request().Context(), dbWorker); err != nil {
 		logger.Error().Err(err).Msg("error saving worker after status change request")
 		return sendAPIError(e, http.StatusInternalServerError, "error saving worker: %v", err)
diff --git a/internal/manager/api_impl/worker_mgt_test.go b/internal/manager/api_impl/worker_mgt_test.go
index 19f383be..cb259711 100644
--- a/internal/manager/api_impl/worker_mgt_test.go
+++ b/internal/manager/api_impl/worker_mgt_test.go
@@ -162,3 +162,49 @@ func TestRequestWorkerStatusChange(t *testing.T) {
 	assert.NoError(t, err)
 	assertResponseEmpty(t, echo)
 }
+
+func TestRequestWorkerStatusChangeRevert(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	mf := newMockedFlamenco(mockCtrl)
+	worker := testWorker()
+
+	// Mimick that a status change request to 'asleep' was already performed.
+	worker.StatusRequested = api.WorkerStatusAsleep
+	worker.LazyStatusRequest = true
+
+	workerUUID := worker.UUID
+	currentStatus := worker.Status
+
+	mf.persistence.EXPECT().FetchWorker(gomock.Any(), workerUUID).Return(&worker, nil)
+
+	// Perform a request to go to the current worker status. This should cancel
+	// the previous status change request.
+	requestStatus := currentStatus
+	savedWorker := worker
+	savedWorker.StatusRequested = ""
+	savedWorker.LazyStatusRequest = false
+	mf.persistence.EXPECT().SaveWorker(gomock.Any(), &savedWorker).Return(nil)
+
+	// Expect a broadcast of the change
+	mf.broadcaster.EXPECT().BroadcastWorkerUpdate(api.SocketIOWorkerUpdate{
+		Id:           worker.UUID,
+		Nickname:     worker.Name,
+		Status:       currentStatus,
+		Updated:      worker.UpdatedAt,
+		Version:      worker.Software,
+		StatusChange: nil,
+	})
+
+	echo := mf.prepareMockedJSONRequest(api.WorkerStatusChangeRequest{
+		Status: requestStatus,
+
+		// This shouldn't matter; requesting the current status should simply erase
+		// the previous status change request.
+		IsLazy: true,
+	})
+	err := mf.flamenco.RequestWorkerStatusChange(echo, workerUUID)
+	assert.NoError(t, err)
+	assertResponseEmpty(t, echo)
+}