Skip to content

Use runCtx instead of of config.Opts for suggesting suggestions. #5490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/skaffold/app/cmd/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ var createRunner = createNewRunner

func withRunner(ctx context.Context, out io.Writer, action func(runner.Runner, []*latest.SkaffoldConfig) error) error {
runner, config, err := createRunner(out, opts)
sErrors.SetSkaffoldOptions(opts)
if err != nil {
return err
}
Expand All @@ -62,6 +61,7 @@ func createNewRunner(out io.Writer, opts config.SkaffoldOptions) (runner.Runner,
if err != nil {
return nil, nil, err
}
sErrors.SetRunContext(*runCtx)

instrumentation.InitMeterFromConfig(configs)
runner, err := runner.NewForConfig(runCtx)
Expand Down
5 changes: 3 additions & 2 deletions pkg/skaffold/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,13 @@ const (
DefaultRPCPort = 50051
DefaultRPCHTTPPort = 50052

DefaultPortForwardNamespace = "default"
DefaultPortForwardAddress = "127.0.0.1"
DefaultPortForwardAddress = "127.0.0.1"

DefaultProjectDescriptor = "project.toml"

LeeroyAppResponse = "leeroooooy app!!\n"

GithubIssueLink = "https://github.com/GoogleContainerTools/skaffold/issues/new"
)

var (
Expand Down
7 changes: 4 additions & 3 deletions pkg/skaffold/errors/build_problems.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package errors

import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
"github.com/GoogleContainerTools/skaffold/proto/v1"
)

Expand All @@ -36,8 +37,8 @@ var (
getConfigForCurrentContext = config.GetConfigForCurrentKubectx
)

func suggestBuildPushAccessDeniedAction(opts config.SkaffoldOptions) []*proto.Suggestion {
if defaultRepo := opts.DefaultRepo.Value(); defaultRepo != nil {
func suggestBuildPushAccessDeniedAction(runCtx runcontext.RunContext) []*proto.Suggestion {
if defaultRepo := runCtx.Opts.DefaultRepo.Value(); defaultRepo != nil {
suggestions := []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_CHECK_DEFAULT_REPO,
Action: "Check your `--default-repo` value",
Expand All @@ -46,7 +47,7 @@ func suggestBuildPushAccessDeniedAction(opts config.SkaffoldOptions) []*proto.Su
}

// check if global repo is set
if cfg, err := getConfigForCurrentContext(opts.GlobalConfig); err == nil {
if cfg, err := getConfigForCurrentContext(runCtx.Opts.GlobalConfig); err == nil {
if defaultRepo := cfg.DefaultRepo; defaultRepo != "" {
suggestions := []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_CHECK_DEFAULT_REPO_GLOBAL_CONFIG,
Expand Down
28 changes: 7 additions & 21 deletions pkg/skaffold/errors/deploy_problems.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,8 @@ package errors
import (
"fmt"

"github.com/sirupsen/logrus"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/cluster"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
"github.com/GoogleContainerTools/skaffold/proto/v1"
)

Expand All @@ -32,26 +29,15 @@ var (
isMinikube = cluster.GetClient().IsMinikube
)

func suggestDeployFailedAction(opts config.SkaffoldOptions) []*proto.Suggestion {
kubeconfig, parsederr := kubectx.CurrentConfig()
if parsederr != nil {
logrus.Debugf("Error retrieving the config: %q", parsederr)
return []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_CHECK_CLUSTER_CONNECTION,
Action: "Check your connection for the cluster",
}}
}

if isMinikube(kubeconfig.CurrentContext) {
if kubeconfig.CurrentContext == "minikube" {
return []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_CHECK_MINIKUBE_STATUS,
Action: "Check if minikube is running using `minikube status` command and try again",
}}
func suggestDeployFailedAction(runCtx runcontext.RunContext) []*proto.Suggestion {
if isMinikube(runCtx.KubeContext) {
command := "minikube status"
if runCtx.KubeContext != "minikube" {
command = fmt.Sprintf("minikube status -p %s", runCtx.KubeContext)
}
return []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_CHECK_MINIKUBE_STATUS,
Action: fmt.Sprintf("Check if minikube is running using `minikube status -p %s` command and try again.", kubeconfig.CurrentContext),
Action: fmt.Sprintf("Check if minikube is running using `%s` command and try again.", command),
}}
}

Expand Down
41 changes: 17 additions & 24 deletions pkg/skaffold/errors/deploy_problems_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,47 +19,40 @@ package errors
import (
"testing"

"k8s.io/client-go/tools/clientcmd/api"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
"github.com/GoogleContainerTools/skaffold/proto/v1"
"github.com/GoogleContainerTools/skaffold/testutil"
)

func TestSuggestDeployFailedAction(t *testing.T) {
tests := []struct {
description string
opts config.SkaffoldOptions
context api.Config
isminikube bool
context string
isMinikube bool
expected []*proto.Suggestion
}{
{
description: "minicube status",
opts: config.SkaffoldOptions{},
context: api.Config{CurrentContext: "minikube"},
isminikube: true,
description: "minikube status",
context: "minikube",
isMinikube: true,
expected: []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_CHECK_MINIKUBE_STATUS,
Action: "Check if minikube is running using `minikube status` command and try again",
Action: "Check if minikube is running using `minikube status` command and try again.",
}},
},
{
description: "minicube status named ctx",
opts: config.SkaffoldOptions{},
context: api.Config{CurrentContext: "test_cluster"},
isminikube: true,
description: "minikube status named ctx",
context: "test_cluster",
isMinikube: true,
expected: []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_CHECK_MINIKUBE_STATUS,
Action: "Check if minikube is running using `minikube status -p test_cluster` command and try again.",
}},
},
{
description: "gke cluster",
opts: config.SkaffoldOptions{},
context: api.Config{},
isminikube: false,
context: "gke_test",
isMinikube: false,
expected: []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_CHECK_CLUSTER_CONNECTION,
Action: "Check your connection for the cluster",
Expand All @@ -68,13 +61,13 @@ func TestSuggestDeployFailedAction(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&kubectx.CurrentConfig, func() (api.Config, error) {
return test.context, nil
})
runCtx := runcontext.RunContext{
KubeContext: test.context,
}
t.Override(&isMinikube, func(string) bool {
return test.isminikube
return test.isMinikube
})
actual := suggestDeployFailedAction(test.opts)
actual := suggestDeployFailedAction(runCtx)
t.CheckDeepEqual(test.expected, actual)
})
}
Expand Down
26 changes: 9 additions & 17 deletions pkg/skaffold/errors/err_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@ import (
"fmt"
"regexp"

"github.com/sirupsen/logrus"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
"github.com/GoogleContainerTools/skaffold/proto/v1"
)

Expand All @@ -42,7 +39,7 @@ type problem struct {
regexp *regexp.Regexp
description func(error) string
errCode proto.StatusCode
suggestion func(opts config.SkaffoldOptions) []*proto.Suggestion
suggestion func(runCtx runcontext.RunContext) []*proto.Suggestion
}

var (
Expand All @@ -66,7 +63,7 @@ var (
description: func(error) string {
return "Build Cancelled."
},
suggestion: func(config.SkaffoldOptions) []*proto.Suggestion {
suggestion: func(_ runcontext.RunContext) []*proto.Suggestion {
return nil
},
},
Expand All @@ -76,7 +73,7 @@ var (
return "Build Failed"
},
errCode: proto.StatusCode_BUILD_PROJECT_NOT_FOUND,
suggestion: func(config.SkaffoldOptions) []*proto.Suggestion {
suggestion: func(_ runcontext.RunContext) []*proto.Suggestion {
return []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_CHECK_GCLOUD_PROJECT,
Action: "Check your GCR project",
Expand All @@ -93,7 +90,7 @@ var (
}
return "Build Failed. Could not connect to Docker daemon"
},
suggestion: func(config.SkaffoldOptions) []*proto.Suggestion {
suggestion: func(runcontext.RunContext) []*proto.Suggestion {
return []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_CHECK_DOCKER_RUNNING,
Action: "Check if docker is running",
Expand All @@ -115,8 +112,8 @@ var (
return fmt.Sprintf("%s. Ignoring files dependencies for all ONBUILD triggers", pre)
},
errCode: proto.StatusCode_DEVINIT_UNSUPPORTED_V1_MANIFEST,
suggestion: func(opts config.SkaffoldOptions) []*proto.Suggestion {
if opts.Command == dev || opts.Command == debug {
suggestion: func(runCtx runcontext.RunContext) []*proto.Suggestion {
if runCtx.Opts.Command == dev || runCtx.Opts.Command == debug {
return []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_RUN_DOCKER_PULL,
Action: "To avoid, hit Cntrl-C and run `docker pull` to fetch the specified image and retry",
Expand All @@ -133,15 +130,10 @@ var (
errCode: proto.StatusCode_DEPLOY_CLUSTER_CONNECTION_ERR,
description: func(err error) string {
matchExp := re("(?i).*unable to connect.*Get (.*)")
kubeconfig, parsederr := kubectx.CurrentConfig()
if parsederr != nil {
logrus.Debugf("Error retrieving the config: %q", parsederr)
return fmt.Sprintf("Deploy failed. Could not connect to the cluster due to %s", err)
}
if match := matchExp.FindStringSubmatch(fmt.Sprintf("%s", err)); len(match) >= 2 {
return fmt.Sprintf("Deploy Failed. Could not connect to cluster %s due to %s", kubeconfig.CurrentContext, match[1])
return fmt.Sprintf("Deploy Failed. Could not connect to cluster %s due to %s", runCtx.KubeContext, match[1])
}
return fmt.Sprintf("Deploy Failed. Could not connect to %s cluster.", kubeconfig.CurrentContext)
return fmt.Sprintf("Deploy Failed. Could not connect to %s cluster.", runCtx.KubeContext)
},
suggestion: suggestDeployFailedAction,
},
Expand Down
29 changes: 15 additions & 14 deletions pkg/skaffold/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ import (
"strings"
"sync"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
"github.com/GoogleContainerTools/skaffold/proto/v1"
)

Expand All @@ -38,14 +39,14 @@ const (
Cleanup = Phase("Cleanup")

// Report issue text
reportIssueText = "If above error is unexpected, please open an issue https://github.com/GoogleContainerTools/skaffold/issues/new to report this error"
reportIssueText = "If above error is unexpected, please open an issue " + constants.GithubIssueLink + " to report this error"
)

var (
setOptionsOnce sync.Once
skaffoldOpts config.SkaffoldOptions
setRunContextOnce sync.Once
runCtx runcontext.RunContext

reportIssueSuggestion = func(config.SkaffoldOptions) []*proto.Suggestion {
reportIssueSuggestion = func(runcontext.RunContext) []*proto.Suggestion {
return []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_OPEN_ISSUE,
Action: reportIssueText,
Expand All @@ -55,11 +56,11 @@ var (

type Phase string

// SetSkaffoldOptions set Skaffold config options once. These options are used later to
// suggest actionable error messages based on skaffold run context
func SetSkaffoldOptions(opts config.SkaffoldOptions) {
setOptionsOnce.Do(func() {
skaffoldOpts = opts
// SetRunContext set Skaffold runCtx once. This run context is used later to
// suggest actionable error messages based on skaffold command line options and run context
func SetRunContext(rc runcontext.RunContext) {
setRunContextOnce.Do(func() {
runCtx = rc
})
}

Expand All @@ -83,7 +84,7 @@ func ShowAIError(err error) error {
for _, v := range append(knownProblems, knownInitProblems...) {
if v.regexp.MatchString(err.Error()) {
instrumentation.SetErrorCode(v.errCode)
if suggestions := v.suggestion(skaffoldOpts); suggestions != nil {
if suggestions := v.suggestion(runCtx); suggestions != nil {
description := fmt.Sprintf("%s\n", err)
if v.description != nil {
description = strings.Trim(v.description(err), ".")
Expand All @@ -98,9 +99,9 @@ func ShowAIError(err error) error {

func IsOldImageManifestProblem(err error) (string, bool) {
if err != nil && oldImageManifest.regexp.MatchString(err.Error()) {
if s := oldImageManifest.suggestion(skaffoldOpts); s != nil {
if s := oldImageManifest.suggestion(runCtx); s != nil {
return fmt.Sprintf("%s. %s", oldImageManifest.description(err),
concatSuggestions(oldImageManifest.suggestion(skaffoldOpts))), true
concatSuggestions(oldImageManifest.suggestion(runCtx))), true
}
return "", true
}
Expand All @@ -116,7 +117,7 @@ func getErrorCodeFromError(phase Phase, err error) (proto.StatusCode, []*proto.S
if problems, ok := allErrors[phase]; ok {
for _, v := range problems {
if v.regexp.MatchString(err.Error()) {
return v.errCode, v.suggestion(skaffoldOpts)
return v.errCode, v.suggestion(runCtx)
}
}
}
Expand Down
Loading