From 5d54b0f786aed79f58bddc5f226a8f089482e6a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Pa=C3=9F?= <22845248+mpass99@users.noreply.github.com> Date: Thu, 13 Oct 2022 21:51:53 +0100 Subject: [PATCH] Fix wrong environment id at monitoring data for created or updated environments. --- internal/api/environments_test.go | 6 +++--- internal/api/websocket_test.go | 2 +- internal/environment/nomad_environment.go | 6 +++--- internal/environment/nomad_environment_test.go | 4 ++-- internal/environment/nomad_manager_test.go | 18 ++++++++++-------- 5 files changed, 19 insertions(+), 17 deletions(-) diff --git a/internal/api/environments_test.go b/internal/api/environments_test.go index 6babb4a..cc58a89 100644 --- a/internal/api/environments_test.go +++ b/internal/api/environments_test.go @@ -87,10 +87,10 @@ func (s *EnvironmentControllerTestSuite) TestList() { s.Run("returns multiple environments", func() { call.Run(func(args mock.Arguments) { - firstEnvironment, err := environment.NewNomadEnvironment(nil, + firstEnvironment, err := environment.NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, nil, "job \""+nomad.TemplateJobID(tests.DefaultEnvironmentIDAsInteger)+"\" {}") s.Require().NoError(err) - secondEnvironment, err := environment.NewNomadEnvironment(nil, + secondEnvironment, err := environment.NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, nil, "job \""+nomad.TemplateJobID(tests.AnotherEnvironmentIDAsInteger)+"\" {}") s.Require().NoError(err) call.ReturnArguments = mock.Arguments{[]runner.ExecutionEnvironment{firstEnvironment, secondEnvironment}, nil} @@ -149,7 +149,7 @@ func (s *EnvironmentControllerTestSuite) TestGet() { s.Run("returns environment", func() { call.Run(func(args mock.Arguments) { - testEnvironment, err := environment.NewNomadEnvironment(nil, + testEnvironment, err := environment.NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, nil, "job \""+nomad.TemplateJobID(tests.DefaultEnvironmentIDAsInteger)+"\" {}") s.Require().NoError(err) call.ReturnArguments = mock.Arguments{testEnvironment, nil} diff --git a/internal/api/websocket_test.go b/internal/api/websocket_test.go index a611224..406210a 100644 --- a/internal/api/websocket_test.go +++ b/internal/api/websocket_test.go @@ -333,7 +333,7 @@ func newRunnerWithNotMockedRunnerManager(t *testing.T, apiMock *nomad.ExecutorAP runnerID := tests.DefaultRunnerID runnerJob := runner.NewNomadJob(runnerID, nil, apiMock, runnerManager.Return) - e, err := environment.NewNomadEnvironment(apiMock, "job \"template-0\" {}") + e, err := environment.NewNomadEnvironment(0, apiMock, "job \"template-0\" {}") require.NoError(t, err) eID, err := nomad.EnvironmentIDFromRunnerID(runnerID) require.NoError(t, err) diff --git a/internal/environment/nomad_environment.go b/internal/environment/nomad_environment.go index 2950400..b39fd34 100644 --- a/internal/environment/nomad_environment.go +++ b/internal/environment/nomad_environment.go @@ -31,7 +31,7 @@ type NomadEnvironment struct { idleRunners storage.Storage[runner.Runner] } -func NewNomadEnvironment(apiClient nomad.ExecutorAPI, jobHCL string) (*NomadEnvironment, error) { +func NewNomadEnvironment(id dto.EnvironmentID, apiClient nomad.ExecutorAPI, jobHCL string) (*NomadEnvironment, error) { job, err := parseJob(jobHCL) if err != nil { return nil, fmt.Errorf("error parsing Nomad job: %w", err) @@ -39,14 +39,14 @@ func NewNomadEnvironment(apiClient nomad.ExecutorAPI, jobHCL string) (*NomadEnvi e := &NomadEnvironment{apiClient, jobHCL, job, nil} e.idleRunners = storage.NewMonitoredLocalStorage[runner.Runner](monitoring.MeasurementIdleRunnerNomad, - runner.MonitorEnvironmentID[runner.Runner](e.ID()), time.Minute) + runner.MonitorEnvironmentID[runner.Runner](id), time.Minute) return e, nil } func NewNomadEnvironmentFromRequest( apiClient nomad.ExecutorAPI, jobHCL string, id dto.EnvironmentID, request dto.ExecutionEnvironmentRequest) ( *NomadEnvironment, error) { - environment, err := NewNomadEnvironment(apiClient, jobHCL) + environment, err := NewNomadEnvironment(id, apiClient, jobHCL) if err != nil { return nil, err } diff --git a/internal/environment/nomad_environment_test.go b/internal/environment/nomad_environment_test.go index 6a6f4f7..c278108 100644 --- a/internal/environment/nomad_environment_test.go +++ b/internal/environment/nomad_environment_test.go @@ -156,13 +156,13 @@ func TestRegisterTemplateJobReturnsErrorWhenMonitoringEvaluationFails(t *testing func TestParseJob(t *testing.T) { t.Run("parses the given default job", func(t *testing.T) { - environment, err := NewNomadEnvironment(nil, templateEnvironmentJobHCL) + environment, err := NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, nil, 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(nil, "") + environment, err := NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, nil, "") assert.Error(t, err) assert.Nil(t, environment) }) diff --git a/internal/environment/nomad_manager_test.go b/internal/environment/nomad_manager_test.go index 2cd3d7c..61775d8 100644 --- a/internal/environment/nomad_manager_test.go +++ b/internal/environment/nomad_manager_test.go @@ -107,7 +107,7 @@ func TestNewNomadEnvironmentManager(t *testing.T) { t.Run("loads template environment job from file", func(t *testing.T) { templateJobHCL := "job \"" + tests.DefaultTemplateJobID + "\" {}" - _, err := NewNomadEnvironment(nil, templateJobHCL) + _, err := NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, nil, templateJobHCL) require.NoError(t, err) f := createTempFile(t, templateJobHCL) defer os.Remove(f.Name()) @@ -125,7 +125,7 @@ func TestNewNomadEnvironmentManager(t *testing.T) { m, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, f.Name()) require.NoError(t, err) - _, err = NewNomadEnvironment(nil, m.templateEnvironmentHCL) + _, err = NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, nil, m.templateEnvironmentHCL) assert.Error(t, err) }) @@ -152,7 +152,8 @@ func TestNomadEnvironmentManager_Get(t *testing.T) { }) t.Run("Returns environment when it was added before", func(t *testing.T) { - expectedEnvironment, err := NewNomadEnvironment(apiMock, templateEnvironmentJobHCL) + expectedEnvironment, err := + NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, apiMock, templateEnvironmentJobHCL) expectedEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) require.NoError(t, err) runnerManager.StoreEnvironment(expectedEnvironment) @@ -170,7 +171,7 @@ func TestNomadEnvironmentManager_Get(t *testing.T) { }) t.Run("Updates values when environment already known by Poseidon", func(t *testing.T) { - fetchedEnvironment, err := NewNomadEnvironment(nil, templateEnvironmentJobHCL) + fetchedEnvironment, err := NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, nil, templateEnvironmentJobHCL) require.NoError(t, err) fetchedEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) fetchedEnvironment.SetImage("random docker image") @@ -178,7 +179,7 @@ func TestNomadEnvironmentManager_Get(t *testing.T) { call.ReturnArguments = mock.Arguments{[]*nomadApi.Job{fetchedEnvironment.job}, nil} }) - localEnvironment, err := NewNomadEnvironment(nil, templateEnvironmentJobHCL) + localEnvironment, err := NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, nil, templateEnvironmentJobHCL) require.NoError(t, err) localEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) runnerManager.StoreEnvironment(localEnvironment) @@ -194,7 +195,7 @@ func TestNomadEnvironmentManager_Get(t *testing.T) { runnerManager.DeleteEnvironment(tests.DefaultEnvironmentIDAsInteger) t.Run("Adds environment when not already known by Poseidon", func(t *testing.T) { - fetchedEnvironment, err := NewNomadEnvironment(nil, templateEnvironmentJobHCL) + fetchedEnvironment, err := NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, nil, templateEnvironmentJobHCL) require.NoError(t, err) fetchedEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) fetchedEnvironment.SetImage("random docker image") @@ -231,7 +232,7 @@ func TestNomadEnvironmentManager_List(t *testing.T) { }) t.Run("Returns added environment", func(t *testing.T) { - localEnvironment, err := NewNomadEnvironment(apiMock, templateEnvironmentJobHCL) + localEnvironment, err := NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, apiMock, templateEnvironmentJobHCL) require.NoError(t, err) localEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) runnerManager.StoreEnvironment(localEnvironment) @@ -244,7 +245,8 @@ func TestNomadEnvironmentManager_List(t *testing.T) { runnerManager.DeleteEnvironment(tests.DefaultEnvironmentIDAsInteger) t.Run("Fetches new Runners via the api client", func(t *testing.T) { - fetchedEnvironment, err := NewNomadEnvironment(apiMock, templateEnvironmentJobHCL) + fetchedEnvironment, err := + NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, apiMock, templateEnvironmentJobHCL) require.NoError(t, err) fetchedEnvironment.SetID(tests.DefaultEnvironmentIDAsInteger) status := structs.JobStatusRunning