adding verbose debug output for inspect & inspects #206

mikebrow · 2017-11-29T18:06:07Z

Changes inspect (container) and inspects (sandbox) to be verbose by default, and using json format by default per discussion below.

Verbose is a new option for the subject status requests afforded by the CRI api.

I've also added "quiet" options to the these two inspect(s) in case someone does not want verbose, and to otherwise exercise the non verbose status requests.

Signed-off-by: Mike Brown [email protected]

Random-Liu · 2017-11-29T19:08:38Z

Take a look at https://github.com/kubernetes-incubator/cri-tools/blob/master/cmd/crictl/info.go
We should generate similar output, basically json/yaml of status+info.

mikebrow · 2017-11-29T19:59:05Z

Look at https://github.com/kubernetes-incubator/cri-tools/blob/master/cmd/crictl/info.go
We should generate similar output.

It does..

Random-Liu · 2017-11-29T23:25:58Z

cmd/crictl/container.go

@@ -221,6 +221,10 @@ var containerStatusCommand = cli.Command{
 			Name:  "output, o",
 			Usage: "Output format, One of: json|yaml|table",
 		},
+		cli.BoolFlag{


I think for inspect, we should always set verbose=true in crictl.
verbose=false is for kubelet using CRI.

@Random-Liu so you are saying you want a --quiet option for less verbose? or take out the option all-together? There were already other verbose flags so I figured that's what we would want with this verbose option.

I feel like by default we should enable verbose, or else it's really troublesome to do -v every time. :)

As for whether we need a quiet option, I'm not sure.

@Random-Liu I hear ya.. think it over no rush, easy enough to switch it to verbose by default, or quiet mode option. But I think when you see the verbose mode from the CRI-O team you'll be begging to turn down the volume :-) @runcom

? Not following, what about CRI-O? We aren't even using that verbose stuff and we have no plan to output anything.
In any case, I'm in favor of going verbose by default like Lantao

+1 of enabling verbose.

I was just making light of the amount of metadata CRI-O keeps around that could possibly used in a verbose output of status about the container, thus the smiley face :-)

Yeah but that's exactly the verbose intention, despite what runtimes want to output

I vote inspect to be verbose by default for crictl (and provide a silent flag to turn verbose off).
I vote to make the json format the default (and I don't care if you want to kill the table format or keep it as an option).

Random-Liu · 2017-11-29T23:32:51Z

cmd/crictl/container.go

@@ -221,6 +221,10 @@ var containerStatusCommand = cli.Command{
 			Name:  "output, o",
 			Usage: "Output format, One of: json|yaml|table",


@feiskyer @mrunalp @runcom @mikebrow Do you still think we should have the table format?
I think for inspect, we may just need json|yaml. Just like crictl info. See an example here
The problem is that timestamp in CRI is unix timestamp, we may want to do a conversion in crictl.

I'm not a big fan of table, let's drop it in favor of machine readable output

I'm not a big fan of table formats either, but as @Random-Liu says the unix timestamp is not human readable output:

crictl --runtime-endpoint /var/run/cri-containerd.sock inspect -v -o json 44025055b48da66cb67bfc221f800fe50f1256d3e22f1cf714294344b29d582e { "status": { "id": "44025055b48da66cb67bfc221f800fe50f1256d3e22f1cf714294344b29d582e", "metadata": { "name": "busybox", "attempt": 0 }, "state": "CONTAINER_CREATED", "createdAt": "1511994785429097447", "startedAt": "0", "finishedAt": "0", "exitCode": 0, "image": { "image": "docker.io/library/busybox:latest" }, "imageRef": "docker.io/library/busybox@sha256:bbc3a03235220b170ba48a157dd097dd1379299370e1ed99ce976df0355d24f0", "reason": "", "message": "", "logPath": "" },

vs:

crictl --runtime-endpoint /var/run/cri-containerd.sock inspect -v 44025055b48da66cb67bfc221f800fe50f1256d3e22f1cf714294344b29d582e ID: 44025055b48da66cb67bfc221f800fe50f1256d3e22f1cf714294344b29d582e Name: busybox State: CONTAINER_CREATED Created: 28 minutes ago

We can do some conversion there, not a big deal

Yeah, we should do some conversion. :)

Agreed, table is not useful for inspect.

so.. @Random-Liu @runcom @feiskyer wrt. "some conversion"
I suggest converting the timestamps to RFC3339Nano
"2006-01-02T15:04:05.999999999Z07:00"
Thoughts?

Signed-off-by: Mike Brown <[email protected]>

mikebrow · 2017-12-01T14:37:41Z

Ok I've changed inspect to use json format by default, made the default output verbose, and introduced a quiet flag just because. @Random-Liu @runcom @feiskyer

Signed-off-by: Mike Brown <[email protected]>

mikebrow · 2017-12-01T16:53:20Z

I've added a new commit for the inspects (sandbox inspect) changes.

Random-Liu · 2017-12-04T20:04:57Z

/lgtm

I think we don't need the table output, but let's keep it for now in case that someone may like it.

Signed-off-by: Mike Brown <[email protected]>

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 29, 2017

mikebrow force-pushed the verbose-inspect branch from 5ff2724 to 5016f72 Compare November 29, 2017 18:13

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 29, 2017

mikebrow mentioned this pull request Nov 29, 2017

Implement CRI Debug API containerd/cri#359

Closed

4 tasks

Random-Liu self-assigned this Nov 29, 2017

Random-Liu reviewed Nov 29, 2017

View reviewed changes

adding verbose output switch for inspect

faa6682

Signed-off-by: Mike Brown <[email protected]>

mikebrow force-pushed the verbose-inspect branch from 5016f72 to faa6682 Compare December 1, 2017 14:35

added inspects options for verbose debug out

0a7a812

Signed-off-by: Mike Brown <[email protected]>

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 1, 2017

mikebrow mentioned this pull request Dec 1, 2017

adding info map for verbose pod status containerd/cri#452

Merged

mikebrow changed the title ~~adding verbose output switch for inspect~~ adding verbose debug output for inspect & inspects Dec 1, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 4, 2017

adds inspecti options for verbose debug out

0e48c9e

Signed-off-by: Mike Brown <[email protected]>

Random-Liu merged commit 6462875 into kubernetes-sigs:master Dec 4, 2017

yanxuean mentioned this pull request Aug 4, 2018

Fix crictl ps and crictl pods. #363

Merged

spiffxp unassigned Random-Liu Sep 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding verbose debug output for inspect & inspects #206

adding verbose debug output for inspect & inspects #206

mikebrow commented Nov 29, 2017 •

edited

Loading

Random-Liu commented Nov 29, 2017 •

edited

Loading

mikebrow commented Nov 29, 2017

Random-Liu Nov 29, 2017

mikebrow Nov 29, 2017

Random-Liu Nov 29, 2017

mikebrow Nov 30, 2017

runcom Nov 30, 2017

feiskyer Nov 30, 2017

mikebrow Nov 30, 2017 •

edited

Loading

runcom Nov 30, 2017

mikebrow Nov 30, 2017

mikebrow Dec 1, 2017

Random-Liu Nov 29, 2017 •

edited

Loading

runcom Nov 29, 2017

mikebrow Nov 29, 2017

runcom Nov 30, 2017

Random-Liu Nov 30, 2017

feiskyer Nov 30, 2017

mikebrow Dec 1, 2017 •

edited

Loading

mikebrow commented Dec 1, 2017 •

edited

Loading

mikebrow commented Dec 1, 2017

Random-Liu commented Dec 4, 2017

		@@ -221,6 +221,10 @@ var containerStatusCommand = cli.Command{
		Name: "output, o",
		Usage: "Output format, One of: json\|yaml\|table",

adding verbose debug output for inspect & inspects #206

adding verbose debug output for inspect & inspects #206

Conversation

mikebrow commented Nov 29, 2017 • edited Loading

Random-Liu commented Nov 29, 2017 • edited Loading

mikebrow commented Nov 29, 2017

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

mikebrow Nov 30, 2017 • edited Loading

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

Random-Liu Nov 29, 2017 • edited Loading

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

mikebrow Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

mikebrow commented Dec 1, 2017 • edited Loading

mikebrow commented Dec 1, 2017

Random-Liu commented Dec 4, 2017

mikebrow commented Nov 29, 2017 •

edited

Loading

Random-Liu commented Nov 29, 2017 •

edited

Loading

mikebrow Nov 30, 2017 •

edited

Loading

Random-Liu Nov 29, 2017 •

edited

Loading

mikebrow Dec 1, 2017 •

edited

Loading

mikebrow commented Dec 1, 2017 •

edited

Loading