From 32fe47d66935b21f493a139993aa3c6b06fb10f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Pa=C3=9F?= <22845248+mpass99@users.noreply.github.com> Date: Mon, 7 Jun 2021 16:53:10 +0200 Subject: [PATCH] Implement linting issues and merge request comments --- .gitignore | 1 + api/environments_test.go | 4 +- api/runners.go | 2 +- api/runners_test.go | 4 +- environment/manager.go | 10 +- environment/manager_test.go | 30 +++--- nomad/api_querier.go | 22 ++-- nomad/executor_api_mock.go | 2 +- nomad/job.go | 12 +-- nomad/nomad.go | 56 +++++----- nomad/nomad_test.go | 49 +++++---- runner/constants_test.go | 4 +- runner/manager.go | 127 +++++++++++----------- runner/manager_mock.go | 26 ++--- runner/manager_test.go | 177 +++++++++++++++++-------------- runner/nomad_job_storage.go | 17 +-- runner/nomad_job_storage_test.go | 78 +++++++------- runner/runner.go | 13 +-- runner/runner_test.go | 17 +-- runner/storage_test.go | 6 +- tests/constants.go | 24 ++--- tests/e2e/environments_test.go | 6 +- tests/e2e/runners_test.go | 4 +- 23 files changed, 363 insertions(+), 328 deletions(-) diff --git a/.gitignore b/.gitignore index f6dedc8..da19ed7 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ poseidon # Configuration file configuration.yaml +tests/e2e/configuration.yaml # TLS certificate/key *.crt diff --git a/api/environments_test.go b/api/environments_test.go index 3ccde52..55b6272 100644 --- a/api/environments_test.go +++ b/api/environments_test.go @@ -42,7 +42,7 @@ func TestCreateOrUpdateEnvironmentTestSuite(t *testing.T) { func (s *CreateOrUpdateEnvironmentTestSuite) SetupTest() { s.EnvironmentControllerTestSuite.SetupTest() - s.id = tests.DefaultEnvironmentIdAsString + s.id = tests.DefaultEnvironmentIDAsString testURL, err := s.router.Get(createOrUpdateRouteName).URL(executionEnvironmentIDKey, s.id) if err != nil { s.T().Fatal(err) @@ -71,7 +71,7 @@ func (s *CreateOrUpdateEnvironmentTestSuite) TestReturnsBadRequestWhenBadBody() } func (s *CreateOrUpdateEnvironmentTestSuite) TestReturnsInternalServerErrorWhenManagerReturnsError() { - testError := tests.DefaultError + testError := tests.ErrDefault s.manager. On("CreateOrUpdate", s.id, mock.AnythingOfType("dto.ExecutionEnvironmentRequest")). Return(false, testError) diff --git a/api/runners.go b/api/runners.go index 60a4ddd..87f8d6f 100644 --- a/api/runners.go +++ b/api/runners.go @@ -45,7 +45,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 := runner.EnvironmentID(runnerRequest.ExecutionEnvironmentId) nextRunner, err := r.manager.Claim(environmentId) if err != nil { if err == runner.ErrUnknownExecutionEnvironment { diff --git a/api/runners_test.go b/api/runners_test.go index c340829..f3824d5 100644 --- a/api/runners_test.go +++ b/api/runners_test.go @@ -173,13 +173,13 @@ type UpdateFileSystemRouteTestSuite struct { func (s *UpdateFileSystemRouteTestSuite) SetupTest() { s.RunnerRouteTestSuite.SetupTest() - routeUrl, err := s.router.Get(UpdateFileSystemPath).URL(RunnerIdKey, tests.DefaultMockId) + routeUrl, err := s.router.Get(UpdateFileSystemPath).URL(RunnerIdKey, tests.DefaultMockID) if err != nil { s.T().Fatal(err) } s.path = routeUrl.String() s.runnerMock = &runner.RunnerMock{} - s.runnerManager.On("Get", tests.DefaultMockId).Return(s.runnerMock, nil) + s.runnerManager.On("Get", tests.DefaultMockID).Return(s.runnerMock, nil) s.recorder = httptest.NewRecorder() } diff --git a/environment/manager.go b/environment/manager.go index 1471b1f..88bb3a8 100644 --- a/environment/manager.go +++ b/environment/manager.go @@ -28,7 +28,7 @@ type Manager interface { Delete(id string) } -func NewNomadEnvironmentManager(runnerManager runner.Manager, apiClient nomad.ExecutorApi) *NomadEnvironmentManager { +func NewNomadEnvironmentManager(runnerManager runner.Manager, apiClient nomad.ExecutorAPI) *NomadEnvironmentManager { environmentManager := &NomadEnvironmentManager{runnerManager, apiClient, *parseJob(defaultJobHCL)} environmentManager.Load() return environmentManager @@ -36,7 +36,7 @@ func NewNomadEnvironmentManager(runnerManager runner.Manager, apiClient nomad.Ex type NomadEnvironmentManager struct { runnerManager runner.Manager - api nomad.ExecutorApi + api nomad.ExecutorAPI defaultJob nomadApi.Job } @@ -48,7 +48,7 @@ func (m *NomadEnvironmentManager) CreateOrUpdate( if err != nil { return false, err } - exists := m.runnerManager.EnvironmentExists(runner.EnvironmentId(idInt)) + exists := m.runnerManager.EnvironmentExists(runner.EnvironmentID(idInt)) err = m.registerJob(id, request.PrewarmingPoolSize, request.CPULimit, request.MemoryLimit, @@ -57,7 +57,7 @@ func (m *NomadEnvironmentManager) CreateOrUpdate( if err == nil { if !exists { m.runnerManager.RegisterEnvironment( - runner.EnvironmentId(idInt), runner.NomadJobId(id), request.PrewarmingPoolSize) + runner.EnvironmentID(idInt), runner.NomadJobID(id), request.PrewarmingPoolSize) } return !exists, nil } @@ -70,5 +70,5 @@ func (m *NomadEnvironmentManager) Delete(id string) { func (m *NomadEnvironmentManager) Load() { // ToDo: remove create default execution environment for debugging purposes - m.runnerManager.RegisterEnvironment(runner.EnvironmentId(0), "python", 5) + m.runnerManager.RegisterEnvironment(runner.EnvironmentID(0), "python", 5) } diff --git a/environment/manager_test.go b/environment/manager_test.go index 632e9ff..1714aa3 100644 --- a/environment/manager_test.go +++ b/environment/manager_test.go @@ -48,17 +48,17 @@ func (s *CreateOrUpdateTestSuite) SetupTest() { } func (s *CreateOrUpdateTestSuite) mockEnvironmentExists(exists bool) { - s.runnerManagerMock.On("EnvironmentExists", mock.AnythingOfType("EnvironmentId")).Return(exists) + s.runnerManagerMock.On("EnvironmentExists", mock.AnythingOfType("EnvironmentID")).Return(exists) } func (s *CreateOrUpdateTestSuite) mockRegisterEnvironment() *mock.Call { return s.runnerManagerMock.On("RegisterEnvironment", - mock.AnythingOfType("EnvironmentId"), mock.AnythingOfType("NomadJobId"), mock.AnythingOfType("uint")). + mock.AnythingOfType("EnvironmentID"), mock.AnythingOfType("NomadJobID"), mock.AnythingOfType("uint")). Return() } func (s *CreateOrUpdateTestSuite) createJobForRequest() *nomadApi.Job { - return createJob(s.manager.defaultJob, tests.DefaultEnvironmentIdAsString, + return createJob(s.manager.defaultJob, tests.DefaultEnvironmentIDAsString, s.request.PrewarmingPoolSize, s.request.CPULimit, s.request.MemoryLimit, s.request.Image, s.request.NetworkAccess, s.request.ExposedPorts) } @@ -78,7 +78,7 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsRegistersCorrectJob() s.mockEnvironmentExists(true) expectedJob := s.createJobForRequest() - created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request) + created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request) s.NoError(err) s.False(created) s.apiMock.AssertCalled(s.T(), "RegisterNomadJob", expectedJob) @@ -87,16 +87,16 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsRegistersCorrectJob() func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsOccurredErrorIsPassed() { s.mockEnvironmentExists(true) - s.registerNomadJobMockCall.Return("", tests.DefaultError) - created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request) + s.registerNomadJobMockCall.Return("", tests.ErrDefault) + created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request) s.False(created) - s.Equal(tests.DefaultError, err) + s.Equal(tests.ErrDefault, err) } func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsReturnsFalse() { s.mockEnvironmentExists(true) - created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request) + created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request) s.NoError(err) s.False(created) } @@ -107,7 +107,7 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentDoesNotExistRegistersCorrec expectedJob := s.createJobForRequest() - created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request) + created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request) s.NoError(err) s.True(created) s.apiMock.AssertCalled(s.T(), "RegisterNomadJob", expectedJob) @@ -117,20 +117,22 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentDoesNotExistRegistersCorrec s.mockEnvironmentExists(false) s.mockRegisterEnvironment() - created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request) + created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request) s.True(created) s.NoError(err) s.runnerManagerMock.AssertCalled(s.T(), "RegisterEnvironment", - runner.EnvironmentId(tests.DefaultEnvironmentIdAsInteger), runner.NomadJobId(tests.DefaultEnvironmentIdAsString), s.request.PrewarmingPoolSize) + runner.EnvironmentID(tests.DefaultEnvironmentIDAsInteger), + runner.NomadJobID(tests.DefaultEnvironmentIDAsString), + s.request.PrewarmingPoolSize) } func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentDoesNotExistOccurredErrorIsPassedAndNoEnvironmentRegistered() { s.mockEnvironmentExists(false) s.mockRegisterEnvironment() - s.registerNomadJobMockCall.Return("", tests.DefaultError) - created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request) + s.registerNomadJobMockCall.Return("", tests.ErrDefault) + created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request) s.False(created) - s.Equal(tests.DefaultError, err) + s.Equal(tests.ErrDefault, err) s.runnerManagerMock.AssertNotCalled(s.T(), "RegisterEnvironment") } diff --git a/nomad/api_querier.go b/nomad/api_querier.go index 9d30430..159695d 100644 --- a/nomad/api_querier.go +++ b/nomad/api_querier.go @@ -43,14 +43,14 @@ type apiQuerier interface { AllocationStream(ctx context.Context) (<-chan *nomadApi.Events, error) } -// nomadApiClient implements the nomadApiQuerier interface and provides access to a real Nomad API. -type nomadApiClient struct { +// nomadAPIClient implements the nomadApiQuerier interface and provides access to a real Nomad API. +type nomadAPIClient struct { client *nomadApi.Client namespace string queryOptions *nomadApi.QueryOptions } -func (nc *nomadApiClient) init(nomadURL *url.URL, nomadNamespace string) (err error) { +func (nc *nomadAPIClient) init(nomadURL *url.URL, nomadNamespace string) (err error) { nc.client, err = nomadApi.NewClient(&nomadApi.Config{ Address: nomadURL.String(), TLSConfig: &nomadApi.TLSConfig{}, @@ -63,8 +63,8 @@ func (nc *nomadApiClient) init(nomadURL *url.URL, nomadNamespace string) (err er return err } -func (nc *nomadApiClient) DeleteRunner(runnerId string) (err error) { - allocation, _, err := nc.client.Allocations().Info(runnerId, nc.queryOptions) +func (nc *nomadAPIClient) DeleteRunner(runnerID string) (err error) { + allocation, _, err := nc.client.Allocations().Info(runnerID, nc.queryOptions) if err != nil { return } @@ -72,7 +72,7 @@ func (nc *nomadApiClient) DeleteRunner(runnerId string) (err error) { return err } -func (nc *nomadApiClient) ExecuteCommand(allocationID string, +func (nc *nomadAPIClient) ExecuteCommand(allocationID string, ctx context.Context, command []string, tty bool, stdin io.Reader, stdout, stderr io.Writer) (int, error) { allocation, _, err := nc.client.Allocations().Info(allocationID, nil) @@ -82,12 +82,12 @@ func (nc *nomadApiClient) ExecuteCommand(allocationID string, return nc.client.Allocations().Exec(ctx, allocation, TaskName, tty, command, stdin, stdout, stderr, nil, nil) } -func (nc *nomadApiClient) loadRunners(jobId string) (allocationListStub []*nomadApi.AllocationListStub, err error) { - allocationListStub, _, err = nc.client.Jobs().Allocations(jobId, true, nc.queryOptions) +func (nc *nomadAPIClient) loadRunners(jobID string) (allocationListStub []*nomadApi.AllocationListStub, err error) { + allocationListStub, _, err = nc.client.Jobs().Allocations(jobID, true, nc.queryOptions) return } -func (nc *nomadApiClient) RegisterNomadJob(job *nomadApi.Job) (string, error) { +func (nc *nomadAPIClient) RegisterNomadJob(job *nomadApi.Job) (string, error) { job.Namespace = &nc.namespace resp, _, err := nc.client.Jobs().Register(job, nil) if err != nil { @@ -102,7 +102,7 @@ func (nc *nomadApiClient) RegisterNomadJob(job *nomadApi.Job) (string, error) { return resp.EvalID, nil } -func (nc *nomadApiClient) EvaluationStream(evalID string, ctx context.Context) (stream <-chan *nomadApi.Events, err error) { +func (nc *nomadAPIClient) EvaluationStream(evalID string, ctx context.Context) (stream <-chan *nomadApi.Events, err error) { stream, err = nc.client.EventStream().Stream( ctx, map[nomadApi.Topic][]string{ @@ -113,7 +113,7 @@ func (nc *nomadApiClient) EvaluationStream(evalID string, ctx context.Context) ( return } -func (nc *nomadApiClient) AllocationStream(ctx context.Context) (stream <-chan *nomadApi.Events, err error) { +func (nc *nomadAPIClient) AllocationStream(ctx context.Context) (stream <-chan *nomadApi.Events, err error) { stream, err = nc.client.EventStream().Stream( ctx, map[nomadApi.Topic][]string{ diff --git a/nomad/executor_api_mock.go b/nomad/executor_api_mock.go index 6e628c3..536f6a4 100644 --- a/nomad/executor_api_mock.go +++ b/nomad/executor_api_mock.go @@ -14,7 +14,7 @@ import ( url "net/url" ) -// ExecutorApiMock is an autogenerated mock type for the ExecutorApi type +// ExecutorApiMock is an autogenerated mock type for the ExecutorAPI type type ExecutorApiMock struct { mock.Mock } diff --git a/nomad/job.go b/nomad/job.go index 857f066..fb1be82 100644 --- a/nomad/job.go +++ b/nomad/job.go @@ -11,25 +11,25 @@ const ( ) // LoadJobList loads the list of jobs from the Nomad api. -func (nc *nomadApiClient) LoadJobList() (list []*nomadApi.JobListStub, err error) { +func (nc *nomadAPIClient) LoadJobList() (list []*nomadApi.JobListStub, err error) { list, _, err = nc.client.Jobs().List(nc.queryOptions) return } // JobScale returns the scale of the passed job. -func (nc *nomadApiClient) JobScale(jobId string) (jobScale uint, err error) { - status, _, err := nc.client.Jobs().ScaleStatus(jobId, nc.queryOptions) +func (nc *nomadAPIClient) JobScale(jobID string) (jobScale uint, err error) { + status, _, err := nc.client.Jobs().ScaleStatus(jobID, nc.queryOptions) if err != nil { return } // ToDo: Consider counting also the placed and desired allocations - jobScale = uint(status.TaskGroups[fmt.Sprintf(TaskGroupNameFormat, jobId)].Running) + jobScale = uint(status.TaskGroups[fmt.Sprintf(TaskGroupNameFormat, jobID)].Running) return } // SetJobScale sets the scaling count of the passed job to Nomad. -func (nc *nomadApiClient) SetJobScale(jobId string, count uint, reason string) (err error) { +func (nc *nomadAPIClient) SetJobScale(jobID string, count uint, reason string) (err error) { intCount := int(count) - _, _, err = nc.client.Jobs().Scale(jobId, fmt.Sprintf(TaskGroupNameFormat, jobId), &intCount, reason, false, nil, nil) + _, _, err = nc.client.Jobs().Scale(jobID, fmt.Sprintf(TaskGroupNameFormat, jobID), &intCount, reason, false, nil, nil) return } diff --git a/nomad/nomad.go b/nomad/nomad.go index 1066cf4..62d71a2 100644 --- a/nomad/nomad.go +++ b/nomad/nomad.go @@ -14,50 +14,47 @@ import ( var ( log = logging.GetLogger("nomad") ErrorExecutorCommunicationFailed = errors.New("communication with executor failed") - errEvaluation = errors.New("evaluation could not complete") - errPlacingAllocations = errors.New("failed to place all allocations") + errEvaluation = errors.New("evaluation could not complete") + errPlacingAllocations = errors.New("failed to place all allocations") ) type AllocationProcessor func(*nomadApi.Allocation) -// ExecutorApi provides access to an container orchestration solution -type ExecutorApi interface { +// ExecutorAPI provides access to an container orchestration solution. +type ExecutorAPI interface { apiQuerier // LoadRunners loads all allocations of the specified job which are running and not about to get stopped. LoadRunners(jobID string) (runnerIds []string, err error) // MonitorEvaluation monitors the given evaluation ID. - // It waits until the evaluation reaches one of the states complete, cancelled or failed. + // It waits until the evaluation reaches one of the states complete, canceled or failed. // If the evaluation was not successful, an error containing the failures is returned. // See also https://github.com/hashicorp/nomad/blob/7d5a9ecde95c18da94c9b6ace2565afbfdd6a40d/command/monitor.go#L175 - MonitorEvaluation(evalID string, ctx context.Context) error + MonitorEvaluation(evaluationID string, ctx context.Context) error // WatchAllocations listens on the Nomad event stream for allocation events. // Depending on the incoming event, any of the given function is executed. WatchAllocations(ctx context.Context, onNewAllocation, onDeletedAllocation AllocationProcessor) error } -// APIClient implements the ExecutorApi interface and can be used to perform different operations on the real Executor API and its return values. +// APIClient implements the ExecutorAPI interface and can be used to perform different operations on the real +// Executor API and its return values. type APIClient struct { apiQuerier } // NewExecutorAPI creates a new api client. // One client is usually sufficient for the complete runtime of the API. -func NewExecutorAPI(nomadURL *url.URL, nomadNamespace string) (ExecutorApi, error) { - client := &APIClient{apiQuerier: &nomadApiClient{}} +func NewExecutorAPI(nomadURL *url.URL, nomadNamespace string) (ExecutorAPI, error) { + client := &APIClient{apiQuerier: &nomadAPIClient{}} err := client.init(nomadURL, nomadNamespace) return client, err } // init prepares an apiClient to be able to communicate to a provided Nomad API. -func (a *APIClient) init(nomadURL *url.URL, nomadNamespace string) (err error) { - err = a.apiQuerier.init(nomadURL, nomadNamespace) - if err != nil { - return err - } - return nil +func (a *APIClient) init(nomadURL *url.URL, nomadNamespace string) error { + return a.apiQuerier.init(nomadURL, nomadNamespace) } // LoadRunners loads the allocations of the specified job. @@ -72,11 +69,11 @@ func (a *APIClient) LoadRunners(jobID string) (runnerIds []string, err error) { runnerIds = append(runnerIds, stub.ID) } } - return + return runnerIds, nil } -func (a *APIClient) MonitorEvaluation(evalID string, ctx context.Context) error { - stream, err := a.EvaluationStream(evalID, ctx) +func (a *APIClient) MonitorEvaluation(evaluationID string, ctx context.Context) error { + stream, err := a.EvaluationStream(evaluationID, ctx) if err != nil { return fmt.Errorf("failed retrieving evaluation stream: %w", err) } @@ -120,8 +117,8 @@ func receiveAndHandleNomadAPIEvents(stream <-chan *nomadApi.Events, handler noma } for _, event := range events.Events { // Don't take the address of the loop variable as the underlying value might change - localEvent := event - done, err := handler(&localEvent) + eventCopy := event + done, err := handler(&eventCopy) if err != nil || done { return err } @@ -130,11 +127,12 @@ func receiveAndHandleNomadAPIEvents(stream <-chan *nomadApi.Events, handler noma return nil } -// handleEvaluationEvent is a nomadAPIEventHandler that returns the status of an evaluation in the event. +// handleEvaluationEvent is a nomadAPIEventHandler that returns whether the evaluation described by the event +// was successful. func handleEvaluationEvent(event *nomadApi.Event) (bool, error) { eval, err := event.Evaluation() if err != nil { - return true, fmt.Errorf("failed monitoring evaluation: %w", err) + return true, fmt.Errorf("failed to monitor evaluation: %w", err) } switch eval.Status { case structs.EvalStatusComplete, structs.EvalStatusCancelled, structs.EvalStatusFailed: @@ -149,11 +147,13 @@ func handleEvaluationEvent(event *nomadApi.Event) (bool, error) { // map the state is persisted between multiple calls of this function. func handleAllocationEvent(startTime int64, pendingAllocations map[string]bool, event *nomadApi.Event, onNewAllocation, onDeletedAllocation AllocationProcessor) error { + if event.Type != structs.TypeAllocationUpdated { + return nil + } alloc, err := event.Allocation() if err != nil { - return fmt.Errorf("failed retrieving allocation from event: %w", err) - } - if alloc == nil || event.Type != structs.TypeAllocationUpdated { + return fmt.Errorf("failed to retrieve allocation from event: %w", err) + } else if alloc == nil { return nil } @@ -169,7 +169,7 @@ func handleAllocationEvent(startTime int64, pendingAllocations map[string]bool, case structs.AllocDesiredStatusStop: onDeletedAllocation(alloc) case structs.AllocDesiredStatusRun: - // first event that marks the transition between pending and running + // is first event that marks the transition between pending and running? _, ok := pendingAllocations[alloc.ID] if ok { onNewAllocation(alloc) @@ -194,8 +194,8 @@ func checkEvaluation(eval *nomadApi.Evaluation) (err error) { } } else { err = fmt.Errorf("evaluation %q finished with status %q but %w", eval.ID, eval.Status, errPlacingAllocations) - for tg, metrics := range eval.FailedTGAllocs { - err = fmt.Errorf("%w\n%s: %#v", err, tg, metrics) + for taskGroup, metrics := range eval.FailedTGAllocs { + err = fmt.Errorf("%w\n%s: %#v", err, taskGroup, metrics) } if eval.BlockedEval != "" { err = fmt.Errorf("%w\nEvaluation %q waiting for additional capacity to place remainder", err, eval.BlockedEval) diff --git a/nomad/nomad_test.go b/nomad/nomad_test.go index ba1339f..5c0d47f 100644 --- a/nomad/nomad_test.go +++ b/nomad/nomad_test.go @@ -132,13 +132,13 @@ var ( const TestNamespace = "unit-tests" func TestApiClient_init(t *testing.T) { - client := &APIClient{apiQuerier: &nomadApiClient{}} + client := &APIClient{apiQuerier: &nomadAPIClient{}} err := client.init(&TestURL, TestNamespace) require.Nil(t, err) } func TestApiClientCanNotBeInitializedWithInvalidUrl(t *testing.T) { - client := &APIClient{apiQuerier: &nomadApiClient{}} + client := &APIClient{apiQuerier: &nomadAPIClient{}} err := client.init(&url.URL{ Scheme: "http", Host: "http://127.0.0.1:4646", @@ -147,7 +147,7 @@ func TestApiClientCanNotBeInitializedWithInvalidUrl(t *testing.T) { } func TestNewExecutorApiCanBeCreatedWithoutError(t *testing.T) { - expectedClient := &APIClient{apiQuerier: &nomadApiClient{}} + expectedClient := &APIClient{apiQuerier: &nomadAPIClient{}} err := expectedClient.init(&TestURL, TestNamespace) require.Nil(t, err) @@ -222,19 +222,27 @@ func eventForEvaluation(t *testing.T, eval nomadApi.Evaluation) nomadApi.Event { // simulateNomadEventStream streams the given events sequentially to the stream channel. // It returns how many events have been processed until an error occurred. -func simulateNomadEventStream(stream chan *nomadApi.Events, errChan chan error, events []*nomadApi.Events) (int, error) { +func simulateNomadEventStream( + stream chan *nomadApi.Events, + errChan chan error, + events []*nomadApi.Events, +) (int, error) { eventsProcessed := 0 var e *nomadApi.Events for _, e = range events { select { case err := <-errChan: - close(stream) return eventsProcessed, err case stream <- e: eventsProcessed++ } } - err := <-errChan + // Wait for last event being processed + var err error + select { + case <-time.After(10 * time.Millisecond): + case err = <-errChan: + } return eventsProcessed, err } @@ -298,7 +306,7 @@ func TestApiClient_MonitorEvaluationWithFailingEvent(t *testing.T) { multipleEventsWithPending := nomadApi.Events{Events: []nomadApi.Event{ eventForEvaluation(t, pendingEval), eventForEvaluation(t, eval), }} - eventsWithErr := nomadApi.Events{Err: tests.DefaultError, Events: []nomadApi.Event{{}}} + eventsWithErr := nomadApi.Events{Err: tests.ErrDefault, Events: []nomadApi.Event{{}}} var cases = []struct { streamedEvents []*nomadApi.Events @@ -316,7 +324,7 @@ func TestApiClient_MonitorEvaluationWithFailingEvent(t *testing.T) { "it skips pending evaluation and fail"}, {[]*nomadApi.Events{&multipleEventsWithPending}, 1, evalErr, "it handles multiple events per received event and fails"}, - {[]*nomadApi.Events{&eventsWithErr}, 1, tests.DefaultError, + {[]*nomadApi.Events{&eventsWithErr}, 1, tests.ErrDefault, "it fails with event error when event has error"}, } @@ -498,17 +506,19 @@ func TestHandleAllocationEventBuffersPendingAllocation(t *testing.T) { func TestAPIClient_WatchAllocationsReturnsErrorWhenAllocationStreamCannotBeRetrieved(t *testing.T) { apiMock := &apiQuerierMock{} - apiMock.On("AllocationStream", mock.Anything).Return(nil, tests.DefaultError) + apiMock.On("AllocationStream", mock.Anything).Return(nil, tests.ErrDefault) apiClient := &APIClient{apiMock} noop := func(a *nomadApi.Allocation) {} err := apiClient.WatchAllocations(context.Background(), noop, noop) - assert.ErrorIs(t, err, tests.DefaultError) + assert.ErrorIs(t, err, tests.ErrDefault) } -func TestAPIClient_WatchAllocationsReturnsErrorWhenAllocationCannotBeRetrievedWithoutReceivingFurtherEvents(t *testing.T) { +func TestAPIClient_WatchAllocationsReturnsErrorWhenAllocationCannotBeRetrievedWithoutReceivingFurtherEvents( + t *testing.T) { noop := func(a *nomadApi.Allocation) {} event := nomadApi.Event{ + Type: structs.TypeAllocationUpdated, Topic: nomadApi.TopicAllocation, // This should fail decoding, as Allocation.ID is expected to be a string, not int Payload: map[string]interface{}{"Allocation": map[string]interface{}{"ID": 1}}, @@ -517,7 +527,7 @@ func TestAPIClient_WatchAllocationsReturnsErrorWhenAllocationCannotBeRetrievedWi require.Error(t, err) events := []*nomadApi.Events{{Events: []nomadApi.Event{event}}, {}} - eventsProcessed, err := runAllocationWatching(t, events, noop, noop, context.Background()) + eventsProcessed, err := runAllocationWatching(t, events, noop, noop) assert.Error(t, err) assert.Equal(t, 1, eventsProcessed) } @@ -535,10 +545,7 @@ func assertWatchAllocation(t *testing.T, events []*nomadApi.Events, deletedAllocations = append(deletedAllocations, alloc) } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond) - defer cancel() - - eventsProcessed, err := runAllocationWatching(t, events, onNewAllocation, onDeletedAllocation, ctx) + eventsProcessed, err := runAllocationWatching(t, events, onNewAllocation, onDeletedAllocation) assert.NoError(t, err) assert.Equal(t, len(events), eventsProcessed) @@ -550,13 +557,9 @@ func assertWatchAllocation(t *testing.T, events []*nomadApi.Events, // to the MonitorEvaluation method. It starts the MonitorEvaluation function as a goroutine // and sequentially transfers the events from the given array to a channel simulating the stream. func runAllocationWatching(t *testing.T, events []*nomadApi.Events, - onNewAllocation, onDeletedAllocation AllocationProcessor, ctx context.Context) (eventsProcessed int, err error) { + onNewAllocation, onDeletedAllocation AllocationProcessor) (eventsProcessed int, err error) { t.Helper() stream := make(chan *nomadApi.Events) - go func() { - <-ctx.Done() - close(stream) - }() errChan := asynchronouslyWatchAllocations(stream, onNewAllocation, onDeletedAllocation) return simulateNomadEventStream(stream, errChan, events) } @@ -591,7 +594,7 @@ func eventForAllocation(t *testing.T, alloc *nomadApi.Allocation) nomadApi.Event err := mapstructure.Decode(eventPayload{Allocation: alloc}, &payload) if err != nil { - t.Fatalf("Couldn't encode allocation %v", err) + t.Fatalf("Couldn't decode allocation %v into payload map", err) return nomadApi.Event{} } event := nomadApi.Event{ @@ -604,7 +607,7 @@ func eventForAllocation(t *testing.T, alloc *nomadApi.Allocation) nomadApi.Event func createAllocation(modifyTime int64, clientStatus, desiredStatus string) *nomadApi.Allocation { return &nomadApi.Allocation{ - ID: tests.AllocationID, + ID: tests.DefaultRunnerID, ModifyTime: modifyTime, ClientStatus: clientStatus, DesiredStatus: desiredStatus, diff --git a/runner/constants_test.go b/runner/constants_test.go index 2605a41..bb5fba2 100644 --- a/runner/constants_test.go +++ b/runner/constants_test.go @@ -3,6 +3,6 @@ package runner import "gitlab.hpi.de/codeocean/codemoon/poseidon/tests" const ( - defaultEnvironmentId = EnvironmentId(tests.DefaultEnvironmentIdAsInteger) - anotherEnvironmentId = EnvironmentId(tests.AnotherEnvironmentIdAsInteger) + defaultEnvironmentID = EnvironmentID(tests.DefaultEnvironmentIDAsInteger) + anotherEnvironmentID = EnvironmentID(tests.AnotherEnvironmentIDAsInteger) ) diff --git a/runner/manager.go b/runner/manager.go index 13baccc..7517840 100644 --- a/runner/manager.go +++ b/runner/manager.go @@ -17,29 +17,30 @@ var ( ErrRunnerNotFound = errors.New("no runner found with this id") ) -type EnvironmentId int +type EnvironmentID int -func (e EnvironmentId) toString() string { +func (e EnvironmentID) toString() string { return string(rune(e)) } -type NomadJobId string +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. +// 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 { // RegisterEnvironment adds a new environment that should be managed. - RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount uint) + RegisterEnvironment(id EnvironmentID, nomadJobID NomadJobID, desiredIdleRunnersCount uint) // EnvironmentExists returns whether the environment with the given id exists. - EnvironmentExists(id EnvironmentId) bool + EnvironmentExists(id EnvironmentID) bool // Claim returns a new runner. // It makes sure that the runner is not in use yet and returns an error if no runner could be provided. - Claim(id EnvironmentId) (Runner, error) + Claim(id EnvironmentID) (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. - Get(runnerId string) (Runner, error) + Get(runnerID string) (Runner, error) // Return signals that the runner is no longer used by the caller and can be claimed by someone else. // The runner is deleted or cleaned up for reuse depending on the used executor. @@ -47,12 +48,15 @@ type Manager interface { } type NomadRunnerManager struct { - apiClient nomad.ExecutorApi + apiClient nomad.ExecutorAPI jobs NomadJobStorage usedRunners Storage } -func NewNomadRunnerManager(apiClient nomad.ExecutorApi, ctx context.Context) *NomadRunnerManager { +// NewNomadRunnerManager creates a new runner manager that keeps track of all runners. +// It uses the apiClient for all requests and runs a background task to keep the runners in sync with Nomad. +// If you cancel the context the background synchronization will be stopped. +func NewNomadRunnerManager(apiClient nomad.ExecutorAPI, ctx context.Context) *NomadRunnerManager { m := &NomadRunnerManager{ apiClient, NewLocalNomadJobStorage(), @@ -63,33 +67,34 @@ func NewNomadRunnerManager(apiClient nomad.ExecutorApi, ctx context.Context) *No } type NomadJob struct { - environmentId EnvironmentId - jobId NomadJobId + environmentID EnvironmentID + jobID NomadJobID idleRunners Storage desiredIdleRunnersCount uint } -func (j *NomadJob) Id() EnvironmentId { - return j.environmentId +func (j *NomadJob) ID() EnvironmentID { + return j.environmentID } -func (m *NomadRunnerManager) RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount uint) { +func (m *NomadRunnerManager) RegisterEnvironment(environmentID EnvironmentID, nomadJobID NomadJobID, + desiredIdleRunnersCount uint) { m.jobs.Add(&NomadJob{ - environmentId, - nomadJobId, + environmentID, + nomadJobID, NewLocalRunnerStorage(), desiredIdleRunnersCount, }) - go m.refreshEnvironment(environmentId) + go m.refreshEnvironment(environmentID) } -func (m *NomadRunnerManager) EnvironmentExists(id EnvironmentId) (ok bool) { +func (m *NomadRunnerManager) EnvironmentExists(id EnvironmentID) (ok bool) { _, ok = m.jobs.Get(id) return } -func (m *NomadRunnerManager) Claim(environmentId EnvironmentId) (Runner, error) { - job, ok := m.jobs.Get(environmentId) +func (m *NomadRunnerManager) Claim(environmentID EnvironmentID) (Runner, error) { + job, ok := m.jobs.Get(environmentID) if !ok { return nil, ErrUnknownExecutionEnvironment } @@ -101,8 +106,8 @@ func (m *NomadRunnerManager) Claim(environmentId EnvironmentId) (Runner, error) return runner, nil } -func (m *NomadRunnerManager) Get(runnerId string) (Runner, error) { - runner, ok := m.usedRunners.Get(runnerId) +func (m *NomadRunnerManager) Get(runnerID string) (Runner, error) { + runner, ok := m.usedRunners.Get(runnerID) if !ok { return nil, ErrRunnerNotFound } @@ -119,42 +124,46 @@ func (m *NomadRunnerManager) Return(r Runner) (err error) { } func (m *NomadRunnerManager) updateRunners(ctx context.Context) { - onCreate := func(alloc *nomadApi.Allocation) { - log.WithField("id", alloc.ID).Debug("Allocation started") - - intJobID, err := strconv.Atoi(alloc.JobID) - if err != nil { - return - } - - job, ok := m.jobs.Get(EnvironmentId(intJobID)) - if ok { - job.idleRunners.Add(NewRunner(alloc.ID)) - } + retries := 0 + for ctx.Err() == nil { + err := m.apiClient.WatchAllocations(ctx, m.onAllocationAdded, m.onAllocationStopped) + retries += 1 + log.WithError(err).Errorf("Stopped updating the runners! Retry %v", retries) + <-time.After(time.Second) } - onStop := func(alloc *nomadApi.Allocation) { - log.WithField("id", alloc.ID).Debug("Allocation stopped") +} - intJobID, err := strconv.Atoi(alloc.JobID) - if err != nil { - return - } +func (m *NomadRunnerManager) onAllocationAdded(alloc *nomadApi.Allocation) { + log.WithField("id", alloc.ID).Debug("Allocation started") - job, ok := m.jobs.Get(EnvironmentId(intJobID)) - if ok { - job.idleRunners.Delete(alloc.ID) - m.usedRunners.Delete(alloc.ID) - } - } - - err := m.apiClient.WatchAllocations(ctx, onCreate, onStop) + intJobID, err := strconv.Atoi(alloc.JobID) if err != nil { - log.WithError(err).Error("Failed updating runners") + return + } + + job, ok := m.jobs.Get(EnvironmentID(intJobID)) + if ok { + job.idleRunners.Add(NewNomadAllocation(alloc.ID, m.apiClient)) + } +} + +func (m *NomadRunnerManager) onAllocationStopped(alloc *nomadApi.Allocation) { + log.WithField("id", alloc.ID).Debug("Allocation stopped") + + intJobID, err := strconv.Atoi(alloc.JobID) + if err != nil { + return + } + + m.usedRunners.Delete(alloc.ID) + job, ok := m.jobs.Get(EnvironmentID(intJobID)) + if ok { + job.idleRunners.Delete(alloc.ID) } } // Refresh Big ToDo: Improve this function!! State out that it also rescales the job; Provide context to be terminable... -func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) { +func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentID) { job, ok := m.jobs.Get(id) if !ok { // this environment does not exist @@ -162,7 +171,7 @@ func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) { } var lastJobScaling uint = 0 for { - runners, err := m.apiClient.LoadRunners(string(job.jobId)) + runners, err := m.apiClient.LoadRunners(string(job.jobID)) if err != nil { log.WithError(err).Printf("Failed fetching runners") break @@ -173,7 +182,7 @@ func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) { job.idleRunners.Add(r) } - jobScale, err := m.apiClient.JobScale(string(job.jobId)) + jobScale, err := m.apiClient.JobScale(string(job.jobID)) if err != nil { log.WithError(err).Printf("Failed get allocation count") break @@ -186,7 +195,7 @@ func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) { time.Sleep(50 * time.Millisecond) if requiredRunnerCount != lastJobScaling { log.Printf("Set job scaling %d", requiredRunnerCount) - err = m.apiClient.SetJobScale(string(job.jobId), requiredRunnerCount, "Runner Requested") + err = m.apiClient.SetJobScale(string(job.jobID), requiredRunnerCount, "Runner Requested") if err != nil { log.WithError(err).Printf("Failed set allocation scaling") continue @@ -196,19 +205,19 @@ func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) { } } -func (m *NomadRunnerManager) unusedRunners(environmentId EnvironmentId, fetchedRunnerIds []string) (newRunners []Runner) { +func (m *NomadRunnerManager) unusedRunners(environmentId EnvironmentID, fetchedRunnerIds []string) (newRunners []Runner) { newRunners = make([]Runner, 0) job, ok := m.jobs.Get(environmentId) if !ok { // the environment does not exist, so it won't have any unused runners return } - for _, runnerId := range fetchedRunnerIds { - _, ok := m.usedRunners.Get(runnerId) + for _, runnerID := range fetchedRunnerIds { + _, ok := m.usedRunners.Get(runnerID) if !ok { - _, ok = job.idleRunners.Get(runnerId) + _, ok = job.idleRunners.Get(runnerID) if !ok { - newRunners = append(newRunners, NewNomadAllocation(runnerId, m.apiClient)) + newRunners = append(newRunners, NewNomadAllocation(runnerID, m.apiClient)) } } } diff --git a/runner/manager_mock.go b/runner/manager_mock.go index 2fa0150..df850ff 100644 --- a/runner/manager_mock.go +++ b/runner/manager_mock.go @@ -10,11 +10,11 @@ type ManagerMock struct { } // Claim provides a mock function with given fields: id -func (_m *ManagerMock) Claim(id EnvironmentId) (Runner, error) { +func (_m *ManagerMock) Claim(id EnvironmentID) (Runner, error) { ret := _m.Called(id) var r0 Runner - if rf, ok := ret.Get(0).(func(EnvironmentId) Runner); ok { + if rf, ok := ret.Get(0).(func(EnvironmentID) Runner); ok { r0 = rf(id) } else { if ret.Get(0) != nil { @@ -23,7 +23,7 @@ func (_m *ManagerMock) Claim(id EnvironmentId) (Runner, error) { } var r1 error - if rf, ok := ret.Get(1).(func(EnvironmentId) error); ok { + if rf, ok := ret.Get(1).(func(EnvironmentID) error); ok { r1 = rf(id) } else { r1 = ret.Error(1) @@ -33,11 +33,11 @@ func (_m *ManagerMock) Claim(id EnvironmentId) (Runner, error) { } // EnvironmentExists provides a mock function with given fields: id -func (_m *ManagerMock) EnvironmentExists(id EnvironmentId) bool { +func (_m *ManagerMock) EnvironmentExists(id EnvironmentID) bool { ret := _m.Called(id) var r0 bool - if rf, ok := ret.Get(0).(func(EnvironmentId) bool); ok { + if rf, ok := ret.Get(0).(func(EnvironmentID) bool); ok { r0 = rf(id) } else { r0 = ret.Get(0).(bool) @@ -46,13 +46,13 @@ func (_m *ManagerMock) EnvironmentExists(id EnvironmentId) bool { return r0 } -// Get provides a mock function with given fields: runnerId -func (_m *ManagerMock) Get(runnerId string) (Runner, error) { - ret := _m.Called(runnerId) +// Get provides a mock function with given fields: runnerID +func (_m *ManagerMock) Get(runnerID string) (Runner, error) { + ret := _m.Called(runnerID) var r0 Runner if rf, ok := ret.Get(0).(func(string) Runner); ok { - r0 = rf(runnerId) + r0 = rf(runnerID) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(Runner) @@ -61,7 +61,7 @@ func (_m *ManagerMock) Get(runnerId string) (Runner, error) { var r1 error if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(runnerId) + r1 = rf(runnerID) } else { r1 = ret.Error(1) } @@ -69,9 +69,9 @@ func (_m *ManagerMock) Get(runnerId string) (Runner, error) { return r0, r1 } -// RegisterEnvironment provides a mock function with given fields: environmentId, nomadJobId, desiredIdleRunnersCount -func (_m *ManagerMock) RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount uint) { - _m.Called(environmentId, nomadJobId, desiredIdleRunnersCount) +// RegisterEnvironment provides a mock function with given fields: id, nomadJobID, desiredIdleRunnersCount +func (_m *ManagerMock) RegisterEnvironment(id EnvironmentID, nomadJobID NomadJobID, desiredIdleRunnersCount uint) { + _m.Called(id, nomadJobID, desiredIdleRunnersCount) } // Return provides a mock function with given fields: r diff --git a/runner/manager_test.go b/runner/manager_test.go index ff51f28..0e1a000 100644 --- a/runner/manager_test.go +++ b/runner/manager_test.go @@ -32,90 +32,98 @@ type ManagerTestSuite struct { func (s *ManagerTestSuite) SetupTest() { s.apiMock = &nomad.ExecutorApiMock{} - s.nomadRunnerManager = NewNomadRunnerManager(s.apiMock, context.Background()) - s.exerciseRunner = NewRunner(tests.DefaultRunnerId) - s.mockRunnerQueries([]string{}) + // Instantly closed context to manually start the update process in some cases + ctx, cancel := context.WithCancel(context.Background()) + cancel() + s.nomadRunnerManager = NewNomadRunnerManager(s.apiMock, ctx) + + s.exerciseRunner = NewRunner(tests.DefaultRunnerID) + mockRunnerQueries(s.apiMock, []string{}) s.registerDefaultEnvironment() } -func (s *ManagerTestSuite) mockRunnerQueries(returnedRunnerIds []string) { +func mockRunnerQueries(apiMock *nomad.ExecutorApiMock, returnedRunnerIds []string) { // reset expected calls to allow new mocked return values - s.apiMock.ExpectedCalls = []*mock.Call{} - s.apiMock.On("WatchAllocations", mock.Anything, mock.Anything, mock.Anything).Return(nil) - s.apiMock.On("LoadRunners", tests.DefaultJobId).Return(returnedRunnerIds, nil) - s.apiMock.On("JobScale", tests.DefaultJobId).Return(len(returnedRunnerIds), nil) - s.apiMock.On("SetJobScale", tests.DefaultJobId, mock.AnythingOfType("uint"), "Runner Requested").Return(nil) + apiMock.ExpectedCalls = []*mock.Call{} + 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} + }) + apiMock.On("LoadRunners", 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) } func (s *ManagerTestSuite) registerDefaultEnvironment() { - s.nomadRunnerManager.RegisterEnvironment(defaultEnvironmentId, tests.DefaultJobId, defaultDesiredRunnersCount) + s.nomadRunnerManager.RegisterEnvironment(defaultEnvironmentID, tests.DefaultJobID, defaultDesiredRunnersCount) } func (s *ManagerTestSuite) AddIdleRunnerForDefaultEnvironment(r Runner) { - job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId) + job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID) job.idleRunners.Add(r) } func (s *ManagerTestSuite) waitForRunnerRefresh() { - time.Sleep(100 * time.Millisecond) + <-time.After(100 * time.Millisecond) } func (s *ManagerTestSuite) TestRegisterEnvironmentAddsNewJob() { - s.nomadRunnerManager.RegisterEnvironment(anotherEnvironmentId, tests.DefaultJobId, defaultDesiredRunnersCount) - job, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId) + s.nomadRunnerManager.RegisterEnvironment(anotherEnvironmentID, tests.DefaultJobID, defaultDesiredRunnersCount) + job, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID) s.True(ok) s.NotNil(job) } func (s *ManagerTestSuite) TestClaimReturnsNotFoundErrorIfEnvironmentNotFound() { - runner, err := s.nomadRunnerManager.Claim(EnvironmentId(42)) + runner, err := s.nomadRunnerManager.Claim(EnvironmentID(42)) s.Nil(runner) s.Equal(ErrUnknownExecutionEnvironment, err) } func (s *ManagerTestSuite) TestClaimReturnsRunnerIfAvailable() { s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner) - receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId) + receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID) s.NoError(err) s.Equal(s.exerciseRunner, receivedRunner) } func (s *ManagerTestSuite) TestClaimReturnsErrorIfNoRunnerAvailable() { s.waitForRunnerRefresh() - runner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId) + runner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID) s.Nil(runner) s.Equal(ErrNoRunnersAvailable, err) } func (s *ManagerTestSuite) TestClaimReturnsNoRunnerOfDifferentEnvironment() { s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner) - receivedRunner, err := s.nomadRunnerManager.Claim(anotherEnvironmentId) + receivedRunner, err := s.nomadRunnerManager.Claim(anotherEnvironmentID) s.Nil(receivedRunner) s.Error(err) } func (s *ManagerTestSuite) TestClaimDoesNotReturnTheSameRunnerTwice() { s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner) - s.AddIdleRunnerForDefaultEnvironment(NewRunner(tests.AnotherRunnerId)) + s.AddIdleRunnerForDefaultEnvironment(NewRunner(tests.AnotherRunnerID)) - firstReceivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId) - require.NoError(s.T(), err) - secondReceivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId) - require.NoError(s.T(), err) + firstReceivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID) + s.NoError(err) + secondReceivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID) + s.NoError(err) s.NotEqual(firstReceivedRunner, secondReceivedRunner) } func (s *ManagerTestSuite) TestClaimThrowsAnErrorIfNoRunnersAvailable() { - receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId) + receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID) s.Nil(receivedRunner) s.Error(err) } func (s *ManagerTestSuite) TestClaimAddsRunnerToUsedRunners() { - s.mockRunnerQueries([]string{tests.DefaultRunnerId}) + mockRunnerQueries(s.apiMock, []string{tests.DefaultRunnerID}) s.waitForRunnerRefresh() - receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId) - require.NoError(s.T(), err) + receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID) + s.Require().NoError(err) savedRunner, ok := s.nomadRunnerManager.usedRunners.Get(receivedRunner.Id()) s.True(ok) s.Equal(savedRunner, receivedRunner) @@ -129,7 +137,7 @@ func (s *ManagerTestSuite) TestGetReturnsRunnerIfRunnerIsUsed() { } func (s *ManagerTestSuite) TestGetReturnsErrorIfRunnerNotFound() { - savedRunner, err := s.nomadRunnerManager.Get(tests.DefaultRunnerId) + savedRunner, err := s.nomadRunnerManager.Get(tests.DefaultRunnerID) s.Nil(savedRunner) s.Error(err) } @@ -157,94 +165,107 @@ func (s *ManagerTestSuite) TestReturnReturnsErrorWhenApiCallFailed() { } func (s *ManagerTestSuite) TestRefreshFetchesRunners() { - s.mockRunnerQueries([]string{tests.DefaultRunnerId}) + mockRunnerQueries(s.apiMock, []string{tests.DefaultRunnerID}) s.waitForRunnerRefresh() - s.apiMock.AssertCalled(s.T(), "LoadRunners", tests.DefaultJobId) + s.apiMock.AssertCalled(s.T(), "LoadRunners", tests.DefaultJobID) } func (s *ManagerTestSuite) TestNewRunnersFoundInRefreshAreAddedToIdleRunners() { - s.mockRunnerQueries([]string{tests.DefaultRunnerId}) + mockRunnerQueries(s.apiMock, []string{tests.DefaultRunnerID}) s.waitForRunnerRefresh() - job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId) - _, ok := job.idleRunners.Get(tests.DefaultRunnerId) + job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID) + _, ok := job.idleRunners.Get(tests.DefaultRunnerID) s.True(ok) } func (s *ManagerTestSuite) TestRefreshScalesJob() { - s.mockRunnerQueries([]string{tests.DefaultRunnerId}) + mockRunnerQueries(s.apiMock, []string{tests.DefaultRunnerID}) s.waitForRunnerRefresh() // use one runner to necessitate rescaling - _, _ = s.nomadRunnerManager.Claim(defaultEnvironmentId) + _, _ = s.nomadRunnerManager.Claim(defaultEnvironmentID) s.waitForRunnerRefresh() - s.apiMock.AssertCalled(s.T(), "SetJobScale", tests.DefaultJobId, defaultDesiredRunnersCount, "Runner Requested") + s.apiMock.AssertCalled(s.T(), "SetJobScale", tests.DefaultJobID, defaultDesiredRunnersCount, "Runner Requested") } func (s *ManagerTestSuite) TestRefreshAddsRunnerToPool() { - s.mockRunnerQueries([]string{tests.DefaultRunnerId}) + mockRunnerQueries(s.apiMock, []string{tests.DefaultRunnerID}) s.waitForRunnerRefresh() - job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId) - poolRunner, ok := job.idleRunners.Get(tests.DefaultRunnerId) + job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID) + poolRunner, ok := job.idleRunners.Get(tests.DefaultRunnerID) s.True(ok) - s.Equal(tests.DefaultRunnerId, poolRunner.Id()) + s.Equal(tests.DefaultRunnerID, poolRunner.Id()) } func (s *ManagerTestSuite) TestUpdateRunnersLogsErrorFromWatchAllocation() { var hook *test.Hook logger, hook := test.NewNullLogger() log = logger.WithField("pkg", "runner") - s.modifyMockedCall("WatchAllocations", func(call *mock.Call) { - call.Return(tests.DefaultError) - }) - - s.nomadRunnerManager.updateRunners(context.Background()) - - require.Equal(s.T(), 1, len(hook.Entries)) - s.Equal(logrus.ErrorLevel, hook.LastEntry().Level) - s.Equal(hook.LastEntry().Data[logrus.ErrorKey], tests.DefaultError) -} - -func (s *ManagerTestSuite) TestUpdateRunnersAddsIdleRunner() { - allocation := &nomadApi.Allocation{ID: tests.AllocationID} - defaultJob, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId) - require.True(s.T(), ok) - allocation.JobID = string(defaultJob.jobId) - - _, ok = defaultJob.idleRunners.Get(allocation.ID) - require.False(s.T(), ok) - - s.modifyMockedCall("WatchAllocations", func(call *mock.Call) { + modifyMockedCall(s.apiMock, "WatchAllocations", func(call *mock.Call) { call.Run(func(args mock.Arguments) { - onCreate, ok := args.Get(1).(nomad.AllocationProcessor) - require.True(s.T(), ok) - onCreate(allocation) + call.ReturnArguments = mock.Arguments{tests.ErrDefault} }) }) - s.nomadRunnerManager.updateRunners(context.Background()) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go s.nomadRunnerManager.updateRunners(ctx) + <-time.After(10 * time.Millisecond) + + s.Require().Equal(1, len(hook.Entries)) + s.Equal(logrus.ErrorLevel, hook.LastEntry().Level) + s.Equal(hook.LastEntry().Data[logrus.ErrorKey], tests.ErrDefault) +} + +func (s *ManagerTestSuite) TestUpdateRunnersAddsIdleRunner() { + allocation := &nomadApi.Allocation{ID: tests.DefaultRunnerID} + defaultJob, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID) + s.Require().True(ok) + allocation.JobID = string(defaultJob.jobID) + + _, ok = defaultJob.idleRunners.Get(allocation.ID) + s.Require().False(ok) + + modifyMockedCall(s.apiMock, "WatchAllocations", func(call *mock.Call) { + call.Run(func(args mock.Arguments) { + onCreate, ok := args.Get(1).(nomad.AllocationProcessor) + s.Require().True(ok) + onCreate(allocation) + call.ReturnArguments = mock.Arguments{nil} + }) + }) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go s.nomadRunnerManager.updateRunners(ctx) + <-time.After(10 * time.Millisecond) _, ok = defaultJob.idleRunners.Get(allocation.ID) s.True(ok) } func (s *ManagerTestSuite) TestUpdateRunnersRemovesIdleAndUsedRunner() { - allocation := &nomadApi.Allocation{ID: tests.AllocationID} - defaultJob, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId) - require.True(s.T(), ok) - allocation.JobID = string(defaultJob.jobId) + allocation := &nomadApi.Allocation{ID: tests.DefaultRunnerID} + defaultJob, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID) + s.Require().True(ok) + allocation.JobID = string(defaultJob.jobID) testRunner := NewRunner(allocation.ID) defaultJob.idleRunners.Add(testRunner) s.nomadRunnerManager.usedRunners.Add(testRunner) - s.modifyMockedCall("WatchAllocations", func(call *mock.Call) { + modifyMockedCall(s.apiMock, "WatchAllocations", func(call *mock.Call) { call.Run(func(args mock.Arguments) { onDelete, ok := args.Get(2).(nomad.AllocationProcessor) - require.True(s.T(), ok) + s.Require().True(ok) onDelete(allocation) + call.ReturnArguments = mock.Arguments{nil} }) }) - s.nomadRunnerManager.updateRunners(context.Background()) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go s.nomadRunnerManager.updateRunners(ctx) + <-time.After(10 * time.Millisecond) _, ok = defaultJob.idleRunners.Get(allocation.ID) s.False(ok) @@ -252,18 +273,16 @@ func (s *ManagerTestSuite) TestUpdateRunnersRemovesIdleAndUsedRunner() { s.False(ok) } -func (s *ManagerTestSuite) modifyMockedCall(method string, modifier func(call *mock.Call)) { - for _, c := range s.apiMock.ExpectedCalls { +func modifyMockedCall(apiMock *nomad.ExecutorApiMock, method string, modifier func(call *mock.Call)) { + for _, c := range apiMock.ExpectedCalls { if c.Method == method { modifier(c) } } - s.True(ok) - s.Equal(tests.DefaultRunnerId, poolRunner.Id()) } func (s *ManagerTestSuite) TestWhenEnvironmentDoesNotExistEnvironmentExistsReturnsFalse() { - id := anotherEnvironmentId + id := anotherEnvironmentID _, ok := s.nomadRunnerManager.jobs.Get(id) require.False(s.T(), ok) @@ -271,8 +290,8 @@ func (s *ManagerTestSuite) TestWhenEnvironmentDoesNotExistEnvironmentExistsRetur } func (s *ManagerTestSuite) TestWhenEnvironmentExistsEnvironmentExistsReturnsTrue() { - id := anotherEnvironmentId - s.nomadRunnerManager.jobs.Add(&NomadJob{environmentId: id}) + id := anotherEnvironmentID + s.nomadRunnerManager.jobs.Add(&NomadJob{environmentID: id}) exists := s.nomadRunnerManager.EnvironmentExists(id) s.True(exists) diff --git a/runner/nomad_job_storage.go b/runner/nomad_job_storage.go index 0c7e7a0..c66c1dc 100644 --- a/runner/nomad_job_storage.go +++ b/runner/nomad_job_storage.go @@ -12,10 +12,11 @@ type NomadJobStorage interface { // Get returns a job from the storage. // Iff the job does not exist in the store, ok will be false. - Get(id EnvironmentId) (job *NomadJob, ok bool) + Get(id EnvironmentID) (job *NomadJob, ok bool) - // Delete deletes the job with the passed id from the storage. It does nothing if no job with the id is present in the storage. - Delete(id EnvironmentId) + // Delete deletes the job with the passed id from the storage. It does nothing if no job with the id is present in + // the storage. + Delete(id EnvironmentID) // Length returns the number of currently stored jobs in the storage. Length() int @@ -24,31 +25,31 @@ type NomadJobStorage interface { // localNomadJobStorage stores NomadJob objects in the local application memory. type localNomadJobStorage struct { sync.RWMutex - jobs map[EnvironmentId]*NomadJob + jobs map[EnvironmentID]*NomadJob } // NewLocalNomadJobStorage responds with an empty localNomadJobStorage. // This implementation stores the data thread-safe in the local application memory. func NewLocalNomadJobStorage() *localNomadJobStorage { return &localNomadJobStorage{ - jobs: make(map[EnvironmentId]*NomadJob), + jobs: make(map[EnvironmentID]*NomadJob), } } func (s *localNomadJobStorage) Add(job *NomadJob) { s.Lock() defer s.Unlock() - s.jobs[job.Id()] = job + s.jobs[job.ID()] = job } -func (s *localNomadJobStorage) Get(id EnvironmentId) (job *NomadJob, ok bool) { +func (s *localNomadJobStorage) Get(id EnvironmentID) (job *NomadJob, ok bool) { s.RLock() defer s.RUnlock() job, ok = s.jobs[id] return } -func (s *localNomadJobStorage) Delete(id EnvironmentId) { +func (s *localNomadJobStorage) Delete(id EnvironmentID) { s.Lock() defer s.Unlock() delete(s.jobs, id) diff --git a/runner/nomad_job_storage_test.go b/runner/nomad_job_storage_test.go index f8ce8a8..4feb996 100644 --- a/runner/nomad_job_storage_test.go +++ b/runner/nomad_job_storage_test.go @@ -16,62 +16,62 @@ type JobStoreTestSuite struct { job *NomadJob } -func (suite *JobStoreTestSuite) SetupTest() { - suite.jobStorage = NewLocalNomadJobStorage() - suite.job = &NomadJob{environmentId: defaultEnvironmentId, jobId: tests.DefaultJobId} +func (s *JobStoreTestSuite) SetupTest() { + s.jobStorage = NewLocalNomadJobStorage() + s.job = &NomadJob{environmentID: defaultEnvironmentID, jobID: tests.DefaultJobID} } -func (suite *JobStoreTestSuite) TestAddedJobCanBeRetrieved() { - suite.jobStorage.Add(suite.job) - retrievedJob, ok := suite.jobStorage.Get(suite.job.Id()) - suite.True(ok, "A saved runner should be retrievable") - suite.Equal(suite.job, retrievedJob) +func (s *JobStoreTestSuite) TestAddedJobCanBeRetrieved() { + s.jobStorage.Add(s.job) + retrievedJob, ok := s.jobStorage.Get(s.job.ID()) + s.True(ok, "A saved runner should be retrievable") + s.Equal(s.job, retrievedJob) } -func (suite *JobStoreTestSuite) TestJobWithSameIdOverwritesOldOne() { - otherJobWithSameId := &NomadJob{environmentId: defaultEnvironmentId} +func (s *JobStoreTestSuite) TestJobWithSameIdOverwritesOldOne() { + otherJobWithSameID := &NomadJob{environmentID: defaultEnvironmentID} // assure runner is actually different - otherJobWithSameId.jobId = tests.AnotherJobId - suite.NotEqual(suite.job, otherJobWithSameId) + otherJobWithSameID.jobID = tests.AnotherJobID + s.NotEqual(s.job, otherJobWithSameID) - suite.jobStorage.Add(suite.job) - suite.jobStorage.Add(otherJobWithSameId) - retrievedJob, _ := suite.jobStorage.Get(suite.job.Id()) - suite.NotEqual(suite.job, retrievedJob) - suite.Equal(otherJobWithSameId, retrievedJob) + s.jobStorage.Add(s.job) + s.jobStorage.Add(otherJobWithSameID) + retrievedJob, _ := s.jobStorage.Get(s.job.ID()) + s.NotEqual(s.job, retrievedJob) + s.Equal(otherJobWithSameID, retrievedJob) } -func (suite *JobStoreTestSuite) TestDeletedJobIsNotAccessible() { - suite.jobStorage.Add(suite.job) - suite.jobStorage.Delete(suite.job.Id()) - retrievedRunner, ok := suite.jobStorage.Get(suite.job.Id()) - suite.Nil(retrievedRunner) - suite.False(ok, "A deleted runner should not be accessible") +func (s *JobStoreTestSuite) TestDeletedJobIsNotAccessible() { + s.jobStorage.Add(s.job) + s.jobStorage.Delete(s.job.ID()) + retrievedRunner, ok := s.jobStorage.Get(s.job.ID()) + s.Nil(retrievedRunner) + s.False(ok, "A deleted runner should not be accessible") } -func (suite *JobStoreTestSuite) TestLenOfEmptyPoolIsZero() { - suite.Equal(0, suite.jobStorage.Length()) +func (s *JobStoreTestSuite) TestLenOfEmptyPoolIsZero() { + s.Equal(0, s.jobStorage.Length()) } -func (suite *JobStoreTestSuite) TestLenChangesOnStoreContentChange() { - suite.Run("len increases when job is added", func() { - suite.jobStorage.Add(suite.job) - suite.Equal(1, suite.jobStorage.Length()) +func (s *JobStoreTestSuite) TestLenChangesOnStoreContentChange() { + s.Run("len increases when job is added", func() { + s.jobStorage.Add(s.job) + s.Equal(1, s.jobStorage.Length()) }) - suite.Run("len does not increase when job with same id is added", func() { - suite.jobStorage.Add(suite.job) - suite.Equal(1, suite.jobStorage.Length()) + s.Run("len does not increase when job with same id is added", func() { + s.jobStorage.Add(s.job) + s.Equal(1, s.jobStorage.Length()) }) - suite.Run("len increases again when different job is added", func() { - anotherJob := &NomadJob{environmentId: anotherEnvironmentId} - suite.jobStorage.Add(anotherJob) - suite.Equal(2, suite.jobStorage.Length()) + s.Run("len increases again when different job is added", func() { + anotherJob := &NomadJob{environmentID: anotherEnvironmentID} + s.jobStorage.Add(anotherJob) + s.Equal(2, s.jobStorage.Length()) }) - suite.Run("len decreases when job is deleted", func() { - suite.jobStorage.Delete(suite.job.Id()) - suite.Equal(1, suite.jobStorage.Length()) + s.Run("len decreases when job is deleted", func() { + s.jobStorage.Delete(s.job.ID()) + s.Equal(1, s.jobStorage.Length()) }) } diff --git a/runner/runner.go b/runner/runner.go index 431efc5..df8ced4 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -54,16 +54,11 @@ type Runner interface { type NomadAllocation struct { ExecutionStorage id string - api nomad.ExecutorApi -} - -// NewRunner creates a new runner with the provided id. -func NewRunner(id string) Runner { - return NewNomadAllocation(id, nil) + api nomad.ExecutorAPI } // NewNomadAllocation creates a new Nomad allocation with the provided id. -func NewNomadAllocation(id string, apiClient nomad.ExecutorApi) *NomadAllocation { +func NewNomadAllocation(id string, apiClient nomad.ExecutorAPI) *NomadAllocation { return &NomadAllocation{ id: id, api: apiClient, @@ -189,9 +184,9 @@ func tarHeader(file dto.File) *tar.Header { // This exports private attributes like the id too. func (r *NomadAllocation) MarshalJSON() ([]byte, error) { return json.Marshal(struct { - Id string `json:"runnerId"` + ID string `json:"runnerId"` }{ - Id: r.Id(), + ID: r.Id(), }) } diff --git a/runner/runner_test.go b/runner/runner_test.go index 409348d..986dee1 100644 --- a/runner/runner_test.go +++ b/runner/runner_test.go @@ -75,18 +75,18 @@ func TestFromContextReturnsIsNotOkWhenContextHasNoRunner(t *testing.T) { func TestExecuteCallsAPI(t *testing.T) { apiMock := &nomad.ExecutorApiMock{} apiMock.On("ExecuteCommand", mock.Anything, mock.Anything, mock.Anything, true, mock.Anything, mock.Anything, mock.Anything).Return(0, nil) - runner := NewNomadAllocation(tests.DefaultRunnerId, apiMock) + runner := NewNomadAllocation(tests.DefaultRunnerID, apiMock) request := &dto.ExecutionRequest{Command: "echo 'Hello World!'"} runner.ExecuteInteractively(request, nil, nil, nil) <-time.After(50 * time.Millisecond) - apiMock.AssertCalled(t, "ExecuteCommand", tests.DefaultRunnerId, mock.Anything, request.FullCommand(), true, mock.Anything, mock.Anything, mock.Anything) + apiMock.AssertCalled(t, "ExecuteCommand", tests.DefaultRunnerID, mock.Anything, request.FullCommand(), true, mock.Anything, mock.Anything, mock.Anything) } func TestExecuteReturnsAfterTimeout(t *testing.T) { apiMock := newApiMockWithTimeLimitHandling() - runner := NewNomadAllocation(tests.DefaultRunnerId, apiMock) + runner := NewNomadAllocation(tests.DefaultRunnerID, apiMock) timeLimit := 1 execution := &dto.ExecutionRequest{TimeLimit: timeLimit} @@ -133,8 +133,8 @@ type UpdateFileSystemTestSuite struct { func (s *UpdateFileSystemTestSuite) SetupTest() { s.apiMock = &nomad.ExecutorApiMock{} - s.runner = NewNomadAllocation(tests.DefaultRunnerId, s.apiMock) - s.mockedExecuteCommandCall = s.apiMock.On("ExecuteCommand", tests.DefaultRunnerId, mock.Anything, mock.Anything, false, mock.Anything, mock.Anything, mock.Anything). + s.runner = NewNomadAllocation(tests.DefaultRunnerID, s.apiMock) + s.mockedExecuteCommandCall = s.apiMock.On("ExecuteCommand", tests.DefaultRunnerID, mock.Anything, mock.Anything, false, mock.Anything, mock.Anything, mock.Anything). Run(func(args mock.Arguments) { s.command = args.Get(2).([]string) s.stdin = args.Get(4).(*bytes.Buffer) @@ -160,7 +160,7 @@ func (s *UpdateFileSystemTestSuite) TestUpdateFileSystemForRunnerReturnsErrorIfE } func (s *UpdateFileSystemTestSuite) TestUpdateFileSystemForRunnerReturnsErrorIfApiCallDid() { - s.mockedExecuteCommandCall.Return(0, tests.DefaultError) + s.mockedExecuteCommandCall.Return(0, tests.ErrDefault) copyRequest := &dto.UpdateFileSystemRequest{} err := s.runner.UpdateFileSystem(copyRequest) s.ErrorIs(err, nomad.ErrorExecutorCommunicationFailed) @@ -251,3 +251,8 @@ func (s *UpdateFileSystemTestSuite) readFilesFromTarArchive(tarArchive io.Reader } return files } + +// NewRunner creates a new runner with the provided id. +func NewRunner(id string) Runner { + return NewNomadAllocation(id, nil) +} diff --git a/runner/storage_test.go b/runner/storage_test.go index 6208c89..ca64d3b 100644 --- a/runner/storage_test.go +++ b/runner/storage_test.go @@ -19,8 +19,8 @@ type RunnerPoolTestSuite struct { func (suite *RunnerPoolTestSuite) SetupTest() { suite.runnerStorage = NewLocalRunnerStorage() - suite.runner = NewRunner(tests.DefaultRunnerId) - suite.runner.Add(tests.DefaultExecutionId, &dto.ExecutionRequest{Command: "true"}) + suite.runner = NewRunner(tests.DefaultRunnerID) + suite.runner.Add(tests.DefaultExecutionID, &dto.ExecutionRequest{Command: "true"}) } func (suite *RunnerPoolTestSuite) TestAddedRunnerCanBeRetrieved() { @@ -86,7 +86,7 @@ func (suite *RunnerPoolTestSuite) TestLenChangesOnStoreContentChange() { }) suite.Run("len increases again when different runner is added", func() { - anotherRunner := NewRunner(tests.AnotherRunnerId) + anotherRunner := NewRunner(tests.AnotherRunnerID) suite.runnerStorage.Add(anotherRunner) suite.Equal(2, suite.runnerStorage.Length()) }) diff --git a/tests/constants.go b/tests/constants.go index ce7b617..ccfa85c 100644 --- a/tests/constants.go +++ b/tests/constants.go @@ -3,23 +3,23 @@ package tests import "errors" const ( - NonExistingId = "n0n-3x1st1ng-1d" + NonExistingID = "n0n-3x1st1ng-1d" DefaultFileName = "test.txt" DefaultFileContent = "Hello, Codemoon!" DefaultDirectoryName = "test/" FileNameWithAbsolutePath = "/test.txt" - DefaultEnvironmentIdAsInteger = 0 - DefaultEnvironmentIdAsString = "0" - AnotherEnvironmentIdAsInteger = 42 - AnotherEnvironmentIdAsString = "42" - DefaultJobId = "s0m3-j0b-1d" - AnotherJobId = "4n0th3r-j0b-1d" - DefaultRunnerId = "s0m3-r4nd0m-1d" - AnotherRunnerId = "4n0th3r-runn3r-1d" - DefaultExecutionId = "s0m3-3x3cu710n-1d" - DefaultMockId = "m0ck-1d" + DefaultEnvironmentIDAsInteger = 0 + DefaultEnvironmentIDAsString = "0" + AnotherEnvironmentIDAsInteger = 42 + AnotherEnvironmentIDAsString = "42" + DefaultJobID = DefaultEnvironmentIDAsString + AnotherJobID = AnotherEnvironmentIDAsString + DefaultRunnerID = DefaultJobID + AnotherRunnerID = AnotherJobID + DefaultExecutionID = "s0m3-3x3cu710n-1d" + DefaultMockID = "m0ck-1d" ) var ( - DefaultError = errors.New("an error occurred") + ErrDefault = errors.New("an error occurred") ) diff --git a/tests/e2e/environments_test.go b/tests/e2e/environments_test.go index 15ea725..d483b71 100644 --- a/tests/e2e/environments_test.go +++ b/tests/e2e/environments_test.go @@ -18,7 +18,7 @@ const ( ) func TestCreateOrUpdateEnvironment(t *testing.T) { - path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath, tests.AnotherEnvironmentIdAsString) + path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath, tests.AnotherEnvironmentIDAsString) t.Run("returns bad request with empty body", func(t *testing.T) { resp, err := helpers.HttpPut(path, strings.NewReader("")) @@ -68,7 +68,7 @@ func TestCreateOrUpdateEnvironment(t *testing.T) { }) _, _, err := nomadClient.Jobs().DeregisterOpts( - tests.AnotherEnvironmentIdAsString, &nomadApi.DeregisterOptions{Purge: true}, nil) + tests.AnotherEnvironmentIDAsString, &nomadApi.DeregisterOptions{Purge: true}, nil) if err != nil { t.Fatalf("Error when removing test job %v", err) } @@ -87,7 +87,7 @@ func assertPutReturnsStatusAndZeroContent(t *testing.T, path string, func validateJob(t *testing.T, expected dto.ExecutionEnvironmentRequest) { t.Helper() - job := findNomadJob(t, tests.AnotherEnvironmentIdAsString) + job := findNomadJob(t, tests.AnotherEnvironmentIDAsString) assertEqualValueStringPointer(t, nomadNamespace, job.Namespace) assertEqualValueStringPointer(t, "batch", job.Type) diff --git a/tests/e2e/runners_test.go b/tests/e2e/runners_test.go index e1094c2..5c42385 100644 --- a/tests/e2e/runners_test.go +++ b/tests/e2e/runners_test.go @@ -66,7 +66,7 @@ func (s *E2ETestSuite) TestDeleteRunnerRoute() { }) s.Run("Deleting non-existing runner returns NotFound", func() { - resp, err := helpers.HttpDelete(helpers.BuildURL(api.BasePath, api.RunnersPath, tests.NonExistingId), nil) + resp, err := helpers.HttpDelete(helpers.BuildURL(api.BasePath, api.RunnersPath, tests.NonExistingID), nil) s.NoError(err) s.Equal(http.StatusNotFound, resp.StatusCode) }) @@ -178,7 +178,7 @@ func (s *E2ETestSuite) TestCopyFilesRoute() { }) s.Run("Copying to non-existing runner returns NotFound", func() { - resp, err := helpers.HttpPatch(helpers.BuildURL(api.BasePath, api.RunnersPath, tests.NonExistingId, api.UpdateFileSystemPath), "application/json", bytes.NewReader(copyFilesRequestByteString)) + resp, err := helpers.HttpPatch(helpers.BuildURL(api.BasePath, api.RunnersPath, tests.NonExistingID, api.UpdateFileSystemPath), "application/json", bytes.NewReader(copyFilesRequestByteString)) s.NoError(err) s.Equal(http.StatusNotFound, resp.StatusCode) })