First draft for adding actionable error items Framework

[tejal29]

In this PR,

• Implement a simple regexMatch to describe root cause and actionable error message.
  • This method, takes in to account config.SkaffoldOptions passed on the CLI to suggest better actionable message.
  • From opts.Skaffold we could derive, the CurrentConfig and RunContext if required.
• Hook this method in Builder.NoArgs. ( Notes, ExactArgs is used for skaffold config set/unset and skaffold schema get hence not hooking it up there)
• Finally covered two tests

User facing changes (remove if N/A)
yes. The "logrus.Fatatf(err)is now changed tocolor.Red.Fprintln`
As we add error messages, the sample output will be change as follows:

Old :

Listing files to watch...
....
The push refers to repository [docker.io/library/leeroy-web]
f9123b69b713: Preparing 
531743b7098c: Preparing 
FATA[0006] exiting dev mode because first build failed: pushing image: denied: requested access to the resource is denied 

New:

tejaldesai@tejaldesai-macbookpro2 microservices (add_err_framework)../../out/skaffold dev
Listing files to watch...
....
The push refers to repository [docker.io/library/leeroy-web]
f9123b69b713: Preparing 
531743b7098c: Preparing 
Build Failed. No push access to specified image repository. Trying running with `--default-repo` flag.

Some things considered as follow up actions

• shd we hide this new flow behind a feature flag until we have tackle considerable amount of errors?
  • Not so keen on this. We can start getting some early feedback on even 1 or 2 errors.
• Currently the errMap lives in code. Shd we make it pluggable so its not dependent on release.
  • Maybe in future, we could provide an extension point where we could have a simple text file with solutions which are straight forward.

     regEx: .*undefined flag --releaseName*
     problem: skaffold v1.32.0 does not support Helm 3.
    solution: Try downgrading to helm v2.0.*
    

[codecov]

Codecov Report

Merging #4045 into master will increase coverage by 0.02%.
The diff coverage is 89.65%.

Impacted Files	Coverage Δ
cmd/skaffold/skaffold.go	0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/commands.go	95.12% <71.42%> (-4.88%)	⬇️
pkg/skaffold/errors/buildProblems.go	100.00% <100.00%> (ø)
pkg/skaffold/errors/err_map.go	100.00% <100.00%> (ø)
pkg/skaffold/errors/errors.go	76.19% <100.00%> (+9.52%)	⬆️
...affold/kubernetes/portforward/kubectl_forwarder.go	65.85% <0.00%> (-2.44%)	⬇️

[briandealwis]

Mining our own output seems brittle and would complicate possible moves to a skaffold-core library with separate UI skins (e.g., a Cloud Run specific front-end that hides any knowledge of Kubernetes).

And we should be thinking of the iDEs as a separate skin (IMHO). I'm concerned as the recommendations here aren't going to be suitable for IDE usage. The example Trying running with --default-repo flag. doesn't make sense to the IDE users.

Maybe these need to be codes like SUGGEST_DEFAULT_REPO?

In my ideal world, all of the printfs in pkg/skaffold would be moved into cmd/skaffold and correspond to events (or structured logs?).

[balopat]

Mining our own output seems brittle and would complicate possible moves to a skaffold-core library with separate UI skins (e.g., a Cloud Run specific front-end that hides any knowledge of Kubernetes).

I agree that ideally our error structure should be type-safe way, like in Java. That could make identifying errors and their context more robust, instead of matching on strings.

However, this is not just our output - it is also underlying tools' output, where we do have to do some kind of string matching typically.

And we should be thinking of the iDEs as a separate skin (IMHO). I'm concerned as the recommendations here aren't going to be suitable for IDE usage. The example Trying running with --default-repo flag. doesn't make sense to the IDE users.
Maybe these need to be codes like SUGGEST_DEFAULT_REPO?

I like this point, but skaffold has to cater for CLI only usage too where this would make sense. Would we make these suggestions conditional on a flag and then IDEs can say "we'll take care of suggestions in our own UI" based on events?

In my ideal world, all of the printfs in pkg/skaffold would be moved into cmd/skaffold and correspond to events (or structured logs?).

I agree that we could think about this more as an MVC pattern, where skaffold's Model would be shared with itself and the IDEs, and they can put separate Views on it.

At the extreme we would reuse the Event API for sending all events, and skaffold could log them, IDEs can decide what they want to do with them - we would have for every possible interesting event a proto message.

However, this sounds like a lot of redesign first and then have the implementation. I think starting with this lightweight approach objectively will improve the experience for Skaffold users.

[nkubala]

@briandealwis brings up some great points that I think we need to consider in the future. I'm hesitant to merge this now, but let's go ahead and do it so we can have a starting point to iterate on.