Skip to content

Commit

Permalink
Add user flag to all minikube command invocations (#5649)
Browse files Browse the repository at this point in the history 
* Adding user flag for minikube.

* Adding version to global cache

* Unit tests.

* Merged version check & minikube binary lookup.

* Updates
  • Loading branch information
@PriyaModali
PriyaModali authored Apr 13, 2021
1 parent 5e2a4ec commit 1ef4fd8
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 24 deletions.
69 changes: 58 additions & 11 deletions pkg/skaffold/cluster/minikube.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,35 @@ import (
"os"
"os/exec"
"path/filepath"
"sync"

"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
var (
GetClient = getClient
minikubeVrsionWithUserFlag = semver.MustParse("1.18.0")
)

// To override during tests
var (
minikubeBinaryFunc = minikubeBinary
getClusterInfo = context.GetClusterInfo
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
}{}
)

type Client interface {
Expand All @@ -55,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
}
Expand Down Expand Up @@ -89,22 +103,55 @@ func (clientImpl) MinikubeExec(arg ...string) (*exec.Cmd, error) {
}

func minikubeExec(arg ...string) (*exec.Cmd, error) {
b, err := minikubeBinaryFunc()
b, v, err := FindMinikubeBinary()
if err != nil {
return nil, fmt.Errorf("getting minikube executable: %w", err)
}

if supportsUserFlag(v) {
arg = append(arg, "--user=skaffold")
}
return exec.Command(b, arg...), nil
}

func minikubeBinary() (string, error) {
filename, err := exec.LookPath("minikube")
func supportsUserFlag(ver semver.Version) bool {
return ver.GE(minikubeVrsionWithUserFlag)
}

// Retrieves minikube version
func getCurrentVersion() (semver.Version, error) {
cmd := exec.Command("minikube", "version")
out, err := util.RunCmdOut(cmd)
if err != nil {
return "", errors.New("unable to find minikube executable. Please add it to PATH environment variable")
return semver.Version{}, err
}
if _, err := os.Stat(filename); os.IsNotExist(err) {
return "", fmt.Errorf("unable to find minikube executable. File not found %s", filename)

currentVersion, err := version.ParseVersion(string(out))
if err != nil {
return semver.Version{}, err
}
return filename, nil

return currentVersion, nil
}

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 mk.path, mk.version, mk.err
}

// matchClusterCertPath checks if the cluster certificate for this context is from inside the minikube directory
Expand Down
47 changes: 34 additions & 13 deletions pkg/skaffold/cluster/minikube_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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'",
Expand All @@ -56,41 +58,60 @@ 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",
minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json", fmt.Sprintf(profileStr, "test-cluster", "docker", "172.17.0.3", 8443)),
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", fmt.Sprintf(profileStr, "test-cluster", "hyperkit", "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", `Random error`),
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", fmt.Sprintf(profileStr, "test-cluster", "hyperkit", "192.168.64.11", 8443)),
minikubeProfileCmd: testutil.CmdRunOut("minikube profile list -o json --user=skaffold", fmt.Sprintf(profileStr, "test-cluster", "hyperkit", "192.168.64.11", 8443)),
expected: false,
},
}

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.Version{}
t.Override(&FindMinikubeBinary, 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.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.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)
t.Override(&getClusterInfo, func(string) (*clientcmdapi.Cluster, error) {
Expand Down
5 changes: 5 additions & 0 deletions pkg/skaffold/test/structure/structure_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import (
"io/ioutil"
"testing"

"github.com/blang/semver"

"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"
Expand All @@ -38,6 +41,8 @@ func TestNewRunner(t *testing.T) {

testutil.Run(t, "", func(t *testutil.T) {
tmpDir := t.NewTempDir().Touch("test.yaml")
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")))

cfg := &mockConfig{
Expand Down

0 comments on commit 1ef4fd8

Please sign in to comment.