Add a `skaffold diagnose` command #1109

dgageot · 2018-10-08T09:02:58Z

balopat · 2018-10-08T15:55:10Z

cmd/skaffold/app/cmd/diagnose.go

+			return errors.Wrap(err, "listing artifact dependencies")
+		}
+
+		fmt.Fprintln(out, "Type:", typeOfArtifact(artifact))


would be nice to have the artifact name here as well - on Ahmet's microservices demo this becomes very hard to follow. But - if someone wants to anonymize before sharing, maybe have an "--anonymize" mode where it prints no names but at least the artifact indexes?

Skaffold version: c2b06e54ebd4eabdccf7101d21520a9dae5e050d Configuration version: skaffold/v1alpha4 Number of artifacts: 10 Running a diagnostic on artifacts and their dependencies: Type: Docker - Dependencies to watch: 27 - Time to list dependencies: 18.67081ms - Size of the Docker context: 1940480 - Time to compute mTimes for all dependencies: 204.849µs Type: Docker - Dependencies to watch: 9 - Time to list dependencies: 13.525253ms - Size of the Docker context: 121856 - Time to compute mTimes for all dependencies: 68.634µs Type: Docker - Dependencies to watch: 7 - Time to list dependencies: 11.399753ms - Size of the Docker context: 85504 - Time to compute mTimes for all dependencies: 61.37µs Type: Docker - Dependencies to watch: 8 - Time to list dependencies: 7.941914ms - Size of the Docker context: 95232 - Time to compute mTimes for all dependencies: 64.555µs Type: Docker - Dependencies to watch: 32 - Time to list dependencies: 10.389932ms - Size of the Docker context: 1309184 - Time to compute mTimes for all dependencies: 225.894µs Type: Docker - Dependencies to watch: 4 - Time to list dependencies: 9.034657ms - Size of the Docker context: 7680 - Time to compute mTimes for all dependencies: 34.428µs Type: Docker - Dependencies to watch: 11 - Time to list dependencies: 6.168579ms - Size of the Docker context: 92672 - Time to compute mTimes for all dependencies: 80.195µs Type: Docker - Dependencies to watch: 9 - Time to list dependencies: 11.701616ms - Size of the Docker context: 114176 - Time to compute mTimes for all dependencies: 145.571µs Type: Docker - Dependencies to watch: 8 - Time to list dependencies: 5.532876ms - Size of the Docker context: 94208 - Time to compute mTimes for all dependencies: 52.45µs Type: Docker - Dependencies to watch: 10 - Time to list dependencies: 13.208922ms - Size of the Docker context: 114176 - Time to compute mTimes for all dependencies: 87.346µs Second run (that benefits from cache): Type: Docker - Dependencies to watch: 27 - Time to list dependencies: 1.273089ms - Size of the Docker context: 1940480 - Time to compute mTimes for all dependencies: 151.649µs Type: Docker - Dependencies to watch: 9 - Time to list dependencies: 734.717µs - Size of the Docker context: 121856 - Time to compute mTimes for all dependencies: 54.673µs Type: Docker - Dependencies to watch: 7 - Time to list dependencies: 680.493µs - Size of the Docker context: 85504 - Time to compute mTimes for all dependencies: 39.901µs Type: Docker - Dependencies to watch: 8 - Time to list dependencies: 460.873µs - Size of the Docker context: 95232 - Time to compute mTimes for all dependencies: 45.44µs Type: Docker - Dependencies to watch: 32 - Time to list dependencies: 774.482µs - Size of the Docker context: 1309184 - Time to compute mTimes for all dependencies: 181.169µs Type: Docker - Dependencies to watch: 4 - Time to list dependencies: 496.26µs - Size of the Docker context: 7680 - Time to compute mTimes for all dependencies: 26.045µs Type: Docker - Dependencies to watch: 11 - Time to list dependencies: 643.966µs - Size of the Docker context: 92672 - Time to compute mTimes for all dependencies: 63.188µs Type: Docker - Dependencies to watch: 9 - Time to list dependencies: 746.828µs - Size of the Docker context: 114176 - Time to compute mTimes for all dependencies: 51.068µs Type: Docker - Dependencies to watch: 8 - Time to list dependencies: 390.799µs - Size of the Docker context: 94208 - Time to compute mTimes for all dependencies: 49.544µs Type: Docker - Dependencies to watch: 10 - Time to list dependencies: 792.987µs - Size of the Docker context: 114176 - Time to compute mTimes for all dependencies: 58.997µs

or maybe a --quiet mode that only prints the essential information?

I'm going to improve the output

balopat · 2018-10-08T15:59:59Z

cmd/skaffold/app/cmd/diagnose.go

+		return errors.Wrap(err, "running diagnostic on artifacts")
+	}
+
+	fmt.Fprintln(out, "\nSecond run (that benefits from cache):")


an idea: what if we list the result of the runs for each metric, instead of printing a full run?
i.e.:

Number of artifacts: 2 Running a diagnostic on artifacts and their dependencies: Type: Docker - Dependencies to watch: 2 - Time to list dependencies: 4.809915ms - Size of the Docker context: 3072 - Time to compute mTimes for all dependencies: 19.012µs Type: Docker - Dependencies to watch: 2 - Time to list dependencies: 454.866µs - Size of the Docker context: 3072 - Time to compute mTimes for all dependencies: 16.534µs Second run (that benefits from cache): Type: Docker - Dependencies to watch: 2 - Time to list dependencies: 396.262µs - Size of the Docker context: 3072 - Time to compute mTimes for all dependencies: 16.35µs Type: Docker - Dependencies to watch: 2 - Time to list dependencies: 305.815µs - Size of the Docker context: 3072 - Time to compute mTimes for all dependencies: 29.689µs

it would be this:

Number of artifacts: 2 Running a diagnostic on artifacts and their dependencies: Type: Docker - Dependencies to watch: 2 - Time to list dependencies: 4.809915ms, 396.262µs (cache) - Size of the Docker context: 3072 - Time to compute mTimes for all dependencies: 19.012µs, 16.35µs (cache) Type: Docker - Dependencies to watch: 2 - Time to list dependencies: 454.866µs, 305.815µs (cached) - Size of the Docker context: 3072 - Time to compute mTimes for all dependencies: 16.534µs, 29.689µs (cached)

I'll +1 this, the output of this can get pretty large for bigger projects

balopat · 2018-10-08T16:01:06Z

cmd/skaffold/app/cmd/diagnose.go

+				return errors.Wrap(err, "computing the size of the Docker context")
+			}
+
+			fmt.Fprintln(out, " - Size of the Docker context:", size)


do we want a human readable size format here, or there's a reason you keep bytes? Even if so, I think a bytes unit can be useful.

there are packages out there that will do automatic "humanization" of byte sizes for you, e.g. https://github.com/dustin/go-humanize

nkubala · 2018-10-08T17:52:30Z

cmd/skaffold/app/cmd/diagnose.go

+		return errors.Wrap(err, "running diagnostic on artifacts")
+	}
+
+	fmt.Fprintln(out, "\nSecond run (that benefits from cache):")


I'll +1 this, the output of this can get pretty large for bigger projects

nkubala · 2018-10-08T17:53:20Z

cmd/skaffold/app/cmd/diagnose.go

+			return errors.Wrap(err, "listing artifact dependencies")
+		}
+
+		fmt.Fprintln(out, "Type:", typeOfArtifact(artifact))


or maybe a --quiet mode that only prints the essential information?

nkubala · 2018-10-08T17:56:02Z

cmd/skaffold/app/cmd/diagnose.go

+				return errors.Wrap(err, "computing the size of the Docker context")
+			}
+
+			fmt.Fprintln(out, " - Size of the Docker context:", size)


there are packages out there that will do automatic "humanization" of byte sizes for you, e.g. https://github.com/dustin/go-humanize

nkubala · 2018-10-08T17:56:20Z

cmd/skaffold/app/cmd/diagnose.go

+	return nil
+}
+
+func sizeOfTheDockerContext(a *latest.Artifact) (int64, error) {


nit: sizeOfDockerContext()

nkubala · 2018-10-08T17:58:23Z

cmd/skaffold/app/cmd/diagnose.go

+	return nil
+}
+
+func diagnoseArtifacts(out io.Writer, artifacts []*latest.Artifact) error {


it might also be worth trying to infer some of the builder- and deployer- specific information for each artifact. e.g. docker version for a docker artifact, bazel version for a bazel artifact, kubectl version, helm version, etc.

codecov-io · 2018-10-09T06:22:25Z

Codecov Report

Merging #1109 into master will decrease coverage by 1.17%.
The diff coverage is 5.74%.

@@            Coverage Diff             @@
##           master    #1109      +/-   ##
==========================================
- Coverage   43.54%   42.36%   -1.18%     
==========================================
  Files          84       78       -6     
  Lines        3734     3609     -125     
==========================================
- Hits         1626     1529      -97     
+ Misses       1951     1927      -24     
+ Partials      157      153       -4

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/diagnose.go	`0% <0%> (ø)`
pkg/skaffold/runner/runner.go	`51.47% <0%> (+1.27%)`	⬆️
cmd/skaffold/app/cmd/cmd.go	`0% <0%> (ø)`	⬆️
cmd/skaffold/app/cmd/dev.go	`0% <0%> (ø)`	⬆️
cmd/skaffold/app/cmd/fix.go	`0% <0%> (ø)`	⬆️
pkg/skaffold/watch/changes.go	`97.87% <100%> (ø)`	⬆️
pkg/skaffold/watch/watch.go	`85% <100%> (ø)`	⬆️
pkg/skaffold/util/util.go	`45.09% <0%> (-4%)`	⬇️
pkg/skaffold/deploy/kubectl.go	`51.21% <0%> (-1.17%)`	⬇️
pkg/skaffold/deploy/kubectl/cli.go	`0% <0%> (ø)`	⬆️
... and 13 more

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 01a6ad8...a72da4e. Read the comment docs.

Signed-off-by: David Gageot <[email protected]>

Fixes GoogleContainerTools#1094 Signed-off-by: David Gageot <[email protected]>

dgageot requested review from balopat, nkubala and r2d4 as code owners October 8, 2018 09:02

balopat reviewed Oct 8, 2018

View reviewed changes

nkubala reviewed Oct 8, 2018

View reviewed changes

dgageot force-pushed the diagnose branch from c2b06e5 to 9a1cb2a Compare October 9, 2018 06:07

dgageot added 2 commits October 10, 2018 18:53

Move flags next to the command

f848d4b

Signed-off-by: David Gageot <[email protected]>

Add a skaffold diagnose command

a72da4e

Fixes GoogleContainerTools#1094 Signed-off-by: David Gageot <[email protected]>

dgageot force-pushed the diagnose branch from d8c9104 to a72da4e Compare October 10, 2018 16:56

nkubala approved these changes Oct 10, 2018

View reviewed changes

dgageot merged commit 7407bf1 into GoogleContainerTools:master Oct 10, 2018

dgageot deleted the diagnose branch December 28, 2018 07:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a `skaffold diagnose` command #1109

Add a `skaffold diagnose` command #1109

dgageot commented Oct 8, 2018

balopat Oct 8, 2018

nkubala Oct 8, 2018

dgageot Oct 9, 2018

balopat Oct 8, 2018

nkubala Oct 8, 2018

balopat Oct 8, 2018

nkubala Oct 8, 2018

nkubala Oct 8, 2018

nkubala Oct 8, 2018

nkubala Oct 8, 2018

nkubala Oct 8, 2018

nkubala Oct 8, 2018

codecov-io commented Oct 9, 2018 •

edited

Loading

Add a skaffold diagnose command #1109

Add a skaffold diagnose command #1109

Conversation

dgageot commented Oct 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Oct 9, 2018 • edited Loading

Codecov Report

Add a `skaffold diagnose` command #1109

Add a `skaffold diagnose` command #1109

codecov-io commented Oct 9, 2018 •

edited

Loading