From d6e8d7f7955de48c2eda81db9077b96d0c635bc9 Mon Sep 17 00:00:00 2001 From: balopat Date: Fri, 14 Feb 2020 16:39:14 -0800 Subject: [PATCH 1/2] hidden --minikube-profile flag --- cmd/skaffold/app/cmd/flags.go | 13 +++++++++++++ pkg/skaffold/build/local/local_test.go | 6 +++--- pkg/skaffold/build/local/types.go | 6 +++++- pkg/skaffold/config/options.go | 5 +++++ pkg/skaffold/config/util.go | 5 ++++- pkg/skaffold/docker/client.go | 25 +++++++++++++++++-------- pkg/skaffold/docker/client_test.go | 2 +- 7 files changed, 48 insertions(+), 14 deletions(-) diff --git a/cmd/skaffold/app/cmd/flags.go b/cmd/skaffold/app/cmd/flags.go index c5eada7eecd..d27282a4c73 100644 --- a/cmd/skaffold/app/cmd/flags.go +++ b/cmd/skaffold/app/cmd/flags.go @@ -261,6 +261,15 @@ var FlagRegistry = []Flag{ FlagAddMethod: "StringVar", DefinedOn: []string{"build", "debug", "dev", "run"}, }, + { + Name: "minikube-profile", + Shorthand: "m", + Usage: "forces skaffold use the given minikube-profile and forces building against the docker daemon inside that minikube profile", + Value: &opts.MinikubeProfile, + DefValue: "", + FlagAddMethod: "StringVar", + DefinedOn: []string{"build", "debug", "dev", "run"}, + }, } var commandFlags []*pflag.Flag @@ -295,6 +304,10 @@ func AddFlags(fs *pflag.FlagSet, cmdName string) { } } fs.MarkHidden("status-check") + // this is a temporary solution until we figure out an automated way to detect the + // minikube profile see + // https://github.com/GoogleContainerTools/skaffold/issues/3668 + fs.MarkHidden("minikube-profile") } func hasCmdAnnotation(cmdName string, annotations []string) bool { diff --git a/pkg/skaffold/build/local/local_test.go b/pkg/skaffold/build/local/local_test.go index 5e0a8be7301..c8c62f1d226 100644 --- a/pkg/skaffold/build/local/local_test.go +++ b/pkg/skaffold/build/local/local_test.go @@ -253,7 +253,7 @@ func TestNewBuilder(t *testing.T) { shouldErr bool localBuild latest.LocalBuild expectedBuilder *Builder - localClusterFn func(string) (bool, error) + localClusterFn func(string, string) (bool, error) localDockerFn func(*runcontext.RunContext) (docker.LocalDaemon, error) }{ { @@ -268,7 +268,7 @@ func TestNewBuilder(t *testing.T) { localDockerFn: func(*runcontext.RunContext) (docker.LocalDaemon, error) { return dummyDaemon, nil }, - localClusterFn: func(string) (b bool, e error) { + localClusterFn: func(string, string) (b bool, e error) { b = false //because this is false and localBuild.push is nil return }, @@ -290,7 +290,7 @@ func TestNewBuilder(t *testing.T) { localDockerFn: func(*runcontext.RunContext) (docker.LocalDaemon, error) { return dummyDaemon, nil }, - localClusterFn: func(string) (b bool, e error) { + localClusterFn: func(string, string) (b bool, e error) { b = false return }, diff --git a/pkg/skaffold/build/local/types.go b/pkg/skaffold/build/local/types.go index 5c3e3b3eab0..67db4c7ba9d 100644 --- a/pkg/skaffold/build/local/types.go +++ b/pkg/skaffold/build/local/types.go @@ -59,7 +59,11 @@ func NewBuilder(runCtx *runcontext.RunContext) (*Builder, error) { return nil, errors.Wrap(err, "getting docker client") } - localCluster, err := getLocalCluster(runCtx.Opts.GlobalConfig) + // TODO(https://github.com/GoogleContainerTools/skaffold/issues/3668): + // remove minikubeProfile from here and instead detect it by matching the + // kubecontext API Server to minikube profiles + + localCluster, err := getLocalCluster(runCtx.Opts.GlobalConfig, runCtx.Opts.MinikubeProfile) if err != nil { return nil, errors.Wrap(err, "getting localCluster") } diff --git a/pkg/skaffold/config/options.go b/pkg/skaffold/config/options.go index df90c2eeb37..a42a9c5ecb9 100644 --- a/pkg/skaffold/config/options.go +++ b/pkg/skaffold/config/options.go @@ -66,6 +66,11 @@ type SkaffoldOptions struct { Command string RPCPort int RPCHTTPPort int + + // TODO(https://github.com/GoogleContainerTools/skaffold/issues/3668): + // remove minikubeProfile from here and instead detect it by matching the + // kubecontext API Server to minikube profiles + MinikubeProfile string } // Labels returns a map of labels to be applied to all deployed diff --git a/pkg/skaffold/config/util.go b/pkg/skaffold/config/util.go index 4fb5324a295..43f6890271a 100644 --- a/pkg/skaffold/config/util.go +++ b/pkg/skaffold/config/util.go @@ -160,7 +160,10 @@ func GetDefaultRepo(configFile, cliValue string) (string, error) { return cfg.DefaultRepo, nil } -func GetLocalCluster(configFile string) (bool, error) { +func GetLocalCluster(configFile string, minikubeProfile string) (bool, error) { + if minikubeProfile != "" { + return true, nil + } cfg, err := GetConfigForCurrentKubectx(configFile) if err != nil { return false, err diff --git a/pkg/skaffold/docker/client.go b/pkg/skaffold/docker/client.go index 62b11f3740a..e67b97d6b49 100644 --- a/pkg/skaffold/docker/client.go +++ b/pkg/skaffold/docker/client.go @@ -54,7 +54,7 @@ var ( // NewAPIClientImpl guesses the docker client to use based on current Kubernetes context. func NewAPIClientImpl(runCtx *runcontext.RunContext) (LocalDaemon, error) { dockerAPIClientOnce.Do(func() { - env, apiClient, err := newAPIClient(runCtx.KubeContext) + env, apiClient, err := newAPIClient(runCtx.KubeContext, runCtx.Opts.MinikubeProfile) dockerAPIClient = NewLocalDaemon(apiClient, env, runCtx.Opts.Prune(), runCtx.InsecureRegistries) dockerAPIClientErr = err }) @@ -62,10 +62,14 @@ func NewAPIClientImpl(runCtx *runcontext.RunContext) (LocalDaemon, error) { return dockerAPIClient, dockerAPIClientErr } +// TODO(https://github.com/GoogleContainerTools/skaffold/issues/3668): +// remove minikubeProfile from here and instead detect it by matching the +// kubecontext API Server to minikube profiles + // newAPIClient guesses the docker client to use based on current Kubernetes context. -func newAPIClient(kubeContext string) ([]string, client.CommonAPIClient, error) { - if kubeContext == constants.DefaultMinikubeContext { - return newMinikubeAPIClient() +func newAPIClient(kubeContext string, minikubeProfile string) ([]string, client.CommonAPIClient, error) { + if kubeContext == constants.DefaultMinikubeContext || minikubeProfile != "" { + return newMinikubeAPIClient(minikubeProfile) } return newEnvAPIClient() } @@ -85,8 +89,8 @@ func newEnvAPIClient() ([]string, client.CommonAPIClient, error) { // newMinikubeAPIClient returns a docker client using the environment variables // provided by minikube. -func newMinikubeAPIClient() ([]string, client.CommonAPIClient, error) { - env, err := getMinikubeDockerEnv() +func newMinikubeAPIClient(minikubeProfile string) ([]string, client.CommonAPIClient, error) { + env, err := getMinikubeDockerEnv(minikubeProfile) if err != nil { logrus.Warnf("Could not get minikube docker env, falling back to local docker daemon: %s", err) return newEnvAPIClient() @@ -176,13 +180,18 @@ func getMiniKubeFilename() (string, error) { return "minikube", nil } -func getMinikubeDockerEnv() (map[string]string, error) { +func getMinikubeDockerEnv(minikubeProfile string) (map[string]string, error) { miniKubeFilename, err := getMiniKubeFilename() if err != nil { return nil, errors.Wrap(err, "getting minikube filename") } - cmd := exec.Command(miniKubeFilename, "docker-env", "--shell", "none") + args := []string{"docker-env", "--shell", "none"} + if minikubeProfile != "" { + args = append(args, "-p", minikubeProfile) + } + + cmd := exec.Command(miniKubeFilename, args...) out, err := util.RunCmdOut(cmd) if err != nil { return nil, errors.Wrap(err, "getting minikube env") diff --git a/pkg/skaffold/docker/client_test.go b/pkg/skaffold/docker/client_test.go index b8657336b0a..4df00c18547 100644 --- a/pkg/skaffold/docker/client_test.go +++ b/pkg/skaffold/docker/client_test.go @@ -115,7 +115,7 @@ DOCKER_API_VERSION=1.23`, test.env, )) - env, _, err := newMinikubeAPIClient() + env, _, err := newMinikubeAPIClient("") t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedEnv, env) }) From c5381d53768aa759062f7965328dfbce8f6347c7 Mon Sep 17 00:00:00 2001 From: balopat Date: Wed, 19 Feb 2020 22:11:56 -0800 Subject: [PATCH 2/2] remove shorthand --- cmd/skaffold/app/cmd/flags.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/skaffold/app/cmd/flags.go b/cmd/skaffold/app/cmd/flags.go index d27282a4c73..bbae2f8e378 100644 --- a/cmd/skaffold/app/cmd/flags.go +++ b/cmd/skaffold/app/cmd/flags.go @@ -263,7 +263,6 @@ var FlagRegistry = []Flag{ }, { Name: "minikube-profile", - Shorthand: "m", Usage: "forces skaffold use the given minikube-profile and forces building against the docker daemon inside that minikube profile", Value: &opts.MinikubeProfile, DefValue: "",