Commit 45415f3c authored by Kamil Trzciński's avatar Kamil Trzciński 🔴 Committed by Alessio Caiazza

Always prefer creating new containers when running with Docker Executor

parent e669d73c
......@@ -36,3 +36,4 @@ plugins:
enabled: false
exclude_patterns:
- vendor/
- .gopath/
......@@ -353,7 +353,10 @@ func (b *Build) Run(globalConfig *Config, trace JobTrace) (err error) {
var executor Executor
b.logger = NewBuildLogger(trace, b.Log())
b.logger.Println(fmt.Sprintf("Running with %s\n on %s (%s)", AppVersion.Line(), b.Runner.Name, b.Runner.ShortDescription()))
b.logger.Println("Running with", AppVersion.Line())
if b.Runner != nil && b.Runner.ShortDescription() != "" {
b.logger.Println(" on", b.Runner.Name, b.Runner.ShortDescription())
}
b.CurrentState = BuildRunStatePending
......
......@@ -42,6 +42,18 @@ func GetRemoteSuccessfulBuild() (JobResponse, error) {
return getRemoteBuildResponse("echo Hello World")
}
func GetRemoteSuccessfulBuildWithAfterScript() (JobResponse, error) {
jobResponse, err := getRemoteBuildResponse("echo Hello World")
jobResponse.Steps = append(jobResponse.Steps,
Step{
Name: StepNameAfterScript,
Script: []string{"echo Hello World"},
When: StepWhenAlways,
},
)
return jobResponse, err
}
func GetRemoteSuccessfulBuildWithDumpedVariables() (response JobResponse, err error) {
variableName := "test_dump"
variableValue := "test"
......
This diff is collapsed.
......@@ -11,8 +11,7 @@ import (
type commandExecutor struct {
executor
predefinedContainer *types.ContainerJSON
buildContainer *types.ContainerJSON
buildContainer *types.ContainerJSON
}
func (s *commandExecutor) Prepare(options common.ExecutorPrepareOptions) error {
......@@ -27,41 +26,66 @@ func (s *commandExecutor) Prepare(options common.ExecutorPrepareOptions) error {
return errors.New("Script is not compatible with Docker")
}
prebuildImage, err := s.getPrebuiltImage()
_, err = s.getPrebuiltImage()
if err != nil {
return err
}
_, err = s.getBuildImage()
if err != nil {
return err
}
return nil
}
func (s *commandExecutor) requestNewPredefinedContainer() (*types.ContainerJSON, error) {
prebuildImage, err := s.getPrebuiltImage()
if err != nil {
return nil, err
}
buildImage := common.Image{
Name: prebuildImage.ID,
}
// Start pre-build container which will git clone changes
s.predefinedContainer, err = s.createContainer("predefined", buildImage, common.ContainerCommandBuild, []string{prebuildImage.ID})
containerJSON, err := s.createContainer("predefined", buildImage, common.ContainerCommandBuild, []string{prebuildImage.ID})
if err != nil {
return err
return nil, err
}
// Start build container which will run actual build
s.buildContainer, err = s.createContainer("build", s.Build.Image, s.BuildShell.DockerCommand, []string{})
if err != nil {
return err
return containerJSON, err
}
func (s *commandExecutor) requestBuildContainer() (*types.ContainerJSON, error) {
if s.buildContainer == nil {
var err error
// Start build container which will run actual build
s.buildContainer, err = s.createContainer("build", s.Build.Image, s.BuildShell.DockerCommand, []string{})
if err != nil {
return nil, err
}
}
return nil
return s.buildContainer, nil
}
func (s *commandExecutor) Run(cmd common.ExecutorCommand) error {
s.SetCurrentStage(DockerExecutorStageRun)
var runOn *types.ContainerJSON
var err error
if cmd.Predefined {
runOn = s.predefinedContainer
runOn, err = s.requestNewPredefinedContainer()
} else {
runOn = s.buildContainer
runOn, err = s.requestBuildContainer()
}
if err != nil {
return err
}
s.Debugln("Executing on", runOn.Name, "the", cmd.Script)
s.SetCurrentStage(DockerExecutorStageRun)
return s.watchContainer(cmd.Context, runOn.ID, bytes.NewBufferString(cmd.Script))
}
......
......@@ -128,11 +128,14 @@ func TestDockerCommandWithAllowedImagesRun(t *testing.T) {
}
func isDockerOlderThan17_07(t *testing.T) bool {
cmd := exec.Command("docker", "version", "--format", "{{.Server.Version}}")
output, err := cmd.Output()
require.NoError(t, err, "docker version should return output")
client, err := docker_helpers.New(
docker_helpers.DockerCredentials{}, docker.DockerAPIVersion)
require.NoError(t, err, "should be able to connect to docker")
localVersion, err := version.NewVersion(strings.TrimSpace(string(output)))
types, err := client.Info(context.Background())
require.NoError(t, err, "should be able to get docker info")
localVersion, err := version.NewVersion(types.ServerVersion)
require.NoError(t, err)
checkedVersion, err := version.NewVersion("17.07.0-ce")
......@@ -648,7 +651,7 @@ func waitForDocker(credentials docker_helpers.DockerCredentials) error {
}
for i := 0; i < 20; i++ {
_, err = client.Info(context.TODO())
_, err = client.Info(context.Background())
if err == nil {
break
}
......@@ -866,5 +869,44 @@ func TestDockerCommandWithHelperImageConfig(t *testing.T) {
assert.NoError(t, err)
out := buffer.String()
assert.Contains(t, out, "Pulling docker image "+helperImageConfig)
assert.Contains(t, out, "Using docker image sha256:bbd86c6ba107ae2feb8dbf9024df4b48597c44e1b584a3d901bba91f7fc500e3 for predefined container...")
assert.Contains(t, out, "Using docker image sha256:bbd86c6ba107ae2feb8dbf9024df4b48597c44e1b584a3d901bba91f7fc500e3 for gitlab/gitlab-runner-helper:x86_64-64eea86c ...")
}
func TestDockerCommandWithDoingPruneAndAfterScript(t *testing.T) {
if helpers.SkipIntegrationTests(t, "docker", "info") {
return
}
successfulBuild, err := common.GetRemoteSuccessfulBuildWithAfterScript()
// This scripts removes self-created containers that do exit
// It will fail if: cannot be removed, or no containers is found
// It is assuming that name of each runner created container starts
// with `runner-doprune-`
successfulBuild.Steps[0].Script = common.StepScript{
"docker ps -a -f status=exited | grep runner-doprune-",
"docker rm $(docker ps -a -f status=exited | grep runner-doprune- | awk '{print $1}')",
}
assert.NoError(t, err)
build := &common.Build{
JobResponse: successfulBuild,
Runner: &common.RunnerConfig{
RunnerCredentials: common.RunnerCredentials{
Token: "doprune",
},
RunnerSettings: common.RunnerSettings{
Executor: "docker",
Docker: &common.DockerConfig{
Image: "docker:git",
Volumes: []string{
"/var/run/docker.sock:/var/run/docker.sock",
},
},
},
},
}
err = build.Run(&common.Config{}, &common.Trace{Writer: os.Stdout})
assert.NoError(t, err)
}
......@@ -203,7 +203,7 @@ func testServiceFromNamedImage(t *testing.T, description, imageName, serviceName
Once()
c.On("ImageInspectWithRaw", e.Context, imageName).
Return(types.ImageInspect{}, nil, nil).
Return(types.ImageInspect{ID: "image-id"}, nil, nil).
Twice()
c.On("ContainerRemove", e.Context, containerName, types.ContainerRemoveOptions{RemoveVolumes: true, Force: true}).
......@@ -290,7 +290,7 @@ func TestDockerForExistingImage(t *testing.T) {
Once()
c.On("ImageInspectWithRaw", e.Context, "existing").
Return(types.ImageInspect{}, nil, nil).
Return(types.ImageInspect{ID: "image-id"}, nil, nil).
Once()
image, err := e.pullDockerImage("existing", nil)
......@@ -372,7 +372,7 @@ func TestDockerPolicyModeIfNotPresentForExistingImage(t *testing.T) {
e.setPolicyMode(common.PullPolicyIfNotPresent)
c.On("ImageInspectWithRaw", e.Context, "existing").
Return(types.ImageInspect{}, nil, nil).
Return(types.ImageInspect{ID: "image-id"}, nil, nil).
Once()
image, err := e.getDockerImage("existing")
......@@ -398,7 +398,7 @@ func TestDockerPolicyModeIfNotPresentForNotExistingImage(t *testing.T) {
Once()
c.On("ImageInspectWithRaw", e.Context, "not-existing").
Return(types.ImageInspect{}, nil, nil).
Return(types.ImageInspect{ID: "image-id"}, nil, nil).
Once()
image, err := e.getDockerImage("not-existing")
......@@ -406,7 +406,7 @@ func TestDockerPolicyModeIfNotPresentForNotExistingImage(t *testing.T) {
assert.NotNil(t, image)
c.On("ImageInspectWithRaw", e.Context, "not-existing").
Return(types.ImageInspect{}, nil, nil).
Return(types.ImageInspect{ID: "image-id"}, nil, nil).
Once()
// It shouldn't execute the pull for second time
......@@ -424,7 +424,7 @@ func TestDockerPolicyModeAlwaysForExistingImage(t *testing.T) {
e.setPolicyMode(common.PullPolicyAlways)
c.On("ImageInspectWithRaw", e.Context, "existing").
Return(types.ImageInspect{}, nil, nil).
Return(types.ImageInspect{ID: "image-id"}, nil, nil).
Once()
options := buildImagePullOptions(e, "existing:latest")
......@@ -433,7 +433,7 @@ func TestDockerPolicyModeAlwaysForExistingImage(t *testing.T) {
Once()
c.On("ImageInspectWithRaw", e.Context, "existing").
Return(types.ImageInspect{}, nil, nil).
Return(types.ImageInspect{ID: "image-id"}, nil, nil).
Once()
image, err := e.getDockerImage("existing")
......@@ -450,7 +450,7 @@ func TestDockerPolicyModeAlwaysForLocalOnlyImage(t *testing.T) {
e.setPolicyMode(common.PullPolicyAlways)
c.On("ImageInspectWithRaw", e.Context, "existing").
Return(types.ImageInspect{}, nil, nil).
Return(types.ImageInspect{ID: "image-id"}, nil, nil).
Once()
options := buildImagePullOptions(e, "existing:lastest")
......@@ -472,7 +472,7 @@ func TestDockerGetExistingDockerImageIfPullFails(t *testing.T) {
e.setPolicyMode(common.PullPolicyAlways)
c.On("ImageInspectWithRaw", e.Context, "to-pull").
Return(types.ImageInspect{}, nil, nil).
Return(types.ImageInspect{ID: "image-id"}, nil, nil).
Once()
options := buildImagePullOptions(e, "to-pull")
......
......@@ -542,6 +542,10 @@ func (b *AbstractShell) writeAfterScript(w ShellWriter, info common.ShellScriptI
return nil
}
if len(afterScriptStep.Script) == 0 {
return nil
}
b.writeExports(w, info)
b.writeCdBuildDir(w, info)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment