🎁 Easier debugging, especially for the in-cluster sender

[cardil]

Changes

• 🎁 Easier debugging, especially for the in-cluster sender

/kind enhancement

Fixes #129

Release Note

The errors are easier to debug because the in-cluster jobs and their pods are logged and users are directed to them.

[knative-prow]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[cardil]

This error handler prints errors like this:

[cardil]

Here's an example of successful log file contents: https://gist.github.com/cardil/6151fae7d7b9c5f8ffdcf6469d6ea27b

And here is a failure example: https://gist.github.com/cardil/af129bb83cbfb018b93921babd513467

[cardil]

With ff89790 I added gathering of K8s' events as well. That should collect all the info we'll need for debugging: job, pods, and events.

Here's updated successful log (converted to JSON array, for readability): https://gist.github.com/cardil/d8b7963c10e7b6e3befcc112916965c7

And here is a failure example (also as JSON array): https://gist.github.com/cardil/d1216a51f483f242ed39517f4ede9a9c

[cardil]

Here's how the error handling and the kn client tui package animation plays well:

[cardil]

Using this deadline should be safe (the image download time isn't counting into this deadline), as checked here: https://drive.google.com/file/d/19js79hNTqPJG7CogHH7O6-CRSijDXRNE/view

[codecov]

Codecov Report

Attention: Patch coverage is 74.88987% with 57 lines in your changes missing coverage. Please review.

Project coverage is 68.92%. Comparing base (b7dd9cb) to head (3d07d88).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/k8s/job_gatherer.go	73.58%	9 Missing and 5 partials ⚠️
internal/cli/errors.go	82.35%	6 Missing and 3 partials ⚠️
pkg/sender/in_cluster.go	55.00%	6 Missing and 3 partials ⚠️
pkg/cli/context.go	12.50%	5 Missing and 2 partials ⚠️
pkg/errors/wrap.go	72.72%	4 Missing and 2 partials ⚠️
pkg/k8s/jobrunner.go	87.17%	5 Missing ⚠️
pkg/ics/send.go	0.00%	4 Missing ⚠️
internal/cli/send.go	0.00%	2 Missing ⚠️
internal/cli/build.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #375      +/-   ##
==========================================
+ Coverage   66.52%   68.92%   +2.39%     
==========================================
  Files          48       52       +4     
  Lines        1440     1622     +182     
==========================================
+ Hits          958     1118     +160     
- Misses        412      424      +12     
- Partials       70       80      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cardil]

/test all

[rhuss]

Nice PR, great work!

I do not have much time to review it thoroughly, but I am happy to ack it. @cardil, is there anything you want to add before merging?

[cardil]

@rhuss: PR, great work!

Thanks. The PR works great, so I'm happy to merge it.

BTW. I think we can adopt very similar error processing across all of kn and fun. WDYT?

[dsimansk]

/approve
/lgtm

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil, dsimansk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [cardil,dsimansk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhuss]

BTW. I think we can adopt very similar error processing across all of kn and fun. WDYT?

That sounds like a good idea. Maybe start with talking to the func team, as this has more interactive elements already anyway. Also, we need to be sure, that the spinner is only active if there is a tty. Not sure if the PR deals already with that, that when no tty is available (i.e. used in a pipe or a redirection), then no control characters or colors should be printed.

[cardil]

@rhuss: we need to be sure, that the spinner is only active if there is a tty. Not sure if the PR deals already with that, that when no tty is available (i.e. used in a pipe or a redirection), then no control characters or colors should be printed.

That's already addressed in knative.dev/client/pkg library and in bubblegum library as well.