Skip to content

Commit 39f60fe

Browse files
tejal29tejal29
committed
code review changes and retesting the scenario manually
1 parent 42c839c commit 39f60fe

File tree

5 files changed

+79
-97
lines changed

5 files changed

+79
-97
lines changed

‎pkg/skaffold/deploy/deploy_problems.go

+3-48
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,14 @@ import (
2121
"regexp"
2222

2323
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
24+
deployerr "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/error"
2425
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/types"
2526
sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
2627
"github.com/GoogleContainerTools/skaffold/proto/v1"
2728
)
2829

29-
const (
30-
defaultMinikubeProfile = "minikube"
31-
)
32-
3330
var (
34-
ClusterInternalSystemErr = regexp.MustCompile(".*Internal Server Error")
35-
clusterConnectionErr = regexp.MustCompile("(?i).*unable to connect.*: Get (.*)")
31+
clusterConnectionErr = regexp.MustCompile("(?i).*unable to connect.*: Get (.*)")
3632
)
3733

3834
func suggestDeployFailedAction(cfg interface{}) []*proto.Suggestion {
@@ -42,7 +38,7 @@ func suggestDeployFailedAction(cfg interface{}) []*proto.Suggestion {
4238
}
4339
if deployCfg.MinikubeProfile() != "" {
4440
return []*proto.Suggestion{
45-
checkMinikubeSuggestion(deployCfg),
41+
deployerr.CheckMinikubeStatusSuggestion(deployCfg),
4642
}
4743
}
4844
return []*proto.Suggestion{{
@@ -64,46 +60,5 @@ func init() {
6460
},
6561
Suggestion: suggestDeployFailedAction,
6662
},
67-
{
68-
Regexp: ClusterInternalSystemErr,
69-
ErrCode: proto.StatusCode_DEPLOY_CLUSTER_INTERNAL_SYSTEM_ERR,
70-
Description: func(err error) string {
71-
return fmt.Sprintf("Deploy Failed. %v", err)
72-
},
73-
Suggestion: func(cfg interface{}) []*proto.Suggestion {
74-
deployCfg, ok := cfg.(types.Config)
75-
if !ok {
76-
return nil
77-
}
78-
if deployCfg.MinikubeProfile() != "" {
79-
return []*proto.Suggestion{
80-
checkMinikubeSuggestion(deployCfg),
81-
{
82-
SuggestionCode: proto.SuggestionCode_OPEN_ISSUE,
83-
// TODO: show tip to run minikube logs command and attach logs.
84-
Action: fmt.Sprintf("open an issue at %s", constants.GithubIssueLink),
85-
}}
86-
}
87-
return []*proto.Suggestion{{
88-
SuggestionCode: proto.SuggestionCode_OPEN_ISSUE,
89-
Action: fmt.Sprintf("Something went wrong with your cluster %q. Try again.\nIf this keeps happening please open an issue at %s", deployCfg.GetKubeContext(), constants.GithubIssueLink),
90-
}}
91-
},
92-
},
9363
})
9464
}
95-
96-
func checkMinikubeSuggestion(cfg types.Config) *proto.Suggestion {
97-
return &proto.Suggestion{
98-
SuggestionCode: proto.SuggestionCode_CHECK_MINIKUBE_STATUS,
99-
Action: fmt.Sprintf("Check if minikube is running using %q command and try again",
100-
getMinikubeStatusCommand(cfg.GetKubeContext())),
101-
}
102-
}
103-
104-
func getMinikubeStatusCommand(p string) string {
105-
if p == defaultMinikubeProfile {
106-
return "minikube status"
107-
}
108-
return fmt.Sprintf("minikube status -p %s", p)
109-
}

‎pkg/skaffold/deploy/deploy_problems_test.go

-35
Original file line numberDiff line numberDiff line change
@@ -81,40 +81,6 @@ func TestSuggestDeployFailedAction(t *testing.T) {
8181
}},
8282
},
8383
},
84-
{
85-
description: "internal system error for minikube cluster",
86-
context: "minikube",
87-
isMinikube: true,
88-
err: fmt.Errorf("Error: (Internal Server Error: the server is currently unable to handle the request)"),
89-
expected: "Deploy Failed. Error: (Internal Server Error: the server is currently unable to handle the request). Check if minikube is running using \"minikube status\" command and try again or open an issue at https://github.com/GoogleContainerTools/skaffold/issues/new.",
90-
expectedAE: &proto.ActionableErr{
91-
ErrCode: proto.StatusCode_DEPLOY_CLUSTER_INTERNAL_SYSTEM_ERR,
92-
Message: "Error: (Internal Server Error: the server is currently unable to handle the request)",
93-
Suggestions: []*proto.Suggestion{{
94-
SuggestionCode: proto.SuggestionCode_CHECK_MINIKUBE_STATUS,
95-
Action: "Check if minikube is running using \"minikube status\" command and try again",
96-
},
97-
{
98-
SuggestionCode: proto.SuggestionCode_OPEN_ISSUE,
99-
Action: fmt.Sprintf("open an issue at %s", constants.GithubIssueLink),
100-
},
101-
},
102-
},
103-
},
104-
{
105-
description: "internal system error for k8s cluster",
106-
context: "test_cluster",
107-
err: fmt.Errorf("Error: (Internal Server Error: the server is currently unable to handle the request)"),
108-
expected: "Deploy Failed. Error: (Internal Server Error: the server is currently unable to handle the request). Something went wrong with your cluster \"test_cluster\". Try again.\nIf this keeps happening please open an issue at https://github.com/GoogleContainerTools/skaffold/issues/new.",
109-
expectedAE: &proto.ActionableErr{
110-
ErrCode: proto.StatusCode_DEPLOY_CLUSTER_INTERNAL_SYSTEM_ERR,
111-
Message: "Error: (Internal Server Error: the server is currently unable to handle the request)",
112-
Suggestions: []*proto.Suggestion{{
113-
SuggestionCode: proto.SuggestionCode_OPEN_ISSUE,
114-
Action: "Something went wrong with your cluster \"test_cluster\". Try again.\nIf this keeps happening please open an issue at https://github.com/GoogleContainerTools/skaffold/issues/new",
115-
}},
116-
},
117-
},
11884
}
11985
for _, test := range tests {
12086
testutil.Run(t, test.description, func(t *testutil.T) {
@@ -123,7 +89,6 @@ func TestSuggestDeployFailedAction(t *testing.T) {
12389
cfg.minikube = test.context
12490
}
12591
actual := sErrors.ShowAIError(&cfg, test.err)
126-
fmt.Println(actual.Error(), "\n", test.expected)
12792
t.CheckDeepEqual(test.expected, actual.Error())
12893
actualAE := sErrors.ActionableErr(&cfg, constants.Deploy, test.err)
12994
t.CheckDeepEqual(test.expectedAE, actualAE)

‎pkg/skaffold/deploy/error/errors.go

+56-5
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,26 @@ package error
1818

1919
import (
2020
"fmt"
21+
"regexp"
2122
"strings"
2223

23-
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy"
24+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
25+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/types"
2426
sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
2527
"github.com/GoogleContainerTools/skaffold/proto/v1"
2628
)
2729

2830
const (
29-
executableNotFound = "executable file not found"
30-
notFound = "%s not found"
31+
executableNotFound = "executable file not found"
32+
notFound = "%s not found"
33+
defaultMinikubeProfile = "minikube"
34+
)
35+
36+
var (
37+
clusterInternalSystemErr = regexp.MustCompile(".*Internal Server Error")
38+
39+
// for testing
40+
internalSystemErrSuggestion = internalSystemErrSuggestionFunc
3141
)
3242

3343
// DebugHelperRetrieveErr is thrown when debug helpers could not be retrieved.
@@ -63,12 +73,53 @@ func UserError(err error, sc proto.StatusCode) error {
6373
if sErrors.IsSkaffoldErr(err) {
6474
return err
6575
}
66-
if deploy.ClusterInternalSystemErr.MatchString(err.Error()) {
67-
return err
76+
if clusterInternalSystemErr.MatchString(err.Error()) {
77+
return sErrors.NewProblem(
78+
func(err error) string {
79+
return fmt.Sprintf("Deploy Failed. %v", err)
80+
},
81+
proto.StatusCode_DEPLOY_CLUSTER_INTERNAL_SYSTEM_ERR,
82+
internalSystemErrSuggestion,
83+
err)
6884
}
6985
return sErrors.NewError(err,
7086
proto.ActionableErr{
7187
Message: err.Error(),
7288
ErrCode: sc,
7389
})
7490
}
91+
92+
func CheckMinikubeStatusSuggestion(cfg types.Config) *proto.Suggestion {
93+
return &proto.Suggestion{
94+
SuggestionCode: proto.SuggestionCode_CHECK_MINIKUBE_STATUS,
95+
Action: fmt.Sprintf("Check if minikube is running using %q command and try again",
96+
getMinikubeStatusCommand(cfg.GetKubeContext())),
97+
}
98+
}
99+
100+
func getMinikubeStatusCommand(p string) string {
101+
if p == defaultMinikubeProfile {
102+
return "minikube status"
103+
}
104+
return fmt.Sprintf("minikube status -p %s", p)
105+
}
106+
107+
func internalSystemErrSuggestionFunc(cfg interface{}) []*proto.Suggestion {
108+
deployCfg, ok := cfg.(types.Config)
109+
if !ok {
110+
return nil
111+
}
112+
if deployCfg.MinikubeProfile() != "" {
113+
return []*proto.Suggestion{
114+
CheckMinikubeStatusSuggestion(deployCfg),
115+
{
116+
SuggestionCode: proto.SuggestionCode_OPEN_ISSUE,
117+
// TODO: show tip to run minikube logs command and attach logs.
118+
Action: fmt.Sprintf("open an issue at %s", constants.GithubIssueLink),
119+
}}
120+
}
121+
return []*proto.Suggestion{{
122+
SuggestionCode: proto.SuggestionCode_OPEN_ISSUE,
123+
Action: fmt.Sprintf("Something went wrong with your cluster %q. Try again.\nIf this keeps happening please open an issue at %s", deployCfg.GetKubeContext(), constants.GithubIssueLink),
124+
}}
125+
}

‎pkg/skaffold/deploy/error/errors_test.go

+16-8
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ limitations under the License.
1717
package error
1818

1919
import (
20-
"errors"
2120
"fmt"
2221
"testing"
2322

@@ -39,7 +38,8 @@ func TestUserError(t *testing.T) {
3938
err: fmt.Errorf("Error: (Internal Server Error: the server is currently unable to handle the request)"),
4039
statusCode: proto.StatusCode_DEPLOY_KUSTOMIZE_USER_ERR,
4140
expected: proto.StatusCode_DEPLOY_CLUSTER_INTERNAL_SYSTEM_ERR,
42-
expectedErr: "Error: (Internal Server Error: the server is currently unable to handle the request)",
41+
expectedErr: "Deploy Failed. Error: (Internal Server Error: the server is currently unable to handle the request)." +
42+
" Something went wrong.",
4343
},
4444
{
4545
description: "not an internal system err",
@@ -51,14 +51,22 @@ func TestUserError(t *testing.T) {
5151
}
5252
for _, test := range tests {
5353
testutil.Run(t, test.description, func(t *testutil.T) {
54+
t.Override(&internalSystemErrSuggestion, func(_ interface{}) []*proto.Suggestion {
55+
return []*proto.Suggestion{{
56+
Action: fmt.Sprintf("Something went wrong"),
57+
}}
58+
})
5459
actual := UserError(test.err, test.statusCode)
55-
if sErrors.IsSkaffoldErr(actual) {
56-
var sErr sErrors.ErrDef
57-
if errors.As(actual, &sErr) {
58-
t.CheckDeepEqual(test.expected, sErr.StatusCode())
59-
}
60+
switch actualType := actual.(type) {
61+
case sErrors.ErrDef:
62+
t.CheckDeepEqual(test.expected, actualType.StatusCode())
63+
case sErrors.Problem:
64+
t.CheckDeepEqual(test.expected, actualType.ErrCode)
65+
actualErr := sErrors.ShowAIError(nil, actualType)
66+
t.CheckErrorContains(test.expectedErr, actualErr)
67+
default:
68+
t.CheckErrorContains(test.expectedErr, actualType)
6069
}
61-
t.CheckErrorContains(test.expectedErr, actual)
6270
})
6371
}
6472
}

‎pkg/skaffold/errors/errors.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,17 @@ func ActionableErrV2(cfg interface{}, phase constants.Phase, err error) *protoV2
6969
}
7070

7171
func ShowAIError(cfg interface{}, err error) error {
72+
if uErr := errors.Unwrap(err); uErr != nil {
73+
err = uErr
74+
}
7275
if IsSkaffoldErr(err) {
7376
instrumentation.SetErrorCode(err.(Error).StatusCode())
7477
return err
7578
}
7679

7780
if p, ok := isProblem(err); ok {
7881
instrumentation.SetErrorCode(p.ErrCode)
79-
return p
82+
return p.AIError(cfg, err)
8083
}
8184

8285
for _, problems := range allErrors {

0 commit comments

Comments
 (0)