Only create exactly one new runner when one runner is claimed

Previously we would create as much runners as needed based on the
local idleRunnersCount and the desiredIdleRunnersCount. This is
problematic if two runners are claimed shortly after one another.
As we only add a runner to the idleRunners list once we get the
event from Nomad, the second runner claim in a short timeframe
would create two new runners. This has been fixed now.
This commit is contained in:
sirkrypt0
2021-06-29 09:11:21 +02:00
parent e0e254a6af
commit 50a2a22b74
2 changed files with 14 additions and 4 deletions

View File

@ -171,11 +171,11 @@ func (m *NomadRunnerManager) updateRunnerSpecs(environmentID EnvironmentID, temp
}
func (m *NomadRunnerManager) Claim(environmentID EnvironmentID, duration int) (Runner, error) {
job, ok := m.environments.Get(environmentID)
environment, ok := m.environments.Get(environmentID)
if !ok {
return nil, ErrUnknownExecutionEnvironment
}
runner, ok := job.idleRunners.Sample()
runner, ok := environment.idleRunners.Sample()
if !ok {
return nil, ErrNoRunnersAvailable
}
@ -187,9 +187,9 @@ func (m *NomadRunnerManager) Claim(environmentID EnvironmentID, duration int) (R
runner.SetupTimeout(time.Duration(duration) * time.Second)
err = m.scaleEnvironment(environmentID)
err = m.createRunner(environment)
if err != nil {
log.WithError(err).WithField("environmentID", environmentID).Error("Couldn't scale environment")
log.WithError(err).WithField("environmentID", environmentID).Error("Couldn't create new runner for claimed one")
}
return runner, nil

View File

@ -133,6 +133,16 @@ func (s *ManagerTestSuite) TestClaimAddsRunnerToUsedRunners() {
s.Equal(savedRunner, receivedRunner)
}
func (s *ManagerTestSuite) TestTwoClaimsAddExactlyTwoRunners() {
s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner)
s.AddIdleRunnerForDefaultEnvironment(NewRunner(tests.AnotherRunnerID, s.nomadRunnerManager))
_, err := s.nomadRunnerManager.Claim(defaultEnvironmentID, defaultInactivityTimeout)
s.Require().NoError(err)
_, err = s.nomadRunnerManager.Claim(defaultEnvironmentID, defaultInactivityTimeout)
s.Require().NoError(err)
s.apiMock.AssertNumberOfCalls(s.T(), "RegisterRunnerJob", 2)
}
func (s *ManagerTestSuite) TestGetReturnsRunnerIfRunnerIsUsed() {
s.nomadRunnerManager.usedRunners.Add(s.exerciseRunner)
savedRunner, err := s.nomadRunnerManager.Get(s.exerciseRunner.Id())