diff --git a/api/runners.go b/api/runners.go index 6c62bbd..e146cd1 100644 --- a/api/runners.go +++ b/api/runners.go @@ -1,9 +1,7 @@ package api import ( - "errors" "fmt" - "github.com/google/uuid" "github.com/gorilla/mux" "gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto" "gitlab.hpi.de/codeocean/codemoon/poseidon/config" @@ -11,20 +9,8 @@ import ( "gitlab.hpi.de/codeocean/codemoon/poseidon/environment/pool" "gitlab.hpi.de/codeocean/codemoon/poseidon/runner" "net/http" - "sync" ) -var ( - executions = make(map[string]map[string]dto.ExecutionRequest) - executionsLock = sync.Mutex{} -) - -func allocateExecutionMap(runner runner.Runner) { - executionsLock.Lock() - executions[runner.Id()] = make(map[string]dto.ExecutionRequest) - executionsLock.Unlock() -} - // provideRunner tries to respond with the id of a runner // This runner is then reserved for future use func provideRunner(writer http.ResponseWriter, request *http.Request) { @@ -42,7 +28,6 @@ func provideRunner(writer http.ResponseWriter, request *http.Request) { writeInternalServerError(writer, err, dto.ErrorNomadOverload) return } - allocateExecutionMap(runner) sendJson(writer, &dto.RunnerResponse{Id: runner.Id()}, http.StatusOK) } @@ -55,33 +40,23 @@ func executeCommand(router *mux.Router) func(w http.ResponseWriter, r *http.Requ return } - var scheme string - if config.Config.Server.TLS { - scheme = "wss" - } else { - scheme = "ws" - } - r, ok := runner.FromContext(request.Context()) - if !ok { - log.Fatal("Expected runner in context! Something must be broken ...") - } + var scheme string + if config.Config.Server.TLS { + scheme = "wss" + } else { + scheme = "ws" + } + r, ok := runner.FromContext(request.Context()) + if !ok { + log.Fatal("Expected runner in context! Something must be broken ...") + } - id, err := uuid.NewRandom() + id, err := r.AddExecution(*executionRequest) if err != nil { - log.Printf("Error creating new execution id: %v", err) writeInternalServerError(writer, err, dto.ErrorUnknown) return } - executionsLock.Lock() - runnerExecutions, ok := executions[r.Id()] - if !ok { - writeNotFound(writer, errors.New("runner has not been provided")) - return - } - runnerExecutions[id.String()] = *executionRequest - executionsLock.Unlock() - path, err := router.Get("runner-websocket").URL("runnerId", r.Id()) if err != nil { log.Printf("Error creating runner websocket URL %v", err) diff --git a/api/runners_test.go b/api/runners_test.go index f001bed..00ef112 100644 --- a/api/runners_test.go +++ b/api/runners_test.go @@ -75,7 +75,6 @@ func TestExecuteRoute(t *testing.T) { testRunner := runner.NewExerciseRunner("testRunner") runnerPool.AddRunner(testRunner) - allocateExecutionMap(testRunner) path, err := router.Get("runner-execute").URL("runnerId", testRunner.Id()) if err != nil { @@ -120,8 +119,10 @@ func TestExecuteRoute(t *testing.T) { t.Fatal(err) } executionId := url.Query().Get("executionId") + storedExecutionRequest, ok := testRunner.Execution(runner.ExecutionId(executionId)) - assert.Equal(t, executionRequest, executions[testRunner.Id()][executionId]) + assert.True(t, ok, "No execution request with this id") + assert.Equal(t, executionRequest, storedExecutionRequest) }) }) diff --git a/runner/runner.go b/runner/runner.go index a35b2ec..56b2831 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -3,12 +3,20 @@ package runner import ( "context" "encoding/json" + "github.com/google/uuid" + "gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto" "sync" ) +// Status is the type for the status of a Runner. type Status string + +// ContextKey is the type for keys in a request context. type ContextKey string +// ExecutionId is an id for a execution in a Runner. +type ExecutionId string + const ( StatusReady Status = "ready" StatusRunning Status = "running" @@ -20,32 +28,48 @@ const ( ) type Runner interface { + // SetStatus sets the status of the runner. SetStatus(Status) + + // Status gets the status of the runner. Status() Status + + // Id returns the id of the runner. Id() string + + // Execution looks up an ExecutionId for the runner and returns the associated RunnerRequest. + // If this request does not exits, ok is false, else true. + Execution(ExecutionId) (request dto.ExecutionRequest, ok bool) + + // AddExecution saves the supplied ExecutionRequest for the runner and returns an ExecutionId to retrieve it again. + AddExecution(dto.ExecutionRequest) (ExecutionId, error) + + // DeleteExecution deletes the execution of the runner with the specified id. + DeleteExecution(ExecutionId) } -// ExerciseRunner is an abstraction to communicate with Nomad allocations +// ExerciseRunner is an abstraction to communicate with Nomad allocations. type ExerciseRunner struct { sync.Mutex - id string - status Status - ch chan bool + id string + status Status + ch chan bool + executions map[ExecutionId]dto.ExecutionRequest } -// NewExerciseRunner creates a new exercise runner with the provided id -// As default value the status is ready +// NewExerciseRunner creates a new exercise runner with the provided id. func NewExerciseRunner(id string) *ExerciseRunner { return &ExerciseRunner{ - id: id, - status: StatusReady, - Mutex: sync.Mutex{}, - ch: make(chan bool), + id: id, + status: StatusReady, + Mutex: sync.Mutex{}, + ch: make(chan bool), + executions: make(map[ExecutionId]dto.ExecutionRequest), } } -// MarshalJSON implements json.Marshaler interface -// This exports also private attributes like the id +// MarshalJSON implements json.Marshaler interface. +// This exports private attributes like the id too. func (r *ExerciseRunner) MarshalJSON() ([]byte, error) { return json.Marshal(struct { Id string `json:"runnerId"` @@ -56,29 +80,53 @@ func (r *ExerciseRunner) MarshalJSON() ([]byte, error) { }) } -// SetStatus sets the status thread-safe func (r *ExerciseRunner) SetStatus(status Status) { r.Lock() defer r.Unlock() r.status = status } -// Status returns the status thread-safe func (r *ExerciseRunner) Status() Status { r.Lock() defer r.Unlock() return r.status } -// Id returns the id of the runner func (r *ExerciseRunner) Id() string { return r.id } +func (r *ExerciseRunner) Execution(id ExecutionId) (executionRequest dto.ExecutionRequest, ok bool) { + r.Lock() + defer r.Unlock() + executionRequest, ok = r.executions[id] + return +} + +func (r *ExerciseRunner) AddExecution(request dto.ExecutionRequest) (ExecutionId, error) { + r.Lock() + defer r.Unlock() + idUuid, err := uuid.NewRandom() + if err != nil { + return ExecutionId(""), err + } + id := ExecutionId(idUuid.String()) + r.executions[id] = request + return id, err +} + +func (r *ExerciseRunner) DeleteExecution(id ExecutionId) { + r.Lock() + defer r.Unlock() + delete(r.executions, id) +} + +// NewContext creates a context containing a runner. func NewContext(ctx context.Context, runner Runner) context.Context { return context.WithValue(ctx, runnerContextKey, runner) } +// FromContext returns a runner from a context. func FromContext(ctx context.Context) (Runner, bool) { runner, ok := ctx.Value(runnerContextKey).(Runner) return runner, ok diff --git a/runner/runner_test.go b/runner/runner_test.go index 4330b07..0af109f 100644 --- a/runner/runner_test.go +++ b/runner/runner_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "github.com/stretchr/testify/assert" + "gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto" "testing" ) @@ -32,6 +33,21 @@ func TestMarshalRunner(t *testing.T) { assert.Equal(t, "{\"runnerId\":\"42\",\"status\":\"ready\"}", string(marshal)) } +func TestExecutionRequestIsStored(t *testing.T) { + runner := NewExerciseRunner("42") + executionRequest := dto.ExecutionRequest{ + Command: "command", + TimeLimit: 10, + Environment: nil, + } + id, err := runner.AddExecution(executionRequest) + storedExecutionRunner, ok := runner.Execution(id) + + assert.NoError(t, err, "AddExecution should not produce an error") + assert.True(t, ok, "Getting an execution should not return ok false") + assert.Equal(t, executionRequest, storedExecutionRunner) +} + func TestNewContextReturnsNewContextWithRunner(t *testing.T) { runner := NewExerciseRunner("testRunner") ctx := context.Background()