From 6e9b1b7bc04e29eaf9ad4a19004efb4a3afd7bd9 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 16 Sep 2020 14:55:40 -0700 Subject: [PATCH 01/13] fix mounting for docker driver in windows --- pkg/drivers/kic/oci/types.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index a1d729ac2a4c..d31c74aaf505 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -19,7 +19,8 @@ package oci import ( "errors" "fmt" - "path/filepath" + "path" + "runtime" "strings" ) @@ -105,7 +106,20 @@ type Mount struct { // '[host-path:]container-path[:]' The comma-delimited 'options' are // [rw|ro], [Z], [srhared|rslave|rprivate]. func ParseMountString(spec string) (m Mount, err error) { - switch fields := strings.Split(spec, ":"); len(fields) { + f := strings.Split(spec, ":") + var fields []string + if runtime.GOOS != "windows" { + fields = f + } else { + // Recreate the host path that got split above since + // Windows paths look like C:\path + hpath := fmt.Sprintf("%s:%s", f[0], f[1]) + fields = append(fields, hpath) + for _, opt := range f[2:] { + fields = append(fields, opt) + } + } + switch len(fields) { case 0: err = errors.New("invalid empty spec") case 1: @@ -132,7 +146,7 @@ func ParseMountString(spec string) (m Mount, err error) { fallthrough case 2: m.HostPath, m.ContainerPath = fields[0], fields[1] - if !filepath.IsAbs(m.ContainerPath) { + if !path.IsAbs(m.ContainerPath) { err = fmt.Errorf("'%s' container path must be absolute", m.ContainerPath) } default: From 14a5b0b26418e5fea2505c42d24991840968d832 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 16 Sep 2020 16:04:10 -0700 Subject: [PATCH 02/13] simplify append --- go.mod | 1 - go.sum | 2 -- pkg/drivers/kic/oci/types.go | 4 +--- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 5a5e6025e8a9..225bdf9537f7 100644 --- a/go.mod +++ b/go.mod @@ -72,7 +72,6 @@ require ( github.com/zchee/go-vmnet v0.0.0-20161021174912-97ebf9174097 golang.org/x/build v0.0.0-20190927031335-2835ba2e683f golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37 - golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6 // indirect golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a golang.org/x/sys v0.0.0-20200523222454-059865788121 diff --git a/go.sum b/go.sum index 93738c41ea97..38b4e9aed348 100644 --- a/go.sum +++ b/go.sum @@ -117,8 +117,6 @@ github.com/StackExchange/wmi v0.0.0-20190523213315-cbe66965904d h1:G0m3OIz70MZUW github.com/StackExchange/wmi v0.0.0-20190523213315-cbe66965904d/go.mod h1:3eOhrUMpNV+6aFIbp5/iudMxNCF27Vw2OZgy4xEx0Fg= github.com/VividCortex/ewma v1.1.1 h1:MnEK4VOv6n0RSY4vtRe3h11qjxL3+t0B8yOL8iMXdcM= github.com/VividCortex/ewma v1.1.1/go.mod h1:2Tkkvm3sRDVXaiyucHiACn4cqf7DpdyLvmxzcbUokwA= -github.com/afbjorklund/go-containerregistry v0.0.0-20200602203322-347d93793dc9 h1:EMF82QM65iOfQTQefF5d5SCsSsXsh6aSdcq9U1KC3Lc= -github.com/afbjorklund/go-containerregistry v0.0.0-20200602203322-347d93793dc9/go.mod h1:npTSyywOeILcgWqd+rvtzGWflIPPcBQhYoOONaY4ltM= github.com/afbjorklund/go-containerregistry v0.0.0-20200902152226-fbad78ec2813 h1:0tskN1ipU/BBrpoEIy0rdZS9jf5+wdP6IMRak8Iu/YE= github.com/afbjorklund/go-containerregistry v0.0.0-20200902152226-fbad78ec2813/go.mod h1:npTSyywOeILcgWqd+rvtzGWflIPPcBQhYoOONaY4ltM= github.com/afbjorklund/go-getter v1.4.1-0.20190910175809-eb9f6c26742c h1:18gEt7qzn7CW7qMkfPTFyyotlPbvPQo9o4IDV8jZqP4= diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index d31c74aaf505..8e0b37e688bc 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -115,9 +115,7 @@ func ParseMountString(spec string) (m Mount, err error) { // Windows paths look like C:\path hpath := fmt.Sprintf("%s:%s", f[0], f[1]) fields = append(fields, hpath) - for _, opt := range f[2:] { - fields = append(fields, opt) - } + fields = append(fields, f[2:]...) } switch len(fields) { case 0: From 4a004dd58ec9cee6d5bef70936e3b912b7975cd4 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 16 Sep 2020 16:39:03 -0700 Subject: [PATCH 03/13] simplify more --- pkg/drivers/kic/oci/types.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index 8e0b37e688bc..8135747c7954 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -107,10 +107,8 @@ type Mount struct { // [rw|ro], [Z], [srhared|rslave|rprivate]. func ParseMountString(spec string) (m Mount, err error) { f := strings.Split(spec, ":") - var fields []string - if runtime.GOOS != "windows" { - fields = f - } else { + fields := f + if runtime.GOOS == "windows" { // Recreate the host path that got split above since // Windows paths look like C:\path hpath := fmt.Sprintf("%s:%s", f[0], f[1]) From 39ae4fbdd2ea808c23c6afe446888fd890ddfcd0 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 17 Sep 2020 12:15:54 -0700 Subject: [PATCH 04/13] check for windows style pathing with a regex --- pkg/drivers/kic/oci/types.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index 8135747c7954..debd8468151b 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -20,7 +20,7 @@ import ( "errors" "fmt" "path" - "runtime" + "regexp" "strings" ) @@ -108,11 +108,12 @@ type Mount struct { func ParseMountString(spec string) (m Mount, err error) { f := strings.Split(spec, ":") fields := f - if runtime.GOOS == "windows" { + windows, _ := regexp.MatchString(`^[A-Z]:\\*`, spec) + if windows { // Recreate the host path that got split above since // Windows paths look like C:\path hpath := fmt.Sprintf("%s:%s", f[0], f[1]) - fields = append(fields, hpath) + fields = []string{hpath} fields = append(fields, f[2:]...) } switch len(fields) { From 84ee6d351bddf1097d7003e62068cc3fd167ad1e Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 17 Sep 2020 13:11:01 -0700 Subject: [PATCH 05/13] unit tests --- pkg/drivers/kic/oci/types.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index debd8468151b..b2dad8de4ade 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -108,6 +108,7 @@ type Mount struct { func ParseMountString(spec string) (m Mount, err error) { f := strings.Split(spec, ":") fields := f + // suppressing err is safe here since the regex will always compile windows, _ := regexp.MatchString(`^[A-Z]:\\*`, spec) if windows { // Recreate the host path that got split above since From 3cb6ba0f75da1d040c638fbb4cd8465408ac881e Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 17 Sep 2020 13:16:07 -0700 Subject: [PATCH 06/13] actually add unit tests --- pkg/drivers/kic/oci/types_test.go | 127 ++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 pkg/drivers/kic/oci/types_test.go diff --git a/pkg/drivers/kic/oci/types_test.go b/pkg/drivers/kic/oci/types_test.go new file mode 100644 index 000000000000..38772a3e791d --- /dev/null +++ b/pkg/drivers/kic/oci/types_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2019 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package oci + +import ( + "testing" +) + +func TestParseMountString(t *testing.T) { + testCases := []struct { + Name string + MountString string + ExpectErr bool + ExpectedMount Mount + }{ + { + Name: "basic linux", + MountString: "/foo:/bar", + ExpectErr: false, + ExpectedMount: Mount{ + HostPath: "/foo", + ContainerPath: "/bar", + }, + }, + { + Name: "linux read only", + MountString: "/foo:/bar:ro", + ExpectErr: false, + ExpectedMount: Mount{ + HostPath: "/foo", + ContainerPath: "/bar", + Readonly: true, + }, + }, + { + Name: "windows style", + MountString: "C:\\Windows\\Path:/foo", + ExpectErr: false, + ExpectedMount: Mount{ + HostPath: "C:\\Windows\\Path", + ContainerPath: "/foo", + }, + }, + { + Name: "windows style read/write", + MountString: "C:\\Windows\\Path:/foo:rw", + ExpectErr: false, + ExpectedMount: Mount{ + HostPath: "C:\\Windows\\Path", + ContainerPath: "/foo", + Readonly: false, + }, + }, + { + Name: "container only", + MountString: "/foo", + ExpectErr: false, + ExpectedMount: Mount{ + ContainerPath: "/foo", + }, + }, + { + Name: "selinux relabel & bidirectional propagation", + MountString: "/foo:/bar/baz:Z,rshared", + ExpectErr: false, + ExpectedMount: Mount{ + HostPath: "/foo", + ContainerPath: "/bar/baz", + SelinuxRelabel: true, + Propagation: MountPropagationBidirectional, + }, + }, + { + Name: "invalid mount option", + MountString: "/foo:/bar:Z,bat", + ExpectErr: true, + ExpectedMount: Mount{ + HostPath: "/foo", + ContainerPath: "/bar", + SelinuxRelabel: true, + }, + }, + { + Name: "empty spec", + MountString: "", + ExpectErr: false, + ExpectedMount: Mount{}, + }, + { + Name: "relative container path", + MountString: "/foo/bar:baz/bat:private", + ExpectErr: true, + ExpectedMount: Mount{ + HostPath: "/foo/bar", + ContainerPath: "baz/bat", + Propagation: MountPropagationNone, + }, + }, + } + + for _, tc := range testCases { + mount, err := ParseMountString(tc.MountString) + if err != nil && !tc.ExpectErr { + t.Errorf("Unexpected error for \"%s\": %v", tc.Name, err) + } + if err == nil && tc.ExpectErr { + t.Errorf("Expected error for \"%s\" but didn't get any: %v %v", tc.Name, mount, err) + } + if mount != tc.ExpectedMount { + t.Errorf("Unexpected mount for \"%s\":\n expected %+v\ngot %+v", tc.Name, tc.ExpectedMount, mount) + } + } +} From 775e047517ac8ce4b31c53c92a8e52127ea5eba3 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 17 Sep 2020 13:18:53 -0700 Subject: [PATCH 07/13] dates --- pkg/drivers/kic/oci/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index b2dad8de4ade..96d706572c0e 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Kubernetes Authors All rights reserved. +Copyright 2020 The Kubernetes Authors All rights reserved. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From f097a0de12f9d3bdc5ce6f88015284f412dc30e9 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 17 Sep 2020 13:20:35 -0700 Subject: [PATCH 08/13] dates, take two --- pkg/drivers/kic/oci/types.go | 2 +- pkg/drivers/kic/oci/types_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index 96d706572c0e..b2dad8de4ade 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -1,5 +1,5 @@ /* -Copyright 2020 The Kubernetes Authors All rights reserved. +Copyright 2019 The Kubernetes Authors All rights reserved. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/pkg/drivers/kic/oci/types_test.go b/pkg/drivers/kic/oci/types_test.go index 38772a3e791d..2e2d698025f0 100644 --- a/pkg/drivers/kic/oci/types_test.go +++ b/pkg/drivers/kic/oci/types_test.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Kubernetes Authors All rights reserved. +Copyright 2020 The Kubernetes Authors All rights reserved. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From 86de3640ea34c3abf5201cea1676021287500c83 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 17 Sep 2020 17:09:02 -0700 Subject: [PATCH 09/13] try to fix functional macos test --- .github/workflows/pr.yml | 1 - test/integration/functional_test.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 21cd5e5a04ea..0886a2773785 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -198,7 +198,6 @@ jobs: go-version: '1.14.6' stable: true - name: Install gopogh - shell: bash run: | curl -LO https://github.com/medyagh/gopogh/releases/download/v0.2.4/gopogh-darwin-amd64 diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 52d188190aee..132ca0435a70 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -518,7 +518,7 @@ func validateCacheCmd(ctx context.Context, t *testing.T, profile string) { } img := "minikube-local-cache-test:" + profile - _, err = Run(t, exec.CommandContext(ctx, "docker", "build", "-t", img, dname)) + _, err = Run(t, exec.CommandContext(ctx, "sudo", "docker", "build", "-t", img, dname)) if err != nil { t.Errorf("failed to build docker image: %v", err) } From f3dec28985afb0d251a50f9b1e200822b2da6b69 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Fri, 18 Sep 2020 09:35:49 -0700 Subject: [PATCH 10/13] get docker working on macos machine --- .github/workflows/pr.yml | 4 +++- test/integration/functional_test.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0886a2773785..8fa0c20a419f 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -206,7 +206,9 @@ jobs: shell: bash run: | brew install docker-machine docker - sudo docker --version + docker --version + docker-machine create --driver virtualbox default + eval $(docker-machine env default) - name: Info shell: bash run: | diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 132ca0435a70..52d188190aee 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -518,7 +518,7 @@ func validateCacheCmd(ctx context.Context, t *testing.T, profile string) { } img := "minikube-local-cache-test:" + profile - _, err = Run(t, exec.CommandContext(ctx, "sudo", "docker", "build", "-t", img, dname)) + _, err = Run(t, exec.CommandContext(ctx, "docker", "build", "-t", img, dname)) if err != nil { t.Errorf("failed to build docker image: %v", err) } From 0d1ea6a4b745b35f73e4d434169a899eb259b829 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Fri, 18 Sep 2020 09:52:04 -0700 Subject: [PATCH 11/13] more docker stuff --- .github/workflows/pr.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 8fa0c20a419f..fc35ce2b4d72 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -207,8 +207,7 @@ jobs: run: | brew install docker-machine docker docker --version - docker-machine create --driver virtualbox default - eval $(docker-machine env default) + brew services start docker-machine - name: Info shell: bash run: | From 319e87061e9430da314ca7b0c1859a595cca20cb Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Fri, 18 Sep 2020 13:27:56 -0700 Subject: [PATCH 12/13] revert gh action changes --- .github/workflows/pr.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index fc35ce2b4d72..0886a2773785 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -206,8 +206,7 @@ jobs: shell: bash run: | brew install docker-machine docker - docker --version - brew services start docker-machine + sudo docker --version - name: Info shell: bash run: | From 286e1b656f134b69b8fda7bf613020e7d8ae5be2 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Fri, 18 Sep 2020 13:44:45 -0700 Subject: [PATCH 13/13] revert gh pr yml change --- .github/workflows/pr.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0886a2773785..21cd5e5a04ea 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -198,6 +198,7 @@ jobs: go-version: '1.14.6' stable: true - name: Install gopogh + shell: bash run: | curl -LO https://github.com/medyagh/gopogh/releases/download/v0.2.4/gopogh-darwin-amd64