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

Introduce a context for RetryExponential

Browse Source 
as second criteria (next to the maximum number of attempts) for canceling the retrying. This is required as we started with the previous commit to retry the nomad environment recovery. This always fails for unit tests (as they are not connected to an Nomad cluster). Before, we ignored the one error but the retrying leads to unit test timeouts.
Additionally, we now stop retrying to create a runner when the environment got deleted.
This commit is contained in:
Maximilian Paß
2023-08-16 18:22:44 +02:00
committed by Sebastian Serth
parent 73759f8a3c
commit 13a9da95e5
7 changed files with 68 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
internal/environment/nomad_environment.go
View File

@ -245,9 +245,7 @@ func (n *NomadEnvironment) Sample() (runner.Runner, bool) {
r, ok := n.idleRunners.Sample()
if ok && n.idleRunners.Length() < n.PrewarmingPoolSize() {
go func() {
err := util.RetryExponential(func() error {
return n.createRunner(false)
})
err := util.RetryExponentialContext(n.ctx, func() error { return n.createRunner(false) })
if err != nil {
log.WithError(err).WithField(dto.KeyEnvironmentID, n.ID().ToString()).
Error("Couldn't create new runner for claimed one")

17
internal/environment/nomad_environment_test.go
View File

@ -1,6 +1,7 @@
package environment
import (
"context"
"fmt"
nomadApi "github.com/hashicorp/nomad/api"
"github.com/openHPI/poseidon/internal/nomad"
@ -18,7 +19,7 @@ import (
func TestConfigureNetworkCreatesNewNetworkWhenNoNetworkExists(t *testing.T) {
_, job := helpers.CreateTemplateJob()
defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(job)
environment := &NomadEnvironment{nil, "", job, nil, nil, nil}
environment := &NomadEnvironment{nil, "", job, nil, context.Background(), nil}
if assert.Equal(t, 0, len(defaultTaskGroup.Networks)) {
environment.SetNetworkAccess(true, []uint16{})
@ -30,7 +31,7 @@ func TestConfigureNetworkCreatesNewNetworkWhenNoNetworkExists(t *testing.T) {
func TestConfigureNetworkDoesNotCreateNewNetworkWhenNetworkExists(t *testing.T) {
_, job := helpers.CreateTemplateJob()
defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(job)
environment := &NomadEnvironment{nil, "", job, nil, nil, nil}
environment := &NomadEnvironment{nil, "", job, nil, context.Background(), nil}
networkResource := &nomadApi.NetworkResource{Mode: "cni/secure-bridge"}
defaultTaskGroup.Networks = []*nomadApi.NetworkResource{networkResource}
@ -59,7 +60,7 @@ func TestConfigureNetworkSetsCorrectValues(t *testing.T) {
_, testJob := helpers.CreateTemplateJob()
testTaskGroup := nomad.FindAndValidateDefaultTaskGroup(testJob)
testTask := nomad.FindAndValidateDefaultTask(testTaskGroup)
testEnvironment := &NomadEnvironment{nil, "", job, nil, nil, nil}
testEnvironment := &NomadEnvironment{nil, "", job, nil, context.Background(), nil}
testEnvironment.SetNetworkAccess(false, ports)
mode, ok := testTask.Config["network_mode"]
@ -74,7 +75,7 @@ func TestConfigureNetworkSetsCorrectValues(t *testing.T) {
_, testJob := helpers.CreateTemplateJob()
testTaskGroup := nomad.FindAndValidateDefaultTaskGroup(testJob)
testTask := nomad.FindAndValidateDefaultTask(testTaskGroup)
testEnvironment := &NomadEnvironment{nil, "", testJob, nil, nil, nil}
testEnvironment := &NomadEnvironment{nil, "", testJob, nil, context.Background(), nil}
testEnvironment.SetNetworkAccess(true, ports)
require.Equal(t, 1, len(testTaskGroup.Networks))
@ -133,7 +134,7 @@ func TestRegisterTemplateJobSucceedsWhenMonitoringEvaluationSucceeds(t *testing.
apiClientMock.On("DeleteJob", mock.AnythingOfType("string")).Return(nil)
environment := &NomadEnvironment{apiClientMock, "", &nomadApi.Job{},
storage.NewLocalStorage[runner.Runner](), nil, nil}
storage.NewLocalStorage[runner.Runner](), context.Background(), nil}
environment.SetID(tests.DefaultEnvironmentIDAsInteger)
err := environment.Register()
@ -150,7 +151,7 @@ func TestRegisterTemplateJobReturnsErrorWhenMonitoringEvaluationFails(t *testing
apiClientMock.On("DeleteJob", mock.AnythingOfType("string")).Return(nil)
environment := &NomadEnvironment{apiClientMock, "", &nomadApi.Job{},
storage.NewLocalStorage[runner.Runner](), nil, nil}
storage.NewLocalStorage[runner.Runner](), context.Background(), nil}
environment.SetID(tests.DefaultEnvironmentIDAsInteger)
err := environment.Register()
@ -177,7 +178,7 @@ func TestTwoSampleAddExactlyTwoRunners(t *testing.T) {
_, job := helpers.CreateTemplateJob()
environment := &NomadEnvironment{apiMock, templateEnvironmentJobHCL, job,
storage.NewLocalStorage[runner.Runner](), nil, nil}
storage.NewLocalStorage[runner.Runner](), context.Background(), nil}
environment.SetPrewarmingPoolSize(2)
runner1 := &runner.RunnerMock{}
runner1.On("ID").Return(tests.DefaultRunnerID)
@ -212,7 +213,7 @@ func TestSampleDoesNotSetForcePullFlag(t *testing.T) {
_, job := helpers.CreateTemplateJob()
environment := &NomadEnvironment{apiMock, templateEnvironmentJobHCL, job,
storage.NewLocalStorage[runner.Runner](), nil, nil}
storage.NewLocalStorage[runner.Runner](), context.Background(), nil}
runner1 := &runner.RunnerMock{}
runner1.On("ID").Return(tests.DefaultRunnerID)
environment.AddRunner(runner1)

3
internal/environment/nomad_manager.go
View File

@ -36,6 +36,7 @@ func NewNomadEnvironmentManager(
runnerManager runner.Manager,
apiClient nomad.ExecutorAPI,
templateJobFile string,
ctx context.Context,
) (*NomadEnvironmentManager, error) {
if err := loadTemplateEnvironmentJobHCL(templateJobFile); err != nil {
return nil, err
@ -43,7 +44,7 @@ func NewNomadEnvironmentManager(
m := &NomadEnvironmentManager{&AbstractManager{nil, runnerManager},
apiClient, templateEnvironmentJobHCL}
if err := util.RetryExponential(func() error { return m.Load() }); err != nil {
if err := util.RetryExponentialContext(ctx, func() error { return m.Load() }); err != nil {
log.WithError(err).Error("Error recovering the execution environments")
}
runnerManager.Load()

23
internal/environment/nomad_manager_test.go
View File

@ -95,6 +95,9 @@ func (s *CreateOrUpdateTestSuite) TestCreateOrUpdatesSetsForcePullFlag() {
}
func TestNewNomadEnvironmentManager(t *testing.T) {
disableRecovery, cancel := context.WithCancel(context.Background())
cancel()
executorAPIMock := &nomad.ExecutorAPIMock{}
executorAPIMock.On("LoadEnvironmentJobs").Return([]*nomadApi.Job{}, nil)
@ -104,7 +107,7 @@ func TestNewNomadEnvironmentManager(t *testing.T) {
previousTemplateEnvironmentJobHCL := templateEnvironmentJobHCL
t.Run("returns error if template file does not exist", func(t *testing.T) {
_, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, "/non-existent/file")
_, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, "/non-existent/file", disableRecovery)
assert.Error(t, err)
})
@ -115,7 +118,7 @@ func TestNewNomadEnvironmentManager(t *testing.T) {
f := createTempFile(t, templateJobHCL)
defer os.Remove(f.Name())
m, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, f.Name())
m, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, f.Name(), disableRecovery)
assert.NoError(t, err)
assert.NotNil(t, m)
assert.Equal(t, templateJobHCL, m.templateEnvironmentHCL)
@ -126,7 +129,7 @@ func TestNewNomadEnvironmentManager(t *testing.T) {
f := createTempFile(t, templateJobHCL)
defer os.Remove(f.Name())
m, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, f.Name())
m, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, f.Name(), disableRecovery)
require.NoError(t, err)
_, err = NewNomadEnvironment(tests.DefaultEnvironmentIDAsInteger, nil, m.templateEnvironmentHCL)
assert.Error(t, err)
@ -136,6 +139,9 @@ func TestNewNomadEnvironmentManager(t *testing.T) {
}
func TestNomadEnvironmentManager_Get(t *testing.T) {
disableRecovery, cancel := context.WithCancel(context.Background())
cancel()
apiMock := &nomad.ExecutorAPIMock{}
mockWatchAllocations(apiMock)
apiMock.On("LoadRunnerIDs", mock.AnythingOfType("string")).Return([]string{}, nil)
@ -146,7 +152,7 @@ func TestNomadEnvironmentManager_Get(t *testing.T) {
})
runnerManager := runner.NewNomadRunnerManager(apiMock, context.Background())
m, err := NewNomadEnvironmentManager(runnerManager, apiMock, "")
m, err := NewNomadEnvironmentManager(runnerManager, apiMock, "", disableRecovery)
require.NoError(t, err)
t.Run("Returns error when not found", func(t *testing.T) {
@ -217,6 +223,9 @@ func TestNomadEnvironmentManager_Get(t *testing.T) {
}
func TestNomadEnvironmentManager_List(t *testing.T) {
disableRecovery, cancel := context.WithCancel(context.Background())
cancel()
apiMock := &nomad.ExecutorAPIMock{}
mockWatchAllocations(apiMock)
call := apiMock.On("LoadEnvironmentJobs")
@ -225,7 +234,7 @@ func TestNomadEnvironmentManager_List(t *testing.T) {
})
runnerManager := runner.NewNomadRunnerManager(apiMock, context.Background())
m, err := NewNomadEnvironmentManager(runnerManager, apiMock, "")
m, err := NewNomadEnvironmentManager(runnerManager, apiMock, "", disableRecovery)
require.NoError(t, err)
t.Run("with no environments", func(t *testing.T) {
@ -287,7 +296,7 @@ func TestNomadEnvironmentManager_Load(t *testing.T) {
_, ok := runnerManager.GetEnvironment(tests.DefaultEnvironmentIDAsInteger)
require.False(t, ok)
_, err := NewNomadEnvironmentManager(runnerManager, apiMock, "")
_, err := NewNomadEnvironmentManager(runnerManager, apiMock, "", context.Background())
require.NoError(t, err)
environment, ok := runnerManager.GetEnvironment(tests.DefaultEnvironmentIDAsInteger)
@ -305,7 +314,7 @@ func TestNomadEnvironmentManager_Load(t *testing.T) {
_, ok := runnerManager.GetEnvironment(tests.DefaultEnvironmentIDAsInteger)
require.False(t, ok)
_, err := NewNomadEnvironmentManager(runnerManager, apiMock, "")
_, err := NewNomadEnvironmentManager(runnerManager, apiMock, "", context.Background())
require.NoError(t, err)
_, ok = runnerManager.GetEnvironment(tests.DefaultEnvironmentIDAsInteger)