From 5bec0a66efc8a60a14439d11bd11e85dedd675e4 Mon Sep 17 00:00:00 2001 From: Priya Modali Date: Wed, 7 Apr 2021 10:55:25 -0700 Subject: [PATCH 1/5] Adding user flag for minikube. --- pkg/skaffold/cluster/minikube.go | 31 ++++++++++ pkg/skaffold/cluster/minikube_test.go | 89 ++++++++++++++++----------- pkg/skaffold/constants/constants.go | 2 + 3 files changed, 85 insertions(+), 37 deletions(-) diff --git a/pkg/skaffold/cluster/minikube.go b/pkg/skaffold/cluster/minikube.go index 2a612540d35..66f7034d3ce 100644 --- a/pkg/skaffold/cluster/minikube.go +++ b/pkg/skaffold/cluster/minikube.go @@ -25,12 +25,14 @@ import ( "os/exec" "path/filepath" + "github.com/blang/semver" "github.com/sirupsen/logrus" "k8s.io/client-go/util/homedir" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/version" ) var GetClient = getClient @@ -39,6 +41,7 @@ var GetClient = getClient var ( minikubeBinaryFunc = minikubeBinary getClusterInfo = context.GetClusterInfo + versionCheckFunc = versionCheck ) type Client interface { @@ -93,9 +96,37 @@ func minikubeExec(arg ...string) (*exec.Cmd, error) { if err != nil { return nil, fmt.Errorf("getting minikube executable: %w", err) } + + minVer, err := versionCheckFunc() + if err != nil { + return nil, err + } + if minVer { + arg = append(arg, "--user=skaffold") + } return exec.Command(b, arg...), nil } +// versionCheck checks if the minikube version meet the minimum requirements for the user flag +func versionCheck() (bool, error) { + cmd := exec.Command("minikube", "version") + out, err := util.RunCmdOut(cmd) + if err != nil { + return false, err + } + + currentVersion, err := version.ParseVersion(string(out)) + if err != nil { + return false, err + } + + versionWithFlag, err := semver.Make(constants.MinikubeVersionWithUserFlag) + if err != nil { + return false, err + } + return currentVersion.GE(versionWithFlag), nil +} + func minikubeBinary() (string, error) { filename, err := exec.LookPath("minikube") if err != nil { diff --git a/pkg/skaffold/cluster/minikube_test.go b/pkg/skaffold/cluster/minikube_test.go index b77008d674f..9185b15c1c1 100644 --- a/pkg/skaffold/cluster/minikube_test.go +++ b/pkg/skaffold/cluster/minikube_test.go @@ -31,57 +31,71 @@ import ( func TestClientImpl_IsMinikube(t *testing.T) { home := homedir.HomeDir() tests := []struct { - description string - kubeContext string - certPath string - serverURL string - minikubeProfileCmd util.Command - minikubeNotInPath bool - expected bool + description string + kubeContext string + certPath string + serverURL string + minikubeProfileCmd util.Command + minikubeNotInPath bool + expected bool + minikubeWithUserFlag bool }{ { - description: "context is 'minikube'", - kubeContext: "minikube", - expected: true, + description: "context is 'minikube'", + kubeContext: "minikube", + expected: true, + minikubeWithUserFlag: false, }, { - description: "'minikube' binary not found", - kubeContext: "test-cluster", - minikubeNotInPath: true, - expected: false, + description: "context is 'minikube'", + kubeContext: "minikube", + expected: true, + minikubeWithUserFlag: true, }, { - description: "cluster cert inside minikube dir", - kubeContext: "test-cluster", - certPath: filepath.Join(home, ".minikube", "ca.crt"), - expected: true, + description: "'minikube' binary not found", + kubeContext: "test-cluster", + minikubeNotInPath: true, + expected: false, + minikubeWithUserFlag: true, }, { - description: "cluster cert outside minikube dir", - kubeContext: "test-cluster", - minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json", fmt.Sprintf(profileStr, "test-cluster", "docker", "172.17.0.3", 8443)), - certPath: filepath.Join(home, "foo", "ca.crt"), - expected: false, + description: "cluster cert inside minikube dir", + kubeContext: "test-cluster", + certPath: filepath.Join(home, ".minikube", "ca.crt"), + expected: true, + minikubeWithUserFlag: true, }, { - description: "minikube node ip matches api server url", - kubeContext: "test-cluster", - serverURL: "https://192.168.64.10:8443", - minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json", fmt.Sprintf(profileStr, "test-cluster", "hyperkit", "192.168.64.10", 8443)), - expected: true, + description: "cluster cert outside minikube dir", + kubeContext: "test-cluster", + minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", fmt.Sprintf(profileStr, "test-cluster", "docker", "172.17.0.3", 8443)), + certPath: filepath.Join(home, "foo", "ca.crt"), + expected: false, + minikubeWithUserFlag: true, }, { - description: "cannot parse minikube profile list", - kubeContext: "test-cluster", - minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json", `Random error`), - expected: false, + description: "minikube node ip matches api server url", + kubeContext: "test-cluster", + serverURL: "https://192.168.64.10:8443", + minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", fmt.Sprintf(profileStr, "test-cluster", "hyperkit", "192.168.64.10", 8443)), + expected: true, + minikubeWithUserFlag: true, }, { - description: "minikube node ip different from api server url", - kubeContext: "test-cluster", - serverURL: "https://192.168.64.10:8443", - minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json", fmt.Sprintf(profileStr, "test-cluster", "hyperkit", "192.168.64.11", 8443)), - expected: false, + description: "cannot parse minikube profile list", + kubeContext: "test-cluster", + minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", `Random error`), + expected: false, + minikubeWithUserFlag: true, + }, + { + description: "minikube node ip different from api server url", + kubeContext: "test-cluster", + serverURL: "https://192.168.64.10:8443", + minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", fmt.Sprintf(profileStr, "test-cluster", "hyperkit", "192.168.64.11", 8443)), + expected: false, + minikubeWithUserFlag: true, }, } @@ -92,6 +106,7 @@ func TestClientImpl_IsMinikube(t *testing.T) { } else { t.Override(&minikubeBinaryFunc, func() (string, error) { return "minikube", nil }) } + t.Override(&versionCheckFunc, func() (bool, error) { return test.minikubeWithUserFlag, nil }) t.Override(&util.DefaultExecCommand, test.minikubeProfileCmd) t.Override(&getClusterInfo, func(string) (*clientcmdapi.Cluster, error) { return &clientcmdapi.Cluster{ diff --git a/pkg/skaffold/constants/constants.go b/pkg/skaffold/constants/constants.go index 9e43a8aeeb2..b756ed6beef 100644 --- a/pkg/skaffold/constants/constants.go +++ b/pkg/skaffold/constants/constants.go @@ -51,6 +51,8 @@ const ( DefaultRPCPort = 50051 DefaultRPCHTTPPort = 50052 + MinikubeVersionWithUserFlag = "1.18.0" + DefaultPortForwardAddress = "127.0.0.1" DefaultProjectDescriptor = "project.toml" From f7b8e49c17bb5b377513386c9af405872944d933 Mon Sep 17 00:00:00 2001 From: Priya Modali Date: Wed, 7 Apr 2021 23:06:36 -0700 Subject: [PATCH 2/5] Adding version to global cache --- pkg/skaffold/cluster/minikube.go | 30 ++++++++++++++++++++------- pkg/skaffold/cluster/minikube_test.go | 8 +------ pkg/skaffold/constants/constants.go | 2 -- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/pkg/skaffold/cluster/minikube.go b/pkg/skaffold/cluster/minikube.go index 66f7034d3ce..51333a777d1 100644 --- a/pkg/skaffold/cluster/minikube.go +++ b/pkg/skaffold/cluster/minikube.go @@ -24,6 +24,7 @@ import ( "os" "os/exec" "path/filepath" + "sync" "github.com/blang/semver" "github.com/sirupsen/logrus" @@ -35,13 +36,19 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/version" ) +const ( + MinikubeVersionWithUserFlag = "1.18.0" +) + +var currentVersionOnce sync.Once + var GetClient = getClient // To override during tests var ( - minikubeBinaryFunc = minikubeBinary - getClusterInfo = context.GetClusterInfo - versionCheckFunc = versionCheck + minikubeBinaryFunc = minikubeBinary + getClusterInfo = context.GetClusterInfo + supportsUserFlagFunc = supportsUserFlag ) type Client interface { @@ -97,7 +104,7 @@ func minikubeExec(arg ...string) (*exec.Cmd, error) { return nil, fmt.Errorf("getting minikube executable: %w", err) } - minVer, err := versionCheckFunc() + minVer, err := supportsUserFlagFunc() if err != nil { return nil, err } @@ -108,25 +115,34 @@ func minikubeExec(arg ...string) (*exec.Cmd, error) { } // versionCheck checks if the minikube version meet the minimum requirements for the user flag -func versionCheck() (bool, error) { +func supportsUserFlag() (bool, error) { cmd := exec.Command("minikube", "version") out, err := util.RunCmdOut(cmd) if err != nil { return false, err } - currentVersion, err := version.ParseVersion(string(out)) + currentVersion, err := getCurrentVersion(string(out)) if err != nil { return false, err } - versionWithFlag, err := semver.Make(constants.MinikubeVersionWithUserFlag) + versionWithFlag, err := semver.Make(MinikubeVersionWithUserFlag) if err != nil { return false, err } return currentVersion.GE(versionWithFlag), nil } +func getCurrentVersion(ver string) (semver.Version, error) { + var currentVersion semver.Version + var err error + currentVersionOnce.Do(func() { + currentVersion, err = version.ParseVersion(ver) + }) + return currentVersion, err +} + func minikubeBinary() (string, error) { filename, err := exec.LookPath("minikube") if err != nil { diff --git a/pkg/skaffold/cluster/minikube_test.go b/pkg/skaffold/cluster/minikube_test.go index 9185b15c1c1..3a254cc0c8b 100644 --- a/pkg/skaffold/cluster/minikube_test.go +++ b/pkg/skaffold/cluster/minikube_test.go @@ -40,12 +40,6 @@ func TestClientImpl_IsMinikube(t *testing.T) { expected bool minikubeWithUserFlag bool }{ - { - description: "context is 'minikube'", - kubeContext: "minikube", - expected: true, - minikubeWithUserFlag: false, - }, { description: "context is 'minikube'", kubeContext: "minikube", @@ -106,7 +100,7 @@ func TestClientImpl_IsMinikube(t *testing.T) { } else { t.Override(&minikubeBinaryFunc, func() (string, error) { return "minikube", nil }) } - t.Override(&versionCheckFunc, func() (bool, error) { return test.minikubeWithUserFlag, nil }) + t.Override(&supportsUserFlagFunc, func() (bool, error) { return test.minikubeWithUserFlag, nil }) t.Override(&util.DefaultExecCommand, test.minikubeProfileCmd) t.Override(&getClusterInfo, func(string) (*clientcmdapi.Cluster, error) { return &clientcmdapi.Cluster{ diff --git a/pkg/skaffold/constants/constants.go b/pkg/skaffold/constants/constants.go index b756ed6beef..9e43a8aeeb2 100644 --- a/pkg/skaffold/constants/constants.go +++ b/pkg/skaffold/constants/constants.go @@ -51,8 +51,6 @@ const ( DefaultRPCPort = 50051 DefaultRPCHTTPPort = 50052 - MinikubeVersionWithUserFlag = "1.18.0" - DefaultPortForwardAddress = "127.0.0.1" DefaultProjectDescriptor = "project.toml" From 0297129e4512adf7f788f99c6acac7cf1b0a62fe Mon Sep 17 00:00:00 2001 From: Priya Modali Date: Thu, 8 Apr 2021 07:57:56 -0700 Subject: [PATCH 3/5] Unit tests. --- pkg/skaffold/cluster/minikube_test.go | 84 ++++++++++++--------------- 1 file changed, 38 insertions(+), 46 deletions(-) diff --git a/pkg/skaffold/cluster/minikube_test.go b/pkg/skaffold/cluster/minikube_test.go index 3a254cc0c8b..659d5ec7f81 100644 --- a/pkg/skaffold/cluster/minikube_test.go +++ b/pkg/skaffold/cluster/minikube_test.go @@ -31,65 +31,57 @@ import ( func TestClientImpl_IsMinikube(t *testing.T) { home := homedir.HomeDir() tests := []struct { - description string - kubeContext string - certPath string - serverURL string - minikubeProfileCmd util.Command - minikubeNotInPath bool - expected bool - minikubeWithUserFlag bool + description string + kubeContext string + certPath string + serverURL string + minikubeProfileCmd util.Command + minikubeNotInPath bool + expected bool }{ { - description: "context is 'minikube'", - kubeContext: "minikube", - expected: true, - minikubeWithUserFlag: true, + description: "context is 'minikube'", + kubeContext: "minikube", + expected: true, }, { - description: "'minikube' binary not found", - kubeContext: "test-cluster", - minikubeNotInPath: true, - expected: false, - minikubeWithUserFlag: true, + description: "'minikube' binary not found", + kubeContext: "test-cluster", + minikubeNotInPath: true, + expected: false, }, { - description: "cluster cert inside minikube dir", - kubeContext: "test-cluster", - certPath: filepath.Join(home, ".minikube", "ca.crt"), - expected: true, - minikubeWithUserFlag: true, + description: "cluster cert inside minikube dir", + kubeContext: "test-cluster", + certPath: filepath.Join(home, ".minikube", "ca.crt"), + expected: true, }, { - description: "cluster cert outside minikube dir", - kubeContext: "test-cluster", - minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", fmt.Sprintf(profileStr, "test-cluster", "docker", "172.17.0.3", 8443)), - certPath: filepath.Join(home, "foo", "ca.crt"), - expected: false, - minikubeWithUserFlag: true, + description: "cluster cert outside minikube dir", + kubeContext: "test-cluster", + minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", fmt.Sprintf(profileStr, "test-cluster", "docker", "172.17.0.3", 8443)), + certPath: filepath.Join(home, "foo", "ca.crt"), + expected: false, }, { - description: "minikube node ip matches api server url", - kubeContext: "test-cluster", - serverURL: "https://192.168.64.10:8443", - minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", fmt.Sprintf(profileStr, "test-cluster", "hyperkit", "192.168.64.10", 8443)), - expected: true, - minikubeWithUserFlag: true, + description: "minikube node ip matches api server url", + kubeContext: "test-cluster", + serverURL: "https://192.168.64.10:8443", + minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", fmt.Sprintf(profileStr, "test-cluster", "hyperkit", "192.168.64.10", 8443)), + expected: true, }, { - description: "cannot parse minikube profile list", - kubeContext: "test-cluster", - minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", `Random error`), - expected: false, - minikubeWithUserFlag: true, + description: "cannot parse minikube profile list", + kubeContext: "test-cluster", + minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", `Random error`), + expected: false, }, { - description: "minikube node ip different from api server url", - kubeContext: "test-cluster", - serverURL: "https://192.168.64.10:8443", - minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", fmt.Sprintf(profileStr, "test-cluster", "hyperkit", "192.168.64.11", 8443)), - expected: false, - minikubeWithUserFlag: true, + description: "minikube node ip different from api server url", + kubeContext: "test-cluster", + serverURL: "https://192.168.64.10:8443", + minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", fmt.Sprintf(profileStr, "test-cluster", "hyperkit", "192.168.64.11", 8443)), + expected: false, }, } @@ -100,7 +92,7 @@ func TestClientImpl_IsMinikube(t *testing.T) { } else { t.Override(&minikubeBinaryFunc, func() (string, error) { return "minikube", nil }) } - t.Override(&supportsUserFlagFunc, func() (bool, error) { return test.minikubeWithUserFlag, nil }) + t.Override(&supportsUserFlagFunc, func() (bool, error) { return true, nil }) t.Override(&util.DefaultExecCommand, test.minikubeProfileCmd) t.Override(&getClusterInfo, func(string) (*clientcmdapi.Cluster, error) { return &clientcmdapi.Cluster{ From ffc9c969a13f1483a066031fee54649105a63816 Mon Sep 17 00:00:00 2001 From: Priya Modali Date: Sat, 10 Apr 2021 15:44:50 -0700 Subject: [PATCH 4/5] Merged version check & minikube binary lookup. --- pkg/skaffold/cluster/minikube.go | 82 ++++++++++--------- pkg/skaffold/cluster/minikube_test.go | 40 ++++++--- pkg/skaffold/test/structure/structure_test.go | 5 ++ 3 files changed, 79 insertions(+), 48 deletions(-) diff --git a/pkg/skaffold/cluster/minikube.go b/pkg/skaffold/cluster/minikube.go index 51333a777d1..af8176996ac 100644 --- a/pkg/skaffold/cluster/minikube.go +++ b/pkg/skaffold/cluster/minikube.go @@ -37,18 +37,23 @@ import ( ) const ( - MinikubeVersionWithUserFlag = "1.18.0" + minikubeVersionWithUserFlag = "1.18.0" ) -var currentVersionOnce sync.Once - var GetClient = getClient // To override during tests var ( - minikubeBinaryFunc = minikubeBinary - getClusterInfo = context.GetClusterInfo - supportsUserFlagFunc = supportsUserFlag + minikubeBinaryFunc = minikubeBinary + getClusterInfo = context.GetClusterInfo + GetCurrentVersionFunc = getCurrentVersion + + findOnce sync.Once + mk = struct { + version semver.Version + path string + err error + }{} ) type Client interface { @@ -65,7 +70,7 @@ func getClient() Client { } func (clientImpl) IsMinikube(kubeContext string) bool { - if _, err := minikubeBinaryFunc(); err != nil { + if _, _, err := minikubeBinaryFunc(); err != nil { logrus.Tracef("Minikube cluster not detected: %v", err) return false } @@ -99,59 +104,60 @@ func (clientImpl) MinikubeExec(arg ...string) (*exec.Cmd, error) { } func minikubeExec(arg ...string) (*exec.Cmd, error) { - b, err := minikubeBinaryFunc() + b, v, err := minikubeBinaryFunc() if err != nil { return nil, fmt.Errorf("getting minikube executable: %w", err) } - minVer, err := supportsUserFlagFunc() - if err != nil { - return nil, err - } - if minVer { + isSupported, _ := supportsUserFlag(v) + if isSupported { arg = append(arg, "--user=skaffold") } return exec.Command(b, arg...), nil } -// versionCheck checks if the minikube version meet the minimum requirements for the user flag -func supportsUserFlag() (bool, error) { - cmd := exec.Command("minikube", "version") - out, err := util.RunCmdOut(cmd) +func supportsUserFlag(ver semver.Version) (bool, error) { + versionWithFlag, err := semver.Make(minikubeVersionWithUserFlag) if err != nil { return false, err } + return ver.GE(versionWithFlag), nil +} - currentVersion, err := getCurrentVersion(string(out)) +// Retrieves minikube version +func getCurrentVersion() (semver.Version, error) { + cmd := exec.Command("minikube", "version") + out, err := util.RunCmdOut(cmd) if err != nil { - return false, err + return semver.Version{}, err } - versionWithFlag, err := semver.Make(MinikubeVersionWithUserFlag) + currentVersion, err := version.ParseVersion(string(out)) if err != nil { - return false, err + return semver.Version{}, err } - return currentVersion.GE(versionWithFlag), nil + + return currentVersion, nil } -func getCurrentVersion(ver string) (semver.Version, error) { - var currentVersion semver.Version - var err error - currentVersionOnce.Do(func() { - currentVersion, err = version.ParseVersion(ver) +func minikubeBinary() (string, semver.Version, error) { + findOnce.Do(func() { + filename, err := exec.LookPath("minikube") + if err != nil { + mk.err = errors.New("unable to lookup minikube executable. Please add it to PATH environment variable") + } + if _, err := os.Stat(filename); os.IsNotExist(err) { + mk.err = fmt.Errorf("unable to find minikube executable. File not found %s", filename) + } + mk.path = filename + if v, err := GetCurrentVersionFunc(); err != nil { + mk.err = err + } else { + mk.version = v + } }) - return currentVersion, err -} -func minikubeBinary() (string, error) { - filename, err := exec.LookPath("minikube") - if err != nil { - return "", errors.New("unable to find minikube executable. Please add it to PATH environment variable") - } - if _, err := os.Stat(filename); os.IsNotExist(err) { - return "", fmt.Errorf("unable to find minikube executable. File not found %s", filename) - } - return filename, nil + return mk.path, mk.version, mk.err } // matchClusterCertPath checks if the cluster certificate for this context is from inside the minikube directory diff --git a/pkg/skaffold/cluster/minikube_test.go b/pkg/skaffold/cluster/minikube_test.go index 659d5ec7f81..a9d1f1b251d 100644 --- a/pkg/skaffold/cluster/minikube_test.go +++ b/pkg/skaffold/cluster/minikube_test.go @@ -21,6 +21,7 @@ import ( "path/filepath" "testing" + "github.com/blang/semver" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/client-go/util/homedir" @@ -31,13 +32,14 @@ import ( func TestClientImpl_IsMinikube(t *testing.T) { home := homedir.HomeDir() tests := []struct { - description string - kubeContext string - certPath string - serverURL string - minikubeProfileCmd util.Command - minikubeNotInPath bool - expected bool + description string + kubeContext string + certPath string + serverURL string + minikubeProfileCmd util.Command + minikubeNotInPath bool + expected bool + minikuneWithoutUserFalg bool }{ { description: "context is 'minikube'", @@ -56,6 +58,14 @@ func TestClientImpl_IsMinikube(t *testing.T) { certPath: filepath.Join(home, ".minikube", "ca.crt"), expected: true, }, + { + description: "minikube without user flag", + kubeContext: "test-cluster", + minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", fmt.Sprintf(profileStr, "test-cluster", "docker", "172.17.0.3", 8443)), + certPath: filepath.Join(home, "foo", "ca.crt"), + expected: false, + minikuneWithoutUserFalg: true, + }, { description: "cluster cert outside minikube dir", kubeContext: "test-cluster", @@ -88,11 +98,21 @@ func TestClientImpl_IsMinikube(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { if test.minikubeNotInPath { - t.Override(&minikubeBinaryFunc, func() (string, error) { return "", fmt.Errorf("minikube not in PATH") }) + ver, _ := semver.Make("1.18.0") + t.Override(&minikubeBinaryFunc, func() (string, semver.Version, error) { + return "", ver, fmt.Errorf("minikube not in PATH") + }) } else { - t.Override(&minikubeBinaryFunc, func() (string, error) { return "minikube", nil }) + if test.minikuneWithoutUserFalg { + ver, _ := semver.Make("1.17.0") + t.Override(&minikubeBinaryFunc, func() (string, semver.Version, error) { + return "", ver, fmt.Errorf("minikube not in PATH") + }) + } else { + ver, _ := semver.Make("1.18.1") + t.Override(&minikubeBinaryFunc, func() (string, semver.Version, error) { return "minikube", ver, nil }) + } } - t.Override(&supportsUserFlagFunc, func() (bool, error) { return true, nil }) t.Override(&util.DefaultExecCommand, test.minikubeProfileCmd) t.Override(&getClusterInfo, func(string) (*clientcmdapi.Cluster, error) { return &clientcmdapi.Cluster{ diff --git a/pkg/skaffold/test/structure/structure_test.go b/pkg/skaffold/test/structure/structure_test.go index d88d8ffc4b7..01863eca15a 100644 --- a/pkg/skaffold/test/structure/structure_test.go +++ b/pkg/skaffold/test/structure/structure_test.go @@ -22,7 +22,10 @@ import ( "io/ioutil" "testing" + "github.com/blang/semver" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/cluster" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" @@ -39,6 +42,8 @@ func TestNewRunner(t *testing.T) { testutil.Run(t, "", func(t *testutil.T) { tmpDir := t.NewTempDir().Touch("test.yaml") + t.Override(&cluster.GetCurrentVersionFunc, func() (semver.Version, error) { return semver.Version{}, nil }) + t.Override(&util.DefaultExecCommand, testutil.CmdRun("container-structure-test test -v warn --image "+imageName+" --config "+tmpDir.Path("test.yaml"))) cfg := &mockConfig{ From 529052682d847db34486c22b521faa244af2cecd Mon Sep 17 00:00:00 2001 From: Priya Modali Date: Tue, 13 Apr 2021 09:54:20 -0700 Subject: [PATCH 5/5] Updates --- pkg/skaffold/cluster/minikube.go | 26 +++++++------------ pkg/skaffold/cluster/minikube_test.go | 12 ++++----- pkg/skaffold/test/structure/structure_test.go | 2 +- 3 files changed, 17 insertions(+), 23 deletions(-) diff --git a/pkg/skaffold/cluster/minikube.go b/pkg/skaffold/cluster/minikube.go index af8176996ac..f7dd9cc6573 100644 --- a/pkg/skaffold/cluster/minikube.go +++ b/pkg/skaffold/cluster/minikube.go @@ -36,23 +36,22 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/version" ) -const ( - minikubeVersionWithUserFlag = "1.18.0" +var ( + GetClient = getClient + minikubeVrsionWithUserFlag = semver.MustParse("1.18.0") ) -var GetClient = getClient - // To override during tests var ( - minikubeBinaryFunc = minikubeBinary + FindMinikubeBinary = minikubeBinary getClusterInfo = context.GetClusterInfo GetCurrentVersionFunc = getCurrentVersion findOnce sync.Once mk = struct { + err error // determines if version and path are valid version semver.Version path string - err error }{} ) @@ -70,7 +69,7 @@ func getClient() Client { } func (clientImpl) IsMinikube(kubeContext string) bool { - if _, _, err := minikubeBinaryFunc(); err != nil { + if _, _, err := FindMinikubeBinary(); err != nil { logrus.Tracef("Minikube cluster not detected: %v", err) return false } @@ -104,24 +103,19 @@ func (clientImpl) MinikubeExec(arg ...string) (*exec.Cmd, error) { } func minikubeExec(arg ...string) (*exec.Cmd, error) { - b, v, err := minikubeBinaryFunc() + b, v, err := FindMinikubeBinary() if err != nil { return nil, fmt.Errorf("getting minikube executable: %w", err) } - isSupported, _ := supportsUserFlag(v) - if isSupported { + if supportsUserFlag(v) { arg = append(arg, "--user=skaffold") } return exec.Command(b, arg...), nil } -func supportsUserFlag(ver semver.Version) (bool, error) { - versionWithFlag, err := semver.Make(minikubeVersionWithUserFlag) - if err != nil { - return false, err - } - return ver.GE(versionWithFlag), nil +func supportsUserFlag(ver semver.Version) bool { + return ver.GE(minikubeVrsionWithUserFlag) } // Retrieves minikube version diff --git a/pkg/skaffold/cluster/minikube_test.go b/pkg/skaffold/cluster/minikube_test.go index a9d1f1b251d..fbf4e1916e1 100644 --- a/pkg/skaffold/cluster/minikube_test.go +++ b/pkg/skaffold/cluster/minikube_test.go @@ -98,19 +98,19 @@ func TestClientImpl_IsMinikube(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { if test.minikubeNotInPath { - ver, _ := semver.Make("1.18.0") - t.Override(&minikubeBinaryFunc, func() (string, semver.Version, error) { + ver := semver.Version{} + t.Override(&FindMinikubeBinary, func() (string, semver.Version, error) { return "", ver, fmt.Errorf("minikube not in PATH") }) } else { if test.minikuneWithoutUserFalg { - ver, _ := semver.Make("1.17.0") - t.Override(&minikubeBinaryFunc, func() (string, semver.Version, error) { + ver := semver.Version{Major: 1, Minor: 17, Patch: 0} + t.Override(&FindMinikubeBinary, func() (string, semver.Version, error) { return "", ver, fmt.Errorf("minikube not in PATH") }) } else { - ver, _ := semver.Make("1.18.1") - t.Override(&minikubeBinaryFunc, func() (string, semver.Version, error) { return "minikube", ver, nil }) + ver := semver.Version{Major: 1, Minor: 18, Patch: 1} + t.Override(&FindMinikubeBinary, func() (string, semver.Version, error) { return "minikube", ver, nil }) } } t.Override(&util.DefaultExecCommand, test.minikubeProfileCmd) diff --git a/pkg/skaffold/test/structure/structure_test.go b/pkg/skaffold/test/structure/structure_test.go index d2fe873010d..91c00ff0cde 100644 --- a/pkg/skaffold/test/structure/structure_test.go +++ b/pkg/skaffold/test/structure/structure_test.go @@ -41,7 +41,7 @@ func TestNewRunner(t *testing.T) { testutil.Run(t, "", func(t *testutil.T) { tmpDir := t.NewTempDir().Touch("test.yaml") - t.Override(&cluster.GetCurrentVersionFunc, func() (semver.Version, error) { return semver.Version{}, nil }) + t.Override(&cluster.FindMinikubeBinary, func() (string, semver.Version, error) { return "", semver.Version{}, errors.New("not found") }) t.Override(&util.DefaultExecCommand, testutil.CmdRun("container-structure-test test -v warn --image "+imageName+" --config "+tmpDir.Path("test.yaml")))