cli: add "debug zip" command

[petermattis]

Add "debug zip" command which gathers cluster debug data into a zip
file. Data includes cluster events, node liveness, node status, range
status, node stack traces, log files and SQL schema.

---

This change is 

[petermattis]

I thought there was an issue open for this, but I can't find it right now.

[mberhault]

#12681

[mberhault]

tar seems more fitting, no? is the reason you're picking zip the compressed-by-default?

[petermattis]

No strong reason. Tar would require compression. Nice that zip gives that out of the box.

[mberhault]

maybe add some logging about attempt/success/failure of each item?
eg:

events: OK
liveness: FAIL (error: blah blah)

this would be clearer than failures being logged silently to the corresponding file. We could include that summary in the zip too.

[petermattis]

Errors are logged to the screen. See createError which writes the error both to the archive and to stdout.

[tamird]

Well, the errors do get logger now, though it's a bit scattered

fmt.Printf(" %s: %s\n", name, e)

---

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks pending.

---

pkg/cli/zip.go, line 28 at r1 (raw file):

	"strings"

	"github.com/cockroachdb/cockroach/pkg/server/serverpb"

separate group for this

---

pkg/cli/zip.go, line 66 at r1 (raw file):

func (z *zipper) create(name string) (io.Writer, error) {
	fmt.Printf("  %s\n", name)

did you mean to leave this?

---

pkg/cli/zip.go, line 88 at r1 (raw file):

func (z *zipper) createError(name string, e error) error {
	fmt.Printf("  %s: %s\n", name, e)

ditto

---

pkg/cli/zip.go, line 93 at r1 (raw file):

		return err
	}
	fmt.Fprintf(w, "%s\n", e.Error())

ditto

---

pkg/cli/zip.go, line 107 at r1 (raw file):

	if len(args) != 1 {
		return errors.New("output file argument is required")

this will be confusing if more than one argument is specified.

---

pkg/cli/zip.go, line 153 at r1 (raw file):

	}

	nodes, err := status.Nodes(ctx, &serverpb.NodesRequest{})

use the if := ...; form here that you used above (and almost everywhere below)?

---

pkg/cli/zip.go, line 182 at r1 (raw file):

			}

			logs, err := status.LogFilesList(ctx, &serverpb.LogFilesListRequest{NodeId: id})

ditto

---

pkg/cli/zip.go, line 210 at r1 (raw file):

			}

			ranges, err := status.Ranges(ctx, &serverpb.RangesRequest{NodeId: id})

ditto

---

pkg/cli/zip.go, line 230 at r1 (raw file):

	}

	databases, err := admin.Databases(ctx, &serverpb.DatabasesRequest{})

ditto

---

Comments from Reviewable

[petermattis]

Review status: 2 of 3 files reviewed at latest revision, 9 unresolved discussions.

---

pkg/cli/zip.go, line 28 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

separate group for this

Done, though gofmt did that formatting and I don't think we should fight it.

---

pkg/cli/zip.go, line 66 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

did you mean to leave this?

Yes. This shows progress during creation of the zip.

---

pkg/cli/zip.go, line 88 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

ditto

Yes.

---

pkg/cli/zip.go, line 93 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

ditto

Yes, this includes the error in the zip file so we'll be able to see if there was a problem retrieving some piece of data.

---

pkg/cli/zip.go, line 107 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

this will be confusing if more than one argument is specified.

Do you have a suggestion? This was cribbed from runDebugSSTables.

---

pkg/cli/zip.go, line 153 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

use the if := ...; form here that you used above (and almost everywhere below)?

Done.

---

pkg/cli/zip.go, line 182 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

ditto

Done.

---

pkg/cli/zip.go, line 210 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

ditto

Done.

---

pkg/cli/zip.go, line 230 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

ditto

Done.

---

Comments from Reviewable

[tamird]

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks pending.

---

pkg/cli/zip.go, line 28 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Done, though gofmt did that formatting and I don't think we should fight it.

There's pretty consistent pattern of putting cockroachdb imports at the bottom, but OK. Sadly this change you made is the opposite of the convention.

---

pkg/cli/zip.go, line 93 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Yes, this includes the error in the zip file so we'll be able to see if there was a problem retrieving some piece of data.

Ack. Error() is superfluous.

---

pkg/cli/zip.go, line 107 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Do you have a suggestion? This was cribbed from runDebugSSTables.

Sure. "exactly one argument is required".

---

Comments from Reviewable

[petermattis]

Review status: 2 of 3 files reviewed at latest revision, 3 unresolved discussions.

---

pkg/cli/zip.go, line 28 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

There's pretty consistent pattern of putting cockroachdb imports at the bottom, but OK. Sadly this change you made is the opposite of the convention.

Switched. I'm curious how often you look at the imports. I almost never do. The ordering seems unimportant.

---

pkg/cli/zip.go, line 93 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

Ack. Error() is superfluous.

Done.

---

pkg/cli/zip.go, line 107 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

Sure. "exactly one argument is required".

Done.

---

Comments from Reviewable

[tamird]

---

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[spencerkimball]

Neat.