cli: debug zip with timeout, added dump for crdb_internal.gossip_*

[windchan7]

Now made command debug zip continue past errors with a timeout
(based on peter's timeout commit).

Also dumped information in crdb_internal.gossip_nodes and gossip_liveness
to the output file.

Fixes #23954.

Release note: None

[cockroach-teamcity]

This change is 

[tbg]

LGTM with some comments. As for testing this, I think to merge this PR what you should do is to run ./cockroach debug zip --host definitely.not.a.real.host myfile.zip. You should get a bunch of warnings, but the zip file should be created. I also filed #24473 for a "real" test.

---

Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

---

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

	defer z.close()

	const timeout = 10 * time.Second

Just make this

ctx := func(baseCtx) context.Context {
  timeout := 10 * time.Second
  if cliCtx.cmdTimeout != 0 {
    timeout = cliCtx.cmdTimeout
  }
  ctx, _ = context.WithTimeout(baseCtx, timeout)
  return ctx
}

// ...
if events, err := admin.Events(ctx(), &serverpb.EventsRequest{})

and remove zipTimeoutContext.

It's usually bad practice a) to not call the cancel closures and b) to create a context out of thin air, but I think here both are justifiable.

---

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

	{
		sqlConn, err := getPasswordAndMakeSQLClient("cockroach sql")

Create this early in this method. If it returns an error, the user likely didn't specify the correct flags to set up the connection. (Are the flags even available in this client command)? Log the error as a warning:

log.Warningf(ctx, "unable to open a SQL session. Debug information will be incomplete: %s", err)

---

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

		if err := dumpGossipData(z, sqlConn, queryLiveness, gossipLName); err != nil {
			return err

Log these as warnings.

---

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

		}
		if err := dumpGossipData(z, sqlConn, queryNodes, gossipNName); err != nil {
			return err

Ditto.

---

Comments from Reviewable

[tbg]

Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

---

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

		defer cancel()
		if events, err := admin.Events(ctx, &serverpb.EventsRequest{}); err != nil {
			if err := z.createError(eventsName, err); err != nil {

I think it makes sense to log all of these as log.Warning as well. Otherwise if the user specifies an incorrect host, they'll get a worthless zip file. Better to have some noise in that case.

---

Comments from Reviewable

[tbg]

, thanks @windchan7!

---

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[windchan7]

bors r+

[craig]

🔒 Permission denied

Existing reviewers: click here to make windchan7 a reviewer

[windchan7]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)