From 527aaf713f9b8fe5834a3f3d92b1e9c03bb95425 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maximilian=20Pa=C3=9F?=
 <22845248+mpass99@users.noreply.github.com>
Date: Wed, 14 Jun 2023 22:27:52 +0100
Subject: [PATCH] Fix decreased prewarming pool due to inactivity timer.

When allocations fail and restart they are added again to the idle runners. The bug fixed with this commit is that the inactivity timer was not stopped at the restart. This led to the idle runner being removed when the timer expired.
---
 internal/runner/nomad_manager.go      |  7 ++++--
 internal/runner/nomad_manager_test.go | 36 +++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/internal/runner/nomad_manager.go b/internal/runner/nomad_manager.go
index c8aed76..401ab31 100644
--- a/internal/runner/nomad_manager.go
+++ b/internal/runner/nomad_manager.go
@@ -190,8 +190,11 @@ func (m *NomadRunnerManager) onAllocationStopped(runnerID string) (alreadyRemove
 		return false
 	}
 
-	_, stillActive := m.usedRunners.Get(runnerID)
-	m.usedRunners.Delete(runnerID)
+	r, stillActive := m.usedRunners.Get(runnerID)
+	if stillActive {
+		r.StopTimeout()
+		m.usedRunners.Delete(runnerID)
+	}
 
 	environment, ok := m.environments.Get(environmentID.ToString())
 	if ok {
diff --git a/internal/runner/nomad_manager_test.go b/internal/runner/nomad_manager_test.go
index 4af0f70..427ec19 100644
--- a/internal/runner/nomad_manager_test.go
+++ b/internal/runner/nomad_manager_test.go
@@ -367,6 +367,42 @@ func (s *ManagerTestSuite) TestOnAllocationAdded() {
 	})
 }
 
+func (s *ManagerTestSuite) TestOnAllocationStopped() {
+	s.Run("stops inactivity timer", func() {
+		testStoppedInactivityTimer(s, true)
+	})
+	s.Run("stops inactivity timer - counter check", func() {
+		testStoppedInactivityTimer(s, false)
+	})
+}
+
+func testStoppedInactivityTimer(s *ManagerTestSuite, stopAllocation bool) {
+	s.T().Helper()
+	environment, ok := s.nomadRunnerManager.environments.Get(tests.DefaultEnvironmentIDAsString)
+	s.Require().True(ok)
+	mockIdleRunners(environment.(*ExecutionEnvironmentMock))
+
+	inactivityTimerCalled := false
+	environment.AddRunner(NewNomadJob(tests.DefaultRunnerID, []nomadApi.PortMapping{}, s.apiMock, func(r Runner) error {
+		inactivityTimerCalled = true
+		return nil
+	}))
+
+	runner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID, 1)
+	s.Require().NoError(err)
+	s.Require().False(runner.TimeoutPassed())
+	s.Require().False(inactivityTimerCalled)
+
+	if stopAllocation {
+		alreadyRemoved := s.nomadRunnerManager.onAllocationStopped(runner.ID())
+		s.False(alreadyRemoved)
+	}
+
+	<-time.After(time.Second + tests.ShortTimeout)
+	s.NotEqual(stopAllocation, runner.TimeoutPassed())
+	s.NotEqual(stopAllocation, inactivityTimerCalled)
+}
+
 func TestNomadRunnerManager_Load(t *testing.T) {
 	apiMock := &nomad.ExecutorAPIMock{}
 	mockWatchAllocations(apiMock)