diff --git a/commands/single_test.go b/commands/single_test.go
index badb8ceb14be109495d675704fbc97ec211dbca8..9121d721c8fd6a7d06e050141a5c950d80147cb4 100644
--- a/commands/single_test.go
+++ b/commands/single_test.go
@@ -83,8 +83,11 @@ func mockingExecutionStack(t *testing.T, executorName string, maxBuilds int, job
}
//ExecutorProvider
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Times(maxBuilds + 1)
+
p.On("Create").Return(&e).Times(maxBuilds)
- p.On("GetFeatures", mock.Anything).Times(maxBuilds)
p.On("Acquire", mock.Anything).Return(&common.MockExecutorData{}, nil).Times(maxBuilds)
p.On("Release", mock.Anything, mock.Anything).Return(nil).Times(maxBuilds)
diff --git a/common/build_test.go b/common/build_test.go
index cf21dd6e373180df0fef34155dbf4b5436691693..4d5734f42f89d5c665f3a73b57e5d9bbec636a1a 100644
--- a/common/build_test.go
+++ b/common/build_test.go
@@ -26,8 +26,11 @@ func TestBuildRun(t *testing.T) {
defer p.AssertExpectations(t)
// Create executor only once
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Twice()
+
p.On("Create").Return(&e).Once()
- p.On("GetFeatures", mock.Anything).Once()
// We run everything once
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
@@ -71,8 +74,11 @@ func TestRetryPrepare(t *testing.T) {
defer p.AssertExpectations(t)
// Create executor
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Twice()
+
p.On("Create").Return(&e).Times(3)
- p.On("GetFeatures", mock.Anything).Once()
// Prepare plan
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).
@@ -112,8 +118,11 @@ func TestPrepareFailure(t *testing.T) {
defer p.AssertExpectations(t)
// Create executor
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Twice()
+
p.On("Create").Return(&e).Times(3)
- p.On("GetFeatures", mock.Anything).Once()
// Prepare plan
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).
@@ -144,8 +153,11 @@ func TestPrepareFailureOnBuildError(t *testing.T) {
defer p.AssertExpectations(t)
// Create executor
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Twice()
+
p.On("Create").Return(&e).Times(1)
- p.On("GetFeatures", mock.Anything).Once()
// Prepare plan
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).
@@ -182,8 +194,11 @@ func TestRunFailureRunsAfterScriptAndArtifactsOnFailure(t *testing.T) {
defer p.AssertExpectations(t)
// Create executor
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Twice()
+
p.On("Create").Return(&e).Once()
- p.On("GetFeatures", mock.Anything).Once()
// Prepare plan
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).Return(nil)
@@ -224,8 +239,11 @@ func TestGetSourcesRunFailure(t *testing.T) {
defer p.AssertExpectations(t)
// Create executor
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Twice()
+
p.On("Create").Return(&e).Once()
- p.On("GetFeatures", mock.Anything).Once()
// Prepare plan
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).Return(nil)
@@ -264,8 +282,11 @@ func TestArtifactDownloadRunFailure(t *testing.T) {
defer p.AssertExpectations(t)
// Create executor
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Twice()
+
p.On("Create").Return(&e).Once()
- p.On("GetFeatures", mock.Anything).Once()
// Prepare plan
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).Return(nil)
@@ -306,8 +327,11 @@ func TestArtifactUploadRunFailure(t *testing.T) {
defer p.AssertExpectations(t)
// Create executor
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Twice()
+
p.On("Create").Return(&e).Once()
- p.On("GetFeatures", mock.Anything).Once()
// Prepare plan
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).Return(nil)
@@ -357,8 +381,11 @@ func TestRestoreCacheRunFailure(t *testing.T) {
defer p.AssertExpectations(t)
// Create executor
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Twice()
+
p.On("Create").Return(&e).Once()
- p.On("GetFeatures", mock.Anything).Once()
// Prepare plan
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).Return(nil)
@@ -397,8 +424,11 @@ func TestRunWrongAttempts(t *testing.T) {
defer p.AssertExpectations(t)
// Create executor
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Twice()
+
p.On("Create").Return(&e)
- p.On("GetFeatures", mock.Anything).Once()
// Prepare plan
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).Return(nil)
@@ -433,8 +463,11 @@ func TestRunSuccessOnSecondAttempt(t *testing.T) {
p := MockExecutorProvider{}
// Create executor only once
+ p.On("CanCreate").Return(true).Once()
+ p.On("GetDefaultShell").Return("bash").Once()
+ p.On("GetFeatures", mock.Anything).Return(nil).Twice()
+
p.On("Create").Return(&e).Once()
- p.On("GetFeatures", mock.Anything).Once()
// We run everything once
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
diff --git a/common/executor.go b/common/executor.go
index 1514151ff42f108203174b2416d9f568d3bc2536..2a004c9ec66874c394fed86347a84db4be74146e 100644
--- a/common/executor.go
+++ b/common/executor.go
@@ -2,6 +2,8 @@ package common
import (
"context"
+ "errors"
+ "fmt"
log "github.com/sirupsen/logrus"
)
@@ -47,7 +49,8 @@ type ExecutorProvider interface {
Create() Executor
Acquire(config *RunnerConfig) (ExecutorData, error)
Release(config *RunnerConfig, data ExecutorData) error
- GetFeatures(features *FeaturesInfo)
+ GetFeatures(features *FeaturesInfo) error
+ GetDefaultShell() string
}
type BuildError struct {
@@ -64,9 +67,29 @@ func (b *BuildError) Error() string {
var executors map[string]ExecutorProvider
+func validateExecutorProvider(provider ExecutorProvider) error {
+ if provider.GetDefaultShell() == "" {
+ return errors.New("default shell not implemented")
+ }
+
+ if !provider.CanCreate() {
+ return errors.New("cannot create executor")
+ }
+
+ if err := provider.GetFeatures(&FeaturesInfo{}); err != nil {
+ return fmt.Errorf("cannot get features: %v", err)
+ }
+
+ return nil
+}
+
func RegisterExecutor(executor string, provider ExecutorProvider) {
log.Debugln("Registering", executor, "executor...")
+ if err := validateExecutorProvider(provider); err != nil {
+ panic("Executor cannot be registered: " + err.Error())
+ }
+
if executors == nil {
executors = make(map[string]ExecutorProvider)
}
diff --git a/common/mock_ExecutorProvider.go b/common/mock_ExecutorProvider.go
index af68e2c4786617fefc3cf267b6a17f34a63afd89..2e5488e351e17a371c77a14f3d8f906ce6694878 100644
--- a/common/mock_ExecutorProvider.go
+++ b/common/mock_ExecutorProvider.go
@@ -64,9 +64,32 @@ func (_m *MockExecutorProvider) Create() Executor {
return r0
}
+// GetDefaultShell provides a mock function with given fields:
+func (_m *MockExecutorProvider) GetDefaultShell() string {
+ ret := _m.Called()
+
+ var r0 string
+ if rf, ok := ret.Get(0).(func() string); ok {
+ r0 = rf()
+ } else {
+ r0 = ret.Get(0).(string)
+ }
+
+ return r0
+}
+
// GetFeatures provides a mock function with given fields: features
-func (_m *MockExecutorProvider) GetFeatures(features *FeaturesInfo) {
- _m.Called(features)
+func (_m *MockExecutorProvider) GetFeatures(features *FeaturesInfo) error {
+ ret := _m.Called(features)
+
+ var r0 error
+ if rf, ok := ret.Get(0).(func(*FeaturesInfo) error); ok {
+ r0 = rf(features)
+ } else {
+ r0 = ret.Error(0)
+ }
+
+ return r0
}
// Release provides a mock function with given fields: config, data
diff --git a/common/network.go b/common/network.go
index 5ea21b487dc62f318bb063301de21c4145514fa0..4959740859bbdbcd1ab5e6b9299cd902b50fa2ca 100644
--- a/common/network.go
+++ b/common/network.go
@@ -93,6 +93,7 @@ type VersionInfo struct {
Platform string `json:"platform,omitempty"`
Architecture string `json:"architecture,omitempty"`
Executor string `json:"executor,omitempty"`
+ Shell string `json:"shell,omitempty"`
Features FeaturesInfo `json:"features"`
}
diff --git a/executors/default_executor_provider.go b/executors/default_executor_provider.go
index 716810bb4bf467edd2ad8b80e1c805900cc21fd9..67a4a60cdb40ae747877c7c31e8742d237794815 100644
--- a/executors/default_executor_provider.go
+++ b/executors/default_executor_provider.go
@@ -1,10 +1,15 @@
package executors
-import "gitlab.com/gitlab-org/gitlab-runner/common"
+import (
+ "errors"
+
+ "gitlab.com/gitlab-org/gitlab-runner/common"
+)
type DefaultExecutorProvider struct {
- Creator func() common.Executor
- FeaturesUpdater func(features *common.FeaturesInfo)
+ Creator func() common.Executor
+ FeaturesUpdater func(features *common.FeaturesInfo)
+ DefaultShellName string
}
func (e DefaultExecutorProvider) CanCreate() bool {
@@ -26,8 +31,15 @@ func (e DefaultExecutorProvider) Release(config *common.RunnerConfig, data commo
return nil
}
-func (e DefaultExecutorProvider) GetFeatures(features *common.FeaturesInfo) {
- if e.FeaturesUpdater != nil {
- e.FeaturesUpdater(features)
+func (e DefaultExecutorProvider) GetFeatures(features *common.FeaturesInfo) error {
+ if e.FeaturesUpdater == nil {
+ return errors.New("cannot evaluate features")
}
+
+ e.FeaturesUpdater(features)
+ return nil
+}
+
+func (e DefaultExecutorProvider) GetDefaultShell() string {
+ return e.DefaultShellName
}
diff --git a/executors/docker/executor_docker_command.go b/executors/docker/executor_docker_command.go
index 955d2bfcd4a59ec42ee9f22257333675e441c27b..c01fbac78f59545613c087f07feda250a4e33ce4 100644
--- a/executors/docker/executor_docker_command.go
+++ b/executors/docker/executor_docker_command.go
@@ -121,7 +121,8 @@ func init() {
}
common.RegisterExecutor("docker", executors.DefaultExecutorProvider{
- Creator: creator,
- FeaturesUpdater: featuresUpdater,
+ Creator: creator,
+ FeaturesUpdater: featuresUpdater,
+ DefaultShellName: options.Shell.Shell,
})
}
diff --git a/executors/docker/executor_docker_ssh.go b/executors/docker/executor_docker_ssh.go
index f0e0b3857b88282e58e455c519ed7d5a6ca757ab..740d0a61c4310089df1e6648019ed47e0cf72645 100644
--- a/executors/docker/executor_docker_ssh.go
+++ b/executors/docker/executor_docker_ssh.go
@@ -112,7 +112,8 @@ func init() {
}
common.RegisterExecutor("docker-ssh", executors.DefaultExecutorProvider{
- Creator: creator,
- FeaturesUpdater: featuresUpdater,
+ Creator: creator,
+ FeaturesUpdater: featuresUpdater,
+ DefaultShellName: options.Shell.Shell,
})
}
diff --git a/executors/docker/machine/provider.go b/executors/docker/machine/provider.go
index 40ae6fcef4720de6f3e15fd877720833fecee4ce..9111022e805daf2098487e8c58c894a5e83a261d 100644
--- a/executors/docker/machine/provider.go
+++ b/executors/docker/machine/provider.go
@@ -405,8 +405,12 @@ func (m *machineProvider) CanCreate() bool {
return m.provider.CanCreate()
}
-func (m *machineProvider) GetFeatures(features *common.FeaturesInfo) {
- m.provider.GetFeatures(features)
+func (m *machineProvider) GetFeatures(features *common.FeaturesInfo) error {
+ return m.provider.GetFeatures(features)
+}
+
+func (m *machineProvider) GetDefaultShell() string {
+ return m.provider.GetDefaultShell()
}
func (m *machineProvider) Create() common.Executor {
diff --git a/executors/kubernetes/executor_kubernetes.go b/executors/kubernetes/executor_kubernetes.go
index 7db7157505a18d22610a08178effaca746041b34..35a34e4b8773a0343301bffe11374f89b7a0d98b 100644
--- a/executors/kubernetes/executor_kubernetes.go
+++ b/executors/kubernetes/executor_kubernetes.go
@@ -549,7 +549,8 @@ func featuresFn(features *common.FeaturesInfo) {
func init() {
common.RegisterExecutor("kubernetes", executors.DefaultExecutorProvider{
- Creator: createFn,
- FeaturesUpdater: featuresFn,
+ Creator: createFn,
+ FeaturesUpdater: featuresFn,
+ DefaultShellName: executorOptions.Shell.Shell,
})
}
diff --git a/executors/parallels/executor_parallels.go b/executors/parallels/executor_parallels.go
index 2b0c0520fe6e93e35b800fa044095f113faf589b..91a37d7887f7cac223673d233bb103ff80834c38 100644
--- a/executors/parallels/executor_parallels.go
+++ b/executors/parallels/executor_parallels.go
@@ -342,7 +342,8 @@ func init() {
}
common.RegisterExecutor("parallels", executors.DefaultExecutorProvider{
- Creator: creator,
- FeaturesUpdater: featuresUpdater,
+ Creator: creator,
+ FeaturesUpdater: featuresUpdater,
+ DefaultShellName: options.Shell.Shell,
})
}
diff --git a/executors/shell/executor_shell.go b/executors/shell/executor_shell.go
index 05d14d364543dabab549a573416177196dd00af1..63fdfcad9a7b6f19676ffcd5fa751e3f6e33f206 100644
--- a/executors/shell/executor_shell.go
+++ b/executors/shell/executor_shell.go
@@ -9,12 +9,13 @@ import (
"path/filepath"
"fmt"
+ "time"
+
"github.com/kardianos/osext"
"github.com/sirupsen/logrus"
"gitlab.com/gitlab-org/gitlab-runner/common"
"gitlab.com/gitlab-org/gitlab-runner/executors"
"gitlab.com/gitlab-org/gitlab-runner/helpers"
- "time"
)
type executor struct {
@@ -158,7 +159,8 @@ func init() {
}
common.RegisterExecutor("shell", executors.DefaultExecutorProvider{
- Creator: creator,
- FeaturesUpdater: featuresUpdater,
+ Creator: creator,
+ FeaturesUpdater: featuresUpdater,
+ DefaultShellName: options.Shell.Shell,
})
}
diff --git a/executors/ssh/executor_ssh.go b/executors/ssh/executor_ssh.go
index 1ea829c17bd70e5d812e402f68b78c7f05a3cd5b..295b565e03adf8e1e507d77deef5f1bae60b0e5d 100644
--- a/executors/ssh/executor_ssh.go
+++ b/executors/ssh/executor_ssh.go
@@ -88,7 +88,8 @@ func init() {
}
common.RegisterExecutor("ssh", executors.DefaultExecutorProvider{
- Creator: creator,
- FeaturesUpdater: featuresUpdater,
+ Creator: creator,
+ FeaturesUpdater: featuresUpdater,
+ DefaultShellName: options.Shell.Shell,
})
}
diff --git a/executors/virtualbox/executor_virtualbox.go b/executors/virtualbox/executor_virtualbox.go
index 49de53736a2fd767e498b07e7e91753c33199455..8936df08f8050d26c21b77d8653bd66e387d5baf 100644
--- a/executors/virtualbox/executor_virtualbox.go
+++ b/executors/virtualbox/executor_virtualbox.go
@@ -3,11 +3,12 @@ package virtualbox
import (
"errors"
"fmt"
+ "time"
+
"gitlab.com/gitlab-org/gitlab-runner/common"
"gitlab.com/gitlab-org/gitlab-runner/executors"
"gitlab.com/gitlab-org/gitlab-runner/helpers/ssh"
vbox "gitlab.com/gitlab-org/gitlab-runner/helpers/virtualbox"
- "time"
)
type executor struct {
@@ -321,7 +322,8 @@ func init() {
}
common.RegisterExecutor("virtualbox", executors.DefaultExecutorProvider{
- Creator: creator,
- FeaturesUpdater: featuresUpdater,
+ Creator: creator,
+ FeaturesUpdater: featuresUpdater,
+ DefaultShellName: options.Shell.Shell,
})
}
diff --git a/network/gitlab.go b/network/gitlab.go
index 46c60450f32a47e7a5a703f3d5b00898c3323c6a..f954f1674c21df33120d3638b548dee0d1fa8212 100644
--- a/network/gitlab.go
+++ b/network/gitlab.go
@@ -132,13 +132,18 @@ func (n *GitLabClient) getRunnerVersion(config common.RunnerConfig) common.Versi
Platform: runtime.GOOS,
Architecture: runtime.GOARCH,
Executor: config.Executor,
+ Shell: config.Shell,
}
if executor := common.GetExecutor(config.Executor); executor != nil {
executor.GetFeatures(&info.Features)
+
+ if info.Shell == "" {
+ info.Shell = executor.GetDefaultShell()
+ }
}
- if shell := common.GetShell(config.Shell); shell != nil {
+ if shell := common.GetShell(info.Shell); shell != nil {
shell.GetFeatures(&info.Features)
}
diff --git a/network/gitlab_test.go b/network/gitlab_test.go
index 7366951033d6b3ecbeac200fd28557d5507f8707..7f5c47de68fbfff0a078afb379905528914fb4eb 100644
--- a/network/gitlab_test.go
+++ b/network/gitlab_test.go
@@ -13,6 +13,7 @@ import (
"testing"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
. "gitlab.com/gitlab-org/gitlab-runner/common"
@@ -982,3 +983,56 @@ func TestArtifactsDownload(t *testing.T) {
state = c.DownloadArtifacts(fileNotFoundTokenCredentials, artifactsFileName)
assert.Equal(t, DownloadNotFound, state, "Artifacts should be bit downloaded if it's not found")
}
+
+func TestRunnerVersion(t *testing.T) {
+ c := NewGitLabClient()
+ info := c.getRunnerVersion(RunnerConfig{
+ RunnerSettings: RunnerSettings{
+ Executor: "my-executor",
+ Shell: "my-shell",
+ },
+ })
+
+ assert.NotEmpty(t, info.Name)
+ assert.NotEmpty(t, info.Version)
+ assert.NotEmpty(t, info.Revision)
+ assert.NotEmpty(t, info.Platform)
+ assert.NotEmpty(t, info.Architecture)
+ assert.Equal(t, "my-executor", info.Executor)
+ assert.Equal(t, "my-shell", info.Shell)
+}
+
+func TestRunnerVersionToGetExecutorAndShellFeaturesWithTheDefaultShell(t *testing.T) {
+ executorProvider := MockExecutorProvider{}
+ defer executorProvider.AssertExpectations(t)
+ executorProvider.On("GetDefaultShell").Return("my-default-executor-shell").Twice()
+ executorProvider.On("CanCreate").Return(true).Once()
+ executorProvider.On("GetFeatures", mock.Anything).Return(nil).Run(func(args mock.Arguments) {
+ features := args[0].(*FeaturesInfo)
+ features.Shared = true
+ })
+ RegisterExecutor("my-test-executor", &executorProvider)
+
+ shell := MockShell{}
+ defer shell.AssertExpectations(t)
+ shell.On("GetName").Return("my-default-executor-shell")
+ shell.On("GetFeatures", mock.Anything).Return(nil).Run(func(args mock.Arguments) {
+ features := args[0].(*FeaturesInfo)
+ features.Variables = true
+ })
+ RegisterShell(&shell)
+
+ c := NewGitLabClient()
+ info := c.getRunnerVersion(RunnerConfig{
+ RunnerSettings: RunnerSettings{
+ Executor: "my-test-executor",
+ Shell: "",
+ },
+ })
+
+ assert.Equal(t, "my-test-executor", info.Executor)
+ assert.Equal(t, "my-default-executor-shell", info.Shell)
+ assert.False(t, info.Features.Artifacts, "dry-run that this is not enabled")
+ assert.True(t, info.Features.Shared, "feature is enabled by executor")
+ assert.True(t, info.Features.Variables, "feature is enabled by shell")
+}