From 34d4bb7ea0afd98ed56781c01d28ff447b151b4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Pa=C3=9F?= <22845248+mpass99@users.noreply.github.com> Date: Thu, 21 Oct 2021 10:33:52 +0200 Subject: [PATCH] Implement routes to list, get and delete execution environments * #9 Implement routes to list, get and delete execution environments. A refactoring was required to introduce the ExecutionEnvironment interface. * Fix MR comments, linting issues and bug that lead to e2e test failure * Add e2e tests * Add unit tests --- api/swagger.yaml | 16 + internal/api/environments.go | 111 +++++- internal/api/environments_test.go | 171 ++++++++- internal/api/runners.go | 2 +- internal/api/runners_test.go | 6 +- internal/environment/environment.go | 352 ++++++++++++++++++ internal/environment/environment_test.go | 192 ++++++++++ internal/environment/manager.go | 240 +++++++----- internal/environment/manager_mock.go | 74 +++- internal/environment/manager_test.go | 243 +++++++----- internal/nomad/api_querier.go | 8 +- internal/nomad/api_querier_mock.go | 2 +- internal/nomad/executor_api_mock.go | 33 +- internal/nomad/job.go | 270 +++++--------- internal/nomad/job_test.go | 334 +++++------------ internal/nomad/nomad.go | 20 +- internal/nomad/nomad_test.go | 18 +- internal/runner/constants_test.go | 9 +- internal/runner/execution_environment_mock.go | 255 +++++++++++++ internal/runner/manager.go | 317 +++++----------- internal/runner/manager_mock.go | 85 +++-- internal/runner/manager_test.go | 183 +++++---- internal/runner/nomad_environment_storage.go | 39 +- .../runner/nomad_environment_storage_test.go | 43 ++- internal/runner/runner_test.go | 14 +- internal/runner/storage.go | 2 +- pkg/dto/dto.go | 22 ++ tests/constants.go | 6 +- tests/e2e/e2e_test.go | 2 +- tests/e2e/environments_test.go | 230 +++++++++++- tests/helpers/test_helpers.go | 5 +- 31 files changed, 2239 insertions(+), 1065 deletions(-) create mode 100644 internal/environment/environment.go create mode 100644 internal/environment/environment_test.go create mode 100644 internal/runner/execution_environment_mock.go diff --git a/api/swagger.yaml b/api/swagger.yaml index dee10b5..53227de 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -321,6 +321,14 @@ paths: description: List all execution environments the API is aware of. tags: - execution environment + parameters: + - name: fetch + in: query + description: Specify whether environments should be fetched again from the executor before returning. Otherwise, the data currently in cache is returned. + schema: + type: boolean + default: false + required: false responses: "200": description: Success. Returns all execution environments @@ -350,6 +358,14 @@ paths: description: Get a representation of the execution environment specified by the id. tags: - execution environment + parameters: + - name: fetch + in: query + description: Specify whether the environment should be fetched again from the executor before returning. Otherwise, the data currently in cache is returned. + schema: + type: boolean + default: false + required: false responses: "200": description: Success. Returns the execution environment diff --git a/internal/api/environments.go b/internal/api/environments.go index 33b5834..14bce65 100644 --- a/internal/api/environments.go +++ b/internal/api/environments.go @@ -9,11 +9,16 @@ import ( "github.com/openHPI/poseidon/internal/runner" "github.com/openHPI/poseidon/pkg/dto" "net/http" + "strconv" ) const ( executionEnvironmentIDKey = "executionEnvironmentId" + fetchEnvironmentKey = "fetch" + listRouteName = "list" + getRouteName = "get" createOrUpdateRouteName = "createOrUpdate" + deleteRouteName = "delete" ) var ErrMissingURLParameter = errors.New("url parameter missing") @@ -22,10 +27,82 @@ type EnvironmentController struct { manager environment.Manager } +type ExecutionEnvironmentsResponse struct { + ExecutionEnvironments []runner.ExecutionEnvironment `json:"executionEnvironments"` +} + func (e *EnvironmentController) ConfigureRoutes(router *mux.Router) { environmentRouter := router.PathPrefix(EnvironmentsPath).Subrouter() + environmentRouter.HandleFunc("", e.list).Methods(http.MethodGet).Name(listRouteName) + specificEnvironmentRouter := environmentRouter.Path(fmt.Sprintf("/{%s:[0-9]+}", executionEnvironmentIDKey)).Subrouter() + specificEnvironmentRouter.HandleFunc("", e.get).Methods(http.MethodGet).Name(getRouteName) specificEnvironmentRouter.HandleFunc("", e.createOrUpdate).Methods(http.MethodPut).Name(createOrUpdateRouteName) + specificEnvironmentRouter.HandleFunc("", e.delete).Methods(http.MethodDelete).Name(deleteRouteName) +} + +// list returns all information about available execution environments. +func (e *EnvironmentController) list(writer http.ResponseWriter, request *http.Request) { + fetch, err := parseFetchParameter(request) + if err != nil { + writeBadRequest(writer, err) + return + } + + environments, err := e.manager.List(fetch) + if err != nil { + writeInternalServerError(writer, err, dto.ErrorUnknown) + return + } + + sendJSON(writer, ExecutionEnvironmentsResponse{environments}, http.StatusOK) +} + +// get returns all information about the requested execution environment. +func (e *EnvironmentController) get(writer http.ResponseWriter, request *http.Request) { + environmentID, err := parseEnvironmentID(request) + if err != nil { + // This case is never used as the router validates the id format + writeBadRequest(writer, err) + return + } + fetch, err := parseFetchParameter(request) + if err != nil { + writeBadRequest(writer, err) + return + } + + executionEnvironment, err := e.manager.Get(environmentID, fetch) + if errors.Is(err, runner.ErrUnknownExecutionEnvironment) { + writer.WriteHeader(http.StatusNotFound) + return + } else if err != nil { + writeInternalServerError(writer, err, dto.ErrorUnknown) + return + } + + sendJSON(writer, executionEnvironment, http.StatusOK) +} + +// delete removes the specified execution environment. +func (e *EnvironmentController) delete(writer http.ResponseWriter, request *http.Request) { + environmentID, err := parseEnvironmentID(request) + if err != nil { + // This case is never used as the router validates the id format + writeBadRequest(writer, err) + return + } + + found, err := e.manager.Delete(environmentID) + if err != nil { + writeInternalServerError(writer, err, dto.ErrorUnknown) + return + } else if !found { + writer.WriteHeader(http.StatusNotFound) + return + } + + writer.WriteHeader(http.StatusNoContent) } // createOrUpdate creates/updates an execution environment on the executor. @@ -35,17 +112,12 @@ func (e *EnvironmentController) createOrUpdate(writer http.ResponseWriter, reque writeBadRequest(writer, err) return } - - id, ok := mux.Vars(request)[executionEnvironmentIDKey] - if !ok { - writeBadRequest(writer, fmt.Errorf("could not find %s: %w", executionEnvironmentIDKey, ErrMissingURLParameter)) - return - } - environmentID, err := runner.NewEnvironmentID(id) + environmentID, err := parseEnvironmentID(request) if err != nil { - writeBadRequest(writer, fmt.Errorf("could not update environment: %w", err)) + writeBadRequest(writer, err) return } + created, err := e.manager.CreateOrUpdate(environmentID, *req) if err != nil { writeInternalServerError(writer, err, dto.ErrorUnknown) @@ -57,3 +129,26 @@ func (e *EnvironmentController) createOrUpdate(writer http.ResponseWriter, reque writer.WriteHeader(http.StatusNoContent) } } + +func parseEnvironmentID(request *http.Request) (dto.EnvironmentID, error) { + id, ok := mux.Vars(request)[executionEnvironmentIDKey] + if !ok { + return 0, fmt.Errorf("could not find %s: %w", executionEnvironmentIDKey, ErrMissingURLParameter) + } + environmentID, err := dto.NewEnvironmentID(id) + if err != nil { + return 0, fmt.Errorf("could not update environment: %w", err) + } + return environmentID, nil +} + +func parseFetchParameter(request *http.Request) (fetch bool, err error) { + fetchString := request.FormValue(fetchEnvironmentKey) + if len(fetchString) > 0 { + fetch, err = strconv.ParseBool(fetchString) + if err != nil { + return false, fmt.Errorf("could not parse fetch parameter: %w", err) + } + } + return fetch, nil +} diff --git a/internal/api/environments_test.go b/internal/api/environments_test.go index ab94886..f25372d 100644 --- a/internal/api/environments_test.go +++ b/internal/api/environments_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "github.com/gorilla/mux" "github.com/openHPI/poseidon/internal/environment" + "github.com/openHPI/poseidon/internal/nomad" "github.com/openHPI/poseidon/internal/runner" "github.com/openHPI/poseidon/pkg/dto" "github.com/openHPI/poseidon/tests" @@ -33,10 +34,178 @@ func (s *EnvironmentControllerTestSuite) SetupTest() { s.router = NewRouter(nil, s.manager) } +func (s *EnvironmentControllerTestSuite) TestList() { + call := s.manager.On("List", mock.AnythingOfType("bool")) + call.Run(func(args mock.Arguments) { + call.ReturnArguments = mock.Arguments{[]runner.ExecutionEnvironment{}, nil} + }) + path, err := s.router.Get(listRouteName).URL() + s.Require().NoError(err) + request, err := http.NewRequest(http.MethodGet, path.String(), nil) + s.Require().NoError(err) + + s.Run("with no Environments", func() { + recorder := httptest.NewRecorder() + s.router.ServeHTTP(recorder, request) + s.Equal(http.StatusOK, recorder.Code) + + var environmentsResponse ExecutionEnvironmentsResponse + err = json.NewDecoder(recorder.Result().Body).Decode(&environmentsResponse) + s.Require().NoError(err) + _ = recorder.Result().Body.Close() + + s.Empty(environmentsResponse.ExecutionEnvironments) + }) + s.manager.Calls = []mock.Call{} + + s.Run("with fetch", func() { + recorder := httptest.NewRecorder() + query := path.Query() + query.Set("fetch", "true") + path.RawQuery = query.Encode() + request, err := http.NewRequest(http.MethodGet, path.String(), nil) + s.Require().NoError(err) + + s.router.ServeHTTP(recorder, request) + s.Equal(http.StatusOK, recorder.Code) + s.manager.AssertCalled(s.T(), "List", true) + }) + s.manager.Calls = []mock.Call{} + + s.Run("with bad fetch", func() { + recorder := httptest.NewRecorder() + query := path.Query() + query.Set("fetch", "YouDecide") + path.RawQuery = query.Encode() + request, err := http.NewRequest(http.MethodGet, path.String(), nil) + s.Require().NoError(err) + + s.router.ServeHTTP(recorder, request) + s.Equal(http.StatusBadRequest, recorder.Code) + s.manager.AssertNotCalled(s.T(), "List") + }) + + s.Run("returns multiple environments", func() { + call.Run(func(args mock.Arguments) { + firstEnvironment, err := environment.NewNomadEnvironment( + "job \"" + nomad.TemplateJobID(tests.DefaultEnvironmentIDAsInteger) + "\" {}") + s.Require().NoError(err) + secondEnvironment, err := environment.NewNomadEnvironment( + "job \"" + nomad.TemplateJobID(tests.AnotherEnvironmentIDAsInteger) + "\" {}") + s.Require().NoError(err) + call.ReturnArguments = mock.Arguments{[]runner.ExecutionEnvironment{firstEnvironment, secondEnvironment}, nil} + }) + recorder := httptest.NewRecorder() + s.router.ServeHTTP(recorder, request) + s.Equal(http.StatusOK, recorder.Code) + + paramMap := make(map[string]interface{}) + err := json.NewDecoder(recorder.Result().Body).Decode(¶mMap) + s.Require().NoError(err) + environmentsInterface, ok := paramMap["executionEnvironments"] + s.Require().True(ok) + environments, ok := environmentsInterface.([]interface{}) + s.Require().True(ok) + s.Equal(2, len(environments)) + }) +} + +func (s *EnvironmentControllerTestSuite) TestGet() { + call := s.manager.On("Get", mock.AnythingOfType("dto.EnvironmentID"), mock.AnythingOfType("bool")) + path, err := s.router.Get(getRouteName).URL(executionEnvironmentIDKey, tests.DefaultEnvironmentIDAsString) + s.Require().NoError(err) + request, err := http.NewRequest(http.MethodGet, path.String(), nil) + s.Require().NoError(err) + + s.Run("with unknown environment", func() { + call.Run(func(args mock.Arguments) { + call.ReturnArguments = mock.Arguments{nil, runner.ErrUnknownExecutionEnvironment} + }) + + recorder := httptest.NewRecorder() + s.router.ServeHTTP(recorder, request) + s.Equal(http.StatusNotFound, recorder.Code) + s.manager.AssertCalled(s.T(), "Get", dto.EnvironmentID(0), false) + }) + s.manager.Calls = []mock.Call{} + + s.Run("not found with fetch", func() { + recorder := httptest.NewRecorder() + query := path.Query() + query.Set("fetch", "true") + path.RawQuery = query.Encode() + request, err := http.NewRequest(http.MethodGet, path.String(), nil) + s.Require().NoError(err) + + call.Run(func(args mock.Arguments) { + call.ReturnArguments = mock.Arguments{nil, runner.ErrUnknownExecutionEnvironment} + }) + + s.router.ServeHTTP(recorder, request) + s.Equal(http.StatusNotFound, recorder.Code) + s.manager.AssertCalled(s.T(), "Get", dto.EnvironmentID(0), true) + }) + s.manager.Calls = []mock.Call{} + + s.Run("returns environment", func() { + call.Run(func(args mock.Arguments) { + testEnvironment, err := environment.NewNomadEnvironment( + "job \"" + nomad.TemplateJobID(tests.DefaultEnvironmentIDAsInteger) + "\" {}") + s.Require().NoError(err) + call.ReturnArguments = mock.Arguments{testEnvironment, nil} + }) + + recorder := httptest.NewRecorder() + s.router.ServeHTTP(recorder, request) + s.Equal(http.StatusOK, recorder.Code) + + var environmentParams map[string]interface{} + err := json.NewDecoder(recorder.Result().Body).Decode(&environmentParams) + s.Require().NoError(err) + idInterface, ok := environmentParams["id"] + s.Require().True(ok) + idFloat, ok := idInterface.(float64) + s.Require().True(ok) + s.Equal(tests.DefaultEnvironmentIDAsInteger, int(idFloat)) + }) +} + +func (s *EnvironmentControllerTestSuite) TestDelete() { + call := s.manager.On("Delete", mock.AnythingOfType("dto.EnvironmentID")) + path, err := s.router.Get(deleteRouteName).URL(executionEnvironmentIDKey, tests.DefaultEnvironmentIDAsString) + s.Require().NoError(err) + request, err := http.NewRequest(http.MethodDelete, path.String(), nil) + s.Require().NoError(err) + + s.Run("environment not found", func() { + call.Run(func(args mock.Arguments) { + call.ReturnArguments = mock.Arguments{false, nil} + }) + recorder := httptest.NewRecorder() + s.router.ServeHTTP(recorder, request) + s.Equal(http.StatusNotFound, recorder.Code) + }) + + s.Run("environment deleted", func() { + call.Run(func(args mock.Arguments) { + call.ReturnArguments = mock.Arguments{true, nil} + }) + recorder := httptest.NewRecorder() + s.router.ServeHTTP(recorder, request) + s.Equal(http.StatusNoContent, recorder.Code) + }) + + s.manager.Calls = []mock.Call{} + s.Run("with bad environment id", func() { + _, err := s.router.Get(deleteRouteName).URL(executionEnvironmentIDKey, "MagicNonNumberID") + s.Error(err) + }) +} + type CreateOrUpdateEnvironmentTestSuite struct { EnvironmentControllerTestSuite path string - id runner.EnvironmentID + id dto.EnvironmentID body []byte } diff --git a/internal/api/runners.go b/internal/api/runners.go index 38fe93a..9ed4a6e 100644 --- a/internal/api/runners.go +++ b/internal/api/runners.go @@ -48,7 +48,7 @@ func (r *RunnerController) provide(writer http.ResponseWriter, request *http.Req if err := parseJSONRequestBody(writer, request, runnerRequest); err != nil { return } - environmentID := runner.EnvironmentID(runnerRequest.ExecutionEnvironmentID) + environmentID := dto.EnvironmentID(runnerRequest.ExecutionEnvironmentID) nextRunner, err := r.manager.Claim(environmentID, runnerRequest.InactivityTimeout) if err != nil { switch err { diff --git a/internal/api/runners_test.go b/internal/api/runners_test.go index d251e34..af6400d 100644 --- a/internal/api/runners_test.go +++ b/internal/api/runners_test.go @@ -122,7 +122,7 @@ func (s *ProvideRunnerTestSuite) SetupTest() { } func (s *ProvideRunnerTestSuite) TestValidRequestReturnsRunner() { - s.runnerManager.On("Claim", mock.AnythingOfType("runner.EnvironmentID"), + s.runnerManager.On("Claim", mock.AnythingOfType("dto.EnvironmentID"), mock.AnythingOfType("int")).Return(s.runner, nil) recorder := httptest.NewRecorder() @@ -149,7 +149,7 @@ func (s *ProvideRunnerTestSuite) TestInvalidRequestReturnsBadRequest() { func (s *ProvideRunnerTestSuite) TestWhenExecutionEnvironmentDoesNotExistReturnsNotFound() { s.runnerManager. - On("Claim", mock.AnythingOfType("runner.EnvironmentID"), mock.AnythingOfType("int")). + On("Claim", mock.AnythingOfType("dto.EnvironmentID"), mock.AnythingOfType("int")). Return(nil, runner.ErrUnknownExecutionEnvironment) recorder := httptest.NewRecorder() @@ -158,7 +158,7 @@ func (s *ProvideRunnerTestSuite) TestWhenExecutionEnvironmentDoesNotExistReturns } func (s *ProvideRunnerTestSuite) TestWhenNoRunnerAvailableReturnsNomadOverload() { - s.runnerManager.On("Claim", mock.AnythingOfType("runner.EnvironmentID"), mock.AnythingOfType("int")). + s.runnerManager.On("Claim", mock.AnythingOfType("dto.EnvironmentID"), mock.AnythingOfType("int")). Return(nil, runner.ErrNoRunnersAvailable) recorder := httptest.NewRecorder() diff --git a/internal/environment/environment.go b/internal/environment/environment.go new file mode 100644 index 0000000..6de0c46 --- /dev/null +++ b/internal/environment/environment.go @@ -0,0 +1,352 @@ +package environment + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "github.com/google/uuid" + nomadApi "github.com/hashicorp/nomad/api" + "github.com/hashicorp/nomad/jobspec2" + "github.com/openHPI/poseidon/internal/nomad" + "github.com/openHPI/poseidon/internal/runner" + "github.com/openHPI/poseidon/pkg/dto" + "strconv" +) + +const ( + portNumberBase = 10 +) + +var ( + ErrorUpdatingExecutionEnvironment = errors.New("errors occurred when updating environment") +) + +type NomadEnvironment struct { + jobHCL string + job *nomadApi.Job + idleRunners runner.Storage +} + +func NewNomadEnvironment(jobHCL string) (*NomadEnvironment, error) { + job, err := parseJob(jobHCL) + if err != nil { + return nil, fmt.Errorf("error parsing Nomad job: %w", err) + } + + return &NomadEnvironment{jobHCL, job, runner.NewLocalRunnerStorage()}, nil +} + +func (n *NomadEnvironment) ID() dto.EnvironmentID { + id, err := nomad.EnvironmentIDFromTemplateJobID(*n.job.ID) + if err != nil { + log.WithError(err).Error("Environment ID can not be parsed from Job") + } + return id +} + +func (n *NomadEnvironment) SetID(id dto.EnvironmentID) { + name := nomad.TemplateJobID(id) + n.job.ID = &name + n.job.Name = &name +} + +func (n *NomadEnvironment) PrewarmingPoolSize() uint { + configTaskGroup := nomad.FindOrCreateConfigTaskGroup(n.job) + count, err := strconv.Atoi(configTaskGroup.Meta[nomad.ConfigMetaPoolSizeKey]) + if err != nil { + log.WithError(err).Error("Prewarming pool size can not be parsed from Job") + } + return uint(count) +} + +func (n *NomadEnvironment) SetPrewarmingPoolSize(count uint) { + taskGroup := nomad.FindOrCreateConfigTaskGroup(n.job) + + if taskGroup.Meta == nil { + taskGroup.Meta = make(map[string]string) + } + taskGroup.Meta[nomad.ConfigMetaPoolSizeKey] = strconv.Itoa(int(count)) +} + +func (n *NomadEnvironment) CPULimit() uint { + defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) + defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + return uint(*defaultTask.Resources.CPU) +} + +func (n *NomadEnvironment) SetCPULimit(limit uint) { + defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) + defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + + integerCPULimit := int(limit) + defaultTask.Resources.CPU = &integerCPULimit +} + +func (n *NomadEnvironment) MemoryLimit() uint { + defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) + defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + return uint(*defaultTask.Resources.MemoryMB) +} + +func (n *NomadEnvironment) SetMemoryLimit(limit uint) { + defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) + defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + + integerMemoryLimit := int(limit) + defaultTask.Resources.MemoryMB = &integerMemoryLimit +} + +func (n *NomadEnvironment) Image() string { + defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) + defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + image, ok := defaultTask.Config["image"].(string) + if !ok { + image = "" + } + return image +} + +func (n *NomadEnvironment) SetImage(image string) { + defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) + defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + defaultTask.Config["image"] = image +} + +func (n *NomadEnvironment) NetworkAccess() (allowed bool, ports []uint16) { + defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) + defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + + allowed = defaultTask.Config["network_mode"] != "none" + if len(defaultTaskGroup.Networks) > 0 { + networkResource := defaultTaskGroup.Networks[0] + for _, port := range networkResource.DynamicPorts { + ports = append(ports, uint16(port.To)) + } + } + return allowed, ports +} + +func (n *NomadEnvironment) SetNetworkAccess(allow bool, exposedPorts []uint16) { + defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) + defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + + if len(defaultTaskGroup.Tasks) == 0 { + // This function is only used internally and must be called as last step when configuring the task. + // This error is not recoverable. + log.Fatal("Can't configure network before task has been configured!") + } + + if allow { + var networkResource *nomadApi.NetworkResource + if len(defaultTaskGroup.Networks) == 0 { + networkResource = &nomadApi.NetworkResource{} + defaultTaskGroup.Networks = []*nomadApi.NetworkResource{networkResource} + } else { + networkResource = defaultTaskGroup.Networks[0] + } + // Prefer "bridge" network over "host" to have an isolated network namespace with bridged interface + // instead of joining the host network namespace. + networkResource.Mode = "bridge" + for _, portNumber := range exposedPorts { + port := nomadApi.Port{ + Label: strconv.FormatUint(uint64(portNumber), portNumberBase), + To: int(portNumber), + } + networkResource.DynamicPorts = append(networkResource.DynamicPorts, port) + } + + // Explicitly set mode to override existing settings when updating job from without to with network. + // Don't use bridge as it collides with the bridge mode above. This results in Docker using 'bridge' + // mode, meaning all allocations will be attached to the `docker0` adapter and could reach other + // non-Nomad containers attached to it. This is avoided when using Nomads bridge network mode. + defaultTask.Config["network_mode"] = "" + } else { + // Somehow, we can't set the network mode to none in the NetworkResource on task group level. + // See https://github.com/hashicorp/nomad/issues/10540 + defaultTask.Config["network_mode"] = "none" + // Explicitly set Networks to signal Nomad to remove the possibly existing networkResource + defaultTaskGroup.Networks = []*nomadApi.NetworkResource{} + } +} + +// Register creates a Nomad job based on the default job configuration and the given parameters. +// It registers the job with Nomad and waits until the registration completes. +func (n *NomadEnvironment) Register(apiClient nomad.ExecutorAPI) error { + evalID, err := apiClient.RegisterNomadJob(n.job) + if err != nil { + return fmt.Errorf("couldn't register job: %w", err) + } + err = apiClient.MonitorEvaluation(evalID, context.Background()) + if err != nil { + return fmt.Errorf("error during the monitoring of the environment job: %w", err) + } + return nil +} + +func (n *NomadEnvironment) Delete(apiClient nomad.ExecutorAPI) error { + err := n.removeRunners(apiClient, uint(n.idleRunners.Length())) + if err != nil { + return err + } + err = apiClient.DeleteJob(*n.job.ID) + if err != nil { + return fmt.Errorf("couldn't delete environment job: %w", err) + } + return nil +} + +func (n *NomadEnvironment) Scale(apiClient nomad.ExecutorAPI) error { + required := int(n.PrewarmingPoolSize()) - n.idleRunners.Length() + + if required > 0 { + return n.createRunners(apiClient, uint(required)) + } else { + return n.removeRunners(apiClient, uint(-required)) + } +} + +func (n *NomadEnvironment) UpdateRunnerSpecs(apiClient nomad.ExecutorAPI) error { + runners, err := apiClient.LoadRunnerIDs(n.ID().ToString()) + if err != nil { + return fmt.Errorf("update environment couldn't load runners: %w", err) + } + + var occurredError error + for _, id := range runners { + // avoid taking the address of the loop variable + runnerID := id + updatedRunnerJob := n.DeepCopyJob() + updatedRunnerJob.ID = &runnerID + updatedRunnerJob.Name = &runnerID + + err := apiClient.RegisterRunnerJob(updatedRunnerJob) + if err != nil { + if occurredError == nil { + occurredError = ErrorUpdatingExecutionEnvironment + } + occurredError = fmt.Errorf("%w; new api error for runner %s - %v", occurredError, id, err) + } + } + return occurredError +} + +func (n *NomadEnvironment) Sample(apiClient nomad.ExecutorAPI) (runner.Runner, bool) { + r, ok := n.idleRunners.Sample() + if ok { + err := n.createRunner(apiClient) + if err != nil { + log.WithError(err).WithField("environmentID", n.ID()).Error("Couldn't create new runner for claimed one") + } + } + return r, ok +} + +func (n *NomadEnvironment) AddRunner(r runner.Runner) { + n.idleRunners.Add(r) +} + +func (n *NomadEnvironment) DeleteRunner(id string) { + n.idleRunners.Delete(id) +} + +// MarshalJSON implements the json.Marshaler interface. +// This converts the NomadEnvironment into the expected schema for dto.ExecutionEnvironmentData. +func (n *NomadEnvironment) MarshalJSON() (res []byte, err error) { + networkAccess, exposedPorts := n.NetworkAccess() + + res, err = json.Marshal(dto.ExecutionEnvironmentData{ + ID: int(n.ID()), + ExecutionEnvironmentRequest: dto.ExecutionEnvironmentRequest{ + PrewarmingPoolSize: n.PrewarmingPoolSize(), + CPULimit: n.CPULimit(), + MemoryLimit: n.MemoryLimit(), + Image: n.Image(), + NetworkAccess: networkAccess, + ExposedPorts: exposedPorts, + }, + }) + if err != nil { + return res, fmt.Errorf("couldn't marshal execution environment: %w", err) + } + return res, nil +} + +// DeepCopyJob clones the native Nomad job in a way that it can be used as Runner job. +func (n *NomadEnvironment) DeepCopyJob() *nomadApi.Job { + copyJob, err := parseJob(n.jobHCL) + if err != nil { + log.WithError(err).Error("The HCL of an existing environment should throw no error!") + return nil + } + copyEnvironment := &NomadEnvironment{job: copyJob} + + copyEnvironment.SetConfigFrom(n) + return copyEnvironment.job +} + +func (n *NomadEnvironment) SetConfigFrom(environment runner.ExecutionEnvironment) { + n.SetID(environment.ID()) + n.SetPrewarmingPoolSize(environment.PrewarmingPoolSize()) + n.SetCPULimit(environment.CPULimit()) + n.SetMemoryLimit(environment.MemoryLimit()) + n.SetImage(environment.Image()) + n.SetNetworkAccess(environment.NetworkAccess()) +} + +func parseJob(jobHCL string) (*nomadApi.Job, error) { + config := jobspec2.ParseConfig{ + Body: []byte(jobHCL), + AllowFS: false, + Strict: true, + } + job, err := jobspec2.ParseWithConfig(&config) + if err != nil { + return job, fmt.Errorf("couldn't parse job HCL: %w", err) + } + return job, nil +} + +func (n *NomadEnvironment) createRunners(apiClient nomad.ExecutorAPI, count uint) error { + log.WithField("runnersRequired", count).WithField("id", n.ID()).Debug("Creating new runners") + for i := 0; i < int(count); i++ { + err := n.createRunner(apiClient) + if err != nil { + return fmt.Errorf("couldn't create new runner: %w", err) + } + } + return nil +} + +func (n *NomadEnvironment) createRunner(apiClient nomad.ExecutorAPI) error { + newUUID, err := uuid.NewUUID() + if err != nil { + return fmt.Errorf("failed generating runner id: %w", err) + } + + newRunnerID := nomad.RunnerJobID(n.ID(), newUUID.String()) + template := n.DeepCopyJob() + template.ID = &newRunnerID + template.Name = &newRunnerID + + err = apiClient.RegisterRunnerJob(template) + if err != nil { + return fmt.Errorf("error registering new runner job: %w", err) + } + return nil +} + +func (n *NomadEnvironment) removeRunners(apiClient nomad.ExecutorAPI, count uint) error { + log.WithField("runnersToDelete", count).WithField("id", n.ID()).Debug("Removing idle runners") + for i := 0; i < int(count); i++ { + r, ok := n.idleRunners.Sample() + if !ok { + return fmt.Errorf("could not delete expected idle runner: %w", runner.ErrRunnerNotFound) + } + err := apiClient.DeleteJob(r.ID()) + if err != nil { + return fmt.Errorf("could not delete expected Nomad idle runner: %w", err) + } + } + return nil +} diff --git a/internal/environment/environment_test.go b/internal/environment/environment_test.go new file mode 100644 index 0000000..2c71fa1 --- /dev/null +++ b/internal/environment/environment_test.go @@ -0,0 +1,192 @@ +package environment + +import ( + "fmt" + nomadApi "github.com/hashicorp/nomad/api" + "github.com/openHPI/poseidon/internal/nomad" + "github.com/openHPI/poseidon/internal/runner" + "github.com/openHPI/poseidon/tests" + "github.com/openHPI/poseidon/tests/helpers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "testing" +) + +func TestConfigureNetworkCreatesNewNetworkWhenNoNetworkExists(t *testing.T) { + _, job := helpers.CreateTemplateJob() + defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(job) + environment := &NomadEnvironment{"", job, nil} + + if assert.Equal(t, 0, len(defaultTaskGroup.Networks)) { + environment.SetNetworkAccess(true, []uint16{}) + + assert.Equal(t, 1, len(defaultTaskGroup.Networks)) + } +} + +func TestConfigureNetworkDoesNotCreateNewNetworkWhenNetworkExists(t *testing.T) { + _, job := helpers.CreateTemplateJob() + defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(job) + environment := &NomadEnvironment{"", job, nil} + + networkResource := &nomadApi.NetworkResource{Mode: "bridge"} + defaultTaskGroup.Networks = []*nomadApi.NetworkResource{networkResource} + + if assert.Equal(t, 1, len(defaultTaskGroup.Networks)) { + environment.SetNetworkAccess(true, []uint16{}) + + assert.Equal(t, 1, len(defaultTaskGroup.Networks)) + assert.Equal(t, networkResource, defaultTaskGroup.Networks[0]) + } +} + +func TestConfigureNetworkSetsCorrectValues(t *testing.T) { + _, job := helpers.CreateTemplateJob() + defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(job) + defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + + mode, ok := defaultTask.Config["network_mode"] + assert.True(t, ok) + assert.Equal(t, "none", mode) + assert.Equal(t, 0, len(defaultTaskGroup.Networks)) + + exposedPortsTests := [][]uint16{{}, {1337}, {42, 1337}} + t.Run("with no network access", func(t *testing.T) { + for _, ports := range exposedPortsTests { + _, testJob := helpers.CreateTemplateJob() + testTaskGroup := nomad.FindOrCreateDefaultTaskGroup(testJob) + testTask := nomad.FindOrCreateDefaultTask(testTaskGroup) + testEnvironment := &NomadEnvironment{"", job, nil} + + testEnvironment.SetNetworkAccess(false, ports) + mode, ok := testTask.Config["network_mode"] + assert.True(t, ok) + assert.Equal(t, "none", mode) + assert.Equal(t, 0, len(testTaskGroup.Networks)) + } + }) + + t.Run("with network access", func(t *testing.T) { + for _, ports := range exposedPortsTests { + _, testJob := helpers.CreateTemplateJob() + testTaskGroup := nomad.FindOrCreateDefaultTaskGroup(testJob) + testTask := nomad.FindOrCreateDefaultTask(testTaskGroup) + testEnvironment := &NomadEnvironment{"", testJob, nil} + + testEnvironment.SetNetworkAccess(true, ports) + require.Equal(t, 1, len(testTaskGroup.Networks)) + + networkResource := testTaskGroup.Networks[0] + assert.Equal(t, "bridge", networkResource.Mode) + require.Equal(t, len(ports), len(networkResource.DynamicPorts)) + + assertExpectedPorts(t, ports, networkResource) + + mode, ok := testTask.Config["network_mode"] + assert.True(t, ok) + assert.Equal(t, mode, "") + } + }) +} + +func assertExpectedPorts(t *testing.T, expectedPorts []uint16, networkResource *nomadApi.NetworkResource) { + t.Helper() + for _, expectedPort := range expectedPorts { + found := false + for _, actualPort := range networkResource.DynamicPorts { + if actualPort.To == int(expectedPort) { + found = true + break + } + } + assert.True(t, found, fmt.Sprintf("port list should contain %v", expectedPort)) + } +} + +func TestRegisterFailsWhenNomadJobRegistrationFails(t *testing.T) { + apiClientMock := &nomad.ExecutorAPIMock{} + expectedErr := tests.ErrDefault + + apiClientMock.On("RegisterNomadJob", mock.AnythingOfType("*api.Job")).Return("", expectedErr) + + environment := &NomadEnvironment{"", &nomadApi.Job{}, nil} + environment.SetID(tests.DefaultEnvironmentIDAsInteger) + err := environment.Register(apiClientMock) + + assert.ErrorIs(t, err, expectedErr) + apiClientMock.AssertNotCalled(t, "EvaluationStream") +} + +func TestRegisterTemplateJobSucceedsWhenMonitoringEvaluationSucceeds(t *testing.T) { + apiClientMock := &nomad.ExecutorAPIMock{} + evaluationID := "id" + + stream := make(chan *nomadApi.Events) + readonlyStream := func() <-chan *nomadApi.Events { + return stream + }() + // Immediately close stream to avoid any reading from it resulting in endless wait + close(stream) + + apiClientMock.On("RegisterNomadJob", mock.AnythingOfType("*api.Job")).Return(evaluationID, nil) + apiClientMock.On("MonitorEvaluation", mock.AnythingOfType("string"), mock.Anything).Return(nil) + apiClientMock.On("EvaluationStream", evaluationID, mock.AnythingOfType("*context.emptyCtx")). + Return(readonlyStream, nil) + + environment := &NomadEnvironment{"", &nomadApi.Job{}, nil} + environment.SetID(tests.DefaultEnvironmentIDAsInteger) + err := environment.Register(apiClientMock) + + assert.NoError(t, err) +} + +func TestRegisterTemplateJobReturnsErrorWhenMonitoringEvaluationFails(t *testing.T) { + apiClientMock := &nomad.ExecutorAPIMock{} + evaluationID := "id" + + apiClientMock.On("RegisterNomadJob", mock.AnythingOfType("*api.Job")).Return(evaluationID, nil) + apiClientMock.On("MonitorEvaluation", mock.AnythingOfType("string"), mock.Anything).Return(tests.ErrDefault) + + environment := &NomadEnvironment{"", &nomadApi.Job{}, nil} + environment.SetID(tests.DefaultEnvironmentIDAsInteger) + err := environment.Register(apiClientMock) + + assert.ErrorIs(t, err, tests.ErrDefault) +} + +func TestParseJob(t *testing.T) { + t.Run("parses the given default job", func(t *testing.T) { + environment, err := NewNomadEnvironment(templateEnvironmentJobHCL) + assert.NoError(t, err) + assert.NotNil(t, environment.job) + }) + + t.Run("returns error when given wrong job", func(t *testing.T) { + environment, err := NewNomadEnvironment("") + assert.Error(t, err) + assert.Nil(t, environment) + }) +} + +func TestTwoSampleAddExactlyTwoRunners(t *testing.T) { + apiMock := &nomad.ExecutorAPIMock{} + apiMock.On("RegisterRunnerJob", mock.AnythingOfType("*api.Job")).Return(nil) + + _, job := helpers.CreateTemplateJob() + environment := &NomadEnvironment{templateEnvironmentJobHCL, job, runner.NewLocalRunnerStorage()} + runner1 := &runner.RunnerMock{} + runner1.On("ID").Return(tests.DefaultRunnerID) + runner2 := &runner.RunnerMock{} + runner2.On("ID").Return(tests.AnotherRunnerID) + + environment.AddRunner(runner1) + environment.AddRunner(runner2) + + _, ok := environment.Sample(apiMock) + require.True(t, ok) + _, ok = environment.Sample(apiMock) + require.True(t, ok) + + apiMock.AssertNumberOfCalls(t, "RegisterRunnerJob", 2) +} diff --git a/internal/environment/manager.go b/internal/environment/manager.go index 277384c..7281a4e 100644 --- a/internal/environment/manager.go +++ b/internal/environment/manager.go @@ -3,15 +3,12 @@ package environment import ( _ "embed" "fmt" - nomadApi "github.com/hashicorp/nomad/api" - "github.com/hashicorp/nomad/jobspec2" "github.com/hashicorp/nomad/nomad/structs" "github.com/openHPI/poseidon/internal/nomad" "github.com/openHPI/poseidon/internal/runner" "github.com/openHPI/poseidon/pkg/dto" "github.com/openHPI/poseidon/pkg/logging" "os" - "strconv" ) // templateEnvironmentJobHCL holds our default job in HCL format. @@ -28,13 +25,31 @@ type Manager interface { // It should be called during the startup process (e.g. on creation of the Manager). Load() error + // List returns all environments known by Poseidon. + // When `fetch` is set the environments are fetched from the executor before returning. + List(fetch bool) ([]runner.ExecutionEnvironment, error) + + // Get returns the details of the requested environment. + // When `fetch` is set the requested environment is fetched from the executor before returning. + Get(id dto.EnvironmentID, fetch bool) (runner.ExecutionEnvironment, error) + // CreateOrUpdate creates/updates an execution environment on the executor. // If the job was created, the returned boolean is true, if it was updated, it is false. // If err is not nil, that means the environment was neither created nor updated. CreateOrUpdate( - id runner.EnvironmentID, + id dto.EnvironmentID, request dto.ExecutionEnvironmentRequest, ) (bool, error) + + // Delete removes the specified execution environment. + // Iff the specified environment could not be found Delete returns false. + Delete(id dto.EnvironmentID) (bool, error) +} + +type NomadEnvironmentManager struct { + runnerManager runner.Manager + api nomad.ExecutorAPI + templateEnvironmentHCL string } func NewNomadEnvironmentManager( @@ -45,11 +60,8 @@ func NewNomadEnvironmentManager( if err := loadTemplateEnvironmentJobHCL(templateJobFile); err != nil { return nil, err } - templateEnvironmentJob, err := parseJob(templateEnvironmentJobHCL) - if err != nil { - return nil, err - } - m := &NomadEnvironmentManager{runnerManager, apiClient, *templateEnvironmentJob} + + m := &NomadEnvironmentManager{runnerManager, apiClient, templateEnvironmentJobHCL} if err := m.Load(); err != nil { log.WithError(err).Error("Error recovering the execution environments") } @@ -57,6 +69,121 @@ func NewNomadEnvironmentManager( return m, nil } +func (m *NomadEnvironmentManager) Get(id dto.EnvironmentID, fetch bool) ( + executionEnvironment runner.ExecutionEnvironment, err error) { + executionEnvironment, ok := m.runnerManager.GetEnvironment(id) + + if fetch { + fetchedEnvironment, err := fetchEnvironment(id, m.api) + switch { + case err != nil: + return nil, err + case fetchedEnvironment == nil: + _, err = m.Delete(id) + if err != nil { + return nil, err + } + ok = false + case !ok: + m.runnerManager.SetEnvironment(fetchedEnvironment) + executionEnvironment = fetchedEnvironment + ok = true + default: + executionEnvironment.SetConfigFrom(fetchedEnvironment) + } + } + + if !ok { + err = runner.ErrUnknownExecutionEnvironment + } + return executionEnvironment, err +} + +func (m *NomadEnvironmentManager) List(fetch bool) ([]runner.ExecutionEnvironment, error) { + if fetch { + err := m.Load() + if err != nil { + return nil, err + } + } + return m.runnerManager.ListEnvironments(), nil +} + +func (m *NomadEnvironmentManager) CreateOrUpdate(id dto.EnvironmentID, request dto.ExecutionEnvironmentRequest) ( + created bool, err error) { + environment, ok := m.runnerManager.GetEnvironment(id) + if !ok { + environment, err = NewNomadEnvironment(m.templateEnvironmentHCL) + if err != nil { + return false, fmt.Errorf("error creating Nomad environment: %w", err) + } + environment.SetID(id) + } + + environment.SetPrewarmingPoolSize(request.PrewarmingPoolSize) + environment.SetCPULimit(request.CPULimit) + environment.SetMemoryLimit(request.MemoryLimit) + environment.SetImage(request.Image) + environment.SetNetworkAccess(request.NetworkAccess, request.ExposedPorts) + created = m.runnerManager.SetEnvironment(environment) + + err = environment.Register(m.api) + if err != nil { + return false, fmt.Errorf("error registering template job in API: %w", err) + } + err = environment.UpdateRunnerSpecs(m.api) + if err != nil { + return false, fmt.Errorf("error updating runner jobs in API: %w", err) + } + err = environment.Scale(m.api) + if err != nil { + return false, fmt.Errorf("error scaling template job in API: %w", err) + } + + return created, nil +} + +func (m *NomadEnvironmentManager) Delete(id dto.EnvironmentID) (bool, error) { + executionEnvironment, ok := m.runnerManager.GetEnvironment(id) + if !ok { + return false, nil + } + m.runnerManager.DeleteEnvironment(id) + err := executionEnvironment.Delete(m.api) + if err != nil { + return true, fmt.Errorf("could not delete environment: %w", err) + } + return true, nil +} + +func (m *NomadEnvironmentManager) Load() error { + templateJobs, err := m.api.LoadEnvironmentJobs() + if err != nil { + return fmt.Errorf("couldn't load template jobs: %w", err) + } + + for _, job := range templateJobs { + jobLogger := log.WithField("jobID", *job.ID) + if *job.Status != structs.JobStatusRunning { + jobLogger.Info("Job not running, skipping ...") + continue + } + configTaskGroup := nomad.FindOrCreateConfigTaskGroup(job) + if configTaskGroup == nil { + jobLogger.Info("Couldn't find config task group in job, skipping ...") + continue + } + environment := &NomadEnvironment{ + jobHCL: templateEnvironmentJobHCL, + job: job, + idleRunners: runner.NewLocalRunnerStorage(), + } + m.runnerManager.SetEnvironment(environment) + jobLogger.Info("Successfully recovered environment") + } + return nil +} + // loadTemplateEnvironmentJobHCL loads the template environment job HCL from the given path. // If the path is empty, the embedded default file is used. func loadTemplateEnvironmentJobHCL(path string) error { @@ -71,84 +198,25 @@ func loadTemplateEnvironmentJobHCL(path string) error { return nil } -type NomadEnvironmentManager struct { - runnerManager runner.Manager - api nomad.ExecutorAPI - templateEnvironmentJob nomadApi.Job -} - -func (m *NomadEnvironmentManager) CreateOrUpdate( - id runner.EnvironmentID, - request dto.ExecutionEnvironmentRequest, -) (bool, error) { - templateJob, err := m.api.RegisterTemplateJob(&m.templateEnvironmentJob, runner.TemplateJobID(id), - request.PrewarmingPoolSize, request.CPULimit, request.MemoryLimit, - request.Image, request.NetworkAccess, request.ExposedPorts) - +func fetchEnvironment(id dto.EnvironmentID, apiClient nomad.ExecutorAPI) (runner.ExecutionEnvironment, error) { + environments, err := apiClient.LoadEnvironmentJobs() if err != nil { - return false, fmt.Errorf("error registering template job in API: %w", err) + return nil, fmt.Errorf("error fetching the environment jobs: %w", err) } - - created, err := m.runnerManager.CreateOrUpdateEnvironment(id, request.PrewarmingPoolSize, templateJob, true) - if err != nil { - return created, fmt.Errorf("error updating environment in runner manager: %w", err) + var fetchedEnvironment runner.ExecutionEnvironment + for _, job := range environments { + environmentID, err := nomad.EnvironmentIDFromTemplateJobID(*job.ID) + if err != nil { + log.WithError(err).Warn("Cannot parse environment id of loaded environment") + continue + } + if id == environmentID { + fetchedEnvironment = &NomadEnvironment{ + jobHCL: templateEnvironmentJobHCL, + job: job, + idleRunners: runner.NewLocalRunnerStorage(), + } + } } - return created, nil -} - -func (m *NomadEnvironmentManager) Load() error { - templateJobs, err := m.api.LoadEnvironmentJobs() - if err != nil { - return fmt.Errorf("couldn't load template jobs: %w", err) - } - - for _, job := range templateJobs { - jobLogger := log.WithField("jobID", *job.ID) - if *job.Status != structs.JobStatusRunning { - jobLogger.Info("Job not running, skipping ...") - continue - } - configTaskGroup := nomad.FindConfigTaskGroup(job) - if configTaskGroup == nil { - jobLogger.Info("Couldn't find config task group in job, skipping ...") - continue - } - desiredIdleRunnersCount, err := strconv.Atoi(configTaskGroup.Meta[nomad.ConfigMetaPoolSizeKey]) - if err != nil { - jobLogger.Infof("Couldn't convert pool size to int: %v, skipping ...", err) - continue - } - environmentIDString, err := runner.EnvironmentIDFromTemplateJobID(*job.ID) - if err != nil { - jobLogger.WithError(err).Error("Couldn't retrieve environment id from template job") - } - environmentID, err := runner.NewEnvironmentID(environmentIDString) - if err != nil { - jobLogger.WithField("environmentID", environmentIDString). - WithError(err). - Error("Couldn't retrieve environmentID from string") - continue - } - _, err = m.runnerManager.CreateOrUpdateEnvironment(environmentID, uint(desiredIdleRunnersCount), job, false) - if err != nil { - jobLogger.WithError(err).Info("Could not recover job.") - continue - } - jobLogger.Info("Successfully recovered environment") - } - return nil -} - -func parseJob(jobHCL string) (*nomadApi.Job, error) { - config := jobspec2.ParseConfig{ - Body: []byte(jobHCL), - AllowFS: false, - Strict: true, - } - job, err := jobspec2.ParseWithConfig(&config) - if err != nil { - return nil, fmt.Errorf("error parsing Nomad job: %w", err) - } - - return job, nil + return fetchedEnvironment, nil } diff --git a/internal/environment/manager_mock.go b/internal/environment/manager_mock.go index 63ac152..32b0bdc 100644 --- a/internal/environment/manager_mock.go +++ b/internal/environment/manager_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.8.0. DO NOT EDIT. +// Code generated by mockery v2.9.4. DO NOT EDIT. package environment @@ -15,18 +15,18 @@ type ManagerMock struct { } // CreateOrUpdate provides a mock function with given fields: id, request -func (_m *ManagerMock) CreateOrUpdate(id runner.EnvironmentID, request dto.ExecutionEnvironmentRequest) (bool, error) { +func (_m *ManagerMock) CreateOrUpdate(id dto.EnvironmentID, request dto.ExecutionEnvironmentRequest) (bool, error) { ret := _m.Called(id, request) var r0 bool - if rf, ok := ret.Get(0).(func(runner.EnvironmentID, dto.ExecutionEnvironmentRequest) bool); ok { + if rf, ok := ret.Get(0).(func(dto.EnvironmentID, dto.ExecutionEnvironmentRequest) bool); ok { r0 = rf(id, request) } else { r0 = ret.Get(0).(bool) } var r1 error - if rf, ok := ret.Get(1).(func(runner.EnvironmentID, dto.ExecutionEnvironmentRequest) error); ok { + if rf, ok := ret.Get(1).(func(dto.EnvironmentID, dto.ExecutionEnvironmentRequest) error); ok { r1 = rf(id, request) } else { r1 = ret.Error(1) @@ -36,8 +36,70 @@ func (_m *ManagerMock) CreateOrUpdate(id runner.EnvironmentID, request dto.Execu } // Delete provides a mock function with given fields: id -func (_m *ManagerMock) Delete(id string) { - _m.Called(id) +func (_m *ManagerMock) Delete(id dto.EnvironmentID) (bool, error) { + ret := _m.Called(id) + + var r0 bool + if rf, ok := ret.Get(0).(func(dto.EnvironmentID) bool); ok { + r0 = rf(id) + } else { + r0 = ret.Get(0).(bool) + } + + var r1 error + if rf, ok := ret.Get(1).(func(dto.EnvironmentID) error); ok { + r1 = rf(id) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Get provides a mock function with given fields: id, fetch +func (_m *ManagerMock) Get(id dto.EnvironmentID, fetch bool) (runner.ExecutionEnvironment, error) { + ret := _m.Called(id, fetch) + + var r0 runner.ExecutionEnvironment + if rf, ok := ret.Get(0).(func(dto.EnvironmentID, bool) runner.ExecutionEnvironment); ok { + r0 = rf(id, fetch) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(runner.ExecutionEnvironment) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(dto.EnvironmentID, bool) error); ok { + r1 = rf(id, fetch) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// List provides a mock function with given fields: fetch +func (_m *ManagerMock) List(fetch bool) ([]runner.ExecutionEnvironment, error) { + ret := _m.Called(fetch) + + var r0 []runner.ExecutionEnvironment + if rf, ok := ret.Get(0).(func(bool) []runner.ExecutionEnvironment); ok { + r0 = rf(fetch) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]runner.ExecutionEnvironment) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(bool) error); ok { + r1 = rf(fetch) + } else { + r1 = ret.Error(1) + } + + return r0, r1 } // Load provides a mock function with given fields: diff --git a/internal/environment/manager_test.go b/internal/environment/manager_test.go index c935bf4..12983d6 100644 --- a/internal/environment/manager_test.go +++ b/internal/environment/manager_test.go @@ -1,7 +1,9 @@ package environment import ( + "context" nomadApi "github.com/hashicorp/nomad/api" + "github.com/hashicorp/nomad/nomad/structs" "github.com/openHPI/poseidon/internal/nomad" "github.com/openHPI/poseidon/internal/runner" "github.com/openHPI/poseidon/pkg/dto" @@ -12,6 +14,7 @@ import ( "github.com/stretchr/testify/suite" "os" "testing" + "time" ) type CreateOrUpdateTestSuite struct { @@ -20,7 +23,7 @@ type CreateOrUpdateTestSuite struct { apiMock nomad.ExecutorAPIMock request dto.ExecutionEnvironmentRequest manager *NomadEnvironmentManager - environmentID runner.EnvironmentID + environmentID dto.EnvironmentID } func TestCreateOrUpdateTestSuite(t *testing.T) { @@ -41,97 +44,22 @@ func (s *CreateOrUpdateTestSuite) SetupTest() { } s.manager = &NomadEnvironmentManager{ - runnerManager: &s.runnerManagerMock, - api: &s.apiMock, + runnerManager: &s.runnerManagerMock, + api: &s.apiMock, + templateEnvironmentHCL: templateEnvironmentJobHCL, } - s.environmentID = runner.EnvironmentID(tests.DefaultEnvironmentIDAsInteger) -} - -func (s *CreateOrUpdateTestSuite) mockRegisterTemplateJob(job *nomadApi.Job, err error) { - s.apiMock.On("RegisterTemplateJob", - mock.AnythingOfType("*api.Job"), mock.AnythingOfType("string"), - mock.AnythingOfType("uint"), mock.AnythingOfType("uint"), mock.AnythingOfType("uint"), - mock.AnythingOfType("string"), mock.AnythingOfType("bool"), mock.AnythingOfType("[]uint16")). - Return(job, err) -} - -func (s *CreateOrUpdateTestSuite) mockCreateOrUpdateEnvironment(created bool, err error) { - s.runnerManagerMock.On("CreateOrUpdateEnvironment", mock.AnythingOfType("EnvironmentID"), - mock.AnythingOfType("uint"), mock.AnythingOfType("*api.Job"), mock.AnythingOfType("bool")). - Return(created, err) -} - -func (s *CreateOrUpdateTestSuite) TestRegistersCorrectTemplateJob() { - s.mockRegisterTemplateJob(&nomadApi.Job{}, nil) - s.mockCreateOrUpdateEnvironment(true, nil) - - _, err := s.manager.CreateOrUpdate(s.environmentID, s.request) - s.NoError(err) - - s.apiMock.AssertCalled(s.T(), "RegisterTemplateJob", - &s.manager.templateEnvironmentJob, runner.TemplateJobID(s.environmentID), - s.request.PrewarmingPoolSize, s.request.CPULimit, s.request.MemoryLimit, - s.request.Image, s.request.NetworkAccess, s.request.ExposedPorts) -} - -func (s *CreateOrUpdateTestSuite) TestReturnsErrorWhenRegisterTemplateJobReturnsError() { - s.mockRegisterTemplateJob(nil, tests.ErrDefault) - - created, err := s.manager.CreateOrUpdate(s.environmentID, s.request) - s.ErrorIs(err, tests.ErrDefault) - s.False(created) -} - -func (s *CreateOrUpdateTestSuite) TestCreatesOrUpdatesCorrectEnvironment() { - templateJobID := tests.DefaultJobID - templateJob := &nomadApi.Job{ID: &templateJobID} - s.mockRegisterTemplateJob(templateJob, nil) - s.mockCreateOrUpdateEnvironment(true, nil) - - _, err := s.manager.CreateOrUpdate(s.environmentID, s.request) - s.NoError(err) - s.runnerManagerMock.AssertCalled(s.T(), "CreateOrUpdateEnvironment", - s.environmentID, s.request.PrewarmingPoolSize, templateJob, true) + s.environmentID = dto.EnvironmentID(tests.DefaultEnvironmentIDAsInteger) } func (s *CreateOrUpdateTestSuite) TestReturnsErrorIfCreatesOrUpdateEnvironmentReturnsError() { - s.mockRegisterTemplateJob(&nomadApi.Job{}, nil) - s.mockCreateOrUpdateEnvironment(false, tests.ErrDefault) - _, err := s.manager.CreateOrUpdate(runner.EnvironmentID(tests.DefaultEnvironmentIDAsInteger), s.request) + s.apiMock.On("RegisterNomadJob", mock.AnythingOfType("*api.Job")).Return("", tests.ErrDefault) + s.runnerManagerMock.On("GetEnvironment", mock.AnythingOfType("dto.EnvironmentID")).Return(nil, false) + s.runnerManagerMock.On("SetEnvironment", mock.AnythingOfType("*environment.NomadEnvironment")).Return(true) + _, err := s.manager.CreateOrUpdate(dto.EnvironmentID(tests.DefaultEnvironmentIDAsInteger), s.request) s.ErrorIs(err, tests.ErrDefault) } -func (s *CreateOrUpdateTestSuite) TestReturnsTrueIfCreatesOrUpdateEnvironmentReturnsTrue() { - s.mockRegisterTemplateJob(&nomadApi.Job{}, nil) - s.mockCreateOrUpdateEnvironment(true, nil) - created, err := s.manager.CreateOrUpdate(runner.EnvironmentID(tests.DefaultEnvironmentIDAsInteger), s.request) - s.Require().NoError(err) - s.True(created) -} - -func (s *CreateOrUpdateTestSuite) TestReturnsFalseIfCreatesOrUpdateEnvironmentReturnsFalse() { - s.mockRegisterTemplateJob(&nomadApi.Job{}, nil) - s.mockCreateOrUpdateEnvironment(false, nil) - created, err := s.manager.CreateOrUpdate(runner.EnvironmentID(tests.DefaultEnvironmentIDAsInteger), s.request) - s.Require().NoError(err) - s.False(created) -} - -func TestParseJob(t *testing.T) { - t.Run("parses the given default job", func(t *testing.T) { - job, err := parseJob(templateEnvironmentJobHCL) - assert.NoError(t, err) - assert.NotNil(t, job) - }) - - t.Run("returns error when given wrong job", func(t *testing.T) { - job, err := parseJob("") - assert.Error(t, err) - assert.Nil(t, job) - }) -} - func TestNewNomadEnvironmentManager(t *testing.T) { executorAPIMock := &nomad.ExecutorAPIMock{} executorAPIMock.On("LoadEnvironmentJobs").Return([]*nomadApi.Job{}, nil) @@ -148,7 +76,7 @@ func TestNewNomadEnvironmentManager(t *testing.T) { t.Run("loads template environment job from file", func(t *testing.T) { templateJobHCL := "job \"test\" {}" - expectedJob, err := parseJob(templateJobHCL) + _, err := NewNomadEnvironment(templateJobHCL) require.NoError(t, err) f := createTempFile(t, templateJobHCL) defer os.Remove(f.Name()) @@ -156,8 +84,7 @@ func TestNewNomadEnvironmentManager(t *testing.T) { m, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, f.Name()) assert.NoError(t, err) assert.NotNil(t, m) - assert.Equal(t, templateJobHCL, templateEnvironmentJobHCL) - assert.Equal(t, *expectedJob, m.templateEnvironmentJob) + assert.Equal(t, templateJobHCL, m.templateEnvironmentHCL) }) t.Run("returns error if template file is invalid", func(t *testing.T) { @@ -165,13 +92,153 @@ func TestNewNomadEnvironmentManager(t *testing.T) { f := createTempFile(t, templateJobHCL) defer os.Remove(f.Name()) - _, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, f.Name()) + m, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, f.Name()) + require.NoError(t, err) + _, err = NewNomadEnvironment(m.templateEnvironmentHCL) assert.Error(t, err) }) templateEnvironmentJobHCL = previousTemplateEnvironmentJobHCL } +func TestNomadEnvironmentManager_Get(t *testing.T) { + apiMock := &nomad.ExecutorAPIMock{} + mockWatchAllocations(apiMock) + call := apiMock.On("LoadEnvironmentJobs") + call.Run(func(args mock.Arguments) { + call.ReturnArguments = mock.Arguments{[]*nomadApi.Job{}, nil} + }) + + runnerManager := runner.NewNomadRunnerManager(apiMock, context.Background()) + m, err := NewNomadEnvironmentManager(runnerManager, apiMock, "") + require.NoError(t, err) + + t.Run("Returns error when not found", func(t *testing.T) { + _, err := m.Get(tests.DefaultEnvironmentIDAsInteger, false) + assert.Error(t, err) + }) + + t.Run("Returns environment when it was added before", func(t *testing.T) { + expectedEnvironment, err := NewNomadEnvironment(templateEnvironmentJobHCL) + expectedEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) + require.NoError(t, err) + runnerManager.SetEnvironment(expectedEnvironment) + + environment, err := m.Get(tests.DefaultEnvironmentIDAsInteger, false) + assert.NoError(t, err) + assert.Equal(t, expectedEnvironment, environment) + }) + + t.Run("Fetch", func(t *testing.T) { + apiMock.On("DeleteJob", mock.AnythingOfType("string")).Return(nil) + t.Run("Returns error when not found", func(t *testing.T) { + _, err := m.Get(tests.DefaultEnvironmentIDAsInteger, true) + assert.Error(t, err) + }) + + t.Run("Updates values when environment already known by Poseidon", func(t *testing.T) { + fetchedEnvironment, err := NewNomadEnvironment(templateEnvironmentJobHCL) + require.NoError(t, err) + fetchedEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) + fetchedEnvironment.SetImage("random docker image") + call.Run(func(args mock.Arguments) { + call.ReturnArguments = mock.Arguments{[]*nomadApi.Job{fetchedEnvironment.job}, nil} + }) + + localEnvironment, err := NewNomadEnvironment(templateEnvironmentJobHCL) + require.NoError(t, err) + localEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) + runnerManager.SetEnvironment(localEnvironment) + + environment, err := m.Get(tests.DefaultEnvironmentIDAsInteger, false) + assert.NoError(t, err) + assert.NotEqual(t, fetchedEnvironment.Image(), environment.Image()) + + environment, err = m.Get(tests.DefaultEnvironmentIDAsInteger, true) + assert.NoError(t, err) + assert.Equal(t, fetchedEnvironment.Image(), environment.Image()) + }) + runnerManager.DeleteEnvironment(tests.DefaultEnvironmentIDAsInteger) + + t.Run("Adds environment when not already known by Poseidon", func(t *testing.T) { + fetchedEnvironment, err := NewNomadEnvironment(templateEnvironmentJobHCL) + require.NoError(t, err) + fetchedEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) + fetchedEnvironment.SetImage("random docker image") + call.Run(func(args mock.Arguments) { + call.ReturnArguments = mock.Arguments{[]*nomadApi.Job{fetchedEnvironment.job}, nil} + }) + + _, err = m.Get(tests.DefaultEnvironmentIDAsInteger, false) + assert.Error(t, err) + + environment, err := m.Get(tests.DefaultEnvironmentIDAsInteger, true) + assert.NoError(t, err) + assert.Equal(t, fetchedEnvironment.Image(), environment.Image()) + }) + }) +} + +func TestNomadEnvironmentManager_List(t *testing.T) { + apiMock := &nomad.ExecutorAPIMock{} + mockWatchAllocations(apiMock) + call := apiMock.On("LoadEnvironmentJobs") + call.Run(func(args mock.Arguments) { + call.ReturnArguments = mock.Arguments{[]*nomadApi.Job{}, nil} + }) + + runnerManager := runner.NewNomadRunnerManager(apiMock, context.Background()) + m, err := NewNomadEnvironmentManager(runnerManager, apiMock, "") + require.NoError(t, err) + + t.Run("with no environments", func(t *testing.T) { + environments, err := m.List(true) + assert.NoError(t, err) + assert.Empty(t, environments) + }) + + t.Run("Returns added environment", func(t *testing.T) { + localEnvironment, err := NewNomadEnvironment(templateEnvironmentJobHCL) + require.NoError(t, err) + localEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) + runnerManager.SetEnvironment(localEnvironment) + + environments, err := m.List(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(environments)) + assert.Equal(t, localEnvironment, environments[0]) + }) + runnerManager.DeleteEnvironment(tests.DefaultEnvironmentIDAsInteger) + + t.Run("Fetches new Runners via the api client", func(t *testing.T) { + fetchedEnvironment, err := NewNomadEnvironment(templateEnvironmentJobHCL) + require.NoError(t, err) + fetchedEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) + status := structs.JobStatusRunning + fetchedEnvironment.job.Status = &status + call.Run(func(args mock.Arguments) { + call.ReturnArguments = mock.Arguments{[]*nomadApi.Job{fetchedEnvironment.job}, nil} + }) + + environments, err := m.List(false) + assert.NoError(t, err) + assert.Empty(t, environments) + + environments, err = m.List(true) + assert.NoError(t, err) + assert.Equal(t, 1, len(environments)) + assert.Equal(t, fetchedEnvironment, environments[0]) + }) +} + +func mockWatchAllocations(apiMock *nomad.ExecutorAPIMock) { + call := apiMock.On("WatchAllocations", mock.Anything, mock.Anything, mock.Anything) + call.Run(func(args mock.Arguments) { + <-time.After(10 * time.Minute) // 10 minutes is the default test timeout + call.ReturnArguments = mock.Arguments{nil} + }) +} + func createTempFile(t *testing.T, content string) *os.File { t.Helper() f, err := os.CreateTemp("", "test") diff --git a/internal/nomad/api_querier.go b/internal/nomad/api_querier.go index bb2c8ed..7684228 100644 --- a/internal/nomad/api_querier.go +++ b/internal/nomad/api_querier.go @@ -27,8 +27,8 @@ type apiQuerier interface { // SetJobScale sets the scaling count of the passed job to Nomad. SetJobScale(jobID string, count uint, reason string) (err error) - // DeleteRunner deletes the runner with the given ID. - DeleteRunner(runnerID string) (err error) + // DeleteJob deletes the Job with the given ID. + DeleteJob(jobID string) (err error) // Execute runs a command in the passed job. Execute(jobID string, ctx context.Context, command []string, tty bool, @@ -82,8 +82,8 @@ func (nc *nomadAPIClient) init(nomadConfig *config.Nomad) (err error) { return nil } -func (nc *nomadAPIClient) DeleteRunner(runnerID string) (err error) { - _, _, err = nc.client.Jobs().Deregister(runnerID, true, nc.writeOptions()) +func (nc *nomadAPIClient) DeleteJob(jobID string) (err error) { + _, _, err = nc.client.Jobs().Deregister(jobID, true, nc.writeOptions()) return } diff --git a/internal/nomad/api_querier_mock.go b/internal/nomad/api_querier_mock.go index cff6e97..d27e0e2 100644 --- a/internal/nomad/api_querier_mock.go +++ b/internal/nomad/api_querier_mock.go @@ -42,7 +42,7 @@ func (_m *apiQuerierMock) AllocationStream(ctx context.Context) (<-chan *api.Eve } // DeleteRunner provides a mock function with given fields: runnerID -func (_m *apiQuerierMock) DeleteRunner(runnerID string) error { +func (_m *apiQuerierMock) DeleteJob(runnerID string) error { ret := _m.Called(runnerID) var r0 error diff --git a/internal/nomad/executor_api_mock.go b/internal/nomad/executor_api_mock.go index 0fa13fe..852dca4 100644 --- a/internal/nomad/executor_api_mock.go +++ b/internal/nomad/executor_api_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v0.0.0-dev. DO NOT EDIT. +// Code generated by mockery v2.9.4. DO NOT EDIT. package nomad @@ -41,13 +41,13 @@ func (_m *ExecutorAPIMock) AllocationStream(ctx context.Context) (<-chan *api.Ev return r0, r1 } -// DeleteRunner provides a mock function with given fields: runnerID -func (_m *ExecutorAPIMock) DeleteRunner(runnerID string) error { - ret := _m.Called(runnerID) +// DeleteJob provides a mock function with given fields: jobID +func (_m *ExecutorAPIMock) DeleteJob(jobID string) error { + ret := _m.Called(jobID) var r0 error if rf, ok := ret.Get(0).(func(string) error); ok { - r0 = rf(runnerID) + r0 = rf(jobID) } else { r0 = ret.Error(0) } @@ -319,29 +319,6 @@ func (_m *ExecutorAPIMock) RegisterRunnerJob(template *api.Job) error { return r0 } -// RegisterTemplateJob provides a mock function with given fields: defaultJob, id, prewarmingPoolSize, cpuLimit, memoryLimit, image, networkAccess, exposedPorts -func (_m *ExecutorAPIMock) RegisterTemplateJob(defaultJob *api.Job, id string, prewarmingPoolSize uint, cpuLimit uint, memoryLimit uint, image string, networkAccess bool, exposedPorts []uint16) (*api.Job, error) { - ret := _m.Called(defaultJob, id, prewarmingPoolSize, cpuLimit, memoryLimit, image, networkAccess, exposedPorts) - - var r0 *api.Job - if rf, ok := ret.Get(0).(func(*api.Job, string, uint, uint, uint, string, bool, []uint16) *api.Job); ok { - r0 = rf(defaultJob, id, prewarmingPoolSize, cpuLimit, memoryLimit, image, networkAccess, exposedPorts) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*api.Job) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(*api.Job, string, uint, uint, uint, string, bool, []uint16) error); ok { - r1 = rf(defaultJob, id, prewarmingPoolSize, cpuLimit, memoryLimit, image, networkAccess, exposedPorts) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // SetJobScale provides a mock function with given fields: jobID, count, reason func (_m *ExecutorAPIMock) SetJobScale(jobID string, count uint, reason string) error { ret := _m.Called(jobID, count, reason) diff --git a/internal/nomad/job.go b/internal/nomad/job.go index b5e9f3c..7f18a2f 100644 --- a/internal/nomad/job.go +++ b/internal/nomad/job.go @@ -5,7 +5,9 @@ import ( "errors" "fmt" nomadApi "github.com/hashicorp/nomad/api" + "github.com/openHPI/poseidon/pkg/dto" "strconv" + "strings" ) const ( @@ -24,73 +26,19 @@ const ( ConfigMetaUnusedValue = "false" ConfigMetaTimeoutKey = "timeout" ConfigMetaPoolSizeKey = "prewarmingPoolSize" + TemplateJobNameParts = 2 ) var ( - TaskArgs = []string{"infinity"} - ErrorConfigTaskGroupNotFound = errors.New("config task group not found in job") + ErrorInvalidJobID = errors.New("invalid job id") + TaskArgs = []string{"infinity"} ) -// FindConfigTaskGroup returns the config task group of a job. -// The config task group should be included in all jobs. -func FindConfigTaskGroup(job *nomadApi.Job) *nomadApi.TaskGroup { - for _, tg := range job.TaskGroups { - if *tg.Name == ConfigTaskGroupName { - return tg - } - } - return nil -} - -func SetMetaConfigValue(job *nomadApi.Job, key, value string) error { - configTaskGroup := FindConfigTaskGroup(job) - if configTaskGroup == nil { - return ErrorConfigTaskGroupNotFound - } - configTaskGroup.Meta[key] = value - return nil -} - -// RegisterTemplateJob creates a Nomad job based on the default job configuration and the given parameters. -// It registers the job with Nomad and waits until the registration completes. -func (a *APIClient) RegisterTemplateJob( - basisJob *nomadApi.Job, - id string, - prewarmingPoolSize, cpuLimit, memoryLimit uint, - image string, - networkAccess bool, - exposedPorts []uint16) (*nomadApi.Job, error) { - job := CreateTemplateJob(basisJob, id, prewarmingPoolSize, - cpuLimit, memoryLimit, image, networkAccess, exposedPorts) - evalID, err := a.apiQuerier.RegisterNomadJob(job) - if err != nil { - return nil, fmt.Errorf("couldn't register template job: %w", err) - } - return job, a.MonitorEvaluation(evalID, context.Background()) -} - -// CreateTemplateJob creates a Nomad job based on the default job configuration and the given parameters. -// It registers the job with Nomad and waits until the registration completes. -func CreateTemplateJob( - basisJob *nomadApi.Job, - id string, - prewarmingPoolSize, cpuLimit, memoryLimit uint, - image string, - networkAccess bool, - exposedPorts []uint16) *nomadApi.Job { - job := *basisJob - job.ID = &id - job.Name = &id - - var taskGroup = createTaskGroup(&job, TaskGroupName) - configureTask(taskGroup, TaskName, cpuLimit, memoryLimit, image, networkAccess, exposedPorts) - storeTemplateConfiguration(&job, prewarmingPoolSize) - - return &job -} - func (a *APIClient) RegisterRunnerJob(template *nomadApi.Job) error { - storeRunnerConfiguration(template) + taskGroup := FindOrCreateConfigTaskGroup(template) + + taskGroup.Meta = make(map[string]string) + taskGroup.Meta[ConfigMetaUsedKey] = ConfigMetaUnusedValue evalID, err := a.apiQuerier.RegisterNomadJob(template) if err != nil { @@ -99,126 +47,37 @@ func (a *APIClient) RegisterRunnerJob(template *nomadApi.Job) error { return a.MonitorEvaluation(evalID, context.Background()) } -func createTaskGroup(job *nomadApi.Job, name string) *nomadApi.TaskGroup { - var taskGroup *nomadApi.TaskGroup - if len(job.TaskGroups) == 0 { - taskGroup = nomadApi.NewTaskGroup(name, TaskCount) - job.TaskGroups = []*nomadApi.TaskGroup{taskGroup} - } else { - taskGroup = job.TaskGroups[0] - taskGroup.Name = &name - count := TaskCount - taskGroup.Count = &count +func FindTaskGroup(job *nomadApi.Job, name string) *nomadApi.TaskGroup { + for _, tg := range job.TaskGroups { + if *tg.Name == name { + return tg + } } + return nil +} + +func FindOrCreateDefaultTaskGroup(job *nomadApi.Job) *nomadApi.TaskGroup { + taskGroup := FindTaskGroup(job, TaskGroupName) + if taskGroup == nil { + taskGroup = nomadApi.NewTaskGroup(TaskGroupName, TaskCount) + job.AddTaskGroup(taskGroup) + } + FindOrCreateDefaultTask(taskGroup) return taskGroup } -const portNumberBase = 10 - -func configureNetwork(taskGroup *nomadApi.TaskGroup, networkAccess bool, exposedPorts []uint16) { - if len(taskGroup.Tasks) == 0 { - // This function is only used internally and must be called as last step when configuring the task. - // This error is not recoverable. - log.Fatal("Can't configure network before task has been configured!") - } - task := taskGroup.Tasks[0] - - if task.Config == nil { - task.Config = make(map[string]interface{}) - } - - if networkAccess { - var networkResource *nomadApi.NetworkResource - if len(taskGroup.Networks) == 0 { - networkResource = &nomadApi.NetworkResource{} - taskGroup.Networks = []*nomadApi.NetworkResource{networkResource} - } else { - networkResource = taskGroup.Networks[0] - } - // Prefer "bridge" network over "host" to have an isolated network namespace with bridged interface - // instead of joining the host network namespace. - networkResource.Mode = "bridge" - for _, portNumber := range exposedPorts { - port := nomadApi.Port{ - Label: strconv.FormatUint(uint64(portNumber), portNumberBase), - To: int(portNumber), - } - networkResource.DynamicPorts = append(networkResource.DynamicPorts, port) - } - - // Explicitly set mode to override existing settings when updating job from without to with network. - // Don't use bridge as it collides with the bridge mode above. This results in Docker using 'bridge' - // mode, meaning all allocations will be attached to the `docker0` adapter and could reach other - // non-Nomad containers attached to it. This is avoided when using Nomads bridge network mode. - task.Config["network_mode"] = "" - } else { - // Somehow, we can't set the network mode to none in the NetworkResource on task group level. - // See https://github.com/hashicorp/nomad/issues/10540 - task.Config["network_mode"] = "none" - // Explicitly set Networks to signal Nomad to remove the possibly existing networkResource - taskGroup.Networks = []*nomadApi.NetworkResource{} - } -} - -func configureTask( - taskGroup *nomadApi.TaskGroup, - name string, - cpuLimit, memoryLimit uint, - image string, - networkAccess bool, - exposedPorts []uint16) { - var task *nomadApi.Task - if len(taskGroup.Tasks) == 0 { - task = nomadApi.NewTask(name, TaskDriver) - taskGroup.Tasks = []*nomadApi.Task{task} - } else { - task = taskGroup.Tasks[0] - task.Name = name - } - integerCPULimit := int(cpuLimit) - integerMemoryLimit := int(memoryLimit) - if task.Resources == nil { - task.Resources = nomadApi.DefaultResources() - } - task.Resources.CPU = &integerCPULimit - task.Resources.MemoryMB = &integerMemoryLimit - - if task.Config == nil { - task.Config = make(map[string]interface{}) - } - task.Config["image"] = image - task.Config["command"] = TaskCommand - task.Config["args"] = TaskArgs - - configureNetwork(taskGroup, networkAccess, exposedPorts) -} - -func storeTemplateConfiguration(job *nomadApi.Job, prewarmingPoolSize uint) { - taskGroup := findOrCreateConfigTaskGroup(job) - - taskGroup.Meta = make(map[string]string) - taskGroup.Meta[ConfigMetaPoolSizeKey] = strconv.Itoa(int(prewarmingPoolSize)) -} - -func storeRunnerConfiguration(job *nomadApi.Job) { - taskGroup := findOrCreateConfigTaskGroup(job) - - taskGroup.Meta = make(map[string]string) - taskGroup.Meta[ConfigMetaUsedKey] = ConfigMetaUnusedValue -} - -func findOrCreateConfigTaskGroup(job *nomadApi.Job) *nomadApi.TaskGroup { - taskGroup := FindConfigTaskGroup(job) +func FindOrCreateConfigTaskGroup(job *nomadApi.Job) *nomadApi.TaskGroup { + taskGroup := FindTaskGroup(job, ConfigTaskGroupName) if taskGroup == nil { taskGroup = nomadApi.NewTaskGroup(ConfigTaskGroupName, 0) job.AddTaskGroup(taskGroup) } - createConfigTaskIfNotPresent(taskGroup) + FindOrCreateConfigTask(taskGroup) return taskGroup } -// createConfigTaskIfNotPresent ensures that a dummy task is in the task group so that the group is accepted by Nomad. -func createConfigTaskIfNotPresent(taskGroup *nomadApi.TaskGroup) { +// FindOrCreateConfigTask ensures that a dummy task is in the task group so that the group is accepted by Nomad. +func FindOrCreateConfigTask(taskGroup *nomadApi.TaskGroup) *nomadApi.Task { var task *nomadApi.Task for _, t := range taskGroup.Tasks { if t.Name == ConfigTaskName { @@ -236,4 +95,75 @@ func createConfigTaskIfNotPresent(taskGroup *nomadApi.TaskGroup) { task.Config = make(map[string]interface{}) } task.Config["command"] = ConfigTaskCommand + return task +} + +// FindOrCreateDefaultTask ensures that a default task is in the task group in that the executions are made. +func FindOrCreateDefaultTask(taskGroup *nomadApi.TaskGroup) *nomadApi.Task { + var task *nomadApi.Task + for _, t := range taskGroup.Tasks { + if t.Name == TaskName { + task = t + break + } + } + + if task == nil { + task = nomadApi.NewTask(TaskName, TaskDriver) + taskGroup.Tasks = append(taskGroup.Tasks, task) + } + + if task.Resources == nil { + task.Resources = nomadApi.DefaultResources() + } + + if task.Config == nil { + task.Config = make(map[string]interface{}) + } + task.Config["command"] = TaskCommand + task.Config["args"] = TaskArgs + return task +} + +// IsEnvironmentTemplateID checks if the passed job id belongs to a template job. +func IsEnvironmentTemplateID(jobID string) bool { + parts := strings.Split(jobID, "-") + if len(parts) != TemplateJobNameParts || parts[0] != TemplateJobPrefix { + return false + } + + _, err := EnvironmentIDFromTemplateJobID(jobID) + return err == nil +} + +// RunnerJobID returns the nomad job id of the runner with the given environmentID and id. +func RunnerJobID(environmentID dto.EnvironmentID, id string) string { + return fmt.Sprintf("%d-%s", environmentID, id) +} + +// TemplateJobID returns the id of the nomad job for the environment with the given id. +func TemplateJobID(id dto.EnvironmentID) string { + return fmt.Sprintf("%s-%d", TemplateJobPrefix, id) +} + +// EnvironmentIDFromRunnerID returns the environment id that is part of the passed runner job id. +func EnvironmentIDFromRunnerID(jobID string) (dto.EnvironmentID, error) { + return partOfJobID(jobID, 0) +} + +// EnvironmentIDFromTemplateJobID returns the environment id that is part of the passed environment job id. +func EnvironmentIDFromTemplateJobID(id string) (dto.EnvironmentID, error) { + return partOfJobID(id, 1) +} + +func partOfJobID(id string, part uint) (dto.EnvironmentID, error) { + parts := strings.Split(id, "-") + if len(parts) == 0 { + return 0, fmt.Errorf("empty job id: %w", ErrorInvalidJobID) + } + environmentID, err := strconv.Atoi(parts[part]) + if err != nil { + return 0, fmt.Errorf("invalid environment id par %v: %w", err, ErrorInvalidJobID) + } + return dto.EnvironmentID(environmentID), nil } diff --git a/internal/nomad/job_test.go b/internal/nomad/job_test.go index a5a3924..ed64c14 100644 --- a/internal/nomad/job_test.go +++ b/internal/nomad/job_test.go @@ -1,279 +1,121 @@ package nomad import ( - "fmt" nomadApi "github.com/hashicorp/nomad/api" - "github.com/openHPI/poseidon/tests" + "github.com/openHPI/poseidon/pkg/dto" "github.com/openHPI/poseidon/tests/helpers" - "github.com/sirupsen/logrus" - "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - "strconv" "testing" ) -func createTestTaskGroup() *nomadApi.TaskGroup { - return nomadApi.NewTaskGroup("taskGroup", 1) -} - -func createTestTask() *nomadApi.Task { - return nomadApi.NewTask("task", "docker") -} - -func createTestResources() *nomadApi.Resources { - result := nomadApi.DefaultResources() - expectedCPULimit := 1337 - expectedMemoryLimit := 42 - result.CPU = &expectedCPULimit - result.MemoryMB = &expectedMemoryLimit - return result -} - -func TestCreateTaskGroupCreatesNewTaskGroupWhenJobHasNoTaskGroup(t *testing.T) { - job := nomadApi.NewBatchJob("test", "test", "test", 1) - - if assert.Equal(t, 0, len(job.TaskGroups)) { - expectedTaskGroup := createTestTaskGroup() - taskGroup := createTaskGroup(job, *expectedTaskGroup.Name) - - assert.Equal(t, *expectedTaskGroup, *taskGroup) - assert.Equal(t, []*nomadApi.TaskGroup{taskGroup}, job.TaskGroups, "it should add the task group to the job") - } -} - -func TestCreateTaskGroupOverwritesOptionsWhenJobHasTaskGroup(t *testing.T) { - job := nomadApi.NewBatchJob("test", "test", "test", 1) - existingTaskGroup := createTestTaskGroup() - existingTaskGroup.Meta = map[string]string{"field": "should still exist"} - newTaskGroupList := []*nomadApi.TaskGroup{existingTaskGroup} - job.TaskGroups = newTaskGroupList - - newName := *existingTaskGroup.Name + "longerName" - taskGroup := createTaskGroup(job, newName) - - // create a new copy to avoid changing the original one as it is a pointer - expectedTaskGroup := *existingTaskGroup - expectedTaskGroup.Name = &newName - - assert.Equal(t, expectedTaskGroup, *taskGroup) - assert.Equal(t, newTaskGroupList, job.TaskGroups, "it should not modify the jobs task group list") -} - -func TestConfigureNetworkFatalsWhenNoTaskExists(t *testing.T) { - logger, hook := test.NewNullLogger() - logger.ExitFunc = func(i int) { - panic(i) - } - log = logger.WithField("pkg", "job_test") - taskGroup := createTestTaskGroup() - if assert.Equal(t, 0, len(taskGroup.Tasks)) { - assert.Panics(t, func() { - configureNetwork(taskGroup, false, nil) - }) - assert.Equal(t, logrus.FatalLevel, hook.LastEntry().Level) - } -} - -func TestConfigureNetworkCreatesNewNetworkWhenNoNetworkExists(t *testing.T) { - taskGroup := createTestTaskGroup() - task := createTestTask() - taskGroup.Tasks = []*nomadApi.Task{task} - - if assert.Equal(t, 0, len(taskGroup.Networks)) { - configureNetwork(taskGroup, true, []uint16{}) - - assert.Equal(t, 1, len(taskGroup.Networks)) - } -} - -func TestConfigureNetworkDoesNotCreateNewNetworkWhenNetworkExists(t *testing.T) { - taskGroup := createTestTaskGroup() - task := createTestTask() - taskGroup.Tasks = []*nomadApi.Task{task} - networkResource := &nomadApi.NetworkResource{Mode: "bridge"} - taskGroup.Networks = []*nomadApi.NetworkResource{networkResource} - - if assert.Equal(t, 1, len(taskGroup.Networks)) { - configureNetwork(taskGroup, true, []uint16{}) - - assert.Equal(t, 1, len(taskGroup.Networks)) - assert.Equal(t, networkResource, taskGroup.Networks[0]) - } -} - -func TestConfigureNetworkSetsCorrectValues(t *testing.T) { - taskGroup := createTestTaskGroup() - task := createTestTask() - _, ok := task.Config["network_mode"] - - require.False(t, ok, "Test tasks network_mode should not be set") - - taskGroup.Tasks = []*nomadApi.Task{task} - exposedPortsTests := [][]uint16{{}, {1337}, {42, 1337}} - - t.Run("with no network access", func(t *testing.T) { - for _, ports := range exposedPortsTests { - testTaskGroup := *taskGroup - testTask := *task - testTaskGroup.Tasks = []*nomadApi.Task{&testTask} - - configureNetwork(&testTaskGroup, false, ports) - mode, ok := testTask.Config["network_mode"] - assert.True(t, ok) - assert.Equal(t, "none", mode) - assert.Equal(t, 0, len(testTaskGroup.Networks)) - } +func TestFindTaskGroup(t *testing.T) { + t.Run("Returns nil if task group not found", func(t *testing.T) { + group := FindTaskGroup(&nomadApi.Job{}, TaskGroupName) + assert.Nil(t, group) }) - t.Run("with network access", func(t *testing.T) { - for _, ports := range exposedPortsTests { - testTaskGroup := *taskGroup - testTask := *task - testTaskGroup.Tasks = []*nomadApi.Task{&testTask} - - configureNetwork(&testTaskGroup, true, ports) - require.Equal(t, 1, len(testTaskGroup.Networks)) - - networkResource := testTaskGroup.Networks[0] - assert.Equal(t, "bridge", networkResource.Mode) - require.Equal(t, len(ports), len(networkResource.DynamicPorts)) - - assertExpectedPorts(t, ports, networkResource) - - mode, ok := testTask.Config["network_mode"] - assert.True(t, ok) - assert.Equal(t, mode, "") - } + t.Run("Finds task group when existent", func(t *testing.T) { + _, job := helpers.CreateTemplateJob() + group := FindTaskGroup(job, TaskGroupName) + assert.NotNil(t, group) }) } -func assertExpectedPorts(t *testing.T, expectedPorts []uint16, networkResource *nomadApi.NetworkResource) { - t.Helper() - for _, expectedPort := range expectedPorts { - found := false - for _, actualPort := range networkResource.DynamicPorts { - if actualPort.To == int(expectedPort) { - found = true - break - } - } - assert.True(t, found, fmt.Sprintf("port list should contain %v", expectedPort)) - } +func TestFindOrCreateDefaultTask(t *testing.T) { + t.Run("Adds default task group when not set", func(t *testing.T) { + job := &nomadApi.Job{} + group := FindOrCreateDefaultTaskGroup(job) + assert.NotNil(t, group) + assert.Equal(t, TaskGroupName, *group.Name) + assert.Equal(t, 1, len(job.TaskGroups)) + assert.Equal(t, group, job.TaskGroups[0]) + assert.Equal(t, TaskCount, *group.Count) + }) + + t.Run("Does not modify task group when already set", func(t *testing.T) { + job := &nomadApi.Job{} + groupName := TaskGroupName + expectedGroup := &nomadApi.TaskGroup{Name: &groupName} + job.TaskGroups = []*nomadApi.TaskGroup{expectedGroup} + + group := FindOrCreateDefaultTaskGroup(job) + assert.NotNil(t, group) + assert.Equal(t, 1, len(job.TaskGroups)) + assert.Equal(t, expectedGroup, group) + }) } -func TestConfigureTaskWhenNoTaskExists(t *testing.T) { - taskGroup := createTestTaskGroup() - require.Equal(t, 0, len(taskGroup.Tasks)) +func TestFindOrCreateConfigTaskGroup(t *testing.T) { + t.Run("Adds config task group when not set", func(t *testing.T) { + job := &nomadApi.Job{} + group := FindOrCreateConfigTaskGroup(job) + assert.NotNil(t, group) + assert.Equal(t, group, job.TaskGroups[0]) + assert.Equal(t, 1, len(job.TaskGroups)) - expectedResources := createTestResources() - expectedTaskGroup := *taskGroup - expectedTask := nomadApi.NewTask("task", TaskDriver) - expectedTask.Resources = expectedResources - expectedImage := "python:latest" - expectedCommand := "sleep" - expectedArgs := []string{"infinity"} - expectedTask.Config = map[string]interface{}{ - "image": expectedImage, "command": expectedCommand, "args": expectedArgs, "network_mode": "none"} - expectedTaskGroup.Tasks = []*nomadApi.Task{expectedTask} - expectedTaskGroup.Networks = []*nomadApi.NetworkResource{} + assert.Equal(t, ConfigTaskGroupName, *group.Name) + assert.Equal(t, 0, *group.Count) + }) - configureTask(taskGroup, expectedTask.Name, - uint(*expectedResources.CPU), uint(*expectedResources.MemoryMB), - expectedImage, false, []uint16{}) + t.Run("Does not modify task group when already set", func(t *testing.T) { + job := &nomadApi.Job{} + groupName := ConfigTaskGroupName + expectedGroup := &nomadApi.TaskGroup{Name: &groupName} + job.TaskGroups = []*nomadApi.TaskGroup{expectedGroup} - assert.Equal(t, expectedTaskGroup, *taskGroup) + group := FindOrCreateConfigTaskGroup(job) + assert.NotNil(t, group) + assert.Equal(t, 1, len(job.TaskGroups)) + assert.Equal(t, expectedGroup, group) + }) } -func TestConfigureTaskWhenTaskExists(t *testing.T) { - taskGroup := createTestTaskGroup() - task := createTestTask() - task.Config = map[string]interface{}{"my_custom_config": "should not be overwritten"} - taskGroup.Tasks = []*nomadApi.Task{task} - require.Equal(t, 1, len(taskGroup.Tasks)) +func TestFindOrCreateTask(t *testing.T) { + t.Run("Does not modify default task when already set", func(t *testing.T) { + groupName := TaskGroupName + group := &nomadApi.TaskGroup{Name: &groupName} + expectedTask := &nomadApi.Task{Name: TaskName} + group.Tasks = []*nomadApi.Task{expectedTask} - expectedResources := createTestResources() - expectedTaskGroup := *taskGroup - expectedTask := *task - expectedTask.Resources = expectedResources - expectedImage := "python:latest" - expectedTask.Config["image"] = expectedImage - expectedTask.Config["network_mode"] = "none" - expectedTaskGroup.Tasks = []*nomadApi.Task{&expectedTask} - expectedTaskGroup.Networks = []*nomadApi.NetworkResource{} + task := FindOrCreateDefaultTask(group) + assert.NotNil(t, task) + assert.Equal(t, 1, len(group.Tasks)) + assert.Equal(t, expectedTask, task) + }) - configureTask(taskGroup, expectedTask.Name, - uint(*expectedResources.CPU), uint(*expectedResources.MemoryMB), - expectedImage, false, []uint16{}) + t.Run("Does not modify config task when already set", func(t *testing.T) { + groupName := ConfigTaskGroupName + group := &nomadApi.TaskGroup{Name: &groupName} + expectedTask := &nomadApi.Task{Name: ConfigTaskName} + group.Tasks = []*nomadApi.Task{expectedTask} - assert.Equal(t, expectedTaskGroup, *taskGroup) - assert.Equal(t, task, taskGroup.Tasks[0], "it should not create a new task") + task := FindOrCreateConfigTask(group) + assert.NotNil(t, task) + assert.Equal(t, 1, len(group.Tasks)) + assert.Equal(t, expectedTask, task) + }) } -func TestCreateTemplateJobSetsAllGivenArguments(t *testing.T) { - base, testJob := helpers.CreateTemplateJob() - prewarmingPoolSize, err := strconv.Atoi(testJob.TaskGroups[1].Meta[ConfigMetaPoolSizeKey]) - require.NoError(t, err) - job := CreateTemplateJob( - base, - tests.DefaultJobID, - uint(prewarmingPoolSize), - uint(*testJob.TaskGroups[0].Tasks[0].Resources.CPU), - uint(*testJob.TaskGroups[0].Tasks[0].Resources.MemoryMB), - testJob.TaskGroups[0].Tasks[0].Config["image"].(string), - false, - nil, - ) - assert.Equal(t, *testJob, *job) +func TestIsEnvironmentTemplateID(t *testing.T) { + assert.True(t, IsEnvironmentTemplateID("template-42")) + assert.False(t, IsEnvironmentTemplateID("template-42-100")) + assert.False(t, IsEnvironmentTemplateID("job-42")) + assert.False(t, IsEnvironmentTemplateID("template-top")) } -func TestRegisterTemplateJobFailsWhenNomadJobRegistrationFails(t *testing.T) { - apiMock := apiQuerierMock{} - expectedErr := tests.ErrDefault - - apiMock.On("RegisterNomadJob", mock.AnythingOfType("*api.Job")).Return("", expectedErr) - - apiClient := &APIClient{&apiMock} - - _, err := apiClient.RegisterTemplateJob(&nomadApi.Job{}, tests.DefaultJobID, - 1, 2, 3, "image", false, []uint16{}) - assert.ErrorIs(t, err, expectedErr) - apiMock.AssertNotCalled(t, "EvaluationStream") +func TestRunnerJobID(t *testing.T) { + assert.Equal(t, "0-RANDOM-UUID", RunnerJobID(0, "RANDOM-UUID")) } -func TestRegisterTemplateJobSucceedsWhenMonitoringEvaluationSucceeds(t *testing.T) { - apiMock := apiQuerierMock{} - evaluationID := "id" +func TestTemplateJobID(t *testing.T) { + assert.Equal(t, "template-42", TemplateJobID(42)) +} - stream := make(chan *nomadApi.Events) - readonlyStream := func() <-chan *nomadApi.Events { - return stream - }() - // Immediately close stream to avoid any reading from it resulting in endless wait - close(stream) - - apiMock.On("RegisterNomadJob", mock.AnythingOfType("*api.Job")).Return(evaluationID, nil) - apiMock.On("EvaluationStream", evaluationID, mock.AnythingOfType("*context.emptyCtx")). - Return(readonlyStream, nil) - - apiClient := &APIClient{&apiMock} - - _, err := apiClient.RegisterTemplateJob(&nomadApi.Job{}, tests.DefaultJobID, - 1, 2, 3, "image", false, []uint16{}) +func TestEnvironmentIDFromRunnerID(t *testing.T) { + id, err := EnvironmentIDFromRunnerID("42-RANDOM-UUID") assert.NoError(t, err) -} - -func TestRegisterTemplateJobReturnsErrorWhenMonitoringEvaluationFails(t *testing.T) { - apiMock := apiQuerierMock{} - evaluationID := "id" - - apiMock.On("RegisterNomadJob", mock.AnythingOfType("*api.Job")).Return(evaluationID, nil) - apiMock.On("EvaluationStream", evaluationID, mock.AnythingOfType("*context.emptyCtx")).Return(nil, tests.ErrDefault) - - apiClient := &APIClient{&apiMock} - - _, err := apiClient.RegisterTemplateJob(&nomadApi.Job{}, tests.DefaultJobID, - 1, 2, 3, "image", false, []uint16{}) - assert.ErrorIs(t, err, tests.ErrDefault) + assert.Equal(t, dto.EnvironmentID(42), id) + + _, err = EnvironmentIDFromRunnerID("") + assert.Error(t, err) } diff --git a/internal/nomad/nomad.go b/internal/nomad/nomad.go index 79a566e..6fc225b 100644 --- a/internal/nomad/nomad.go +++ b/internal/nomad/nomad.go @@ -25,7 +25,7 @@ var ( type AllocationProcessor func(*nomadApi.Allocation) -// ExecutorAPI provides access to an container orchestration solution. +// ExecutorAPI provides access to a container orchestration solution. type ExecutorAPI interface { apiQuerier @@ -42,12 +42,6 @@ type ExecutorAPI interface { // LoadRunnerPortMappings returns the mapped ports of the runner. LoadRunnerPortMappings(runnerID string) ([]nomadApi.PortMapping, error) - // RegisterTemplateJob creates a template job based on the default job configuration and the given parameters. - // It registers the job and waits until the registration completes. - RegisterTemplateJob(defaultJob *nomadApi.Job, id string, - prewarmingPoolSize, cpuLimit, memoryLimit uint, - image string, networkAccess bool, exposedPorts []uint16) (*nomadApi.Job, error) - // RegisterRunnerJob creates a runner job based on the template job. // It registers the job and waits until the registration completes. RegisterRunnerJob(template *nomadApi.Job) error @@ -278,14 +272,10 @@ func (a *APIClient) MarkRunnerAsUsed(runnerID string, duration int) error { if err != nil { return fmt.Errorf("couldn't retrieve job info: %w", err) } - err = SetMetaConfigValue(job, ConfigMetaUsedKey, ConfigMetaUsedValue) - if err != nil { - return fmt.Errorf("couldn't update runner in job as used: %w", err) - } - err = SetMetaConfigValue(job, ConfigMetaTimeoutKey, strconv.Itoa(duration)) - if err != nil { - return fmt.Errorf("couldn't update runner in job with timeout: %w", err) - } + configTaskGroup := FindOrCreateConfigTaskGroup(job) + configTaskGroup.Meta[ConfigMetaUsedKey] = ConfigMetaUsedValue + configTaskGroup.Meta[ConfigMetaTimeoutKey] = strconv.Itoa(duration) + _, err = a.RegisterNomadJob(job) if err != nil { return fmt.Errorf("couldn't update runner config: %w", err) diff --git a/internal/nomad/nomad_test.go b/internal/nomad/nomad_test.go index 7a56eb0..77ec38e 100644 --- a/internal/nomad/nomad_test.go +++ b/internal/nomad/nomad_test.go @@ -15,7 +15,6 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "io" - "net/url" "regexp" "strings" "testing" @@ -38,15 +37,15 @@ type LoadRunnersTestSuite struct { } func (s *LoadRunnersTestSuite) SetupTest() { - s.jobID = tests.DefaultJobID + s.jobID = tests.DefaultRunnerID s.mock = &apiQuerierMock{} s.nomadAPIClient = APIClient{apiQuerier: s.mock} - s.availableRunner = newJobListStub(tests.DefaultJobID, structs.JobStatusRunning, 1) - s.anotherAvailableRunner = newJobListStub(tests.AnotherJobID, structs.JobStatusRunning, 1) - s.pendingRunner = newJobListStub(tests.DefaultJobID+"-1", structs.JobStatusPending, 0) - s.deadRunner = newJobListStub(tests.AnotherJobID+"-1", structs.JobStatusDead, 0) + s.availableRunner = newJobListStub(tests.DefaultRunnerID, structs.JobStatusRunning, 1) + s.anotherAvailableRunner = newJobListStub(tests.AnotherRunnerID, structs.JobStatusRunning, 1) + s.pendingRunner = newJobListStub(tests.DefaultRunnerID+"-1", structs.JobStatusPending, 0) + s.deadRunner = newJobListStub(tests.AnotherRunnerID+"-1", structs.JobStatusDead, 0) } func newJobListStub(id, status string, amountRunning int) *nomadApi.JobListStub { @@ -122,13 +121,6 @@ func (s *LoadRunnersTestSuite) TestReturnsAllAvailableRunners() { s.Contains(returnedIds, s.anotherAvailableRunner.ID) } -var ( - TestURL = url.URL{ - Scheme: "http", - Host: "127.0.0.1:4646", - } -) - const TestNamespace = "unit-tests" const TestNomadToken = "n0m4d-t0k3n" const TestDefaultAddress = "127.0.0.1" diff --git a/internal/runner/constants_test.go b/internal/runner/constants_test.go index 9225b91..420f26c 100644 --- a/internal/runner/constants_test.go +++ b/internal/runner/constants_test.go @@ -1,9 +1,12 @@ package runner -import "github.com/openHPI/poseidon/tests" +import ( + "github.com/openHPI/poseidon/pkg/dto" + "github.com/openHPI/poseidon/tests" +) const ( - defaultEnvironmentID = EnvironmentID(tests.DefaultEnvironmentIDAsInteger) - anotherEnvironmentID = EnvironmentID(tests.AnotherEnvironmentIDAsInteger) + defaultEnvironmentID = dto.EnvironmentID(tests.DefaultEnvironmentIDAsInteger) + anotherEnvironmentID = dto.EnvironmentID(tests.AnotherEnvironmentIDAsInteger) defaultInactivityTimeout = 0 ) diff --git a/internal/runner/execution_environment_mock.go b/internal/runner/execution_environment_mock.go new file mode 100644 index 0000000..69276c4 --- /dev/null +++ b/internal/runner/execution_environment_mock.go @@ -0,0 +1,255 @@ +// Code generated by mockery v2.9.4. DO NOT EDIT. + +package runner + +import ( + dto "github.com/openHPI/poseidon/pkg/dto" + mock "github.com/stretchr/testify/mock" + + nomad "github.com/openHPI/poseidon/internal/nomad" +) + +// ExecutionEnvironmentMock is an autogenerated mock type for the ExecutionEnvironment type +type ExecutionEnvironmentMock struct { + mock.Mock +} + +// AddRunner provides a mock function with given fields: r +func (_m *ExecutionEnvironmentMock) AddRunner(r Runner) { + _m.Called(r) +} + +// CPULimit provides a mock function with given fields: +func (_m *ExecutionEnvironmentMock) CPULimit() uint { + ret := _m.Called() + + var r0 uint + if rf, ok := ret.Get(0).(func() uint); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint) + } + + return r0 +} + +// Delete provides a mock function with given fields: apiClient +func (_m *ExecutionEnvironmentMock) Delete(apiClient nomad.ExecutorAPI) error { + ret := _m.Called(apiClient) + + var r0 error + if rf, ok := ret.Get(0).(func(nomad.ExecutorAPI) error); ok { + r0 = rf(apiClient) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// DeleteRunner provides a mock function with given fields: id +func (_m *ExecutionEnvironmentMock) DeleteRunner(id string) { + _m.Called(id) +} + +// ID provides a mock function with given fields: +func (_m *ExecutionEnvironmentMock) ID() dto.EnvironmentID { + ret := _m.Called() + + var r0 dto.EnvironmentID + if rf, ok := ret.Get(0).(func() dto.EnvironmentID); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(dto.EnvironmentID) + } + + return r0 +} + +// Image provides a mock function with given fields: +func (_m *ExecutionEnvironmentMock) Image() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// MarshalJSON provides a mock function with given fields: +func (_m *ExecutionEnvironmentMock) MarshalJSON() ([]byte, error) { + ret := _m.Called() + + var r0 []byte + if rf, ok := ret.Get(0).(func() []byte); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]byte) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MemoryLimit provides a mock function with given fields: +func (_m *ExecutionEnvironmentMock) MemoryLimit() uint { + ret := _m.Called() + + var r0 uint + if rf, ok := ret.Get(0).(func() uint); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint) + } + + return r0 +} + +// NetworkAccess provides a mock function with given fields: +func (_m *ExecutionEnvironmentMock) NetworkAccess() (bool, []uint16) { + ret := _m.Called() + + var r0 bool + if rf, ok := ret.Get(0).(func() bool); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(bool) + } + + var r1 []uint16 + if rf, ok := ret.Get(1).(func() []uint16); ok { + r1 = rf() + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).([]uint16) + } + } + + return r0, r1 +} + +// PrewarmingPoolSize provides a mock function with given fields: +func (_m *ExecutionEnvironmentMock) PrewarmingPoolSize() uint { + ret := _m.Called() + + var r0 uint + if rf, ok := ret.Get(0).(func() uint); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint) + } + + return r0 +} + +// Register provides a mock function with given fields: apiClient +func (_m *ExecutionEnvironmentMock) Register(apiClient nomad.ExecutorAPI) error { + ret := _m.Called(apiClient) + + var r0 error + if rf, ok := ret.Get(0).(func(nomad.ExecutorAPI) error); ok { + r0 = rf(apiClient) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Sample provides a mock function with given fields: apiClient +func (_m *ExecutionEnvironmentMock) Sample(apiClient nomad.ExecutorAPI) (Runner, bool) { + ret := _m.Called(apiClient) + + var r0 Runner + if rf, ok := ret.Get(0).(func(nomad.ExecutorAPI) Runner); ok { + r0 = rf(apiClient) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(Runner) + } + } + + var r1 bool + if rf, ok := ret.Get(1).(func(nomad.ExecutorAPI) bool); ok { + r1 = rf(apiClient) + } else { + r1 = ret.Get(1).(bool) + } + + return r0, r1 +} + +// Scale provides a mock function with given fields: apiClient +func (_m *ExecutionEnvironmentMock) Scale(apiClient nomad.ExecutorAPI) error { + ret := _m.Called(apiClient) + + var r0 error + if rf, ok := ret.Get(0).(func(nomad.ExecutorAPI) error); ok { + r0 = rf(apiClient) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SetCPULimit provides a mock function with given fields: limit +func (_m *ExecutionEnvironmentMock) SetCPULimit(limit uint) { + _m.Called(limit) +} + +// SetConfigFrom provides a mock function with given fields: environment +func (_m *ExecutionEnvironmentMock) SetConfigFrom(environment ExecutionEnvironment) { + _m.Called(environment) +} + +// SetID provides a mock function with given fields: id +func (_m *ExecutionEnvironmentMock) SetID(id dto.EnvironmentID) { + _m.Called(id) +} + +// SetImage provides a mock function with given fields: image +func (_m *ExecutionEnvironmentMock) SetImage(image string) { + _m.Called(image) +} + +// SetMemoryLimit provides a mock function with given fields: limit +func (_m *ExecutionEnvironmentMock) SetMemoryLimit(limit uint) { + _m.Called(limit) +} + +// SetNetworkAccess provides a mock function with given fields: allow, ports +func (_m *ExecutionEnvironmentMock) SetNetworkAccess(allow bool, ports []uint16) { + _m.Called(allow, ports) +} + +// SetPrewarmingPoolSize provides a mock function with given fields: count +func (_m *ExecutionEnvironmentMock) SetPrewarmingPoolSize(count uint) { + _m.Called(count) +} + +// UpdateRunnerSpecs provides a mock function with given fields: apiClient +func (_m *ExecutionEnvironmentMock) UpdateRunnerSpecs(apiClient nomad.ExecutorAPI) error { + ret := _m.Called(apiClient) + + var r0 error + if rf, ok := ret.Get(0).(func(nomad.ExecutorAPI) error); ok { + r0 = rf(apiClient) + } else { + r0 = ret.Error(0) + } + + return r0 +} diff --git a/internal/runner/manager.go b/internal/runner/manager.go index b096375..0bf5cc1 100644 --- a/internal/runner/manager.go +++ b/internal/runner/manager.go @@ -2,52 +2,89 @@ package runner import ( "context" + "encoding/json" "errors" "fmt" - "github.com/google/uuid" nomadApi "github.com/hashicorp/nomad/api" "github.com/openHPI/poseidon/internal/nomad" + "github.com/openHPI/poseidon/pkg/dto" "github.com/openHPI/poseidon/pkg/logging" "github.com/sirupsen/logrus" "strconv" - "strings" "time" ) var ( - log = logging.GetLogger("runner") - ErrUnknownExecutionEnvironment = errors.New("execution environment not found") - ErrNoRunnersAvailable = errors.New("no runners available for this execution environment") - ErrRunnerNotFound = errors.New("no runner found with this id") - ErrorUpdatingExecutionEnvironment = errors.New("errors occurred when updating environment") - ErrorInvalidJobID = errors.New("invalid job id") + log = logging.GetLogger("runner") + ErrUnknownExecutionEnvironment = errors.New("execution environment not found") + ErrNoRunnersAvailable = errors.New("no runners available for this execution environment") + ErrRunnerNotFound = errors.New("no runner found with this id") ) -type EnvironmentID int +// ExecutionEnvironment are groups of runner that share the configuration stored in the environment. +type ExecutionEnvironment interface { + json.Marshaler -func NewEnvironmentID(id string) (EnvironmentID, error) { - environment, err := strconv.Atoi(id) - return EnvironmentID(environment), err + // ID returns the id of the environment. + ID() dto.EnvironmentID + SetID(id dto.EnvironmentID) + // PrewarmingPoolSize sets the number of idle runner of this environment that should be prewarmed. + PrewarmingPoolSize() uint + SetPrewarmingPoolSize(count uint) + // CPULimit sets the share of cpu that a runner should receive at minimum. + CPULimit() uint + SetCPULimit(limit uint) + // MemoryLimit sets the amount of memory that should be available for each runner. + MemoryLimit() uint + SetMemoryLimit(limit uint) + // Image sets the image of the runner, e.g. Docker image. + Image() string + SetImage(image string) + // NetworkAccess sets if a runner should have network access and if ports should be mapped. + NetworkAccess() (bool, []uint16) + SetNetworkAccess(allow bool, ports []uint16) + // SetConfigFrom copies all above attributes from the passed environment to the object itself. + SetConfigFrom(environment ExecutionEnvironment) + + // Register saves this environment at the executor. + Register(apiClient nomad.ExecutorAPI) error + // Delete removes this environment and all it's runner from the executor and Poseidon itself. + Delete(apiClient nomad.ExecutorAPI) error + // Scale manages if the executor has enough idle runner according to the PrewarmingPoolSize. + Scale(apiClient nomad.ExecutorAPI) error + // UpdateRunnerSpecs updates all Runner of the passed environment to have the same definition as the environment. + UpdateRunnerSpecs(apiClient nomad.ExecutorAPI) error + + // Sample returns and removes an arbitrary available runner. + // ok is true iff a runner was returned. + Sample(apiClient nomad.ExecutorAPI) (r Runner, ok bool) + // AddRunner adds an existing runner to the idle runners of the environment. + AddRunner(r Runner) + // DeleteRunner removes an idle runner from the environment. + DeleteRunner(id string) } -func (e EnvironmentID) toString() string { - return strconv.Itoa(int(e)) -} - -type NomadJobID string - // Manager keeps track of the used and unused runners of all execution environments in order to provide unused // runners to new clients and ensure no runner is used twice. type Manager interface { - // CreateOrUpdateEnvironment creates the given environment if it does not exist. Otherwise, it updates - // the existing environment and all runners. Iff a new Environment has been created, it returns true. - // Iff scale is true, runners are created until the desiredIdleRunnersCount is reached. - CreateOrUpdateEnvironment(id EnvironmentID, desiredIdleRunnersCount uint, templateJob *nomadApi.Job, - scale bool) (bool, error) + // ListEnvironments returns all execution environments known by Poseidon. + ListEnvironments() []ExecutionEnvironment + + // GetEnvironment returns the details of the requested environment. + // Iff the requested environment is not stored it returns false. + GetEnvironment(id dto.EnvironmentID) (ExecutionEnvironment, bool) + + // SetEnvironment stores the environment in Poseidons memory. + // It returns true iff a new environment is stored and false iff an existing environment was updated. + SetEnvironment(environment ExecutionEnvironment) bool + + // DeleteEnvironment removes the specified execution environment in Poseidons memory. + // It does nothing if the specified environment can not be found. + DeleteEnvironment(id dto.EnvironmentID) // Claim returns a new runner. The runner is deleted after duration seconds if duration is not 0. // It makes sure that the runner is not in use yet and returns an error if no runner could be provided. - Claim(id EnvironmentID, duration int) (Runner, error) + Claim(id dto.EnvironmentID, duration int) (Runner, error) // Get returns the used runner with the given runnerId. // If no runner with the given runnerId is currently used, it returns an error. @@ -64,7 +101,7 @@ type Manager interface { type NomadRunnerManager struct { apiClient nomad.ExecutorAPI - environments NomadEnvironmentStorage + environments EnvironmentStorage usedRunners Storage } @@ -74,107 +111,37 @@ type NomadRunnerManager struct { func NewNomadRunnerManager(apiClient nomad.ExecutorAPI, ctx context.Context) *NomadRunnerManager { m := &NomadRunnerManager{ apiClient, - NewLocalNomadEnvironmentStorage(), + NewLocalEnvironmentStorage(), NewLocalRunnerStorage(), } go m.keepRunnersSynced(ctx) return m } -type NomadEnvironment struct { - environmentID EnvironmentID - idleRunners Storage - desiredIdleRunnersCount uint - templateJob *nomadApi.Job +func (m *NomadRunnerManager) ListEnvironments() []ExecutionEnvironment { + return m.environments.List() } -func (j *NomadEnvironment) ID() EnvironmentID { - return j.environmentID +func (m *NomadRunnerManager) GetEnvironment(id dto.EnvironmentID) (ExecutionEnvironment, bool) { + return m.environments.Get(id) } -func (m *NomadRunnerManager) CreateOrUpdateEnvironment(id EnvironmentID, desiredIdleRunnersCount uint, - templateJob *nomadApi.Job, scale bool) (bool, error) { - _, ok := m.environments.Get(id) - if !ok { - return true, m.registerEnvironment(id, desiredIdleRunnersCount, templateJob, scale) - } - return false, m.updateEnvironment(id, desiredIdleRunnersCount, templateJob, scale) +func (m *NomadRunnerManager) SetEnvironment(environment ExecutionEnvironment) bool { + _, ok := m.environments.Get(environment.ID()) + m.environments.Add(environment) + return !ok } -func (m *NomadRunnerManager) registerEnvironment(environmentID EnvironmentID, desiredIdleRunnersCount uint, - templateJob *nomadApi.Job, scale bool) error { - m.environments.Add(&NomadEnvironment{ - environmentID, - NewLocalRunnerStorage(), - desiredIdleRunnersCount, - templateJob, - }) - if scale { - err := m.scaleEnvironment(environmentID) - if err != nil { - return fmt.Errorf("couldn't upscale environment %w", err) - } - } - return nil +func (m *NomadRunnerManager) DeleteEnvironment(id dto.EnvironmentID) { + m.environments.Delete(id) } -// updateEnvironment updates all runners of the specified environment. This is required as attributes like the -// CPULimit or MemoryMB could be changed in the new template job. -func (m *NomadRunnerManager) updateEnvironment(id EnvironmentID, desiredIdleRunnersCount uint, - newTemplateJob *nomadApi.Job, scale bool) error { - environment, ok := m.environments.Get(id) - if !ok { - return ErrUnknownExecutionEnvironment - } - environment.desiredIdleRunnersCount = desiredIdleRunnersCount - environment.templateJob = newTemplateJob - err := nomad.SetMetaConfigValue(newTemplateJob, nomad.ConfigMetaPoolSizeKey, - strconv.Itoa(int(desiredIdleRunnersCount))) - if err != nil { - return fmt.Errorf("update environment couldn't update template environment: %w", err) - } - - err = m.updateRunnerSpecs(id, newTemplateJob) - if err != nil { - return err - } - - if scale { - err = m.scaleEnvironment(id) - } - return err -} - -func (m *NomadRunnerManager) updateRunnerSpecs(environmentID EnvironmentID, templateJob *nomadApi.Job) error { - runners, err := m.apiClient.LoadRunnerIDs(environmentID.toString()) - if err != nil { - return fmt.Errorf("update environment couldn't load runners: %w", err) - } - - var occurredError error - for _, id := range runners { - // avoid taking the address of the loop variable - runnerID := id - updatedRunnerJob := *templateJob - updatedRunnerJob.ID = &runnerID - updatedRunnerJob.Name = &runnerID - err := m.apiClient.RegisterRunnerJob(&updatedRunnerJob) - if err != nil { - if occurredError == nil { - occurredError = ErrorUpdatingExecutionEnvironment - } - occurredError = fmt.Errorf("%w; new api error for runner %s - %v", occurredError, id, err) - } - } - return occurredError -} - -func (m *NomadRunnerManager) Claim(environmentID EnvironmentID, duration int) (Runner, error) { +func (m *NomadRunnerManager) Claim(environmentID dto.EnvironmentID, duration int) (Runner, error) { environment, ok := m.environments.Get(environmentID) if !ok { return nil, ErrUnknownExecutionEnvironment } - runner, ok := environment.idleRunners.Sample() + runner, ok := environment.Sample(m.apiClient) if !ok { return nil, ErrNoRunnersAvailable } @@ -185,12 +152,6 @@ func (m *NomadRunnerManager) Claim(environmentID EnvironmentID, duration int) (R } runner.SetupTimeout(time.Duration(duration) * time.Second) - - err = m.createRunner(environment) - if err != nil { - log.WithError(err).WithField("environmentID", environmentID).Error("Couldn't create new runner for claimed one") - } - return runner, nil } @@ -204,7 +165,7 @@ func (m *NomadRunnerManager) Get(runnerID string) (Runner, error) { func (m *NomadRunnerManager) Return(r Runner) error { r.StopTimeout() - err := m.apiClient.DeleteRunner(r.ID()) + err := m.apiClient.DeleteJob(r.ID()) if err != nil { return fmt.Errorf("error deleting runner in Nomad: %w", err) } @@ -215,24 +176,23 @@ func (m *NomadRunnerManager) Return(r Runner) error { func (m *NomadRunnerManager) Load() { for _, environment := range m.environments.List() { environmentLogger := log.WithField("environmentID", environment.ID()) - runnerJobs, err := m.apiClient.LoadRunnerJobs(environment.ID().toString()) + runnerJobs, err := m.apiClient.LoadRunnerJobs(environment.ID().ToString()) if err != nil { environmentLogger.WithError(err).Warn("Error fetching the runner jobs") } for _, job := range runnerJobs { m.loadSingleJob(job, environmentLogger, environment) } - err = m.scaleEnvironment(environment.ID()) + err = environment.Scale(m.apiClient) if err != nil { - environmentLogger.Error("Couldn't scale environment") + environmentLogger.WithError(err).Error("Couldn't scale environment") } } } func (m *NomadRunnerManager) loadSingleJob(job *nomadApi.Job, environmentLogger *logrus.Entry, - environment *NomadEnvironment, -) { - configTaskGroup := nomad.FindConfigTaskGroup(job) + environment ExecutionEnvironment) { + configTaskGroup := nomad.FindTaskGroup(job, nomad.ConfigTaskGroupName) if configTaskGroup == nil { environmentLogger.Infof("Couldn't find config task group in job %s, skipping ...", *job.ID) return @@ -253,7 +213,7 @@ func (m *NomadRunnerManager) loadSingleJob(job *nomadApi.Job, environmentLogger newJob.SetupTimeout(time.Duration(timeout) * time.Second) } } else { - environment.idleRunners.Add(newJob) + environment.AddRunner(newJob) } } @@ -270,137 +230,38 @@ func (m *NomadRunnerManager) keepRunnersSynced(ctx context.Context) { func (m *NomadRunnerManager) onAllocationAdded(alloc *nomadApi.Allocation) { log.WithField("id", alloc.JobID).Debug("Runner started") - if IsEnvironmentTemplateID(alloc.JobID) { + if nomad.IsEnvironmentTemplateID(alloc.JobID) { return } - environmentID, err := EnvironmentIDFromJobID(alloc.JobID) + environmentID, err := nomad.EnvironmentIDFromRunnerID(alloc.JobID) if err != nil { log.WithError(err).Warn("Allocation could not be added") return } - job, ok := m.environments.Get(environmentID) + environment, ok := m.environments.Get(environmentID) if ok { var mappedPorts []nomadApi.PortMapping if alloc.AllocatedResources != nil { mappedPorts = alloc.AllocatedResources.Shared.Ports } - job.idleRunners.Add(NewNomadJob(alloc.JobID, mappedPorts, m.apiClient, m)) + environment.AddRunner(NewNomadJob(alloc.JobID, mappedPorts, m.apiClient, m)) } } func (m *NomadRunnerManager) onAllocationStopped(alloc *nomadApi.Allocation) { log.WithField("id", alloc.JobID).Debug("Runner stopped") - environmentID, err := EnvironmentIDFromJobID(alloc.JobID) + environmentID, err := nomad.EnvironmentIDFromRunnerID(alloc.JobID) if err != nil { log.WithError(err).Warn("Stopped allocation can not be handled") return } m.usedRunners.Delete(alloc.JobID) - job, ok := m.environments.Get(environmentID) + environment, ok := m.environments.Get(environmentID) if ok { - job.idleRunners.Delete(alloc.JobID) + environment.DeleteRunner(alloc.JobID) } } - -// scaleEnvironment makes sure that the amount of idle runners is at least the desiredIdleRunnersCount. -func (m *NomadRunnerManager) scaleEnvironment(id EnvironmentID) error { - environment, ok := m.environments.Get(id) - if !ok { - return ErrUnknownExecutionEnvironment - } - - required := int(environment.desiredIdleRunnersCount) - environment.idleRunners.Length() - - if required > 0 { - return m.createRunners(environment, uint(required)) - } else { - return m.removeRunners(environment, uint(-required)) - } -} - -func (m *NomadRunnerManager) createRunners(environment *NomadEnvironment, count uint) error { - log.WithField("runnersRequired", count).WithField("id", environment.ID()).Debug("Creating new runners") - for i := 0; i < int(count); i++ { - err := m.createRunner(environment) - if err != nil { - return fmt.Errorf("couldn't create new runner: %w", err) - } - } - return nil -} - -func (m *NomadRunnerManager) createRunner(environment *NomadEnvironment) error { - newUUID, err := uuid.NewUUID() - if err != nil { - return fmt.Errorf("failed generating runner id: %w", err) - } - newRunnerID := RunnerJobID(environment.ID(), newUUID.String()) - - template := *environment.templateJob - template.ID = &newRunnerID - template.Name = &newRunnerID - - err = m.apiClient.RegisterRunnerJob(&template) - if err != nil { - return fmt.Errorf("error registering new runner job: %w", err) - } - return nil -} - -func (m *NomadRunnerManager) removeRunners(environment *NomadEnvironment, count uint) error { - log.WithField("runnersToDelete", count).WithField("id", environment.ID()).Debug("Removing idle runners") - for i := 0; i < int(count); i++ { - r, ok := environment.idleRunners.Sample() - if !ok { - return fmt.Errorf("could not delete expected idle runner: %w", ErrRunnerNotFound) - } - err := m.apiClient.DeleteRunner(r.ID()) - if err != nil { - return fmt.Errorf("could not delete expected Nomad idle runner: %w", err) - } - } - return nil -} - -// RunnerJobID returns the nomad job id of the runner with the given environmentID and id. -func RunnerJobID(environmentID EnvironmentID, id string) string { - return fmt.Sprintf("%d-%s", environmentID, id) -} - -// EnvironmentIDFromJobID returns the environment id that is part of the passed job id. -func EnvironmentIDFromJobID(jobID string) (EnvironmentID, error) { - parts := strings.Split(jobID, "-") - if len(parts) == 0 { - return 0, fmt.Errorf("empty job id: %w", ErrorInvalidJobID) - } - environmentID, err := strconv.Atoi(parts[0]) - if err != nil { - return 0, fmt.Errorf("invalid environment id par %v: %w", err, ErrorInvalidJobID) - } - return EnvironmentID(environmentID), nil -} - -const templateJobNameParts = 2 - -// TemplateJobID returns the id of the template job for the environment with the given id. -func TemplateJobID(id EnvironmentID) string { - return fmt.Sprintf("%s-%d", nomad.TemplateJobPrefix, id) -} - -// IsEnvironmentTemplateID checks if the passed job id belongs to a template job. -func IsEnvironmentTemplateID(jobID string) bool { - parts := strings.Split(jobID, "-") - return len(parts) == templateJobNameParts && parts[0] == nomad.TemplateJobPrefix -} - -func EnvironmentIDFromTemplateJobID(id string) (string, error) { - parts := strings.Split(id, "-") - if len(parts) < templateJobNameParts { - return "", fmt.Errorf("invalid template job id: %w", ErrorInvalidJobID) - } - return parts[1], nil -} diff --git a/internal/runner/manager_mock.go b/internal/runner/manager_mock.go index 142243c..1edd092 100644 --- a/internal/runner/manager_mock.go +++ b/internal/runner/manager_mock.go @@ -1,9 +1,9 @@ -// Code generated by mockery v0.0.0-dev. DO NOT EDIT. +// Code generated by mockery v2.9.4. DO NOT EDIT. package runner import ( - api "github.com/hashicorp/nomad/api" + dto "github.com/openHPI/poseidon/pkg/dto" mock "github.com/stretchr/testify/mock" ) @@ -13,11 +13,11 @@ type ManagerMock struct { } // Claim provides a mock function with given fields: id, duration -func (_m *ManagerMock) Claim(id EnvironmentID, duration int) (Runner, error) { +func (_m *ManagerMock) Claim(id dto.EnvironmentID, duration int) (Runner, error) { ret := _m.Called(id, duration) var r0 Runner - if rf, ok := ret.Get(0).(func(EnvironmentID, int) Runner); ok { + if rf, ok := ret.Get(0).(func(dto.EnvironmentID, int) Runner); ok { r0 = rf(id, duration) } else { if ret.Get(0) != nil { @@ -26,7 +26,7 @@ func (_m *ManagerMock) Claim(id EnvironmentID, duration int) (Runner, error) { } var r1 error - if rf, ok := ret.Get(1).(func(EnvironmentID, int) error); ok { + if rf, ok := ret.Get(1).(func(dto.EnvironmentID, int) error); ok { r1 = rf(id, duration) } else { r1 = ret.Error(1) @@ -35,25 +35,9 @@ func (_m *ManagerMock) Claim(id EnvironmentID, duration int) (Runner, error) { return r0, r1 } -// CreateOrUpdateEnvironment provides a mock function with given fields: id, desiredIdleRunnersCount, templateJob, scale -func (_m *ManagerMock) CreateOrUpdateEnvironment(id EnvironmentID, desiredIdleRunnersCount uint, templateJob *api.Job, scale bool) (bool, error) { - ret := _m.Called(id, desiredIdleRunnersCount, templateJob, scale) - - var r0 bool - if rf, ok := ret.Get(0).(func(EnvironmentID, uint, *api.Job, bool) bool); ok { - r0 = rf(id, desiredIdleRunnersCount, templateJob, scale) - } else { - r0 = ret.Get(0).(bool) - } - - var r1 error - if rf, ok := ret.Get(1).(func(EnvironmentID, uint, *api.Job, bool) error); ok { - r1 = rf(id, desiredIdleRunnersCount, templateJob, scale) - } else { - r1 = ret.Error(1) - } - - return r0, r1 +// DeleteEnvironment provides a mock function with given fields: id +func (_m *ManagerMock) DeleteEnvironment(id dto.EnvironmentID) { + _m.Called(id) } // Get provides a mock function with given fields: runnerID @@ -79,6 +63,45 @@ func (_m *ManagerMock) Get(runnerID string) (Runner, error) { return r0, r1 } +// GetEnvironment provides a mock function with given fields: id +func (_m *ManagerMock) GetEnvironment(id dto.EnvironmentID) (ExecutionEnvironment, bool) { + ret := _m.Called(id) + + var r0 ExecutionEnvironment + if rf, ok := ret.Get(0).(func(dto.EnvironmentID) ExecutionEnvironment); ok { + r0 = rf(id) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(ExecutionEnvironment) + } + } + + var r1 bool + if rf, ok := ret.Get(1).(func(dto.EnvironmentID) bool); ok { + r1 = rf(id) + } else { + r1 = ret.Get(1).(bool) + } + + return r0, r1 +} + +// ListEnvironments provides a mock function with given fields: +func (_m *ManagerMock) ListEnvironments() []ExecutionEnvironment { + ret := _m.Called() + + var r0 []ExecutionEnvironment + if rf, ok := ret.Get(0).(func() []ExecutionEnvironment); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]ExecutionEnvironment) + } + } + + return r0 +} + // Load provides a mock function with given fields: func (_m *ManagerMock) Load() { _m.Called() @@ -97,3 +120,17 @@ func (_m *ManagerMock) Return(r Runner) error { return r0 } + +// SetEnvironment provides a mock function with given fields: environment +func (_m *ManagerMock) SetEnvironment(environment ExecutionEnvironment) bool { + ret := _m.Called(environment) + + var r0 bool + if rf, ok := ret.Get(0).(func(ExecutionEnvironment) bool); ok { + r0 = rf(environment) + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} diff --git a/internal/runner/manager_test.go b/internal/runner/manager_test.go index c7d3a21..393d0ef 100644 --- a/internal/runner/manager_test.go +++ b/internal/runner/manager_test.go @@ -4,30 +4,26 @@ import ( "context" nomadApi "github.com/hashicorp/nomad/api" "github.com/openHPI/poseidon/internal/nomad" + "github.com/openHPI/poseidon/pkg/dto" "github.com/openHPI/poseidon/tests" - "github.com/openHPI/poseidon/tests/helpers" "github.com/sirupsen/logrus" "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" - "strconv" "testing" "time" ) -const ( - defaultDesiredRunnersCount uint = 5 -) - func TestGetNextRunnerTestSuite(t *testing.T) { suite.Run(t, new(ManagerTestSuite)) } type ManagerTestSuite struct { suite.Suite - apiMock *nomad.ExecutorAPIMock - nomadRunnerManager *NomadRunnerManager - exerciseRunner Runner + apiMock *nomad.ExecutorAPIMock + nomadRunnerManager *NomadRunnerManager + exerciseEnvironment *ExecutionEnvironmentMock + exerciseRunner Runner } func (s *ManagerTestSuite) SetupTest() { @@ -39,7 +35,8 @@ func (s *ManagerTestSuite) SetupTest() { s.nomadRunnerManager = NewNomadRunnerManager(s.apiMock, ctx) s.exerciseRunner = NewRunner(tests.DefaultRunnerID, s.nomadRunnerManager) - s.registerDefaultEnvironment() + s.exerciseEnvironment = &ExecutionEnvironmentMock{} + s.setDefaultEnvironment() } func mockRunnerQueries(apiMock *nomad.ExecutorAPIMock, returnedRunnerIds []string) { @@ -52,44 +49,66 @@ func mockRunnerQueries(apiMock *nomad.ExecutorAPIMock, returnedRunnerIds []strin }) apiMock.On("LoadEnvironmentJobs").Return([]*nomadApi.Job{}, nil) apiMock.On("MarkRunnerAsUsed", mock.AnythingOfType("string"), mock.AnythingOfType("int")).Return(nil) - apiMock.On("LoadRunnerIDs", tests.DefaultJobID).Return(returnedRunnerIds, nil) - apiMock.On("JobScale", tests.DefaultJobID).Return(uint(len(returnedRunnerIds)), nil) - apiMock.On("SetJobScale", tests.DefaultJobID, mock.AnythingOfType("uint"), "Runner Requested").Return(nil) + apiMock.On("LoadRunnerIDs", tests.DefaultRunnerID).Return(returnedRunnerIds, nil) + apiMock.On("JobScale", tests.DefaultRunnerID).Return(uint(len(returnedRunnerIds)), nil) + apiMock.On("SetJobScale", tests.DefaultRunnerID, mock.AnythingOfType("uint"), "Runner Requested").Return(nil) apiMock.On("RegisterRunnerJob", mock.Anything).Return(nil) apiMock.On("MonitorEvaluation", mock.Anything, mock.Anything).Return(nil) } -func (s *ManagerTestSuite) registerDefaultEnvironment() { - err := s.nomadRunnerManager.registerEnvironment(defaultEnvironmentID, 0, &nomadApi.Job{}, true) - s.Require().NoError(err) +func mockIdleRunners(environmentMock *ExecutionEnvironmentMock) { + idleRunner := NewLocalRunnerStorage() + environmentMock.On("AddRunner", mock.Anything).Run(func(args mock.Arguments) { + r, ok := args.Get(0).(Runner) + if !ok { + return + } + idleRunner.Add(r) + }) + sampleCall := environmentMock.On("Sample", mock.Anything) + sampleCall.Run(func(args mock.Arguments) { + r, ok := idleRunner.Sample() + sampleCall.ReturnArguments = mock.Arguments{r, ok} + }) + deleteCall := environmentMock.On("DeleteRunner", mock.AnythingOfType("string")) + deleteCall.Run(func(args mock.Arguments) { + id, ok := args.Get(0).(string) + if !ok { + return + } + idleRunner.Delete(id) + }) } -func (s *ManagerTestSuite) AddIdleRunnerForDefaultEnvironment(r Runner) { - job, _ := s.nomadRunnerManager.environments.Get(defaultEnvironmentID) - job.idleRunners.Add(r) +func (s *ManagerTestSuite) setDefaultEnvironment() { + s.exerciseEnvironment.On("ID").Return(defaultEnvironmentID) + created := s.nomadRunnerManager.SetEnvironment(s.exerciseEnvironment) + s.Require().True(created) } func (s *ManagerTestSuite) waitForRunnerRefresh() { <-time.After(100 * time.Millisecond) } -func (s *ManagerTestSuite) TestRegisterEnvironmentAddsNewJob() { - err := s.nomadRunnerManager. - registerEnvironment(anotherEnvironmentID, defaultDesiredRunnersCount, &nomadApi.Job{}, true) - s.Require().NoError(err) - job, ok := s.nomadRunnerManager.environments.Get(defaultEnvironmentID) +func (s *ManagerTestSuite) TestSetEnvironmentAddsNewEnvironment() { + anotherEnvironment := &ExecutionEnvironmentMock{} + anotherEnvironment.On("ID").Return(anotherEnvironmentID) + created := s.nomadRunnerManager.SetEnvironment(anotherEnvironment) + s.Require().True(created) + + job, ok := s.nomadRunnerManager.environments.Get(anotherEnvironmentID) s.True(ok) s.NotNil(job) } func (s *ManagerTestSuite) TestClaimReturnsNotFoundErrorIfEnvironmentNotFound() { - runner, err := s.nomadRunnerManager.Claim(EnvironmentID(42), defaultInactivityTimeout) + runner, err := s.nomadRunnerManager.Claim(anotherEnvironmentID, defaultInactivityTimeout) s.Nil(runner) s.Equal(ErrUnknownExecutionEnvironment, err) } func (s *ManagerTestSuite) TestClaimReturnsRunnerIfAvailable() { - s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner) + s.exerciseEnvironment.On("Sample", mock.Anything).Return(s.exerciseRunner, true) receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID, defaultInactivityTimeout) s.NoError(err) s.Equal(s.exerciseRunner, receivedRunner) @@ -97,21 +116,23 @@ func (s *ManagerTestSuite) TestClaimReturnsRunnerIfAvailable() { func (s *ManagerTestSuite) TestClaimReturnsErrorIfNoRunnerAvailable() { s.waitForRunnerRefresh() + s.exerciseEnvironment.On("Sample", mock.Anything).Return(nil, false) runner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID, defaultInactivityTimeout) s.Nil(runner) s.Equal(ErrNoRunnersAvailable, err) } func (s *ManagerTestSuite) TestClaimReturnsNoRunnerOfDifferentEnvironment() { - s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner) + s.exerciseEnvironment.On("Sample", mock.Anything).Return(s.exerciseRunner, true) receivedRunner, err := s.nomadRunnerManager.Claim(anotherEnvironmentID, defaultInactivityTimeout) s.Nil(receivedRunner) s.Error(err) } func (s *ManagerTestSuite) TestClaimDoesNotReturnTheSameRunnerTwice() { - s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner) - s.AddIdleRunnerForDefaultEnvironment(NewRunner(tests.AnotherRunnerID, s.nomadRunnerManager)) + s.exerciseEnvironment.On("Sample", mock.Anything).Return(s.exerciseRunner, true).Once() + s.exerciseEnvironment.On("Sample", mock.Anything). + Return(NewRunner(tests.AnotherRunnerID, s.nomadRunnerManager), true).Once() firstReceivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID, defaultInactivityTimeout) s.NoError(err) @@ -120,14 +141,8 @@ func (s *ManagerTestSuite) TestClaimDoesNotReturnTheSameRunnerTwice() { s.NotEqual(firstReceivedRunner, secondReceivedRunner) } -func (s *ManagerTestSuite) TestClaimThrowsAnErrorIfNoRunnersAvailable() { - receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID, defaultInactivityTimeout) - s.Nil(receivedRunner) - s.Error(err) -} - func (s *ManagerTestSuite) TestClaimAddsRunnerToUsedRunners() { - s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner) + s.exerciseEnvironment.On("Sample", mock.Anything).Return(s.exerciseRunner, true) receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID, defaultInactivityTimeout) s.Require().NoError(err) savedRunner, ok := s.nomadRunnerManager.usedRunners.Get(receivedRunner.ID()) @@ -135,16 +150,6 @@ func (s *ManagerTestSuite) TestClaimAddsRunnerToUsedRunners() { s.Equal(savedRunner, receivedRunner) } -func (s *ManagerTestSuite) TestTwoClaimsAddExactlyTwoRunners() { - s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner) - s.AddIdleRunnerForDefaultEnvironment(NewRunner(tests.AnotherRunnerID, s.nomadRunnerManager)) - _, err := s.nomadRunnerManager.Claim(defaultEnvironmentID, defaultInactivityTimeout) - s.Require().NoError(err) - _, err = s.nomadRunnerManager.Claim(defaultEnvironmentID, defaultInactivityTimeout) - s.Require().NoError(err) - s.apiMock.AssertNumberOfCalls(s.T(), "RegisterRunnerJob", 2) -} - func (s *ManagerTestSuite) TestGetReturnsRunnerIfRunnerIsUsed() { s.nomadRunnerManager.usedRunners.Add(s.exerciseRunner) savedRunner, err := s.nomadRunnerManager.Get(s.exerciseRunner.ID()) @@ -159,7 +164,7 @@ func (s *ManagerTestSuite) TestGetReturnsErrorIfRunnerNotFound() { } func (s *ManagerTestSuite) TestReturnRemovesRunnerFromUsedRunners() { - s.apiMock.On("DeleteRunner", mock.AnythingOfType("string")).Return(nil) + s.apiMock.On("DeleteJob", mock.AnythingOfType("string")).Return(nil) s.nomadRunnerManager.usedRunners.Add(s.exerciseRunner) err := s.nomadRunnerManager.Return(s.exerciseRunner) s.Nil(err) @@ -168,14 +173,14 @@ func (s *ManagerTestSuite) TestReturnRemovesRunnerFromUsedRunners() { } func (s *ManagerTestSuite) TestReturnCallsDeleteRunnerApiMethod() { - s.apiMock.On("DeleteRunner", mock.AnythingOfType("string")).Return(nil) + s.apiMock.On("DeleteJob", mock.AnythingOfType("string")).Return(nil) err := s.nomadRunnerManager.Return(s.exerciseRunner) s.Nil(err) - s.apiMock.AssertCalled(s.T(), "DeleteRunner", s.exerciseRunner.ID()) + s.apiMock.AssertCalled(s.T(), "DeleteJob", s.exerciseRunner.ID()) } func (s *ManagerTestSuite) TestReturnReturnsErrorWhenApiCallFailed() { - s.apiMock.On("DeleteRunner", mock.AnythingOfType("string")).Return(tests.ErrDefault) + s.apiMock.On("DeleteJob", mock.AnythingOfType("string")).Return(tests.ErrDefault) err := s.nomadRunnerManager.Return(s.exerciseRunner) s.Error(err) } @@ -204,9 +209,10 @@ func (s *ManagerTestSuite) TestUpdateRunnersAddsIdleRunner() { allocation := &nomadApi.Allocation{ID: tests.DefaultRunnerID} environment, ok := s.nomadRunnerManager.environments.Get(defaultEnvironmentID) s.Require().True(ok) - allocation.JobID = environment.environmentID.toString() + allocation.JobID = environment.ID().ToString() + mockIdleRunners(environment.(*ExecutionEnvironmentMock)) - _, ok = environment.idleRunners.Get(allocation.ID) + _, ok = environment.Sample(s.apiMock) s.Require().False(ok) modifyMockedCall(s.apiMock, "WatchAllocations", func(call *mock.Call) { @@ -223,17 +229,18 @@ func (s *ManagerTestSuite) TestUpdateRunnersAddsIdleRunner() { go s.nomadRunnerManager.keepRunnersSynced(ctx) <-time.After(10 * time.Millisecond) - _, ok = environment.idleRunners.Get(allocation.JobID) + _, ok = environment.Sample(s.apiMock) s.True(ok) } func (s *ManagerTestSuite) TestUpdateRunnersRemovesIdleAndUsedRunner() { - allocation := &nomadApi.Allocation{JobID: tests.DefaultJobID} + allocation := &nomadApi.Allocation{JobID: tests.DefaultRunnerID} environment, ok := s.nomadRunnerManager.environments.Get(defaultEnvironmentID) s.Require().True(ok) + mockIdleRunners(environment.(*ExecutionEnvironmentMock)) testRunner := NewRunner(allocation.JobID, s.nomadRunnerManager) - environment.idleRunners.Add(testRunner) + environment.AddRunner(testRunner) s.nomadRunnerManager.usedRunners.Add(testRunner) modifyMockedCall(s.apiMock, "WatchAllocations", func(call *mock.Call) { @@ -250,33 +257,12 @@ func (s *ManagerTestSuite) TestUpdateRunnersRemovesIdleAndUsedRunner() { go s.nomadRunnerManager.keepRunnersSynced(ctx) <-time.After(10 * time.Millisecond) - _, ok = environment.idleRunners.Get(allocation.JobID) + _, ok = environment.Sample(s.apiMock) s.False(ok) _, ok = s.nomadRunnerManager.usedRunners.Get(allocation.JobID) s.False(ok) } -func (s *ManagerTestSuite) TestUpdateEnvironmentRemovesIdleRunnersWhenScalingDown() { - _, job := helpers.CreateTemplateJob() - initialRunners := uint(40) - updatedRunners := uint(10) - err := s.nomadRunnerManager.registerEnvironment(anotherEnvironmentID, initialRunners, job, true) - s.Require().NoError(err) - s.apiMock.AssertNumberOfCalls(s.T(), "RegisterRunnerJob", int(initialRunners)) - environment, ok := s.nomadRunnerManager.environments.Get(anotherEnvironmentID) - s.Require().True(ok) - for i := 0; i < int(initialRunners); i++ { - environment.idleRunners.Add(NewRunner("active-runner-"+strconv.Itoa(i), s.nomadRunnerManager)) - } - - s.apiMock.On("LoadRunnerIDs", anotherEnvironmentID.toString()).Return([]string{}, nil) - s.apiMock.On("DeleteRunner", mock.AnythingOfType("string")).Return(nil) - - err = s.nomadRunnerManager.updateEnvironment(tests.AnotherEnvironmentIDAsInteger, updatedRunners, job, true) - s.Require().NoError(err) - s.apiMock.AssertNumberOfCalls(s.T(), "DeleteRunner", int(initialRunners-updatedRunners)) -} - func modifyMockedCall(apiMock *nomad.ExecutorAPIMock, method string, modifier func(call *mock.Call)) { for _, c := range apiMock.ExpectedCalls { if c.Method == method { @@ -286,13 +272,16 @@ func modifyMockedCall(apiMock *nomad.ExecutorAPIMock, method string, modifier fu } func (s *ManagerTestSuite) TestOnAllocationAdded() { - s.registerDefaultEnvironment() s.Run("does not add environment template id job", func() { - alloc := &nomadApi.Allocation{JobID: TemplateJobID(tests.DefaultEnvironmentIDAsInteger)} - s.nomadRunnerManager.onAllocationAdded(alloc) - job, ok := s.nomadRunnerManager.environments.Get(tests.DefaultEnvironmentIDAsInteger) + environment, ok := s.nomadRunnerManager.environments.Get(tests.DefaultEnvironmentIDAsInteger) s.True(ok) - s.Zero(job.idleRunners.Length()) + mockIdleRunners(environment.(*ExecutionEnvironmentMock)) + + alloc := &nomadApi.Allocation{JobID: nomad.TemplateJobID(tests.DefaultEnvironmentIDAsInteger)} + s.nomadRunnerManager.onAllocationAdded(alloc) + + _, ok = environment.Sample(s.apiMock) + s.False(ok) }) s.Run("does not panic when environment id cannot be parsed", func() { alloc := &nomadApi.Allocation{JobID: ""} @@ -301,46 +290,52 @@ func (s *ManagerTestSuite) TestOnAllocationAdded() { }) }) s.Run("does not panic when environment does not exist", func() { - nonExistentEnvironment := EnvironmentID(1234) + nonExistentEnvironment := dto.EnvironmentID(1234) _, ok := s.nomadRunnerManager.environments.Get(nonExistentEnvironment) s.Require().False(ok) - alloc := &nomadApi.Allocation{JobID: RunnerJobID(nonExistentEnvironment, "1-1-1-1")} + alloc := &nomadApi.Allocation{JobID: nomad.RunnerJobID(nonExistentEnvironment, "1-1-1-1")} s.NotPanics(func() { s.nomadRunnerManager.onAllocationAdded(alloc) }) }) s.Run("adds correct job", func() { s.Run("without allocated resources", func() { + environment, ok := s.nomadRunnerManager.environments.Get(tests.DefaultEnvironmentIDAsInteger) + s.True(ok) + mockIdleRunners(environment.(*ExecutionEnvironmentMock)) + alloc := &nomadApi.Allocation{ - JobID: tests.DefaultJobID, + JobID: tests.DefaultRunnerID, AllocatedResources: nil, } s.nomadRunnerManager.onAllocationAdded(alloc) - job, ok := s.nomadRunnerManager.environments.Get(tests.DefaultEnvironmentIDAsInteger) - s.True(ok) - runner, ok := job.idleRunners.Get(tests.DefaultJobID) + + runner, ok := environment.Sample(s.apiMock) s.True(ok) nomadJob, ok := runner.(*NomadJob) s.True(ok) - s.Equal(nomadJob.id, tests.DefaultJobID) + s.Equal(nomadJob.id, tests.DefaultRunnerID) s.Empty(nomadJob.portMappings) }) s.Run("with mapped ports", func() { + environment, ok := s.nomadRunnerManager.environments.Get(tests.DefaultEnvironmentIDAsInteger) + s.True(ok) + mockIdleRunners(environment.(*ExecutionEnvironmentMock)) + alloc := &nomadApi.Allocation{ - JobID: tests.DefaultJobID, + JobID: tests.DefaultRunnerID, AllocatedResources: &nomadApi.AllocatedResources{ Shared: nomadApi.AllocatedSharedResources{Ports: tests.DefaultPortMappings}, }, } s.nomadRunnerManager.onAllocationAdded(alloc) - job, ok := s.nomadRunnerManager.environments.Get(tests.DefaultEnvironmentIDAsInteger) - s.True(ok) - runner, ok := job.idleRunners.Get(tests.DefaultJobID) + + runner, ok := environment.Sample(s.apiMock) s.True(ok) nomadJob, ok := runner.(*NomadJob) s.True(ok) - s.Equal(nomadJob.id, tests.DefaultJobID) + s.Equal(nomadJob.id, tests.DefaultRunnerID) s.Equal(nomadJob.portMappings, tests.DefaultPortMappings) }) }) diff --git a/internal/runner/nomad_environment_storage.go b/internal/runner/nomad_environment_storage.go index 94c6e92..c376eb3 100644 --- a/internal/runner/nomad_environment_storage.go +++ b/internal/runner/nomad_environment_storage.go @@ -1,74 +1,75 @@ package runner import ( + "github.com/openHPI/poseidon/pkg/dto" "sync" ) -// NomadEnvironmentStorage is an interface for storing Nomad environments. -type NomadEnvironmentStorage interface { +// EnvironmentStorage is an interface for storing environments. +type EnvironmentStorage interface { // List returns all environments stored in this storage. - List() []*NomadEnvironment + List() []ExecutionEnvironment // Add adds an environment to the storage. // It overwrites the old environment if one with the same id was already stored. - Add(environment *NomadEnvironment) + Add(environment ExecutionEnvironment) // Get returns an environment from the storage. // Iff the environment does not exist in the store, ok will be false. - Get(id EnvironmentID) (environment *NomadEnvironment, ok bool) + Get(id dto.EnvironmentID) (environment ExecutionEnvironment, ok bool) // Delete deletes the environment with the passed id from the storage. It does nothing if no environment with the id // is present in the storage. - Delete(id EnvironmentID) + Delete(id dto.EnvironmentID) // Length returns the number of currently stored environments in the storage. Length() int } -// localNomadEnvironmentStorage stores NomadEnvironment objects in the local application memory. -type localNomadEnvironmentStorage struct { +// localEnvironmentStorage stores ExecutionEnvironment objects in the local application memory. +type localEnvironmentStorage struct { sync.RWMutex - environments map[EnvironmentID]*NomadEnvironment + environments map[dto.EnvironmentID]ExecutionEnvironment } -// NewLocalNomadEnvironmentStorage responds with an empty localNomadEnvironmentStorage. +// NewLocalEnvironmentStorage responds with an empty localEnvironmentStorage. // This implementation stores the data thread-safe in the local application memory. -func NewLocalNomadEnvironmentStorage() *localNomadEnvironmentStorage { - return &localNomadEnvironmentStorage{ - environments: make(map[EnvironmentID]*NomadEnvironment), +func NewLocalEnvironmentStorage() *localEnvironmentStorage { + return &localEnvironmentStorage{ + environments: make(map[dto.EnvironmentID]ExecutionEnvironment), } } -func (s *localNomadEnvironmentStorage) List() []*NomadEnvironment { +func (s *localEnvironmentStorage) List() []ExecutionEnvironment { s.RLock() defer s.RUnlock() - values := make([]*NomadEnvironment, 0, len(s.environments)) + values := make([]ExecutionEnvironment, 0, len(s.environments)) for _, v := range s.environments { values = append(values, v) } return values } -func (s *localNomadEnvironmentStorage) Add(environment *NomadEnvironment) { +func (s *localEnvironmentStorage) Add(environment ExecutionEnvironment) { s.Lock() defer s.Unlock() s.environments[environment.ID()] = environment } -func (s *localNomadEnvironmentStorage) Get(id EnvironmentID) (environment *NomadEnvironment, ok bool) { +func (s *localEnvironmentStorage) Get(id dto.EnvironmentID) (environment ExecutionEnvironment, ok bool) { s.RLock() defer s.RUnlock() environment, ok = s.environments[id] return } -func (s *localNomadEnvironmentStorage) Delete(id EnvironmentID) { +func (s *localEnvironmentStorage) Delete(id dto.EnvironmentID) { s.Lock() defer s.Unlock() delete(s.environments, id) } -func (s *localNomadEnvironmentStorage) Length() int { +func (s *localEnvironmentStorage) Length() int { s.RLock() defer s.RUnlock() return len(s.environments) diff --git a/internal/runner/nomad_environment_storage_test.go b/internal/runner/nomad_environment_storage_test.go index ae4bcc0..1b89152 100644 --- a/internal/runner/nomad_environment_storage_test.go +++ b/internal/runner/nomad_environment_storage_test.go @@ -1,7 +1,6 @@ package runner import ( - nomadApi "github.com/hashicorp/nomad/api" "github.com/stretchr/testify/suite" "testing" ) @@ -12,13 +11,15 @@ func TestEnvironmentStoreTestSuite(t *testing.T) { type EnvironmentStoreTestSuite struct { suite.Suite - environmentStorage *localNomadEnvironmentStorage - environment *NomadEnvironment + environmentStorage *localEnvironmentStorage + environment *ExecutionEnvironmentMock } func (s *EnvironmentStoreTestSuite) SetupTest() { - s.environmentStorage = NewLocalNomadEnvironmentStorage() - s.environment = &NomadEnvironment{environmentID: defaultEnvironmentID} + s.environmentStorage = NewLocalEnvironmentStorage() + environmentMock := &ExecutionEnvironmentMock{} + environmentMock.On("ID").Return(defaultEnvironmentID) + s.environment = environmentMock } func (s *EnvironmentStoreTestSuite) TestAddedEnvironmentCanBeRetrieved() { @@ -29,8 +30,8 @@ func (s *EnvironmentStoreTestSuite) TestAddedEnvironmentCanBeRetrieved() { } func (s *EnvironmentStoreTestSuite) TestEnvironmentWithSameIdOverwritesOldOne() { - otherEnvironmentWithSameID := &NomadEnvironment{environmentID: defaultEnvironmentID} - otherEnvironmentWithSameID.templateJob = &nomadApi.Job{} + otherEnvironmentWithSameID := &ExecutionEnvironmentMock{} + otherEnvironmentWithSameID.On("ID").Return(defaultEnvironmentID) s.NotEqual(s.environment, otherEnvironmentWithSameID) s.environmentStorage.Add(s.environment) @@ -64,7 +65,8 @@ func (s *EnvironmentStoreTestSuite) TestLenChangesOnStoreContentChange() { }) s.Run("len increases again when different environment is added", func() { - anotherEnvironment := &NomadEnvironment{environmentID: anotherEnvironmentID} + anotherEnvironment := &ExecutionEnvironmentMock{} + anotherEnvironment.On("ID").Return(anotherEnvironmentID) s.environmentStorage.Add(anotherEnvironment) s.Equal(2, s.environmentStorage.Length()) }) @@ -74,3 +76,28 @@ func (s *EnvironmentStoreTestSuite) TestLenChangesOnStoreContentChange() { s.Equal(1, s.environmentStorage.Length()) }) } + +func (s *EnvironmentStoreTestSuite) TestListEnvironments() { + s.Run("list returns empty array", func() { + environments := s.environmentStorage.List() + s.Empty(environments) + }) + + s.Run("list returns one environment", func() { + s.environmentStorage.Add(s.environment) + + environments := s.environmentStorage.List() + s.Equal(1, len(environments)) + s.Equal(defaultEnvironmentID, environments[0].ID()) + }) + + s.Run("list returns multiple environments", func() { + anotherEnvironment := &ExecutionEnvironmentMock{} + anotherEnvironment.On("ID").Return(anotherEnvironmentID) + s.environmentStorage.Add(s.environment) + s.environmentStorage.Add(anotherEnvironment) + + environments := s.environmentStorage.List() + s.Equal(2, len(environments)) + }) +} diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index 70fc9ab..b36ba0b 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -25,27 +25,27 @@ import ( const defaultExecutionID = "execution-id" func TestIdIsStored(t *testing.T) { - runner := NewNomadJob(tests.DefaultJobID, nil, nil, nil) - assert.Equal(t, tests.DefaultJobID, runner.ID()) + runner := NewNomadJob(tests.DefaultRunnerID, nil, nil, nil) + assert.Equal(t, tests.DefaultRunnerID, runner.ID()) } func TestMappedPortsAreStoredCorrectly(t *testing.T) { - runner := NewNomadJob(tests.DefaultJobID, tests.DefaultPortMappings, nil, nil) + runner := NewNomadJob(tests.DefaultRunnerID, tests.DefaultPortMappings, nil, nil) assert.Equal(t, tests.DefaultMappedPorts, runner.MappedPorts()) - runner = NewNomadJob(tests.DefaultJobID, nil, nil, nil) + runner = NewNomadJob(tests.DefaultRunnerID, nil, nil, nil) assert.Empty(t, runner.MappedPorts()) } func TestMarshalRunner(t *testing.T) { - runner := NewNomadJob(tests.DefaultJobID, nil, nil, nil) + runner := NewNomadJob(tests.DefaultRunnerID, nil, nil, nil) marshal, err := json.Marshal(runner) assert.NoError(t, err) - assert.Equal(t, "{\"runnerId\":\""+tests.DefaultJobID+"\"}", string(marshal)) + assert.Equal(t, "{\"runnerId\":\""+tests.DefaultRunnerID+"\"}", string(marshal)) } func TestExecutionRequestIsStored(t *testing.T) { - runner := NewNomadJob(tests.DefaultJobID, nil, nil, nil) + runner := NewNomadJob(tests.DefaultRunnerID, nil, nil, nil) executionRequest := &dto.ExecutionRequest{ Command: "command", TimeLimit: 10, diff --git a/internal/runner/storage.go b/internal/runner/storage.go index 519f7bd..74452b4 100644 --- a/internal/runner/storage.go +++ b/internal/runner/storage.go @@ -6,7 +6,7 @@ import ( // Storage is an interface for storing runners. type Storage interface { - // Add adds an runner to the storage. + // Add adds a runner to the storage. // It overwrites the old runner if one with the same id was already stored. Add(Runner) diff --git a/pkg/dto/dto.go b/pkg/dto/dto.go index ed1a7ea..1291889 100644 --- a/pkg/dto/dto.go +++ b/pkg/dto/dto.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "path" + "strconv" "strings" ) @@ -31,6 +32,27 @@ func (er *ExecutionRequest) FullCommand() []string { return command } +// EnvironmentID is an id of an environment. +type EnvironmentID int + +// NewEnvironmentID parses a string into an EnvironmentID. +func NewEnvironmentID(id string) (EnvironmentID, error) { + environment, err := strconv.Atoi(id) + return EnvironmentID(environment), err +} + +// ToString pareses an EnvironmentID back to a string. +func (e EnvironmentID) ToString() string { + return strconv.Itoa(int(e)) +} + +// ExecutionEnvironmentData is the expected json structure of the response body +// for routes returning an execution environment. +type ExecutionEnvironmentData struct { + ExecutionEnvironmentRequest + ID int `json:"id"` +} + // ExecutionEnvironmentRequest is the expected json structure of the request body // for the create execution environment function. type ExecutionEnvironmentRequest struct { diff --git a/tests/constants.go b/tests/constants.go index 26a0e34..a32bbe3 100644 --- a/tests/constants.go +++ b/tests/constants.go @@ -20,10 +20,8 @@ const ( AnotherEnvironmentIDAsString = "42" DefaultUUID = "MY-DEFAULT-RANDOM-UUID" AnotherUUID = "another-uuid-43" - DefaultJobID = DefaultEnvironmentIDAsString + "-" + DefaultUUID - AnotherJobID = AnotherEnvironmentIDAsString + "-" + AnotherUUID - DefaultRunnerID = DefaultJobID - AnotherRunnerID = AnotherJobID + DefaultRunnerID = DefaultEnvironmentIDAsString + "-" + DefaultUUID + AnotherRunnerID = AnotherEnvironmentIDAsString + "-" + AnotherUUID DefaultExecutionID = "s0m3-3x3cu710n-1d" DefaultMockID = "m0ck-1d" ShortTimeout = 100 * time.Millisecond diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 235aee0..37adde2 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -65,7 +65,7 @@ func TestMain(m *testing.M) { <-time.After(10 * time.Second) code := m.Run() - cleanupJobsForEnvironment(&testing.T{}, "0") + cleanupJobsForEnvironment(&testing.T{}, tests.DefaultEnvironmentIDAsString) os.Exit(code) } diff --git a/tests/e2e/environments_test.go b/tests/e2e/environments_test.go index 8cecb81..ac30951 100644 --- a/tests/e2e/environments_test.go +++ b/tests/e2e/environments_test.go @@ -1,9 +1,10 @@ package e2e import ( + "encoding/json" nomadApi "github.com/hashicorp/nomad/api" "github.com/openHPI/poseidon/internal/api" - "github.com/openHPI/poseidon/internal/runner" + "github.com/openHPI/poseidon/internal/nomad" "github.com/openHPI/poseidon/pkg/dto" "github.com/openHPI/poseidon/tests" "github.com/openHPI/poseidon/tests/helpers" @@ -65,7 +66,213 @@ func TestCreateOrUpdateEnvironment(t *testing.T) { validateJob(t, request) }) - cleanupJobsForEnvironment(t, tests.AnotherEnvironmentIDAsString) + deleteEnvironment(t, tests.AnotherEnvironmentIDAsString) +} + +func TestListEnvironments(t *testing.T) { + path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath) + + t.Run("returns list with one element", func(t *testing.T) { + response, err := http.Get(path) //nolint:gosec // because we build this path right above + require.NoError(t, err) + + assert.Equal(t, http.StatusOK, response.StatusCode) + environmentsArray := assertEnvironmentArrayInResponse(t, response) + assert.Equal(t, 1, len(environmentsArray)) + }) + + t.Run("returns list including the default environment", func(t *testing.T) { + response, err := http.Get(path) //nolint:gosec // because we build this path right above + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + + environmentsArray := assertEnvironmentArrayInResponse(t, response) + require.Equal(t, 1, len(environmentsArray)) + + assertEnvironment(t, environmentsArray[0], tests.DefaultEnvironmentIDAsInteger) + }) + + t.Run("Added environments can be retrieved without fetch", func(t *testing.T) { + createEnvironment(t, tests.AnotherEnvironmentIDAsString) + + response, err := http.Get(path) //nolint:gosec // because we build this path right above + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + + environmentsArray := assertEnvironmentArrayInResponse(t, response) + require.Equal(t, 2, len(environmentsArray)) + foundIDs := parseIDsFromEnvironments(t, environmentsArray) + assert.Contains(t, foundIDs, dto.EnvironmentID(tests.AnotherEnvironmentIDAsInteger)) + }) + deleteEnvironment(t, tests.AnotherEnvironmentIDAsString) + + t.Run("Added environments can be retrieved with fetch", func(t *testing.T) { + // Add environment without Poseidon + _, job := helpers.CreateTemplateJob() + jobID := nomad.TemplateJobID(tests.AnotherEnvironmentIDAsInteger) + job.ID = &jobID + job.Name = &jobID + _, _, err := nomadClient.Jobs().Register(job, nil) + require.NoError(t, err) + + // List without fetch should not include the added environment + response, err := http.Get(path) //nolint:gosec // because we build this path right above + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + environmentsArray := assertEnvironmentArrayInResponse(t, response) + require.Equal(t, 1, len(environmentsArray)) + assertEnvironment(t, environmentsArray[0], tests.DefaultEnvironmentIDAsInteger) + + // List with fetch should include the added environment + response, err = http.Get(path + "?fetch=true") //nolint:gosec // because we build this path right above + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + environmentsArray = assertEnvironmentArrayInResponse(t, response) + require.Equal(t, 2, len(environmentsArray)) + foundIDs := parseIDsFromEnvironments(t, environmentsArray) + assert.Contains(t, foundIDs, dto.EnvironmentID(tests.AnotherEnvironmentIDAsInteger)) + }) + deleteEnvironment(t, tests.AnotherEnvironmentIDAsString) +} + +func TestGetEnvironment(t *testing.T) { + t.Run("returns the default environment", func(t *testing.T) { + path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath, tests.DefaultEnvironmentIDAsString) + response, err := http.Get(path) //nolint:gosec // because we build this path right above + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + + environment := getEnvironmentFromResponse(t, response) + assertEnvironment(t, environment, tests.DefaultEnvironmentIDAsInteger) + }) + + t.Run("Added environments can be retrieved without fetch", func(t *testing.T) { + createEnvironment(t, tests.AnotherEnvironmentIDAsString) + + path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath, tests.AnotherEnvironmentIDAsString) + response, err := http.Get(path) //nolint:gosec // because we build this path right above + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + + environment := getEnvironmentFromResponse(t, response) + assertEnvironment(t, environment, tests.AnotherEnvironmentIDAsInteger) + }) + deleteEnvironment(t, tests.AnotherEnvironmentIDAsString) + + t.Run("Added environments can be retrieved with fetch", func(t *testing.T) { + // Add environment without Poseidon + _, job := helpers.CreateTemplateJob() + jobID := nomad.TemplateJobID(tests.AnotherEnvironmentIDAsInteger) + job.ID = &jobID + job.Name = &jobID + _, _, err := nomadClient.Jobs().Register(job, nil) + require.NoError(t, err) + + // List without fetch should not include the added environment + path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath, tests.AnotherEnvironmentIDAsString) + response, err := http.Get(path) //nolint:gosec // because we build this path right above + require.NoError(t, err) + require.Equal(t, http.StatusNotFound, response.StatusCode) + + // List with fetch should include the added environment + response, err = http.Get(path + "?fetch=true") //nolint:gosec // because we build this path right above + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + environment := getEnvironmentFromResponse(t, response) + assertEnvironment(t, environment, tests.AnotherEnvironmentIDAsInteger) + }) + deleteEnvironment(t, tests.AnotherEnvironmentIDAsString) +} + +func TestDeleteEnvironment(t *testing.T) { + t.Run("Removes added environment", func(t *testing.T) { + createEnvironment(t, tests.AnotherEnvironmentIDAsString) + + path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath, tests.AnotherEnvironmentIDAsString) + response, err := helpers.HTTPDelete(path, nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusNoContent, response.StatusCode) + }) + + t.Run("Removes Nomad Job", func(t *testing.T) { + createEnvironment(t, tests.AnotherEnvironmentIDAsString) + + // Expect created Nomad job + jobID := nomad.TemplateJobID(tests.AnotherEnvironmentIDAsInteger) + job, _, err := nomadClient.Jobs().Info(jobID, nil) + assert.NoError(t, err) + assert.Equal(t, jobID, *job.ID) + + // Delete the job + path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath, tests.AnotherEnvironmentIDAsString) + response, err := helpers.HTTPDelete(path, nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusNoContent, response.StatusCode) + + // Expect not to find the Nomad job + _, _, err = nomadClient.Jobs().Info(jobID, nil) + assert.Error(t, err) + }) +} + +func parseIDsFromEnvironments(t *testing.T, environments []interface{}) (ids []dto.EnvironmentID) { + t.Helper() + for _, environment := range environments { + id, _ := parseEnvironment(t, environment) + ids = append(ids, id) + } + return ids +} + +func assertEnvironment(t *testing.T, environment interface{}, expectedID dto.EnvironmentID) { + t.Helper() + id, defaultEnvironmentParams := parseEnvironment(t, environment) + + assert.Equal(t, expectedID, id) + expectedKeys := []string{"prewarmingPoolSize", "cpuLimit", "memoryLimit", "image", "networkAccess", "exposedPorts"} + for _, key := range expectedKeys { + _, ok := defaultEnvironmentParams[key] + assert.True(t, ok) + } +} + +func parseEnvironment(t *testing.T, environment interface{}) (id dto.EnvironmentID, params map[string]interface{}) { + t.Helper() + environmentParams, ok := environment.(map[string]interface{}) + require.True(t, ok) + idInterface, ok := environmentParams["id"] + require.True(t, ok) + idFloat, ok := idInterface.(float64) + require.True(t, ok) + return dto.EnvironmentID(int(idFloat)), environmentParams +} + +func assertEnvironmentArrayInResponse(t *testing.T, response *http.Response) []interface{} { + t.Helper() + paramMap := make(map[string]interface{}) + err := json.NewDecoder(response.Body).Decode(¶mMap) + require.NoError(t, err) + environments, ok := paramMap["executionEnvironments"] + assert.True(t, ok) + environmentsArray, ok := environments.([]interface{}) + assert.True(t, ok) + return environmentsArray +} + +func getEnvironmentFromResponse(t *testing.T, response *http.Response) interface{} { + t.Helper() + var environment interface{} + err := json.NewDecoder(response.Body).Decode(&environment) + require.NoError(t, err) + return environment +} + +//nolint:unparam // Because its more clear if the environment id is written in the real test +func deleteEnvironment(t *testing.T, id string) { + t.Helper() + path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath, id) + _, err := helpers.HTTPDelete(path, nil) + require.NoError(t, err) } func cleanupJobsForEnvironment(t *testing.T, environmentID string) { @@ -84,6 +291,21 @@ func cleanupJobsForEnvironment(t *testing.T, environmentID string) { } } +//nolint:unparam // Because its more clear if the environment id is written in the real test +func createEnvironment(t *testing.T, environmentID string) { + t.Helper() + path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath, environmentID) + request := dto.ExecutionEnvironmentRequest{ + PrewarmingPoolSize: 1, + CPULimit: 100, + MemoryLimit: 100, + Image: *testDockerImage, + NetworkAccess: false, + ExposedPorts: nil, + } + assertPutReturnsStatusAndZeroContent(t, path, request, http.StatusCreated) +} + func assertPutReturnsStatusAndZeroContent(t *testing.T, path string, request dto.ExecutionEnvironmentRequest, status int) { t.Helper() @@ -133,9 +355,9 @@ func validateJob(t *testing.T, expected dto.ExecutionEnvironmentRequest) { } } -func findTemplateJob(t *testing.T, id runner.EnvironmentID) *nomadApi.Job { +func findTemplateJob(t *testing.T, id dto.EnvironmentID) *nomadApi.Job { t.Helper() - job, _, err := nomadClient.Jobs().Info(runner.TemplateJobID(id), nil) + job, _, err := nomadClient.Jobs().Info(nomad.TemplateJobID(id), nil) if err != nil { t.Fatalf("Error retrieving Nomad job: %v", err) } diff --git a/tests/helpers/test_helpers.go b/tests/helpers/test_helpers.go index b2be89f..86b9294 100644 --- a/tests/helpers/test_helpers.go +++ b/tests/helpers/test_helpers.go @@ -174,8 +174,9 @@ func HTTPPutJSON(url string, body interface{}) (response *http.Response, err err const templateJobPriority = 100 func CreateTemplateJob() (base, job *nomadApi.Job) { - base = nomadApi.NewBatchJob(tests.DefaultJobID, tests.DefaultJobID, "region-name", templateJobPriority) - job = nomadApi.NewBatchJob(tests.DefaultJobID, tests.DefaultJobID, "region-name", templateJobPriority) + base = nomadApi.NewBatchJob(tests.DefaultRunnerID, tests.DefaultRunnerID, "global", templateJobPriority) + job = nomadApi.NewBatchJob(tests.DefaultRunnerID, tests.DefaultRunnerID, "global", templateJobPriority) + job.Datacenters = []string{"dc1"} configTaskGroup := nomadApi.NewTaskGroup("config", 0) configTaskGroup.Meta = make(map[string]string) configTaskGroup.Meta["prewarmingPoolSize"] = "0"