Dump metadata as a subcommand for virtletctl

[jellonek]

This PR provides a base for multi command binary virtletctl with beginning of first command, dump of metadata store from all instances of virtlet containers on the cluster.

---

This change is 

[ivan4th]

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.

---

pkg/tools/common.go, line 67 at r1 (raw file):

	stdin io.Reader, stdout, stderr io.Writer,
	command ...string,
) (int, error) {

ExecInContainer maybe?

---

pkg/tools/dumpmetadata.go, line 63 at r1 (raw file):

		stdin := bytes.NewBufferString("")

		fmt.Printf("pod: %s\n", pod.Name)

"Virtlet pod name:"

---

pkg/tools/dumpmetadata.go, line 82 at r1 (raw file):

			continue
		}
		f, err := ioutil.TempFile("/tmp", "virtlet-")

maybe it would be better to dump it to stdout, like all kubectl commands behave for example? That would provide much more flexibility

---

pkg/tools/subcommands.go, line 55 at r1 (raw file):

func ParseFlags(command string) error {
	flag.StringVar(&kubeconfig, "kubeconfig", "~/.kube/config", "absolute path to the kubeconfig file")
	flag.StringVar(&master, "master", "http://127.0.0.1:8080", "master url")

The default value must be blank and the description needs to be something like "master url (overrides the value from kubeconfig)". Otherwise the apiserver url from ~/.kube/config is never used

---

Comments from Reviewable

[ivan4th]

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.

---

pkg/tools/dumpmetadata.go, line 82 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

maybe it would be better to dump it to stdout, like all kubectl commands behave for example? That would provide much more flexibility

Hmm oops it saves binary metadata. I thought there may be some kind of human-readable dump of it...

---

Comments from Reviewable

[jellonek]

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.

---

pkg/tools/common.go, line 67 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

ExecInContainer maybe?

Yep, better name.

---

pkg/tools/dumpmetadata.go, line 63 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

"Virtlet pod name:"

OK.

---

pkg/tools/dumpmetadata.go, line 82 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

Hmm oops it saves binary metadata. I thought there may be some kind of human-readable dump of it...

Nope. it's WIP for this moment. Finally data will be read from these files and dumped in human readable form to stdout.

---

pkg/tools/subcommands.go, line 55 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

The default value must be blank and the description needs to be something like "master url (overrides the value from kubeconfig)". Otherwise the apiserver url from ~/.kube/config is never used

Will do that!

---

Comments from Reviewable

[ivan4th]

I'd suggest moving the dumper to pkg/metadata and adding a simple test for it (compare output for a prefilled db to some expected output), given that there are problems in the code.

---

Review status: 0 of 5 files reviewed at latest revision, 6 unresolved discussions.

---

pkg/tools/dumpmetadata.go, line 114 at r2 (raw file):

}

func dumpMetadata(fname string) error {

I'd make a Dumper struct that should contain the output stream and indent level. Dumper.indent(N) should return a new dumper with indent level increased. Make all print... funcs methods of the dumper

---

pkg/tools/dumpmetadata.go, line 141 at r2 (raw file):

	for image := range images {
		printlnIndented(2, image)

This is probably a bug? It'll just print numbers starting from 0.

---

pkg/tools/dumpmetadata.go, line 149 at r2 (raw file):

func dumpSandbox(podid string, sandbox *metadata.PodSandboxInfo, s metadata.MetadataStore) error {
	printlnIndented(2, "Sandbox id: %s", podid)
	printlnIndented(0, spew.Sdump(sandbox))

This will probably look rather mangled. First of all, spew output should be split into separate lines via strings.Split and then these lines displayed with indent added. Second, spew sdump looks quite messy by default, I'd suggest cleaning it up a little bit, like this https://github.com/Mirantis/criproxy/blob/b6769d9d6fc4b2d3b9e191acce425e57c0a3d0e0/pkg/proxy/proxy.go#L519-L526 (although we probably need to have smth better than vanilla go-spew at some point)

---

pkg/tools/dumpmetadata.go, line 174 at r2 (raw file):

func printlnIndented(level int, format string, data ...interface{}) {
	printIndentedWith(" ", 2*level, format+"\n", data...)

maybe printIndented(level, format+"\n", data...) would be better (so there's no unneeded repetition of " ", 2*level with an added risk of inconsistency if we want to change the indent at some point)? Also I'd probably move " " to a const

---

Comments from Reviewable

[jellonek]

Review status: 0 of 5 files reviewed at latest revision, 6 unresolved discussions.

---

pkg/tools/dumpmetadata.go, line 114 at r2 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

I'd make a Dumper struct that should contain the output stream and indent level. Dumper.indent(N) should return a new dumper with indent level increased. Make all print... funcs methods of the dumper

Good idea but would like to do that as a followup.

---

pkg/tools/dumpmetadata.go, line 141 at r2 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

This is probably a bug? It'll just print numbers starting from 0.

No, it's map so that iterates over the keys which are string. Give it a try ;)

---

pkg/tools/dumpmetadata.go, line 149 at r2 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

This will probably look rather mangled. First of all, spew output should be split into separate lines via strings.Split and then these lines displayed with indent added. Second, spew sdump looks quite messy by default, I'd suggest cleaning it up a little bit, like this https://github.com/Mirantis/criproxy/blob/b6769d9d6fc4b2d3b9e191acce425e57c0a3d0e0/pkg/proxy/proxy.go#L519-L526 (although we probably need to have smth better than vanilla go-spew at some point)

It was used to have a fast effect. Level 0 is used simply to not add any indentation for first line.
I'm not sure if we should waste more time on that, it's only a debug util with potential quite small use.

---

pkg/tools/dumpmetadata.go, line 174 at r2 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

maybe printIndented(level, format+"\n", data...) would be better (so there's no unneeded repetition of " ", 2*level with an added risk of inconsistency if we want to change the indent at some point)? Also I'd probably move " " to a const

Good idea. Done.

---

Comments from Reviewable

[jellonek]

Review status: 0 of 5 files reviewed at latest revision, 6 unresolved discussions.

---

pkg/tools/dumpmetadata.go, line 141 at r2 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

No, it's map so that iterates over the keys which are string. Give it a try ;)

Example output:

  Images:
    cirros@sha256:0615af1caf345e605e27d5419f8d4c6d087b3b41ff4b979aec094ce0399f04ab

---

Comments from Reviewable

[jellonek]

Review status: 0 of 5 files reviewed at latest revision, 6 unresolved discussions.

---

pkg/tools/subcommands.go, line 55 at r1 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

Will do that!

Done.

---

Comments from Reviewable

[ivan4th]

Reviewed 2 of 5 files at r1, 1 of 2 files at r2.
Review status: 3 of 5 files reviewed at latest revision, 3 unresolved discussions.

---

pkg/tools/dumpmetadata.go, line 114 at r2 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

Good idea but would like to do that as a followup.

Ok.

---

pkg/tools/dumpmetadata.go, line 141 at r2 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

Example output:

  Images:
    cirros@sha256:0615af1caf345e605e27d5419f8d4c6d087b3b41ff4b979aec094ce0399f04ab

Sorry, dunno why I thought it was a slice. My bad.

---

pkg/tools/dumpmetadata.go, line 149 at r2 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

It was used to have a fast effect. Level 0 is used simply to not add any indentation for first line.
I'm not sure if we should waste more time on that, it's only a debug util with potential quite small use.

Ok let's keep it this way for now.

---

Comments from Reviewable

[ivan4th]

---

Review status: 3 of 5 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[lukaszo]

---

Review status: 3 of 5 files reviewed at latest revision, 1 unresolved discussion.

---

pkg/tools/common.go, line 59 at r3 (raw file):

}

// ExecCommandOnContainer given a pod, container, namespace and command

ExecInContainer

---

Comments from Reviewable