Unverified Commit c5bc5753 authored by Tomasz Maczukin's avatar Tomasz Maczukin Committed by Tomasz Maczukin

Merge branch 'make-volumes-to-work-on-linux-docker-on-windows' into 'master'

Make volumes to work on linux docker on windows

Closes #4251

See merge request gitlab-org/gitlab-runner!1363
parent a419fbe4
......@@ -13,3 +13,5 @@ const waitForContainerTimeout = 15 * time.Second
const osTypeLinux = "linux"
const osTypeWindows = "windows"
const metadataOSType = "OSType"
......@@ -52,8 +52,9 @@ var errVolumesManagerUndefined = errors.New("volumesManager is undefined")
type executor struct {
executors.AbstractExecutor
client docker_helpers.Client
info types.Info
client docker_helpers.Client
volumeParser parser.Parser
info types.Info
temporary []string // IDs of containers that should be removed
......@@ -987,8 +988,8 @@ func (e *executor) connectDocker() error {
// validateOSType checks if the ExecutorOptions metadata matches with the docker
// info response.
func (e *executor) validateOSType() error {
executorOSType, ok := e.ExecutorOptions.Metadata["OSType"]
if !ok {
executorOSType := e.ExecutorOptions.Metadata[metadataOSType]
if executorOSType == "" {
return common.MakeBuildError("%s does not have any OSType specified", e.Config.Executor)
}
......@@ -1142,21 +1143,14 @@ func (e *executor) Prepare(options common.ExecutorPrepareOptions) error {
return nil
}
var (
buildDirectoryNotAbsoluteErr = common.MakeBuildError("build directory needs to be an absolute path")
buildDirectoryIsRootPathErr = common.MakeBuildError("build directory needs to be a non-root path")
)
func (e *executor) prepareBuildsDir(options common.ExecutorPrepareOptions) error {
volumeParser, err := parser.New(e.info)
if err != nil {
return fmt.Errorf("couldn't create volumes parser: %v", err)
if e.volumeParser == nil {
return common.MakeBuildError("missing volume parser")
}
isHostMounted, err := volumes.IsHostMountedVolume(volumeParser, e.RootDir(), options.Config.Docker.Volumes...)
isHostMounted, err := volumes.IsHostMountedVolume(e.volumeParser, e.RootDir(), options.Config.Docker.Volumes...)
if err != nil {
return err
return &common.BuildError{Inner: err}
}
// We need to set proper value for e.SharedBuildsDir because
......@@ -1168,14 +1162,6 @@ func (e *executor) prepareBuildsDir(options common.ExecutorPrepareOptions) error
e.SharedBuildsDir = true
}
if !filepath.IsAbs(e.RootDir()) {
return buildDirectoryNotAbsoluteErr
}
if e.RootDir() == "/" {
return buildDirectoryIsRootPathErr
}
return nil
}
......
......@@ -9,6 +9,7 @@ import (
"gitlab.com/gitlab-org/gitlab-runner/common"
"gitlab.com/gitlab-org/gitlab-runner/executors"
"gitlab.com/gitlab-org/gitlab-runner/executors/docker/internal/volumes/parser"
)
type commandExecutor struct {
......@@ -115,7 +116,7 @@ func init() {
},
ShowHostname: true,
Metadata: map[string]string{
"OSType": osTypeLinux,
metadataOSType: osTypeLinux,
},
}
......@@ -125,6 +126,7 @@ func init() {
AbstractExecutor: executors.AbstractExecutor{
ExecutorOptions: options,
},
volumeParser: parser.NewLinuxParser(),
},
}
e.SetCurrentStage(common.ExecutorStageCreated)
......
......@@ -3,6 +3,7 @@ package docker
import (
"gitlab.com/gitlab-org/gitlab-runner/common"
"gitlab.com/gitlab-org/gitlab-runner/executors"
"gitlab.com/gitlab-org/gitlab-runner/executors/docker/internal/volumes/parser"
)
func init() {
......@@ -18,7 +19,7 @@ func init() {
},
ShowHostname: true,
Metadata: map[string]string{
"OSType": osTypeWindows,
metadataOSType: osTypeWindows,
},
}
......@@ -28,6 +29,7 @@ func init() {
AbstractExecutor: executors.AbstractExecutor{
ExecutorOptions: options,
},
volumeParser: parser.NewWindowsParser(),
},
}
e.SetCurrentStage(common.ExecutorStageCreated)
......
......@@ -7,6 +7,7 @@ import (
"gitlab.com/gitlab-org/gitlab-runner/common"
"gitlab.com/gitlab-org/gitlab-runner/executors"
"gitlab.com/gitlab-org/gitlab-runner/executors/docker/internal/volumes/parser"
"gitlab.com/gitlab-org/gitlab-runner/helpers/ssh"
)
......@@ -93,7 +94,7 @@ func init() {
},
ShowHostname: true,
Metadata: map[string]string{
"OSType": osTypeLinux,
metadataOSType: osTypeLinux,
},
}
......@@ -103,6 +104,7 @@ func init() {
AbstractExecutor: executors.AbstractExecutor{
ExecutorOptions: options,
},
volumeParser: parser.NewLinuxParser(),
},
}
e.SetCurrentStage(common.ExecutorStageCreated)
......
......@@ -205,6 +205,7 @@ func testServiceFromNamedImage(t *testing.T, description, imageName, serviceName
OSType: helperimage.OSTypeLinux,
Architecture: "amd64",
},
volumeParser: parser.NewLinuxParser(),
}
options := buildImagePullOptions(e, imageName)
......@@ -567,54 +568,54 @@ func TestDockerGetExistingDockerImageIfPullFails(t *testing.T) {
func TestPrepareBuildsDir(t *testing.T) {
tests := map[string]struct {
osType string
parser parser.Parser
rootDir string
volumes []string
expectedSharedBuildsDir bool
expectedError error
expectedError string
}{
"rootDir mounted as host based volume": {
osType: parser.OSTypeLinux,
parser: parser.NewLinuxParser(),
rootDir: "/build",
volumes: []string{"/build:/build"},
expectedSharedBuildsDir: true,
},
"rootDir mounted as container based volume": {
osType: parser.OSTypeLinux,
parser: parser.NewLinuxParser(),
rootDir: "/build",
volumes: []string{"/build"},
expectedSharedBuildsDir: false,
},
"rootDir not mounted as volume": {
osType: parser.OSTypeLinux,
parser: parser.NewLinuxParser(),
rootDir: "/build",
volumes: []string{"/folder:/folder"},
expectedSharedBuildsDir: false,
},
"rootDir's parent mounted as volume": {
osType: parser.OSTypeLinux,
parser: parser.NewLinuxParser(),
rootDir: "/build/other/directory",
volumes: []string{"/build/:/build"},
expectedSharedBuildsDir: true,
},
"rootDir is not an absolute path": {
osType: parser.OSTypeLinux,
parser: parser.NewLinuxParser(),
rootDir: "builds",
expectedError: buildDirectoryNotAbsoluteErr,
expectedError: "build directory needs to be an absolute path",
},
"rootDir is /": {
osType: parser.OSTypeLinux,
parser: parser.NewLinuxParser(),
rootDir: "/",
expectedError: buildDirectoryIsRootPathErr,
expectedError: "build directory needs to be a non-root path",
},
"error on volume parsing": {
osType: parser.OSTypeLinux,
parser: parser.NewLinuxParser(),
rootDir: "/build",
volumes: []string{""},
expectedError: parser.NewInvalidVolumeSpecErr(""),
expectedError: "invalid volume specification",
},
"error on volume parser creation": {
expectedError: errors.New(`couldn't create volumes parser: unsupported OSType ""`),
expectedError: `missing volume parser`,
},
}
......@@ -637,13 +638,17 @@ func TestPrepareBuildsDir(t *testing.T) {
AbstractExecutor: executors.AbstractExecutor{
Config: c,
},
info: types.Info{
OSType: test.osType,
},
volumeParser: test.parser,
}
err := e.prepareBuildsDir(options)
assert.Equal(t, test.expectedError, err)
if test.expectedError != "" {
assert.Error(t, err)
assert.Contains(t, err.Error(), test.expectedError)
return
}
assert.NoError(t, err)
assert.Equal(t, test.expectedSharedBuildsDir, e.SharedBuildsDir)
})
}
......@@ -1408,10 +1413,11 @@ func TestDockerWatchOn_1_12_4(t *testing.T) {
AbstractExecutor: executors.AbstractExecutor{
ExecutorOptions: executors.ExecutorOptions{
Metadata: map[string]string{
"OSType": "linux",
metadataOSType: osTypeLinux,
},
},
},
volumeParser: parser.NewLinuxParser(),
}
e.Context = context.Background()
e.Build = &common.Build{
......@@ -1486,6 +1492,7 @@ func prepareTestDockerConfiguration(t *testing.T, dockerConfig *common.DockerCon
e := &executor{}
e.client = c
e.volumeParser = parser.NewLinuxParser()
e.info = types.Info{
OSType: helperimage.OSTypeLinux,
Architecture: "amd64",
......@@ -1733,21 +1740,21 @@ func TestCheckOSType(t *testing.T) {
}{
"executor and docker info mismatch": {
executorMetadata: map[string]string{
"OSType": "windows",
metadataOSType: osTypeWindows,
},
dockerInfoOSType: "linux",
dockerInfoOSType: osTypeLinux,
expectedErr: "executor requires OSType=windows, but Docker Engine supports only OSType=linux",
},
"executor and docker info match": {
executorMetadata: map[string]string{
"OSType": "linux",
metadataOSType: osTypeLinux,
},
dockerInfoOSType: "linux",
dockerInfoOSType: osTypeLinux,
expectedErr: "",
},
"executor OSType not defined": {
executorMetadata: nil,
dockerInfoOSType: "linux",
dockerInfoOSType: osTypeLinux,
expectedErr: " does not have any OSType specified",
},
}
......
......@@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"path/filepath"
"gitlab.com/gitlab-org/gitlab-runner/executors/docker/internal/volumes/parser"
)
......@@ -74,9 +73,14 @@ func (m *manager) Create(volume string) error {
}
func (m *manager) addHostVolume(volume *parser.Volume) error {
volume.Destination = m.getAbsoluteContainerPath(volume.Destination)
var err error
err := m.managedVolumes.Add(volume.Destination)
volume.Destination, err = m.getAbsoluteContainerPath(volume.Destination)
if err != nil {
return err
}
err = m.managedVolumes.Add(volume.Destination)
if err != nil {
return err
}
......@@ -86,16 +90,21 @@ func (m *manager) addHostVolume(volume *parser.Volume) error {
return nil
}
func (m *manager) getAbsoluteContainerPath(dir string) string {
if filepath.IsAbs(dir) {
return dir
func (m *manager) getAbsoluteContainerPath(dir string) (string, error) {
if m.parser.Path().IsRoot(dir) {
return "", errDirectoryIsRootPath
}
if m.parser.Path().IsAbs(dir) {
return dir, nil
}
return filepath.Join(m.config.BaseContainerPath, dir)
return m.parser.Path().Join(m.config.BaseContainerPath, dir), nil
}
func (m *manager) appendVolumeBind(volume *parser.Volume) {
m.logger.Debugln(fmt.Sprintf("Using host-based %q for %q...", volume.Source, volume.Destination))
m.volumeBindings = append(m.volumeBindings, volume.Definition())
}
......@@ -118,19 +127,20 @@ func (m *manager) addCacheVolume(volume *parser.Volume) error {
}
func (m *manager) createHostBasedCacheVolume(containerPath string) error {
containerPath = m.getAbsoluteContainerPath(containerPath)
var err error
err := m.managedVolumes.Add(containerPath)
containerPath, err = m.getAbsoluteContainerPath(containerPath)
if err != nil {
return err
}
hostPath := filepath.Join(m.config.CacheDir, m.config.UniqueName, hashContainerPath(containerPath))
hostPath, err = filepath.Abs(hostPath)
err = m.managedVolumes.Add(containerPath)
if err != nil {
return err
}
hostPath := m.parser.Path().Join(m.config.CacheDir, m.config.UniqueName, hashContainerPath(containerPath))
m.appendVolumeBind(&parser.Volume{
Source: hostPath,
Destination: containerPath,
......@@ -140,9 +150,12 @@ func (m *manager) createHostBasedCacheVolume(containerPath string) error {
}
func (m *manager) createContainerBasedCacheVolume(containerPath string) (string, error) {
containerPath = m.getAbsoluteContainerPath(containerPath)
containerPath, err := m.getAbsoluteContainerPath(containerPath)
if err != nil {
return "", err
}
err := m.managedVolumes.Add(containerPath)
err = m.managedVolumes.Add(containerPath)
if err != nil {
return "", err
}
......
......@@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitlab-runner/executors/docker/internal/volumes/parser"
"gitlab.com/gitlab-org/gitlab-runner/helpers/path"
)
func newDebugLoggerMock() *mockDebugLogger {
......@@ -51,8 +52,9 @@ func addCacheContainerManager(manager *manager) *MockCacheContainersManager {
func addParser(manager *manager) *parser.MockParser {
parserMock := new(parser.MockParser)
manager.parser = parserMock
parserMock.On("Path").Return(path.NewUnixPath())
manager.parser = parserMock
return parserMock
}
......@@ -101,6 +103,12 @@ func TestDefaultManager_CreateUserVolumes_HostVolume(t *testing.T) {
parsedVolume: &parser.Volume{Source: "/host/new", Destination: "/my/path", Mode: "ro"},
expectedBinding: []string{"/host:/duplicated", "/host/new:/my/path:ro"},
},
"root volume specified": {
volume: "/host/new:/:ro",
parsedVolume: &parser.Volume{Source: "/host/new", Destination: "/", Mode: "ro"},
expectedBinding: []string{"/host:/duplicated"},
expectedError: errDirectoryIsRootPath,
},
}
for testName, testCase := range testCases {
......@@ -451,8 +459,9 @@ func TestDefaultManager_CreateUserVolumes_CacheVolume_ContainerBased_WithError(t
func TestDefaultManager_CreateUserVolumes_ParserError(t *testing.T) {
m := newDefaultManager(ManagerConfig{})
volumeParser := addParser(m)
volumeParser := new(parser.MockParser)
defer volumeParser.AssertExpectations(t)
m.parser = volumeParser
volumeParser.On("ParseVolume", "volume").
Return(nil, errors.New("parser-test-error")).
......
......@@ -2,9 +2,13 @@ package parser
import (
"regexp"
"gitlab.com/gitlab-org/gitlab-runner/helpers/path"
)
type baseParser struct{}
type baseParser struct {
path path.Path
}
// The way how matchesToVolumeSpecParts parses the volume mount specification and assigns
// parts was inspired by how Docker Engine's `windowsParser` is created. The original sources
......@@ -41,3 +45,7 @@ func (p *baseParser) matchesToVolumeSpecParts(spec string, specExp *regexp.Regex
return parts, nil
}
func (p *baseParser) Path() path.Path {
return p.path
}
......@@ -2,6 +2,8 @@ package parser
import (
"regexp"
"gitlab.com/gitlab-org/gitlab-runner/helpers/path"
)
const (
......@@ -18,8 +20,12 @@ type linuxParser struct {
baseParser
}
func newLinuxParser() Parser {
return new(linuxParser)
func NewLinuxParser() Parser {
return &linuxParser{
baseParser: baseParser{
path: path.NewUnixPath(),
},
}
}
func (p *linuxParser) ParseVolume(spec string) (*Volume, error) {
......
......@@ -68,7 +68,7 @@ func TestLinuxParser_ParseVolume(t *testing.T) {
for testName, testCase := range testCases {
t.Run(testName, func(t *testing.T) {
parser := newLinuxParser()
parser := NewLinuxParser()
parts, err := parser.ParseVolume(testCase.volumeSpec)
if testCase.expectedError == nil {
......
......@@ -3,6 +3,7 @@
package parser
import mock "github.com/stretchr/testify/mock"
import path "gitlab.com/gitlab-org/gitlab-runner/helpers/path"
// MockParser is an autogenerated mock type for the Parser type
type MockParser struct {
......@@ -31,3 +32,19 @@ func (_m *MockParser) ParseVolume(spec string) (*Volume, error) {
return r0, r1
}
// Path provides a mock function with given fields:
func (_m *MockParser) Path() path.Path {
ret := _m.Called()
var r0 path.Path
if rf, ok := ret.Get(0).(func() path.Path); ok {
r0 = rf()
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(path.Path)
}
}
return r0
}
package parser
import (
"github.com/docker/docker/api/types"
"gitlab.com/gitlab-org/gitlab-runner/helpers/docker/errors"
)
const (
OSTypeLinux = "linux"
OSTypeWindows = "windows"
)
import "gitlab.com/gitlab-org/gitlab-runner/helpers/path"
type Parser interface {
ParseVolume(spec string) (*Volume, error)
}
type parserFactory func() Parser
var supportedOSTypes = map[string]parserFactory{
OSTypeLinux: newLinuxParser,
OSTypeWindows: newWindowsParser,
}
func New(info types.Info) (Parser, error) {
factory, ok := supportedOSTypes[info.OSType]
if !ok {
return nil, errors.NewErrOSNotSupported(info.OSType)
}
return factory(), nil
Path() path.Path
}
package parser
import (
"testing"
"github.com/docker/docker/api/types"
"github.com/stretchr/testify/assert"
"gitlab.com/gitlab-org/gitlab-runner/helpers/docker/errors"
)
func TestNew(t *testing.T) {
testCases := map[string]struct {
expectedParserType interface{}
expectedError error
}{
OSTypeLinux: {expectedParserType: &linuxParser{}, expectedError: nil},
OSTypeWindows: {expectedParserType: &windowsParser{}, expectedError: nil},
"unsupported": {expectedParserType: nil, expectedError: errors.NewErrOSNotSupported("unsupported")},
}
for osType, testCase := range testCases {
t.Run(osType, func(t *testing.T) {
parser, err := New(types.Info{OSType: osType})
assert.IsType(t, testCase.expectedParserType, parser)
if testCase.expectedError == nil {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, testCase.expectedError.Error())
}
})
}
}
// +build windows
package parser
import (
"regexp"
"gitlab.com/gitlab-org/gitlab-runner/helpers/path"
)
// The specification of regular expression used for parsing Windows volumes
......@@ -67,8 +71,12 @@ type windowsParser struct {
baseParser
}
func newWindowsParser() Parser {
return new(windowsParser)
func NewWindowsParser() Parser {
return &windowsParser{
baseParser: baseParser{
path: path.NewWindowsPath(),
},