Adding distinct exit codes for cluster connection failures. #4933

PriyaModali · 2020-10-23T18:04:31Z

Fixes: #4645
Related: Tracking issue #4921
Description
Adding distinct exit codes for cluster connection failures.

User facing changes
These changes will enable Skaffold dev, deploy, to return distinct error codes when kubernetes cluster could not be reached.

Ex. before error messages:

Unable to connect to the server: net/http: TLS handshake timeout

exiting dev mode because first deploy failed: unable to connect to Kubernetes: Get "https://192.168.64.3:8443/version?timeout=32s": net/http: TLS handshake timeout

Ex. after error messages:

Deploy Failed. Could not connect to <cluster name> cluster.

Deploy Failed. Could not connect to cluster test_cluster due to \"https://192.168.64.3:8443/version?timeout=32s\": net/http: TLS handshake timeout. Check your connection for the cluster.

pkg/skaffold/errors/deployProblems.go

pkg/skaffold/errors/err_map.go

proto/skaffold.proto

briandealwis · 2020-10-23T18:31:06Z

pkg/skaffold/errors/err_map.go

+		regexp:  re(fmt.Sprintf(".*%s.* Uanable to connect: .*", ClusterConnectErrPrefix)),
+		errCode: proto.StatusCode_DEPLOY_CLUSTER_CONNECTION_ERR,
+		description: func(error) string {
+			return "Deploy Failed."


I think this gets turned into the log entry, which is then reflected to the output on the IDEs. Maybe "Unable to connect to cluster"?

The description is not used in the Event API but only for the console.
The ActionableErr.Message which is the raw error is used.

I made a conscious decision to send the raw error to IDEs to they could do any extra processing of the error message.
Maybe it makes sense to send the description or we could change or add another field to the ActionableErr e.g.
ProcessedMessage or ExtractedMessage along with the raw error text in Message.

WDYT?

I only wondered if Skaffold would swallow the Unable to connect and instead report Deploy Failed.

ah. got it. something like this

Deploy Failed. Check your connection for the cluster test_cluster.

Vs what we have right now.

Deploy Failed. Could not connect to cluster test_cluster due to \"https://192.168.64.3:8443/version?timeout=32s\": net/http: TLS handshake timeout. Check your connection for the cluster.

This change makes the message more readable / actionable.

pkg/skaffold/errors/errors.go

proto/skaffold.proto

google-cla · 2020-10-24T01:34:24Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

tejal29 · 2020-10-24T01:34:45Z

@googlebot I consent

codecov · 2020-10-24T01:44:33Z

Codecov Report

Merging #4933 into master will increase coverage by 0.00%.
The diff coverage is 67.74%.

@@           Coverage Diff           @@
##           master    #4933   +/-   ##
=======================================
  Coverage   72.19%   72.19%           
=======================================
  Files         362      363    +1     
  Lines       12715    12749   +34     
=======================================
+ Hits         9179     9204   +25     
- Misses       2855     2863    +8     
- Partials      681      682    +1

Impacted Files	Coverage Δ
pkg/skaffold/errors/err_map.go	`87.17% <55.55%> (-9.49%)`	⬇️
pkg/skaffold/errors/deploy_problems.go	`71.42% <71.42%> (ø)`
pkg/skaffold/errors/errors.go	`96.87% <100.00%> (+0.10%)`	⬆️
pkg/skaffold/schema/defaults/defaults.go	`87.34% <0.00%> (-1.75%)`	⬇️
pkg/skaffold/runner/dev.go	`67.66% <0.00%> (ø)`
pkg/skaffold/runner/build_deploy.go	`68.88% <0.00%> (ø)`
pkg/skaffold/docker/image.go	`81.93% <0.00%> (+1.32%)`	⬆️
...ffold/kubernetes/portforward/resource_forwarder.go	`86.00% <0.00%> (+3.50%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9010a6...8d5ae6a. Read the comment docs.

pkg/skaffold/errors/errors_test.go

briandealwis · 2020-10-26T17:20:16Z

BTW: this code path is never run on the first iteration of dev. I tried pausing minikube and then skaffold dev. I see an error reported

exiting dev mode because first deploy failed: unable to connect to Kubernetes: Get "https://192.168.64.2:8443/version?timeout=32s": net/http: TLS handshake timeout

But there are no events:

{"result":{"timestamp":"2020-10-26T17:17:44.630814Z","event":{"metaEvent":{"entry":"Starting Skaffold: \u0026{Version: ConfigVersion:skaffold/v2beta9 GitVersion: GitCommit: GitTreeState: BuildDate: GoVersion:go1.14.3 Compiler:gc Platform:darwin/amd64}","metadata":{"build":{"numberOfArtifacts":1,"builders":[{"type":"DOCKER","count":1}],"type":"LOCAL"},"deploy":{"deployers":[{"type":"KUBECTL","count":1}],"cluster":"MINIKUBE"}}}}}}
{"result":{"timestamp":"2020-10-26T17:17:45.067668Z","event":{"devLoopEvent":{"status":"In Progress"}},"entry":"Update initiated"}}
event.DevLoopFailedInPhase: phase=Deploy deployState=status:"Not Started" autoTrigger:true 
{"result":{"timestamp":"2020-10-26T17:17:55.194247Z","event":{"devLoopEvent":{"status":"Failed","err":{"message":"unable to connect to Kubernetes: Get \"https://192.168.64.2:8443/version?timeout=32s\": net/http: TLS handshake timeout"}}},"entry":"Update failed with error code OK"}}

tejal29 · 2020-10-26T18:12:11Z

BTW: this code path is never run on the first iteration of dev. I tried pausing minikube and then skaffold dev. I see an error reported

exiting dev mode because first deploy failed: unable to connect to Kubernetes: Get "https://192.168.64.2:8443/version?timeout=32s": net/http: TLS handshake timeout

But there are no events:

{"result":{"timestamp":"2020-10-26T17:17:44.630814Z","event":{"metaEvent":{"entry":"Starting Skaffold: \u0026{Version: ConfigVersion:skaffold/v2beta9 GitVersion: GitCommit: GitTreeState: BuildDate: GoVersion:go1.14.3 Compiler:gc Platform:darwin/amd64}","metadata":{"build":{"numberOfArtifacts":1,"builders":[{"type":"DOCKER","count":1}],"type":"LOCAL"},"deploy":{"deployers":[{"type":"KUBECTL","count":1}],"cluster":"MINIKUBE"}}}}}}
{"result":{"timestamp":"2020-10-26T17:17:45.067668Z","event":{"devLoopEvent":{"status":"In Progress"}},"entry":"Update initiated"}}
event.DevLoopFailedInPhase: phase=Deploy deployState=status:"Not Started" autoTrigger:true 
{"result":{"timestamp":"2020-10-26T17:17:55.194247Z","event":{"devLoopEvent":{"status":"Failed","err":{"message":"unable to connect to Kubernetes: Get \"https://192.168.64.2:8443/version?timeout=32s\": net/http: TLS handshake timeout"}}},"entry":"Update failed with error code OK"}}

@PriyaModali Can you please verify?

tejal29 · 2020-10-26T23:52:38Z

Currently, we suggest "Check cluster connection" as a suggestion.

@gsquared94 added a PR to detect if a context is isMinikube in this PR #4701.

We could use this functionality to suggest

If cluster.IsMinikube(opts.KubeContext) is true and opts.KubeContext == minkube, then suggestion should be

Check if minikube is running using `minikube status` command and try again

If cluster.IsMinikube(opts.KubeContext) is true and opts.KubeContext != minkube e.g. "cloud-run-dev-internal`, then suggestion should be

Check if minikube is running using `minikube status -p cloud-run-dev-internal` command and try again.

if cluster.IsMinikube(opts.KubeContext) is false and opts.KubeContext== "gke_tejal-test_us-central1-a_dump " then suggestion should be

Check your cluster connection for cluster gke_tejal-test_us-central1-a_dump.

It would be great, if for conditions 1 and 2, if we can actually run minikube status command and

if minikube is paused, suggest users to run minikube unpause -p <>
if minikube stopped, then suggest users to run minikube start -p <>.

You can follow up the above two changes in another PR and create an issue for this.

Updated the deploy error fromat.

z

…into deploy_err

pkg/skaffold/errors/errors_test.go

pkg/skaffold/errors/err_map.go

…fold into deploy_err

PriyaModali · 2020-10-29T18:37:21Z

BTW: this code path is never run on the first iteration of dev. I tried pausing minikube and then skaffold dev. I see an error reported

exiting dev mode because first deploy failed: unable to connect to Kubernetes: Get "https://192.168.64.2:8443/version?timeout=32s": net/http: TLS handshake timeout

But there are no events:

{"result":{"timestamp":"2020-10-26T17:17:44.630814Z","event":{"metaEvent":{"entry":"Starting Skaffold: \u0026{Version: ConfigVersion:skaffold/v2beta9 GitVersion: GitCommit: GitTreeState: BuildDate: GoVersion:go1.14.3 Compiler:gc Platform:darwin/amd64}","metadata":{"build":{"numberOfArtifacts":1,"builders":[{"type":"DOCKER","count":1}],"type":"LOCAL"},"deploy":{"deployers":[{"type":"KUBECTL","count":1}],"cluster":"MINIKUBE"}}}}}}
{"result":{"timestamp":"2020-10-26T17:17:45.067668Z","event":{"devLoopEvent":{"status":"In Progress"}},"entry":"Update initiated"}}
event.DevLoopFailedInPhase: phase=Deploy deployState=status:"Not Started" autoTrigger:true 
{"result":{"timestamp":"2020-10-26T17:17:55.194247Z","event":{"devLoopEvent":{"status":"Failed","err":{"message":"unable to connect to Kubernetes: Get \"https://192.168.64.2:8443/version?timeout=32s\": net/http: TLS handshake timeout"}}},"entry":"Update failed with error code OK"}}

Looks like accidentally deleted the error display logic during the merge process. Added it back.

BTW: this code path is never run on the first iteration of dev. I tried pausing minikube and then skaffold dev. I see an error reported

exiting dev mode because first deploy failed: unable to connect to Kubernetes: Get "https://192.168.64.2:8443/version?timeout=32s": net/http: TLS handshake timeout

But there are no events:

{"result":{"timestamp":"2020-10-26T17:17:44.630814Z","event":{"metaEvent":{"entry":"Starting Skaffold: \u0026{Version: ConfigVersion:skaffold/v2beta9 GitVersion: GitCommit: GitTreeState: BuildDate: GoVersion:go1.14.3 Compiler:gc Platform:darwin/amd64}","metadata":{"build":{"numberOfArtifacts":1,"builders":[{"type":"DOCKER","count":1}],"type":"LOCAL"},"deploy":{"deployers":[{"type":"KUBECTL","count":1}],"cluster":"MINIKUBE"}}}}}}
{"result":{"timestamp":"2020-10-26T17:17:45.067668Z","event":{"devLoopEvent":{"status":"In Progress"}},"entry":"Update initiated"}}
event.DevLoopFailedInPhase: phase=Deploy deployState=status:"Not Started" autoTrigger:true 
{"result":{"timestamp":"2020-10-26T17:17:55.194247Z","event":{"devLoopEvent":{"status":"Failed","err":{"message":"unable to connect to Kubernetes: Get \"https://192.168.64.2:8443/version?timeout=32s\": net/http: TLS handshake timeout"}}},"entry":"Update failed with error code OK"}}

@PriyaModali Can you please verify?

BTW: this code path is never run on the first iteration of dev. I tried pausing minikube and then skaffold dev. I see an error reported

exiting dev mode because first deploy failed: unable to connect to Kubernetes: Get "https://192.168.64.2:8443/version?timeout=32s": net/http: TLS handshake timeout

But there are no events:

{"result":{"timestamp":"2020-10-26T17:17:44.630814Z","event":{"metaEvent":{"entry":"Starting Skaffold: \u0026{Version: ConfigVersion:skaffold/v2beta9 GitVersion: GitCommit: GitTreeState: BuildDate: GoVersion:go1.14.3 Compiler:gc Platform:darwin/amd64}","metadata":{"build":{"numberOfArtifacts":1,"builders":[{"type":"DOCKER","count":1}],"type":"LOCAL"},"deploy":{"deployers":[{"type":"KUBECTL","count":1}],"cluster":"MINIKUBE"}}}}}}
{"result":{"timestamp":"2020-10-26T17:17:45.067668Z","event":{"devLoopEvent":{"status":"In Progress"}},"entry":"Update initiated"}}
event.DevLoopFailedInPhase: phase=Deploy deployState=status:"Not Started" autoTrigger:true 
{"result":{"timestamp":"2020-10-26T17:17:55.194247Z","event":{"devLoopEvent":{"status":"Failed","err":{"message":"unable to connect to Kubernetes: Get \"https://192.168.64.2:8443/version?timeout=32s\": net/http: TLS handshake timeout"}}},"entry":"Update failed with error code OK"}}

BTW: this code path is never run on the first iteration of dev. I tried pausing minikube and then skaffold dev. I see an error reported

exiting dev mode because first deploy failed: unable to connect to Kubernetes: Get "https://192.168.64.2:8443/version?timeout=32s": net/http: TLS handshake timeout

But there are no events:

{"result":{"timestamp":"2020-10-26T17:17:44.630814Z","event":{"metaEvent":{"entry":"Starting Skaffold: \u0026{Version: ConfigVersion:skaffold/v2beta9 GitVersion: GitCommit: GitTreeState: BuildDate: GoVersion:go1.14.3 Compiler:gc Platform:darwin/amd64}","metadata":{"build":{"numberOfArtifacts":1,"builders":[{"type":"DOCKER","count":1}],"type":"LOCAL"},"deploy":{"deployers":[{"type":"KUBECTL","count":1}],"cluster":"MINIKUBE"}}}}}}
{"result":{"timestamp":"2020-10-26T17:17:45.067668Z","event":{"devLoopEvent":{"status":"In Progress"}},"entry":"Update initiated"}}
event.DevLoopFailedInPhase: phase=Deploy deployState=status:"Not Started" autoTrigger:true 
{"result":{"timestamp":"2020-10-26T17:17:55.194247Z","event":{"devLoopEvent":{"status":"Failed","err":{"message":"unable to connect to Kubernetes: Get \"https://192.168.64.2:8443/version?timeout=32s\": net/http: TLS handshake timeout"}}},"entry":"Update failed with error code OK"}}

@PriyaModali Can you please verify?

Looks like accidentally deleted the error display logging during the merge. Added it back.

pkg/skaffold/errors/err_map.go

proto/skaffold.proto

pkg/skaffold/errors/errors_test.go

pkg/skaffold/errors/deploy_problems.go

tejal29

Use evaluated kubeContext to check if cluster is local/minikube.
Mock isMinkube and add all variations of tests.

isMinikube=true, clusterName = minikube
isMinikube=true, clusterName = some_random
isMinikube=false, cluterName = 'test_cluster" (what you have right now)

pkg/skaffold/errors/deploy_problems.go

pkg/skaffold/errors/err_map.go

pkg/skaffold/errors/deploy_problems.go

tejal29 · 2020-11-02T19:32:21Z

Currently, we suggest "Check cluster connection" as a suggestion.

@gsquared94 added a PR to detect if a context is isMinikube in this PR #4701.

We could use this functionality to suggest

If cluster.IsMinikube(opts.KubeContext) is true and opts.KubeContext == minkube, then suggestion should be
Check if minikube is running using `minikube status` command and try again
If cluster.IsMinikube(opts.KubeContext) is true and opts.KubeContext != minkube e.g. "cloud-run-dev-internal`, then suggestion should be
Check if minikube is running using `minikube status -p cloud-run-dev-internal` command and try again.
if cluster.IsMinikube(opts.KubeContext) is false and opts.KubeContext== "gke_tejal-test_us-central1-a_dump " then suggestion should be
Check your cluster connection for cluster gke_tejal-test_us-central1-a_dump.
It would be great, if for conditions 1 and 2, if we can actually run minikube status command and

if minikube is paused, suggest users to run minikube unpause -p <>

if minikube stopped, then suggest users to run minikube start -p <>.

You can follow up the above two changes in another PR and create an issue for this.

We spoke in our 1:1 on Friday, to switch opts.KubeContext with the currentConfig() value

pkg/skaffold/errors/deploy_problems.go

tejal29

This code Looks good to me expect for 1 nit.

pkg/skaffold/errors/deploy_problems.go

…fold into deploy_err

Adding distinct exit codes for cluster connection failures.

564c6bc

PriyaModali requested a review from a team as a code owner October 23, 2020 18:04

PriyaModali requested a review from nkubala October 23, 2020 18:04

google-cla bot added the cla: yes label Oct 23, 2020

pull-request-size bot added the size/XL label Oct 23, 2020

tejal29 reviewed Oct 23, 2020

View reviewed changes

pkg/skaffold/errors/deployProblems.go Outdated Show resolved Hide resolved

tejal29 reviewed Oct 23, 2020

View reviewed changes

pkg/skaffold/errors/deployProblems.go Outdated Show resolved Hide resolved

briandealwis reviewed Oct 23, 2020

View reviewed changes

pkg/skaffold/errors/err_map.go Outdated Show resolved Hide resolved

briandealwis reviewed Oct 23, 2020

View reviewed changes

tejal29 mentioned this pull request Oct 23, 2020

Adding distinct exit codes for Kubernetes connection failures. #4921

Closed

Adding unit tests.

0f17f71

tejal29 reviewed Oct 23, 2020

View reviewed changes

pkg/skaffold/errors/errors.go Outdated Show resolved Hide resolved

tejal29 reviewed Oct 23, 2020

View reviewed changes

pkg/skaffold/errors/errors.go Outdated Show resolved Hide resolved

tejal29 reviewed Oct 23, 2020

View reviewed changes

proto/skaffold.proto Outdated Show resolved Hide resolved

tejal29 force-pushed the deploy_err branch from ce050e0 to afaa476 Compare October 24, 2020 01:34

google-cla bot added cla: no and removed cla: yes labels Oct 24, 2020

google-cla bot added cla: yes and removed cla: no labels Oct 24, 2020

Merge branch 'master' into deploy_err

03506c5

tejal29 reviewed Oct 24, 2020

View reviewed changes

pkg/skaffold/errors/errors_test.go Show resolved Hide resolved

PriyaModali added 3 commits October 24, 2020 08:28

Reaming files and updatig unit test to work with latest design changes.

99d0b86

Merging the new deploy cluster error in err_map and updated tests.

5ff9bff

Cleaning up deploy cluster error files.

aa17382

PriyaModali force-pushed the deploy_err branch from afaa476 to aa17382 Compare October 26, 2020 07:10

PriyaModali added 3 commits October 26, 2020 20:12

Updated the known problens with deploy errors.

6a108ee

Updated the deploy error fromat.

Updating error message format.

10c9058

z

Merge branch 'deploy_err' of https://github.com/PriyaModali/skaffold …

c375278

…into deploy_err

tejal29 suggested changes Oct 27, 2020

View reviewed changes

pkg/skaffold/errors/errors_test.go Show resolved Hide resolved

pkg/skaffold/errors/err_map.go Outdated Show resolved Hide resolved

pkg/skaffold/errors/err_map.go Outdated Show resolved Hide resolved

PriyaModali added 2 commits October 28, 2020 15:14

Updating error message with kubecontext.

6d9f51c

Merge branch 'master' of https://github.com/GoogleContainerTools/skaf…

2f3d36e

…fold into deploy_err

Updating error message.

87e4e74

tejal29 suggested changes Oct 29, 2020

View reviewed changes

PriyaModali added 2 commits November 2, 2020 08:17

Updated the logic & test case with modified error messages.

ccb071d

Code re-arrange.

35ff3e3

tejal29 reviewed Nov 2, 2020

View reviewed changes

pkg/skaffold/errors/deploy_problems.go Outdated Show resolved Hide resolved

tejal29 suggested changes Nov 2, 2020

View reviewed changes

pkg/skaffold/errors/deploy_problems.go Outdated Show resolved Hide resolved

pkg/skaffold/errors/err_map.go Outdated Show resolved Hide resolved

pkg/skaffold/errors/err_map.go Outdated Show resolved Hide resolved

pkg/skaffold/errors/deploy_problems.go Outdated Show resolved Hide resolved

tejal29 reviewed Nov 2, 2020

View reviewed changes

pkg/skaffold/errors/deploy_problems.go Outdated Show resolved Hide resolved

tejal29 suggested changes Nov 2, 2020

View reviewed changes

pkg/skaffold/errors/deploy_problems.go Outdated Show resolved Hide resolved

PriyaModali added 3 commits November 2, 2020 15:09

Updated existing unit test.

d0f716b

Adding unit tests.

fb0d476

Fixed messages.

627de26

tejal29 reviewed Nov 3, 2020

View reviewed changes

pkg/skaffold/errors/deploy_problems.go Outdated Show resolved Hide resolved

pkg/skaffold/errors/deploy_problems.go Outdated Show resolved Hide resolved

Addressing lint.

6ab843c

tejal29 approved these changes Nov 3, 2020

View reviewed changes

PriyaModali added 4 commits November 3, 2020 11:11

Merge branch 'master' of https://github.com/GoogleContainerTools/skaf…

9b5fb24

…fold into deploy_err

Updated imports grouping.

e018818

Updating suggestion code.

be83e75

Updating the import order.

8d5ae6a

PriyaModali merged commit cefe8e0 into GoogleContainerTools:master Nov 3, 2020

fmotrifork mentioned this pull request Nov 25, 2020

skaffold 1.17.0 fishworks/fish-food#1082

Merged

tejal29 mentioned this pull request Mar 4, 2021

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding distinct exit codes for cluster connection failures. #4933

Adding distinct exit codes for cluster connection failures. #4933

PriyaModali commented Oct 23, 2020 •

edited by tejal29

Loading

briandealwis Oct 23, 2020

tejal29 Oct 24, 2020 •

edited

Loading

briandealwis Oct 26, 2020

tejal29 Nov 3, 2020

PriyaModali Nov 3, 2020

google-cla bot commented Oct 24, 2020

tejal29 commented Oct 24, 2020

codecov bot commented Oct 24, 2020 •

edited

Loading

briandealwis commented Oct 26, 2020

tejal29 commented Oct 26, 2020

tejal29 commented Oct 26, 2020 •

edited

Loading

PriyaModali commented Oct 29, 2020 •

edited

Loading

tejal29 left a comment

tejal29 commented Nov 2, 2020

tejal29 left a comment

Adding distinct exit codes for cluster connection failures. #4933

Adding distinct exit codes for cluster connection failures. #4933

Conversation

PriyaModali commented Oct 23, 2020 • edited by tejal29 Loading

briandealwis Oct 23, 2020

Choose a reason for hiding this comment

tejal29 Oct 24, 2020 • edited Loading

Choose a reason for hiding this comment

briandealwis Oct 26, 2020

Choose a reason for hiding this comment

tejal29 Nov 3, 2020

Choose a reason for hiding this comment

PriyaModali Nov 3, 2020

Choose a reason for hiding this comment

google-cla bot commented Oct 24, 2020

tejal29 commented Oct 24, 2020

codecov bot commented Oct 24, 2020 • edited Loading

Codecov Report

briandealwis commented Oct 26, 2020

tejal29 commented Oct 26, 2020

tejal29 commented Oct 26, 2020 • edited Loading

PriyaModali commented Oct 29, 2020 • edited Loading

tejal29 left a comment

Choose a reason for hiding this comment

tejal29 commented Nov 2, 2020

tejal29 left a comment

Choose a reason for hiding this comment

PriyaModali commented Oct 23, 2020 •

edited by tejal29

Loading

tejal29 Oct 24, 2020 •

edited

Loading

codecov bot commented Oct 24, 2020 •

edited

Loading

tejal29 commented Oct 26, 2020 •

edited

Loading

PriyaModali commented Oct 29, 2020 •

edited

Loading