Skip to content

Commit 193c06c

Browse files
tejal29tejal29
authored
Use runCtx instead of of config.Opts for suggesting suggestions. (#5490)
* Use runcontext.Context to suggest suggestions for error codes. Fix deploy tests * use constants * address comments
1 parent 2b41d3f commit 193c06c

File tree

10 files changed

+80
-102
lines changed

10 files changed

+80
-102
lines changed

‎cmd/skaffold/app/cmd/runner.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ var createRunner = createNewRunner
4646

4747
func withRunner(ctx context.Context, out io.Writer, action func(runner.Runner, []*latest.SkaffoldConfig) error) error {
4848
runner, config, err := createRunner(out, opts)
49-
sErrors.SetSkaffoldOptions(opts)
5049
if err != nil {
5150
return err
5251
}
@@ -62,6 +61,7 @@ func createNewRunner(out io.Writer, opts config.SkaffoldOptions) (runner.Runner,
6261
if err != nil {
6362
return nil, nil, err
6463
}
64+
sErrors.SetRunContext(*runCtx)
6565

6666
instrumentation.InitMeterFromConfig(configs)
6767
runner, err := runner.NewForConfig(runCtx)

‎pkg/skaffold/constants/constants.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,13 @@ const (
5151
DefaultRPCPort = 50051
5252
DefaultRPCHTTPPort = 50052
5353

54-
DefaultPortForwardNamespace = "default"
55-
DefaultPortForwardAddress = "127.0.0.1"
54+
DefaultPortForwardAddress = "127.0.0.1"
5655

5756
DefaultProjectDescriptor = "project.toml"
5857

5958
LeeroyAppResponse = "leeroooooy app!!\n"
59+
60+
GithubIssueLink = "https://github.com/GoogleContainerTools/skaffold/issues/new"
6061
)
6162

6263
var (

‎pkg/skaffold/errors/build_problems.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package errors
1818

1919
import (
2020
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
21+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
2122
"github.com/GoogleContainerTools/skaffold/proto/v1"
2223
)
2324

@@ -36,8 +37,8 @@ var (
3637
getConfigForCurrentContext = config.GetConfigForCurrentKubectx
3738
)
3839

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

4849
// check if global repo is set
49-
if cfg, err := getConfigForCurrentContext(opts.GlobalConfig); err == nil {
50+
if cfg, err := getConfigForCurrentContext(runCtx.Opts.GlobalConfig); err == nil {
5051
if defaultRepo := cfg.DefaultRepo; defaultRepo != "" {
5152
suggestions := []*proto.Suggestion{{
5253
SuggestionCode: proto.SuggestionCode_CHECK_DEFAULT_REPO_GLOBAL_CONFIG,

‎pkg/skaffold/errors/deploy_problems.go

+7-21
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,8 @@ package errors
1919
import (
2020
"fmt"
2121

22-
"github.com/sirupsen/logrus"
23-
2422
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/cluster"
25-
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
26-
kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
23+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
2724
"github.com/GoogleContainerTools/skaffold/proto/v1"
2825
)
2926

@@ -32,26 +29,15 @@ var (
3229
isMinikube = cluster.GetClient().IsMinikube
3330
)
3431

35-
func suggestDeployFailedAction(opts config.SkaffoldOptions) []*proto.Suggestion {
36-
kubeconfig, parsederr := kubectx.CurrentConfig()
37-
if parsederr != nil {
38-
logrus.Debugf("Error retrieving the config: %q", parsederr)
39-
return []*proto.Suggestion{{
40-
SuggestionCode: proto.SuggestionCode_CHECK_CLUSTER_CONNECTION,
41-
Action: "Check your connection for the cluster",
42-
}}
43-
}
44-
45-
if isMinikube(kubeconfig.CurrentContext) {
46-
if kubeconfig.CurrentContext == "minikube" {
47-
return []*proto.Suggestion{{
48-
SuggestionCode: proto.SuggestionCode_CHECK_MINIKUBE_STATUS,
49-
Action: "Check if minikube is running using `minikube status` command and try again",
50-
}}
32+
func suggestDeployFailedAction(runCtx runcontext.RunContext) []*proto.Suggestion {
33+
if isMinikube(runCtx.KubeContext) {
34+
command := "minikube status"
35+
if runCtx.KubeContext != "minikube" {
36+
command = fmt.Sprintf("minikube status -p %s", runCtx.KubeContext)
5137
}
5238
return []*proto.Suggestion{{
5339
SuggestionCode: proto.SuggestionCode_CHECK_MINIKUBE_STATUS,
54-
Action: fmt.Sprintf("Check if minikube is running using `minikube status -p %s` command and try again.", kubeconfig.CurrentContext),
40+
Action: fmt.Sprintf("Check if minikube is running using `%s` command and try again.", command),
5541
}}
5642
}
5743

‎pkg/skaffold/errors/deploy_problems_test.go

+17-24
Original file line numberDiff line numberDiff line change
@@ -19,47 +19,40 @@ package errors
1919
import (
2020
"testing"
2121

22-
"k8s.io/client-go/tools/clientcmd/api"
23-
24-
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
25-
kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
22+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
2623
"github.com/GoogleContainerTools/skaffold/proto/v1"
2724
"github.com/GoogleContainerTools/skaffold/testutil"
2825
)
2926

3027
func TestSuggestDeployFailedAction(t *testing.T) {
3128
tests := []struct {
3229
description string
33-
opts config.SkaffoldOptions
34-
context api.Config
35-
isminikube bool
30+
context string
31+
isMinikube bool
3632
expected []*proto.Suggestion
3733
}{
3834
{
39-
description: "minicube status",
40-
opts: config.SkaffoldOptions{},
41-
context: api.Config{CurrentContext: "minikube"},
42-
isminikube: true,
35+
description: "minikube status",
36+
context: "minikube",
37+
isMinikube: true,
4338
expected: []*proto.Suggestion{{
4439
SuggestionCode: proto.SuggestionCode_CHECK_MINIKUBE_STATUS,
45-
Action: "Check if minikube is running using `minikube status` command and try again",
40+
Action: "Check if minikube is running using `minikube status` command and try again.",
4641
}},
4742
},
4843
{
49-
description: "minicube status named ctx",
50-
opts: config.SkaffoldOptions{},
51-
context: api.Config{CurrentContext: "test_cluster"},
52-
isminikube: true,
44+
description: "minikube status named ctx",
45+
context: "test_cluster",
46+
isMinikube: true,
5347
expected: []*proto.Suggestion{{
5448
SuggestionCode: proto.SuggestionCode_CHECK_MINIKUBE_STATUS,
5549
Action: "Check if minikube is running using `minikube status -p test_cluster` command and try again.",
5650
}},
5751
},
5852
{
5953
description: "gke cluster",
60-
opts: config.SkaffoldOptions{},
61-
context: api.Config{},
62-
isminikube: false,
54+
context: "gke_test",
55+
isMinikube: false,
6356
expected: []*proto.Suggestion{{
6457
SuggestionCode: proto.SuggestionCode_CHECK_CLUSTER_CONNECTION,
6558
Action: "Check your connection for the cluster",
@@ -68,13 +61,13 @@ func TestSuggestDeployFailedAction(t *testing.T) {
6861
}
6962
for _, test := range tests {
7063
testutil.Run(t, test.description, func(t *testutil.T) {
71-
t.Override(&kubectx.CurrentConfig, func() (api.Config, error) {
72-
return test.context, nil
73-
})
64+
runCtx := runcontext.RunContext{
65+
KubeContext: test.context,
66+
}
7467
t.Override(&isMinikube, func(string) bool {
75-
return test.isminikube
68+
return test.isMinikube
7669
})
77-
actual := suggestDeployFailedAction(test.opts)
70+
actual := suggestDeployFailedAction(runCtx)
7871
t.CheckDeepEqual(test.expected, actual)
7972
})
8073
}

‎pkg/skaffold/errors/err_map.go

+9-17
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,7 @@ import (
2020
"fmt"
2121
"regexp"
2222

23-
"github.com/sirupsen/logrus"
24-
25-
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
26-
kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
23+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
2724
"github.com/GoogleContainerTools/skaffold/proto/v1"
2825
)
2926

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

4845
var (
@@ -66,7 +63,7 @@ var (
6663
description: func(error) string {
6764
return "Build Cancelled."
6865
},
69-
suggestion: func(config.SkaffoldOptions) []*proto.Suggestion {
66+
suggestion: func(_ runcontext.RunContext) []*proto.Suggestion {
7067
return nil
7168
},
7269
},
@@ -76,7 +73,7 @@ var (
7673
return "Build Failed"
7774
},
7875
errCode: proto.StatusCode_BUILD_PROJECT_NOT_FOUND,
79-
suggestion: func(config.SkaffoldOptions) []*proto.Suggestion {
76+
suggestion: func(_ runcontext.RunContext) []*proto.Suggestion {
8077
return []*proto.Suggestion{{
8178
SuggestionCode: proto.SuggestionCode_CHECK_GCLOUD_PROJECT,
8279
Action: "Check your GCR project",
@@ -93,7 +90,7 @@ var (
9390
}
9491
return "Build Failed. Could not connect to Docker daemon"
9592
},
96-
suggestion: func(config.SkaffoldOptions) []*proto.Suggestion {
93+
suggestion: func(runcontext.RunContext) []*proto.Suggestion {
9794
return []*proto.Suggestion{{
9895
SuggestionCode: proto.SuggestionCode_CHECK_DOCKER_RUNNING,
9996
Action: "Check if docker is running",
@@ -115,8 +112,8 @@ var (
115112
return fmt.Sprintf("%s. Ignoring files dependencies for all ONBUILD triggers", pre)
116113
},
117114
errCode: proto.StatusCode_DEVINIT_UNSUPPORTED_V1_MANIFEST,
118-
suggestion: func(opts config.SkaffoldOptions) []*proto.Suggestion {
119-
if opts.Command == dev || opts.Command == debug {
115+
suggestion: func(runCtx runcontext.RunContext) []*proto.Suggestion {
116+
if runCtx.Opts.Command == dev || runCtx.Opts.Command == debug {
120117
return []*proto.Suggestion{{
121118
SuggestionCode: proto.SuggestionCode_RUN_DOCKER_PULL,
122119
Action: "To avoid, hit Cntrl-C and run `docker pull` to fetch the specified image and retry",
@@ -133,15 +130,10 @@ var (
133130
errCode: proto.StatusCode_DEPLOY_CLUSTER_CONNECTION_ERR,
134131
description: func(err error) string {
135132
matchExp := re("(?i).*unable to connect.*Get (.*)")
136-
kubeconfig, parsederr := kubectx.CurrentConfig()
137-
if parsederr != nil {
138-
logrus.Debugf("Error retrieving the config: %q", parsederr)
139-
return fmt.Sprintf("Deploy failed. Could not connect to the cluster due to %s", err)
140-
}
141133
if match := matchExp.FindStringSubmatch(fmt.Sprintf("%s", err)); len(match) >= 2 {
142-
return fmt.Sprintf("Deploy Failed. Could not connect to cluster %s due to %s", kubeconfig.CurrentContext, match[1])
134+
return fmt.Sprintf("Deploy Failed. Could not connect to cluster %s due to %s", runCtx.KubeContext, match[1])
143135
}
144-
return fmt.Sprintf("Deploy Failed. Could not connect to %s cluster.", kubeconfig.CurrentContext)
136+
return fmt.Sprintf("Deploy Failed. Could not connect to %s cluster.", runCtx.KubeContext)
145137
},
146138
suggestion: suggestDeployFailedAction,
147139
},

‎pkg/skaffold/errors/errors.go

+15-14
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ import (
2222
"strings"
2323
"sync"
2424

25-
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
25+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
2626
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation"
27+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
2728
"github.com/GoogleContainerTools/skaffold/proto/v1"
2829
)
2930

@@ -38,14 +39,14 @@ const (
3839
Cleanup = Phase("Cleanup")
3940

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

4445
var (
45-
setOptionsOnce sync.Once
46-
skaffoldOpts config.SkaffoldOptions
46+
setRunContextOnce sync.Once
47+
runCtx runcontext.RunContext
4748

48-
reportIssueSuggestion = func(config.SkaffoldOptions) []*proto.Suggestion {
49+
reportIssueSuggestion = func(runcontext.RunContext) []*proto.Suggestion {
4950
return []*proto.Suggestion{{
5051
SuggestionCode: proto.SuggestionCode_OPEN_ISSUE,
5152
Action: reportIssueText,
@@ -55,11 +56,11 @@ var (
5556

5657
type Phase string
5758

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

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

99100
func IsOldImageManifestProblem(err error) (string, bool) {
100101
if err != nil && oldImageManifest.regexp.MatchString(err.Error()) {
101-
if s := oldImageManifest.suggestion(skaffoldOpts); s != nil {
102+
if s := oldImageManifest.suggestion(runCtx); s != nil {
102103
return fmt.Sprintf("%s. %s", oldImageManifest.description(err),
103-
concatSuggestions(oldImageManifest.suggestion(skaffoldOpts))), true
104+
concatSuggestions(oldImageManifest.suggestion(runCtx))), true
104105
}
105106
return "", true
106107
}
@@ -116,7 +117,7 @@ func getErrorCodeFromError(phase Phase, err error) (proto.StatusCode, []*proto.S
116117
if problems, ok := allErrors[phase]; ok {
117118
for _, v := range problems {
118119
if v.regexp.MatchString(err.Error()) {
119-
return v.errCode, v.suggestion(skaffoldOpts)
120+
return v.errCode, v.suggestion(runCtx)
120121
}
121122
}
122123
}

0 commit comments

Comments
 (0)