diff --git a/internal/manager/api_impl/worker_mgt.go b/internal/manager/api_impl/worker_mgt.go index 64edf2a2..b58db7eb 100644 --- a/internal/manager/api_impl/worker_mgt.go +++ b/internal/manager/api_impl/worker_mgt.go @@ -91,11 +91,9 @@ func (f *Flamenco) RequestWorkerStatusChange(e echo.Context, workerUUID string) 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 + dbWorker.StatusChangeClear() } else { - dbWorker.StatusRequested = change.Status - dbWorker.LazyStatusRequest = change.IsLazy + dbWorker.StatusChangeRequest(change.Status, change.IsLazy) } // Store the status change. diff --git a/internal/manager/api_impl/worker_mgt_test.go b/internal/manager/api_impl/worker_mgt_test.go index cb259711..aab4e2d2 100644 --- a/internal/manager/api_impl/worker_mgt_test.go +++ b/internal/manager/api_impl/worker_mgt_test.go @@ -25,7 +25,7 @@ func TestFetchWorkers(t *testing.T) { worker2.ID = 4 worker2.UUID = "f07b6d53-16ec-40a8-a7b4-a9cc8547f790" worker2.Status = api.WorkerStatusAwake - worker2.StatusRequested = api.WorkerStatusAsleep + worker2.StatusChangeRequest(api.WorkerStatusAsleep, false) mf.persistence.EXPECT().FetchWorkers(gomock.Any()). Return([]*persistence.Worker{&worker1, &worker2}, nil) @@ -104,7 +104,7 @@ func TestFetchWorker(t *testing.T) { // Test with worker that does have a status change requested. requestedStatus := api.WorkerStatusAsleep - worker.StatusRequested = requestedStatus + worker.StatusChangeRequest(requestedStatus, false) mf.persistence.EXPECT().FetchWorker(gomock.Any(), workerUUID).Return(&worker, nil) echo = mf.prepareMockedRequest(nil) @@ -137,8 +137,7 @@ func TestRequestWorkerStatusChange(t *testing.T) { requestStatus := api.WorkerStatusAsleep savedWorker := worker - savedWorker.StatusRequested = requestStatus - savedWorker.LazyStatusRequest = true + savedWorker.StatusChangeRequest(requestStatus, true) mf.persistence.EXPECT().SaveWorker(gomock.Any(), &savedWorker).Return(nil) // Expect a broadcast of the change @@ -171,8 +170,7 @@ func TestRequestWorkerStatusChangeRevert(t *testing.T) { worker := testWorker() // Mimick that a status change request to 'asleep' was already performed. - worker.StatusRequested = api.WorkerStatusAsleep - worker.LazyStatusRequest = true + worker.StatusChangeRequest(api.WorkerStatusAsleep, true) workerUUID := worker.UUID currentStatus := worker.Status @@ -183,8 +181,7 @@ func TestRequestWorkerStatusChangeRevert(t *testing.T) { // the previous status change request. requestStatus := currentStatus savedWorker := worker - savedWorker.StatusRequested = "" - savedWorker.LazyStatusRequest = false + savedWorker.StatusChangeClear() mf.persistence.EXPECT().SaveWorker(gomock.Any(), &savedWorker).Return(nil) // Expect a broadcast of the change diff --git a/internal/manager/api_impl/workers.go b/internal/manager/api_impl/workers.go index f648063d..539985ec 100644 --- a/internal/manager/api_impl/workers.go +++ b/internal/manager/api_impl/workers.go @@ -150,7 +150,7 @@ func (f *Flamenco) SignOff(e echo.Context) error { prevStatus := w.Status w.Status = api.WorkerStatusOffline if w.StatusRequested == api.WorkerStatusOffline { - w.StatusRequested = "" + w.StatusChangeClear() } // Pass a generic background context, as these changes should be stored even @@ -258,7 +258,7 @@ func (f *Flamenco) WorkerStateChanged(e echo.Context) error { logger.Info().Msg("worker changed status") // Either there was no status change request (and this is a no-op) or the // status change was actually acknowledging the request. - w.StatusRequested = "" + w.StatusChangeClear() } err = f.persist.SaveWorkerStatus(e.Request().Context(), w) diff --git a/internal/manager/api_impl/workers_test.go b/internal/manager/api_impl/workers_test.go index 54a95188..67470552 100644 --- a/internal/manager/api_impl/workers_test.go +++ b/internal/manager/api_impl/workers_test.go @@ -75,7 +75,7 @@ func TestTaskScheduleOtherStatusRequested(t *testing.T) { mf := newMockedFlamenco(mockCtrl) worker := testWorker() - worker.StatusRequested = api.WorkerStatusAsleep + worker.StatusChangeRequest(api.WorkerStatusAsleep, false) // Explicitly NO expected calls to the persistence layer. Since the worker is // not in a state that allows task execution, there should be no DB queries. @@ -355,7 +355,7 @@ func TestMayWorkerRun(t *testing.T) { // Test: unhappy, assigned and runnable but worker should go to bed. { - worker.StatusRequested = api.WorkerStatusAsleep + worker.StatusChangeRequest(api.WorkerStatusAsleep, false) echo := prepareRequest() task.WorkerID = &worker.ID task.Status = api.TaskStatusActive diff --git a/internal/manager/persistence/workers.go b/internal/manager/persistence/workers.go index 32d1cae2..340ef63c 100644 --- a/internal/manager/persistence/workers.go +++ b/internal/manager/persistence/workers.go @@ -34,6 +34,22 @@ func (w *Worker) TaskTypes() []string { return strings.Split(w.SupportedTaskTypes, ",") } +// StatusChangeRequest stores a requested status change on the Worker. +// This just updates the Worker instance, but doesn't store the change in the +// database. +func (w *Worker) StatusChangeRequest(status api.WorkerStatus, isLazyRequest bool) { + w.StatusRequested = status + w.LazyStatusRequest = isLazyRequest +} + +// StatusChangeClear clears the requested status change of the Worker. +// This just updates the Worker instance, but doesn't store the change in the +// database. +func (w *Worker) StatusChangeClear() { + w.StatusRequested = "" + w.LazyStatusRequest = false +} + func (db *DB) CreateWorker(ctx context.Context, w *Worker) error { if err := db.gormDB.WithContext(ctx).Create(w).Error; err != nil { return fmt.Errorf("creating new worker: %w", err)