Skip to content

Commit

Permalink
Add an init phase to detect skaffold errors even before skaffold runn…
Browse files Browse the repository at this point in the history
…er is created. (#4926)

* Introduce a new Phase skaffold "Init" which indicates, skaffold failed in the initial phase.

If, Minikube is not running, and skaffold deploy context is set to "minikube", skaffold fails
when creating a runner.
This failure has no phase attached to it.

Adding a Init Phase, will help reduce errors where "UNKNOWN_ERROR" is seen to "INIT_KNOWNN"

* add tests for init phase problem

* minor style changes

* code review comments

* code review comments

* code review comments: SessionEndEvent -> TerminationEvent
  • Loading branch information
tejal29 authored Oct 21, 2020
1 parent e1e0446 commit 12d64a2
Show file tree
Hide file tree
Showing 14 changed files with 1,099 additions and 290 deletions.
2 changes: 2 additions & 0 deletions cmd/skaffold/app/cmd/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
Expand Down Expand Up @@ -60,6 +61,7 @@ func createNewRunner(opts config.SkaffoldOptions) (runner.Runner, *latest.Skaffo

runner, err := runner.NewForConfig(runCtx)
if err != nil {
event.InititializationFailed(err)
return nil, nil, fmt.Errorf("creating runner: %w", err)
}

Expand Down
268 changes: 242 additions & 26 deletions docs/content/en/api/skaffold.swagger.json

Large diffs are not rendered by default.

46 changes: 38 additions & 8 deletions docs/content/en/docs/references/api/grpc.md
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ It is one of MetaEvent, BuildEvent, DeployEvent, PortEvent, StatusCheckEvent, Re
| fileSyncEvent | [FileSyncEvent](#proto.FileSyncEvent) | | describes the sync status. |
| debuggingContainerEvent | [DebuggingContainerEvent](#proto.DebuggingContainerEvent) | | describes the appearance or disappearance of a debugging container |
| devLoopEvent | [DevLoopEvent](#proto.DevLoopEvent) | | describes a start and end of a dev loop. |
| terminationEvent | [TerminationEvent](#proto.TerminationEvent) | | describes a skaffold termination event |



Expand Down Expand Up @@ -635,6 +636,22 @@ Suggestion defines the action a user needs to recover from an error.



<a name="proto.TerminationEvent"></a>
#### TerminationEvent
`TerminationEvent` marks the end of the skaffold session


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| status | [string](#string) | | status oneof: Completed or Failed |
| err | [ActionableErr](#proto.ActionableErr) | | actionable error message |







<a name="proto.TriggerRequest"></a>
#### TriggerRequest

Expand Down Expand Up @@ -789,15 +806,25 @@ For Cancelled Error code, use range 800 to 850.
| BUILD_UNKNOWN | 506 | Build failed due to unknown reason |
| DEVINIT_UNKNOWN | 507 | Dev Init failed due to unknown reason |
| CLEANUP_UNKNOWN | 508 | Cleanup failed due to unknown reason |
| INIT_UNKNOWN | 510 | Initialization of the Skaffold session failed due to unknown reason(s) |
| SYNC_INIT_ERROR | 601 | File Sync Initialize failure |
| DEVINIT_REGISTER_BUILD_DEPS | 701 | Failed to configure watcher for build dependencies in dev loop |
| DEVINIT_REGISTER_TEST_DEPS | 702 | Failed to configure watcher for test dependencies in dev loop |
| DEVINIT_REGISTER_DEPLOY_DEPS | 703 | Failed to configure watcher for deploy dependencies in dev loop |
| DEVINIT_REGISTER_CONFIG_DEP | 704 | Failed to configure watcher for Skaffold configuration file. |
| STATUSCHECK_USER_CANCELLED | 800 | User cancelled the skaffold dev run |
| STATUSCHECK_DEADLINE_EXCEEDED | 801 | Deadline for status check exceeded |
| BUILD_CANCELLED | 802 | Build cancelled due to user cancellation or one or more build failed. |
| BUILD_CANCELLED | 802 | Build Cancelled |
| DEPLOY_CANCELLED | 803 | Deploy cancelled due to user cancellation or one or more deployers failed. |
| INIT_CREATE_TAGGER_ERROR | 901 | Skaffold was unable to create the configured tagger |
| INIT_MINIKUBE_PAUSED_ERROR | 902 | Skaffold was unable to start as Minikube appears to be paused |
| INIT_MINIKUBE_NOT_RUNNING_ERROR | 903 | Skaffold was unable to start as Minikube appears to be stopped |
| INIT_CREATE_BUILDER_ERROR | 904 | Skaffold was unable to create a configured image builder |
| INIT_CREATE_DEPLOYER_ERROR | 905 | Skaffold was unable to create a configured deployer |
| INIT_CREATE_TEST_DEP_ERROR | 906 | Skaffold was unable to create a configured test |
| INIT_CACHE_ERROR | 907 | Skaffold encountered an error validating the artifact cache |
| INIT_CREATE_WATCH_TRIGGER_ERROR | 908 | Skaffold encountered an error when configuring file watching |
| INIT_CREATE_ARTIFACT_DEP_ERROR | 909 | Skaffold encountered an error when evaluating artifact dependencies |



Expand All @@ -809,13 +836,13 @@ Enum for Suggestion codes
| Name | Number | Description |
| ---- |:------:| ----------- |
| NIL | 0 | default nil suggestion. This is usually set when no error happens. |
| ADD_DEFAULT_REPO | 100 | Build error suggestion codes |
| CHECK_DEFAULT_REPO | 101 | |
| CHECK_DEFAULT_REPO_GLOBAL_CONFIG | 102 | |
| GCLOUD_DOCKER_AUTH_CONFIGURE | 103 | |
| DOCKER_AUTH_CONFIGURE | 104 | |
| CHECK_GCLOUD_PROJECT | 105 | |
| CHECK_DOCKER_RUNNING | 106 | |
| ADD_DEFAULT_REPO | 100 | Add Default Repo |
| CHECK_DEFAULT_REPO | 101 | Verify Default Repo |
| CHECK_DEFAULT_REPO_GLOBAL_CONFIG | 102 | Verify default repo in the global config |
| GCLOUD_DOCKER_AUTH_CONFIGURE | 103 | run gcloud docker auth configure |
| DOCKER_AUTH_CONFIGURE | 104 | Run docker auth configure |
| CHECK_GCLOUD_PROJECT | 105 | Verify Gcloud Project |
| CHECK_DOCKER_RUNNING | 106 | Check if docker is running |
| CHECK_CONTAINER_LOGS | 301 | Container run error |
| CHECK_READINESS_PROBE | 302 | Pod Health check error |
| CHECK_CONTAINER_IMAGE | 303 | Check Container image |
Expand All @@ -828,6 +855,9 @@ Enum for Suggestion codes
| ADDRESS_NODE_NOT_READY | 406 | Node not ready error |
| ADDRESS_FAILED_SCHEDULING | 407 | Scheduler failure error |
| CHECK_HOST_CONNECTION | 408 | Cluster Connectivity error |
| START_MINIKUBE | 501 | Minikube is stopped: use `minikube start` |
| UNPAUSE_MINIKUBE | 502 | Minikube is paused: use `minikube unpause` |
| OPEN_ISSUE | 900 | Open an issue so this situation can be diagnosed |


<!-- end enums -->
Expand Down
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion pkg/skaffold/errors/err_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ var knownBuildProblems = []problem{
suggestion: func(config.SkaffoldOptions) []*proto.Suggestion {
return []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_CHECK_DOCKER_RUNNING,
Action: "Please check if docker is running",
Action: "Check if docker is running",
}}
},
},
Expand Down
73 changes: 57 additions & 16 deletions pkg/skaffold/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,28 @@ import (

// These are phases in a DevLoop
const (
Init = Phase("Init")
Build = Phase("Build")
Deploy = Phase("Deploy")
StatusCheck = Phase("StatusCheck")
FileSync = Phase("FileSync")
DevInit = Phase("DevInit")
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"
)

var (
setOptionsOnce sync.Once
skaffoldOpts config.SkaffoldOptions

reportIssueSuggestion = func(config.SkaffoldOptions) []*proto.Suggestion {
return []*proto.Suggestion{{
SuggestionCode: proto.SuggestionCode_OPEN_ISSUE,
Action: reportIssueText,
}}
}
)

type Phase string
Expand All @@ -61,10 +72,14 @@ func ActionableErr(phase Phase, err error) *proto.ActionableErr {
}

func ShowAIError(err error) error {
for _, v := range knownBuildProblems {
for _, v := range append(knownBuildProblems, knownInitProblems...) {
if v.regexp.MatchString(err.Error()) {
if suggestions := v.suggestion(skaffoldOpts); suggestions != nil {
return fmt.Errorf("%s. %s", strings.Trim(v.description(err), "."), concatSuggestions(suggestions))
description := fmt.Sprintf("%s\n", err)
if v.description != nil {
description = strings.Trim(v.description(err), ".")
}
return fmt.Errorf("%s. %s", description, concatSuggestions(suggestions))
}
return fmt.Errorf(v.description(err))
}
Expand All @@ -73,24 +88,12 @@ func ShowAIError(err error) error {
}

func getErrorCodeFromError(phase Phase, err error) (proto.StatusCode, []*proto.Suggestion) {
switch phase {
case Build:
for _, v := range knownBuildProblems {
if problems, ok := allErrors[phase]; ok {
for _, v := range problems {
if v.regexp.MatchString(err.Error()) {
return v.errCode, v.suggestion(skaffoldOpts)
}
}
return proto.StatusCode_BUILD_UNKNOWN, nil
case Deploy:
return proto.StatusCode_DEPLOY_UNKNOWN, nil
case StatusCheck:
return proto.StatusCode_STATUSCHECK_UNKNOWN, nil
case FileSync:
return proto.StatusCode_SYNC_UNKNOWN, nil
case DevInit:
return proto.StatusCode_DEVINIT_UNKNOWN, nil
case Cleanup:
return proto.StatusCode_CLEANUP_UNKNOWN, nil
}
return proto.StatusCode_UNKNOWN_ERROR, nil
}
Expand All @@ -106,3 +109,41 @@ func concatSuggestions(suggestions []*proto.Suggestion) string {
s.WriteString(".")
return s.String()
}

var allErrors = map[Phase][]problem{
Build: append(knownBuildProblems, problem{
regexp: re(".*"),
errCode: proto.StatusCode_BUILD_UNKNOWN,
suggestion: reportIssueSuggestion,
}),
Init: append(knownInitProblems, problem{
regexp: re(".*"),
errCode: proto.StatusCode_INIT_UNKNOWN,
suggestion: reportIssueSuggestion,
}),
Deploy: {{
regexp: re(".*"),
errCode: proto.StatusCode_DEPLOY_UNKNOWN,
suggestion: reportIssueSuggestion,
}},
StatusCheck: {{
regexp: re(".*"),
errCode: proto.StatusCode_STATUSCHECK_UNKNOWN,
suggestion: reportIssueSuggestion,
}},
FileSync: {{
regexp: re(".*"),
errCode: proto.StatusCode_SYNC_UNKNOWN,
suggestion: reportIssueSuggestion,
}},
DevInit: {{
regexp: re(".*"),
errCode: proto.StatusCode_DEVINIT_UNKNOWN,
suggestion: reportIssueSuggestion,
}},
Cleanup: {{
regexp: re(".*"),
errCode: proto.StatusCode_CLEANUP_UNKNOWN,
suggestion: reportIssueSuggestion,
}},
}
Loading

0 comments on commit 12d64a2

Please sign in to comment.