diff --git a/internal/manager/api_impl/workers.go b/internal/manager/api_impl/workers.go index 0cb5741b..02ef455c 100644 --- a/internal/manager/api_impl/workers.go +++ b/internal/manager/api_impl/workers.go @@ -272,15 +272,6 @@ func (f *Flamenco) ScheduleTask(e echo.Context) error { } // Check that this worker is actually allowed to do work. - requiredStatusToGetTask := api.WorkerStatusAwake - if worker.Status != requiredStatusToGetTask { - logger.Warn(). - Str("workerStatus", string(worker.Status)). - Str("requiredStatus", string(requiredStatusToGetTask)). - Msg("worker asking for task but is in wrong state") - return sendAPIError(e, http.StatusConflict, - fmt.Sprintf("worker is in state %q, requires state %q to execute tasks", worker.Status, requiredStatusToGetTask)) - } if worker.StatusRequested != "" { logger.Warn(). Str("workerStatus", string(worker.Status)). @@ -291,6 +282,16 @@ func (f *Flamenco) ScheduleTask(e echo.Context) error { }) } + requiredStatusToGetTask := api.WorkerStatusAwake + if worker.Status != requiredStatusToGetTask { + logger.Warn(). + Str("workerStatus", string(worker.Status)). + Str("requiredStatus", string(requiredStatusToGetTask)). + Msg("worker asking for task but is in wrong state") + return sendAPIError(e, http.StatusConflict, + fmt.Sprintf("worker is in state %q, requires state %q to execute tasks", worker.Status, requiredStatusToGetTask)) + } + // Get a task to execute: dbTask, err := f.persist.ScheduleTask(e.Request().Context(), worker) if err != nil { diff --git a/internal/manager/api_impl/workers_test.go b/internal/manager/api_impl/workers_test.go index 713a5645..8ac42b63 100644 --- a/internal/manager/api_impl/workers_test.go +++ b/internal/manager/api_impl/workers_test.go @@ -124,6 +124,32 @@ func TestTaskScheduleOtherStatusRequested(t *testing.T) { assertResponseJSON(t, echoCtx, http.StatusLocked, expectBody) } +func TestTaskScheduleOtherStatusRequestedAndBadState(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mf := newMockedFlamenco(mockCtrl) + worker := testWorker() + + // Even when the worker is in a state that doesn't allow execution, if there + // is a status change requested, this should be communicated to the worker. + worker.Status = api.WorkerStatusError + worker.StatusChangeRequest(api.WorkerStatusAwake, false) + + echoCtx := mf.prepareMockedRequest(nil) + requestWorkerStore(echoCtx, &worker) + + // The worker should be marked as 'seen', even when it's in a state that + // doesn't allow task execution. + mf.persistence.EXPECT().WorkerSeen(echoCtx.Request().Context(), &worker) + + err := mf.flamenco.ScheduleTask(echoCtx) + assert.NoError(t, err) + + expectBody := api.WorkerStateChange{StatusRequested: api.WorkerStatusAwake} + assertResponseJSON(t, echoCtx, http.StatusLocked, expectBody) +} + func TestWorkerSignOn(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish()