refactor: move all update checks to single function; enforce honoring updateCheck flag

[gsquared94]

Fixes #4673
Honor update-check flag for all update check notifications. Consolidate function in a single place

[nkubala]

I wonder if this error message is even useful anymore. this will show if I add the line

foo: bar

to my skaffold.yaml, which isn't really helpful, or if I accidentally make a typo like

buildConfigg:
  ...

also, the IDEs have syntax completion for the skaffold.yaml at the version they're using to guide users. WDYT?

[gsquared94]

I wonder if this error message is even useful anymore. this will show if I add the line

foo: bar

to my skaffold.yaml, which isn't really helpful, or if I accidentally make a typo like

buildConfigg:
  ...

also, the IDEs have syntax completion for the skaffold.yaml at the version they're using to guide users. WDYT?

It might seem misleading in those scenarios. But if you look at the errors returned by ParseConfigAndUpgrade function, I think when the error is actually due to older binary or the upgrade function not working this is a reasonable actionable error message. So I think we should keep it.

[nkubala]

added an error to my skaffold.yaml. before this change I saw:

➜  guestbook-1 SKAFFOLD_UPDATE_CHECK=false skaffold dev
WARN[0000] Your Skaffold version might be too old. Download the latest version (1.13.1) from:
  https://github.com/GoogleContainerTools/skaffold/releases/tag/v1.13.1 
parsing skaffold config: unable to parse config: yaml: unmarshal errors:
  line 8: field foo not found in type v2beta4.BuildConfig

now, I see:

➜  guestbook-1 SKAFFOLD_UPDATE_CHECK=false skaffold dev
parsing skaffold config: %!w(<nil>)

so the parse error is getting shadowed.

[gsquared94]

added an error to my skaffold.yaml. before this change I saw:

➜  guestbook-1 SKAFFOLD_UPDATE_CHECK=false skaffold dev
WARN[0000] Your Skaffold version might be too old. Download the latest version (1.13.1) from:
  https://github.com/GoogleContainerTools/skaffold/releases/tag/v1.13.1 
parsing skaffold config: unable to parse config: yaml: unmarshal errors:
  line 8: field foo not found in type v2beta4.BuildConfig

now, I see:

➜  guestbook-1 SKAFFOLD_UPDATE_CHECK=false skaffold dev
parsing skaffold config: %!w(<nil>)

so the parse error is getting shadowed.

Fixed it

[nkubala]

cool, LGTM but there's a linting error. i'll merge when CI is green

[codecov]

Codecov Report

Merging #4677 into master will decrease coverage by 0.03%.
The diff coverage is 68.96%.

@@            Coverage Diff             @@
##           master    #4677      +/-   ##
==========================================
- Coverage   73.21%   73.17%   -0.04%     
==========================================
  Files         339      340       +1     
  Lines       13323    13368      +45     
==========================================
+ Hits         9754     9782      +28     
- Misses       2958     2979      +21     
+ Partials      611      607       -4     

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/cmd.go	68.34% <0.00%> (+4.15%)	⬆️
cmd/skaffold/app/cmd/runner.go	73.17% <16.66%> (-5.78%)	⬇️
pkg/skaffold/update/update.go	43.47% <100.00%> (-10.10%)	⬇️
pkg/skaffold/runner/intent.go	74.19% <0.00%> (-5.12%)	⬇️
pkg/skaffold/runner/new.go	67.11% <0.00%> (ø)
pkg/skaffold/deploy/kpt.go	100.00% <0.00%> (ø)
pkg/skaffold/trigger/triggers.go	37.93% <0.00%> (+5.97%)	⬆️

---

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 283e9c4...fd982cd. Read the comment docs.