Skip to content

Commit

Permalink
Merge pull request #3691 from balopat/quickfix_to_pass_minikube_profile
Browse files Browse the repository at this point in the history
hidden --minikube-profile flag
  • Loading branch information
balopat authored Feb 20, 2020
2 parents a95fcf8 + c5381d5 commit 987ec17
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 14 deletions.
12 changes: 12 additions & 0 deletions cmd/skaffold/app/cmd/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,14 @@ var FlagRegistry = []Flag{
FlagAddMethod: "StringVar",
DefinedOn: []string{"build", "debug", "dev", "run"},
},
{
Name: "minikube-profile",
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
Expand Down Expand Up @@ -295,6 +303,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 {
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/build/local/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}{
{
Expand All @@ -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
},
Expand All @@ -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
},
Expand Down
6 changes: 5 additions & 1 deletion pkg/skaffold/build/local/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/skaffold/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion pkg/skaffold/config/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 17 additions & 8 deletions pkg/skaffold/docker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,22 @@ 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
})

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()
}
Expand All @@ -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()
Expand Down Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/docker/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down

0 comments on commit 987ec17

Please sign in to comment.