From 630a006258ef53dd6c3f4d47e00e69eadd8cda23 Mon Sep 17 00:00:00 2001 From: sirkrypt0 <22522058+sirkrypt0@users.noreply.github.com> Date: Tue, 1 Jun 2021 09:49:45 +0200 Subject: [PATCH] Use more uints Previously we accepted int values although only uint values made sense. We adjusted this to accept uints where appropriate. --- environment/manager.go | 2 +- environment/manager_test.go | 2 +- nomad/api_querier.go | 4 ++-- nomad/api_querier_mock.go | 12 ++++++------ nomad/executor_api_mock.go | 14 +++++++------- nomad/job.go | 9 +++++---- runner/manager.go | 10 +++++----- runner/manager_mock.go | 4 ++-- runner/manager_test.go | 6 +++--- 9 files changed, 32 insertions(+), 31 deletions(-) diff --git a/environment/manager.go b/environment/manager.go index 79fd1d7..1471b1f 100644 --- a/environment/manager.go +++ b/environment/manager.go @@ -57,7 +57,7 @@ func (m *NomadEnvironmentManager) CreateOrUpdate( if err == nil { if !exists { m.runnerManager.RegisterEnvironment( - runner.EnvironmentId(idInt), runner.NomadJobId(id), int(request.PrewarmingPoolSize)) + runner.EnvironmentId(idInt), runner.NomadJobId(id), request.PrewarmingPoolSize) } return !exists, nil } diff --git a/environment/manager_test.go b/environment/manager_test.go index 995b132..632e9ff 100644 --- a/environment/manager_test.go +++ b/environment/manager_test.go @@ -53,7 +53,7 @@ func (s *CreateOrUpdateTestSuite) mockEnvironmentExists(exists bool) { func (s *CreateOrUpdateTestSuite) mockRegisterEnvironment() *mock.Call { return s.runnerManagerMock.On("RegisterEnvironment", - mock.AnythingOfType("EnvironmentId"), mock.AnythingOfType("NomadJobId"), mock.AnythingOfType("int")). + mock.AnythingOfType("EnvironmentId"), mock.AnythingOfType("NomadJobId"), mock.AnythingOfType("uint")). Return() } diff --git a/nomad/api_querier.go b/nomad/api_querier.go index 079a891..909820f 100644 --- a/nomad/api_querier.go +++ b/nomad/api_querier.go @@ -16,10 +16,10 @@ type apiQuerier interface { LoadJobList() (list []*nomadApi.JobListStub, err error) // JobScale returns the scale of the passed job. - JobScale(jobId string) (jobScale int, err error) + JobScale(jobId string) (jobScale uint, err error) // SetJobScale sets the scaling count of the passed job to Nomad. - SetJobScale(jobId string, count int, reason string) (err error) + SetJobScale(jobId string, count uint, reason string) (err error) // DeleteRunner deletes the runner with the given Id. DeleteRunner(runnerId string) (err error) diff --git a/nomad/api_querier_mock.go b/nomad/api_querier_mock.go index ead43bb..b8b3b14 100644 --- a/nomad/api_querier_mock.go +++ b/nomad/api_querier_mock.go @@ -78,14 +78,14 @@ func (_m *apiQuerierMock) ExecuteCommand(allocationID string, ctx context.Contex } // JobScale provides a mock function with given fields: jobId -func (_m *apiQuerierMock) JobScale(jobId string) (int, error) { +func (_m *apiQuerierMock) JobScale(jobId string) (uint, error) { ret := _m.Called(jobId) - var r0 int - if rf, ok := ret.Get(0).(func(string) int); ok { + var r0 uint + if rf, ok := ret.Get(0).(func(string) uint); ok { r0 = rf(jobId) } else { - r0 = ret.Get(0).(int) + r0 = ret.Get(0).(uint) } var r1 error @@ -143,11 +143,11 @@ func (_m *apiQuerierMock) RegisterNomadJob(job *api.Job) (string, error) { } // SetJobScale provides a mock function with given fields: jobId, count, reason -func (_m *apiQuerierMock) SetJobScale(jobId string, count int, reason string) error { +func (_m *apiQuerierMock) SetJobScale(jobId string, count uint, reason string) error { ret := _m.Called(jobId, count, reason) var r0 error - if rf, ok := ret.Get(0).(func(string, int, string) error); ok { + if rf, ok := ret.Get(0).(func(string, uint, string) error); ok { r0 = rf(jobId, count, reason) } else { r0 = ret.Error(0) diff --git a/nomad/executor_api_mock.go b/nomad/executor_api_mock.go index 60800aa..97f90e2 100644 --- a/nomad/executor_api_mock.go +++ b/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.8.0. DO NOT EDIT. package nomad @@ -78,14 +78,14 @@ func (_m *ExecutorApiMock) ExecuteCommand(allocationID string, ctx context.Conte } // JobScale provides a mock function with given fields: jobId -func (_m *ExecutorApiMock) JobScale(jobId string) (int, error) { +func (_m *ExecutorApiMock) JobScale(jobId string) (uint, error) { ret := _m.Called(jobId) - var r0 int - if rf, ok := ret.Get(0).(func(string) int); ok { + var r0 uint + if rf, ok := ret.Get(0).(func(string) uint); ok { r0 = rf(jobId) } else { - r0 = ret.Get(0).(int) + r0 = ret.Get(0).(uint) } var r1 error @@ -180,11 +180,11 @@ func (_m *ExecutorApiMock) RegisterNomadJob(job *api.Job) (string, error) { } // SetJobScale provides a mock function with given fields: jobId, count, reason -func (_m *ExecutorApiMock) SetJobScale(jobId string, count int, reason string) error { +func (_m *ExecutorApiMock) SetJobScale(jobId string, count uint, reason string) error { ret := _m.Called(jobId, count, reason) var r0 error - if rf, ok := ret.Get(0).(func(string, int, string) error); ok { + if rf, ok := ret.Get(0).(func(string, uint, string) error); ok { r0 = rf(jobId, count, reason) } else { r0 = ret.Error(0) diff --git a/nomad/job.go b/nomad/job.go index 4fe11f9..01a63b7 100644 --- a/nomad/job.go +++ b/nomad/job.go @@ -17,18 +17,19 @@ func (nc *nomadApiClient) LoadJobList() (list []*nomadApi.JobListStub, err error } // JobScale returns the scale of the passed job. -func (nc *nomadApiClient) JobScale(jobId string) (jobScale int, err error) { +func (nc *nomadApiClient) JobScale(jobId string) (jobScale uint, err error) { status, _, err := nc.client.Jobs().ScaleStatus(jobId, nil) if err != nil { return } // ToDo: Consider counting also the placed and desired allocations - jobScale = 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 int, reason string) (err error) { - _, _, err = nc.client.Jobs().Scale(jobId, fmt.Sprintf(TaskGroupNameFormat, jobId), &count, reason, false, nil, nil) +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) return } diff --git a/runner/manager.go b/runner/manager.go index 3a0f4aa..497a8fd 100644 --- a/runner/manager.go +++ b/runner/manager.go @@ -25,7 +25,7 @@ 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 { // RegisterEnvironment adds a new environment that should be managed. - RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount int) + RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount uint) // EnvironmentExists returns whether the environment with the given id exists. EnvironmentExists(id EnvironmentId) bool @@ -61,14 +61,14 @@ type NomadJob struct { environmentId EnvironmentId jobId NomadJobId idleRunners Storage - desiredIdleRunnersCount int + desiredIdleRunnersCount uint } func (j *NomadJob) Id() EnvironmentId { return j.environmentId } -func (m *NomadRunnerManager) RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount int) { +func (m *NomadRunnerManager) RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount uint) { m.jobs.Add(&NomadJob{ environmentId, nomadJobId, @@ -120,7 +120,7 @@ func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) { // this environment does not exist return } - lastJobScaling := -1 + var lastJobScaling uint = 0 for { runners, err := m.apiClient.LoadRunners(string(job.jobId)) if err != nil { @@ -138,7 +138,7 @@ func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) { log.WithError(err).Printf("Failed get allocation count") break } - additionallyNeededRunners := job.desiredIdleRunnersCount - job.idleRunners.Length() + 1 + additionallyNeededRunners := job.desiredIdleRunnersCount - uint(job.idleRunners.Length()) + 1 requiredRunnerCount := jobScale if additionallyNeededRunners > 0 { requiredRunnerCount += additionallyNeededRunners diff --git a/runner/manager_mock.go b/runner/manager_mock.go index 58af5f7..2fa0150 100644 --- a/runner/manager_mock.go +++ b/runner/manager_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v0.0.0-dev. DO NOT EDIT. +// Code generated by mockery v2.8.0. DO NOT EDIT. package runner @@ -70,7 +70,7 @@ func (_m *ManagerMock) Get(runnerId string) (Runner, error) { } // RegisterEnvironment provides a mock function with given fields: environmentId, nomadJobId, desiredIdleRunnersCount -func (_m *ManagerMock) RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount int) { +func (_m *ManagerMock) RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount uint) { _m.Called(environmentId, nomadJobId, desiredIdleRunnersCount) } diff --git a/runner/manager_test.go b/runner/manager_test.go index 8de2245..34147b8 100644 --- a/runner/manager_test.go +++ b/runner/manager_test.go @@ -12,7 +12,7 @@ import ( ) const ( - defaultDesiredRunnersCount = 5 + defaultDesiredRunnersCount uint = 5 ) func TestGetNextRunnerTestSuite(t *testing.T) { @@ -38,8 +38,8 @@ func (s *ManagerTestSuite) mockRunnerQueries(returnedRunnerIds []string) { // reset expected calls to allow new mocked return values s.apiMock.ExpectedCalls = []*mock.Call{} 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("int"), "Runner Requested").Return(nil) + s.apiMock.On("JobScale", tests.DefaultJobId).Return(uint(len(returnedRunnerIds)), nil) + s.apiMock.On("SetJobScale", tests.DefaultJobId, mock.AnythingOfType("uint"), "Runner Requested").Return(nil) } func (s *ManagerTestSuite) registerDefaultEnvironment() {