From e465b0aa4f59684ac7dd094692e7a08325c4464c Mon Sep 17 00:00:00 2001
From: Steve Azzopardi <steveazz@outlook.com>
Date: Wed, 23 Jan 2019 15:08:52 +0100
Subject: [PATCH] Update ExecutorProvider interface signature
Update the `Release` signature of the `ExecutorProvider` interface.
Returning an error when releasing a resource is not actionable, and all
of the implementations of `ExecutorProvider` do not return an error.
---
commands/multi.go | 11 ++---------
common/executor.go | 2 +-
common/mock_ExecutorProvider.go | 13 ++-----------
executors/default_executor_provider.go | 4 +---
executors/docker/machine/provider.go | 5 ++---
executors/docker/machine/provider_test.go | 3 +--
6 files changed, 9 insertions(+), 29 deletions(-)
diff --git a/commands/multi.go b/commands/multi.go
index 4f45f7cff..fd32de0e0 100644
--- a/commands/multi.go
+++ b/commands/multi.go
@@ -216,21 +216,14 @@ func (mr *RunCommand) acquireRunnerResources(provider common.ExecutorProvider, r
return nil, func() {}, fmt.Errorf("failed to update executor: %v", err)
}
- releaseProviderFn := func() {
- err := provider.Release(runner, executorData)
- if err != nil {
- logrus.WithError(err).Error("Failed to release executor")
- }
- }
-
if !mr.buildsHelper.acquireBuild(runner) {
- releaseProviderFn()
+ provider.Release(runner, executorData)
return nil, nil, errors.New("failed to request job, runner limit met")
}
releaseFn := func() {
mr.buildsHelper.releaseBuild(runner)
- releaseProviderFn()
+ provider.Release(runner, executorData)
}
return executorData, releaseFn, nil
diff --git a/common/executor.go b/common/executor.go
index f8ceb36c8..8dbed88b2 100644
--- a/common/executor.go
+++ b/common/executor.go
@@ -48,7 +48,7 @@ type ExecutorProvider interface {
CanCreate() bool
Create() Executor
Acquire(config *RunnerConfig) (ExecutorData, error)
- Release(config *RunnerConfig, data ExecutorData) error
+ Release(config *RunnerConfig, data ExecutorData)
GetFeatures(features *FeaturesInfo) error
GetDefaultShell() string
}
diff --git a/common/mock_ExecutorProvider.go b/common/mock_ExecutorProvider.go
index 2e5488e35..47722361f 100644
--- a/common/mock_ExecutorProvider.go
+++ b/common/mock_ExecutorProvider.go
@@ -93,15 +93,6 @@ func (_m *MockExecutorProvider) GetFeatures(features *FeaturesInfo) error {
}
// Release provides a mock function with given fields: config, data
-func (_m *MockExecutorProvider) Release(config *RunnerConfig, data ExecutorData) error {
- ret := _m.Called(config, data)
-
- var r0 error
- if rf, ok := ret.Get(0).(func(*RunnerConfig, ExecutorData) error); ok {
- r0 = rf(config, data)
- } else {
- r0 = ret.Error(0)
- }
-
- return r0
+func (_m *MockExecutorProvider) Release(config *RunnerConfig, data ExecutorData) {
+ _m.Called(config, data)
}
diff --git a/executors/default_executor_provider.go b/executors/default_executor_provider.go
index 67a4a60cd..ebffd49a3 100644
--- a/executors/default_executor_provider.go
+++ b/executors/default_executor_provider.go
@@ -27,9 +27,7 @@ func (e DefaultExecutorProvider) Acquire(config *common.RunnerConfig) (common.Ex
return nil, nil
}
-func (e DefaultExecutorProvider) Release(config *common.RunnerConfig, data common.ExecutorData) error {
- return nil
-}
+func (e DefaultExecutorProvider) Release(config *common.RunnerConfig, data common.ExecutorData) {}
func (e DefaultExecutorProvider) GetFeatures(features *common.FeaturesInfo) error {
if e.FeaturesUpdater == nil {
diff --git a/executors/docker/machine/provider.go b/executors/docker/machine/provider.go
index 9111022e8..8e7f2cbc3 100644
--- a/executors/docker/machine/provider.go
+++ b/executors/docker/machine/provider.go
@@ -379,7 +379,7 @@ func (m *machineProvider) Use(config *common.RunnerConfig, data common.ExecutorD
return
}
-func (m *machineProvider) Release(config *common.RunnerConfig, data common.ExecutorData) error {
+func (m *machineProvider) Release(config *common.RunnerConfig, data common.ExecutorData) {
// Release machine
details, ok := data.(*machineDetails)
if ok {
@@ -393,12 +393,11 @@ func (m *machineProvider) Release(config *common.RunnerConfig, data common.Execu
config.Machine.MaxBuilds > 0 && details.UsedCount >= config.Machine.MaxBuilds {
err := m.remove(details.Name, "Too many builds")
if err == nil {
- return nil
+ return
}
}
details.State = machineStateIdle
}
- return nil
}
func (m *machineProvider) CanCreate() bool {
diff --git a/executors/docker/machine/provider_test.go b/executors/docker/machine/provider_test.go
index d81e500d4..3b0aaf9ea 100644
--- a/executors/docker/machine/provider_test.go
+++ b/executors/docker/machine/provider_test.go
@@ -564,8 +564,7 @@ func TestMachineReleaseIfInvalidDataArePassed(t *testing.T) {
assert.NotNil(t, nd)
assertTotalMachines(t, p, 1, "it creates one machine")
- err = p.Release(nil, nd)
- assert.NoError(t, err, "it should not fail")
+ p.Release(nil, nd)
}
func TestMachineCreationIfFailedToConnect(t *testing.T) {
--
GitLab