From ba6e7035faaa39e6e0dfae246162c35de8bd2599 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maximilian=20Pa=C3=9F?=
 <22845248+mpass99@users.noreply.github.com>
Date: Fri, 22 Oct 2021 14:54:50 +0200
Subject: [PATCH] Mark runner as used now asynchronously.

---
 internal/environment/environment_test.go |  2 ++
 internal/runner/manager.go               | 19 ++++++++++++-------
 internal/runner/manager_test.go          | 17 +++++++++++++++++
 3 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/internal/environment/environment_test.go b/internal/environment/environment_test.go
index 2c71fa1..6c824b4 100644
--- a/internal/environment/environment_test.go
+++ b/internal/environment/environment_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	"testing"
+	"time"
 )
 
 func TestConfigureNetworkCreatesNewNetworkWhenNoNetworkExists(t *testing.T) {
@@ -188,5 +189,6 @@ func TestTwoSampleAddExactlyTwoRunners(t *testing.T) {
 	_, ok = environment.Sample(apiMock)
 	require.True(t, ok)
 
+	<-time.After(tests.ShortTimeout) // New Runners are requested asynchronously
 	apiMock.AssertNumberOfCalls(t, "RegisterRunnerJob", 2)
 }
diff --git a/internal/runner/manager.go b/internal/runner/manager.go
index dab5abd..983c673 100644
--- a/internal/runner/manager.go
+++ b/internal/runner/manager.go
@@ -141,23 +141,28 @@ func (m *NomadRunnerManager) Claim(environmentID dto.EnvironmentID, duration int
 	if !ok {
 		return nil, ErrUnknownExecutionEnvironment
 	}
-	log.Debug("Before Sample")
 	runner, ok := environment.Sample(m.apiClient)
 	if !ok {
 		return nil, ErrNoRunnersAvailable
 	}
+
 	m.usedRunners.Add(runner)
-	log.Debug("Before Mark Runner As Used")
-	err := m.apiClient.MarkRunnerAsUsed(runner.ID(), duration)
-	if err != nil {
-		return nil, fmt.Errorf("can't mark runner as used: %w", err)
-	}
-	log.Debug("After Mark Runner As Used")
+	go m.markRunnerAsUsed(runner, duration)
 
 	runner.SetupTimeout(time.Duration(duration) * time.Second)
 	return runner, nil
 }
 
+func (m *NomadRunnerManager) markRunnerAsUsed(runner Runner, timeoutDuration int) {
+	err := m.apiClient.MarkRunnerAsUsed(runner.ID(), timeoutDuration)
+	if err != nil {
+		err = m.Return(runner)
+		if err != nil {
+			log.WithError(err).WithField("runnerID", runner.ID()).Error("can't mark runner as used and can't return runner")
+		}
+	}
+}
+
 func (m *NomadRunnerManager) Get(runnerID string) (Runner, error) {
 	runner, ok := m.usedRunners.Get(runnerID)
 	if !ok {
diff --git a/internal/runner/manager_test.go b/internal/runner/manager_test.go
index 393d0ef..ed15ffe 100644
--- a/internal/runner/manager_test.go
+++ b/internal/runner/manager_test.go
@@ -150,6 +150,23 @@ func (s *ManagerTestSuite) TestClaimAddsRunnerToUsedRunners() {
 	s.Equal(savedRunner, receivedRunner)
 }
 
+func (s *ManagerTestSuite) TestClaimRemovesRunnerWhenMarkAsUsedFails() {
+	s.exerciseEnvironment.On("Sample", mock.Anything).Return(s.exerciseRunner, true)
+	s.apiMock.On("DeleteJob", mock.AnythingOfType("string")).Return(nil)
+	modifyMockedCall(s.apiMock, "MarkRunnerAsUsed", func(call *mock.Call) {
+		call.Run(func(args mock.Arguments) {
+			call.ReturnArguments = mock.Arguments{tests.ErrDefault}
+		})
+	})
+
+	claimedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID, defaultInactivityTimeout)
+	s.Require().NoError(err)
+	<-time.After(tests.ShortTimeout) // Claimed runners are marked as used asynchronously
+	s.apiMock.AssertCalled(s.T(), "DeleteJob", claimedRunner.ID())
+	_, ok := s.nomadRunnerManager.usedRunners.Get(claimedRunner.ID())
+	s.False(ok)
+}
+
 func (s *ManagerTestSuite) TestGetReturnsRunnerIfRunnerIsUsed() {
 	s.nomadRunnerManager.usedRunners.Add(s.exerciseRunner)
 	savedRunner, err := s.nomadRunnerManager.Get(s.exerciseRunner.ID())