htwkmooc/poseidon
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Update all runners when updating environment

Browse Source 
Previously only the default job would be updated to the newest specs.
Now all Nomad jobs that belong to the given environment are updated
accordingly.
This commit is contained in:
sirkrypt0
2021-06-08 16:48:38 +02:00
committed by Maximilian Paß
parent c7d59810e5
commit 0020590c96
5 changed files with 88 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
environment/manager.go
View File

@ -48,8 +48,6 @@ func (m *NomadEnvironmentManager) CreateOrUpdate(
if err != nil {
return false, err
}
exists := m.runnerManager.EnvironmentExists(runner.EnvironmentID(idInt))
err = m.registerDefaultJob(id,
request.PrewarmingPoolSize, request.CPULimit, request.MemoryLimit,
request.Image, request.NetworkAccess, request.ExposedPorts)
@ -58,15 +56,11 @@ func (m *NomadEnvironmentManager) CreateOrUpdate(
return false, err
}
// TODO: If already exists, make sure to update all existing runners as well
if !exists {
err = m.runnerManager.RegisterEnvironment(
runner.EnvironmentID(idInt), request.PrewarmingPoolSize)
if err != nil {
return false, err
}
created, err := m.runnerManager.CreateOrUpdateEnvironment(runner.EnvironmentID(idInt), request.PrewarmingPoolSize)
if err != nil {
return created, err
}
return !exists, nil
return created, nil
}
func (m *NomadEnvironmentManager) Delete(id string) {
@ -75,7 +69,7 @@ func (m *NomadEnvironmentManager) Delete(id string) {
func (m *NomadEnvironmentManager) Load() {
// ToDo: remove create default execution environment for debugging purposes
err := m.runnerManager.RegisterEnvironment(runner.EnvironmentID(0), 5)
_, err := m.runnerManager.CreateOrUpdateEnvironment(runner.EnvironmentID(0), 5)
if err != nil {
return
}

31
environment/manager_test.go
View File

@ -28,7 +28,7 @@ func TestCreateOrUpdateTestSuite(t *testing.T) {
func (s *CreateOrUpdateTestSuite) SetupTest() {
s.runnerManagerMock = runner.ManagerMock{}
s.runnerManagerMock.On("RegisterEnvironment", mock.Anything, mock.Anything).Return(nil)
s.runnerManagerMock.On("registerEnvironment", mock.Anything, mock.Anything).Return(nil)
s.apiMock = nomad.ExecutorAPIMock{}
s.request = dto.ExecutionEnvironmentRequest{
@ -53,10 +53,10 @@ func (s *CreateOrUpdateTestSuite) mockEnvironmentExists(exists bool) {
s.runnerManagerMock.On("EnvironmentExists", mock.AnythingOfType("EnvironmentID")).Return(exists)
}
func (s *CreateOrUpdateTestSuite) mockRegisterEnvironment() *mock.Call {
return s.runnerManagerMock.On("RegisterEnvironment",
func (s *CreateOrUpdateTestSuite) mockCreateOrUpdateEnvironment(exists bool) *mock.Call {
return s.runnerManagerMock.On("CreateOrUpdateEnvironment",
mock.AnythingOfType("EnvironmentID"), mock.AnythingOfType("uint")).
Return()
Return(!exists, nil)
}
func (s *CreateOrUpdateTestSuite) createJobForRequest() *nomadApi.Job {
@ -77,7 +77,7 @@ func (s *CreateOrUpdateTestSuite) TestFailsOnTooLargeID() {
}
func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsRegistersCorrectJob() {
s.mockEnvironmentExists(true)
s.mockCreateOrUpdateEnvironment(true)
expectedJob := s.createJobForRequest()
created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request)
@ -87,7 +87,7 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsRegistersCorrectJob()
}
func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsOccurredErrorIsPassed() {
s.mockEnvironmentExists(true)
s.mockCreateOrUpdateEnvironment(true)
s.registerNomadJobMockCall.Return("", tests.ErrDefault)
created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request)
@ -96,7 +96,7 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsOccurredErrorIsPassed
}
func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsReturnsFalse() {
s.mockEnvironmentExists(true)
s.mockCreateOrUpdateEnvironment(true)
created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request)
s.NoError(err)
@ -104,8 +104,7 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsReturnsFalse() {
}
func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentDoesNotExistRegistersCorrectJob() {
s.mockEnvironmentExists(false)
s.mockRegisterEnvironment()
s.mockCreateOrUpdateEnvironment(false)
expectedJob := s.createJobForRequest()
@ -116,24 +115,20 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentDoesNotExistRegistersCorrec
}
func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentDoesNotExistRegistersCorrectEnvironment() {
s.mockEnvironmentExists(false)
s.mockRegisterEnvironment()
s.mockCreateOrUpdateEnvironment(false)
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),
s.request.PrewarmingPoolSize)
s.runnerManagerMock.AssertCalled(s.T(), "CreateOrUpdateEnvironment",
runner.EnvironmentID(tests.DefaultEnvironmentIdAsInteger), s.request.PrewarmingPoolSize)
}
func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentDoesNotExistOccurredErrorIsPassedAndNoEnvironmentRegistered() {
s.mockEnvironmentExists(false)
s.mockRegisterEnvironment()
s.mockCreateOrUpdateEnvironment(false)
s.registerNomadJobMockCall.Return("", tests.ErrDefault)
created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request)
s.False(created)
s.Equal(tests.ErrDefault, err)
s.runnerManagerMock.AssertNotCalled(s.T(), "RegisterEnvironment")
s.Equal(tests.DefaultError, err)
}

56
runner/manager.go
View File

@ -9,6 +9,7 @@ import (
"gitlab.hpi.de/codeocean/codemoon/poseidon/logging"
"gitlab.hpi.de/codeocean/codemoon/poseidon/nomad"
"strconv"
"strings"
"time"
)
@ -32,11 +33,9 @@ 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(id EnvironmentID, desiredIdleRunnersCount uint) error
// EnvironmentExists returns whether the environment with the given id exists.
EnvironmentExists(id EnvironmentID) bool
// CreateOrUpdateEnvironment creates the given environment if it does not exist. Otherwise, it updates
// the existing environment and all runners.
CreateOrUpdateEnvironment(environmentID EnvironmentID, desiredIdleRunnersCount uint) (bool, error)
// 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.
@ -81,7 +80,15 @@ func (j *NomadEnvironment) ID() EnvironmentID {
return j.environmentID
}
func (m *NomadRunnerManager) RegisterEnvironment(environmentID EnvironmentID, desiredIdleRunnersCount uint) error {
func (m *NomadRunnerManager) CreateOrUpdateEnvironment(id EnvironmentID, desiredIdleRunnersCount uint) (bool, error) {
_, ok := m.environments.Get(id)
if !ok {
return true, m.registerEnvironment(id, desiredIdleRunnersCount)
}
return false, m.updateEnvironment(id, desiredIdleRunnersCount)
}
func (m *NomadRunnerManager) registerEnvironment(environmentID EnvironmentID, desiredIdleRunnersCount uint) error {
templateJob, err := m.apiClient.LoadTemplateJob(environmentID.toString())
if err != nil {
return fmt.Errorf("couldn't register environment: %w", err)
@ -100,9 +107,40 @@ func (m *NomadRunnerManager) RegisterEnvironment(environmentID EnvironmentID, de
return nil
}
func (m *NomadRunnerManager) EnvironmentExists(id EnvironmentID) (ok bool) {
_, ok = m.environments.Get(id)
return
func (m *NomadRunnerManager) updateEnvironment(id EnvironmentID, desiredIdleRunnersCount uint) error {
environment, ok := m.environments.Get(id)
if !ok {
return ErrUnknownExecutionEnvironment
}
environment.desiredIdleRunnersCount = desiredIdleRunnersCount
templateJob, err := m.apiClient.LoadTemplateJob(id.toString())
if err != nil {
return fmt.Errorf("update environment couldn't load template job: %w", err)
}
environment.templateJob = templateJob
runners, err := m.apiClient.LoadRunners(id.toString())
if err != nil {
return fmt.Errorf("update environment couldn't load runners: %w", err)
}
var occurredErrors []string
for _, id := range runners {
// avoid taking the address of the loop variable
runnerID := id
updatedRunnerJob := *environment.templateJob
updatedRunnerJob.ID = &runnerID
updatedRunnerJob.Name = &runnerID
_, err := m.apiClient.RegisterNomadJob(&updatedRunnerJob)
if err != nil {
occurredErrors = append(occurredErrors, err.Error())
}
}
if len(occurredErrors) > 0 {
errorResult := strings.Join(occurredErrors, "\n")
return fmt.Errorf("%d errors occurred when updating environment: %s", len(occurredErrors), errorResult)
}
return nil
}
func (m *NomadRunnerManager) Claim(environmentID EnvironmentID) (Runner, error) {

40
runner/manager_mock.go
View File

@ -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)
@ -32,27 +32,34 @@ func (_m *ManagerMock) Claim(id EnvironmentID) (Runner, error) {
return r0, r1
}
// EnvironmentExists provides a mock function with given fields: id
func (_m *ManagerMock) EnvironmentExists(id EnvironmentID) bool {
ret := _m.Called(id)
// CreateOrUpdateEnvironment provides a mock function with given fields: environmentId, desiredIdleRunnersCount
func (_m *ManagerMock) CreateOrUpdateEnvironment(environmentId EnvironmentId, desiredIdleRunnersCount uint) (bool, error) {
ret := _m.Called(environmentId, desiredIdleRunnersCount)
var r0 bool
if rf, ok := ret.Get(0).(func(EnvironmentID) bool); ok {
r0 = rf(id)
if rf, ok := ret.Get(0).(func(EnvironmentId, uint) bool); ok {
r0 = rf(environmentId, desiredIdleRunnersCount)
} else {
r0 = ret.Get(0).(bool)
}
return r0
var r1 error
if rf, ok := ret.Get(1).(func(EnvironmentId, uint) error); ok {
r1 = rf(environmentId, desiredIdleRunnersCount)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// 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 +68,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,11 +76,6 @@ func (_m *ManagerMock) Get(runnerID string) (Runner, error) {
return r0, r1
}
// 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
func (_m *ManagerMock) Return(r Runner) error {
ret := _m.Called(r)

21
runner/manager_test.go
View File

@ -7,7 +7,6 @@ import (
"github.com/sirupsen/logrus"
"github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"gitlab.hpi.de/codeocean/codemoon/poseidon/nomad"
"gitlab.hpi.de/codeocean/codemoon/poseidon/tests"
@ -59,7 +58,7 @@ func mockRunnerQueries(apiMock *nomad.ExecutorAPIMock, returnedRunnerIds []strin
}
func (s *ManagerTestSuite) registerDefaultEnvironment() {
err := s.nomadRunnerManager.RegisterEnvironment(defaultEnvironmentId, 0)
err := s.nomadRunnerManager.registerEnvironment(defaultEnvironmentId, 0)
s.Require().NoError(err)
}
@ -73,7 +72,7 @@ func (s *ManagerTestSuite) waitForRunnerRefresh() {
}
func (s *ManagerTestSuite) TestRegisterEnvironmentAddsNewJob() {
err := s.nomadRunnerManager.RegisterEnvironment(anotherEnvironmentId, defaultDesiredRunnersCount)
err := s.nomadRunnerManager.registerEnvironment(anotherEnvironmentId, defaultDesiredRunnersCount)
s.Require().NoError(err)
job, ok := s.nomadRunnerManager.environments.Get(defaultEnvironmentId)
s.True(ok)
@ -251,19 +250,3 @@ func modifyMockedCall(apiMock *nomad.ExecutorAPIMock, method string, modifier fu
}
}
}
func (s *ManagerTestSuite) TestWhenEnvironmentDoesNotExistEnvironmentExistsReturnsFalse() {
id := anotherEnvironmentId
_, ok := s.nomadRunnerManager.environments.Get(id)
require.False(s.T(), ok)
s.False(s.nomadRunnerManager.EnvironmentExists(id))
}
func (s *ManagerTestSuite) TestWhenEnvironmentExistsEnvironmentExistsReturnsTrue() {
id := anotherEnvironmentId
s.nomadRunnerManager.environments.Add(&NomadEnvironment{environmentId: id})
exists := s.nomadRunnerManager.EnvironmentExists(id)
s.True(exists)
}