roachtest: surface cluster config. params when filing an issue

[srosenberg]

A new failed roachtest will result in creating a fresh github issue. (If it's a repeat failure, then a new comment is added.)
The issue description doesn't capture essential information pertaining to the cluster configuration. This could lead to wasted (developer) cycles; e.g., see #68976 (comment)
Some tests randomly enable zfs, encryption, etc.–this context should also be propagated to the issue.

Each roachtest is associated with a TestSpec [1] which contains a ClusterSpec [2]. The latter can be used to extract the cloud provider, instance type, node count, etc., including whether or not zfs was used. (Determining whether or not encryption is enabled is a separate issue [3]; it needs some refactoring to lift it into ClusterSpec.)
The test runner's maybePostGithubIssue already receives TestSpec; the rest is a matter of choosing a compact while descriptive format to capture the additional (cluster) configuration parameters in order to make it easier for the reader.

[1] https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/test/test_interface.go#L20
[2] https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/spec/cluster_spec.go#L41
[3] #79265 (comment)

Jira issue: CRDB-15509

[blathers-crl]

Hi @srosenberg, please add a C-ategory label to your issue. Check out the label system docs.

While you're here, please consider adding an A- label to help keep our repository tidy.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[tbg]

Just a heads up, the cloud has been a part of the issue for a long time:

However, the "message" is often a lengthy log dump and gets truncated, so the cloud is lost:

A goal here should be to avoid "jamming" this metadata into the "message" code block. Instead, we should pass in a tags map[string]string (or something like that) which features prominently above the code block.

We basically have this already:

cockroach/pkg/cmd/internal/issues/issues.go

Lines 217 to 218 in 5e7690d

	// GOFLAGS=-foo TAGS=-race etc.
	Parameters []string

So maybe the task here is to put more parameters into roachtest issues, and to adjust the template to print these in the right place. (They are currently tucked away under the "Help" collapsible, so they're rarely seen).

While we touch the issue template, we should also fix #80852, which is a big paper cut that we pay a tax for every day.

[renatolabs]

Some thoughts on this:

As @tbg mentioned, the cloud and branch names are already displayed in the message block, although that information is often lost when the message is truncated. It seems to me that even if the message was not truncated, we could use some more easily-accessible cluster data to help debug issues.

One possible approach here would be to just add the cluster spec to the Parameters field that @tbg referenced. We wouldn't even have to change the formatting necessarily, and it would look something like this:

Some pros/cons:

• Pro: no changes to existing data structures and formatting code.
• Con: Parameters is a somewhat abstract term (currently it seems tied to just GOFLAGS and TAGS, likely because this was the immediate need when the field was introduced). It's not obvious that the cluster spec would be a "parameter". And a lot of things could be considered "parameters".
• Con: parameters are not grouped, so we'd have infrastructure parameters (cloud name) alongside build flags.

Another option:

Here, "Cluster" gets its own collapsed area. I also chose to use a table here to experiment with that formatting approach, which I find more pleasant than the current bulleted list of parameters.

• Pro: cluster data is all in one place, and devs can see the configuration the test ran in in a quick glance.
• Con: introduction of a new collapsible area in the issue report

In both cases, the actual list of cluster parameters included in the report would include other things (like ZFS, encryption, etc). The screenshots above are only to illustrate the aproaches.

I'm slightly more inclined towards the second option.

---

There are other options, of course, and I'm interested to hear other ways people would prefer to see this formatted.

I'm also aware that not everything in ClusterSpec necessarily needs to be included in the report as they are declared in the test itself and never changed; however, I think including them anyway is good as it reduces friction.

Thoughts?

[tbg]

I like option two as well, though I think including all parameters is too much; we typically don't care about most of them.

Also, worth considering rotated tables:

Machine type	CPUs	HDD	Foo	Bar
asdfasdasfasf	4	500	asfasf	lkjlkjj

I also still don't dislike Option 1. I think with a bit of care it would get the job done just fine without having to put in too much additional work. For example, what if we list the params (in sorted order) like below. I think (a refined version of) that would be my preference, since it just gets the job done (?).

---

roachtest.tpccbench/nodes=9/cpu=4/multi-region failed with artifacts on master @ 16c484aa84d3718bfc82557f8e935ab78e6753b6

Parameters: TAGS=-endtoendenv GOFLAGS=-somegoflags ROACHTEST_Cloud=gce ROACHTEST_InstanceType=n1-standard-2 ROACHTEST_NodeCount=5 ROACHTEST_CPUs=2 ROACHTEST_SSDs=5 ROACHTEST_RAID0=true ROACHTEST_VolumeSize=100GB

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /artifacts/tpccbench/nodes=9/cpu=4/multi-region/run_1
	monitor.go:127,tpcc.go:1096,tpcc.go:931,test_runner.go:876: monitor failure: monitor task failed: Non-zero exit code: 1
blah blah

Help

See: roachtest README

See: How To Investigate (internal)

Same failure on other branches

• roachtest: tpccbench/nodes=9/cpu=4/multi-region failed #80856 roachtest: tpccbench/nodes=9/cpu=4/multi-region failed [C-test-failure O-roachtest O-robot T-bulkio T-kv branch-release-21.1]

This test on roachdash | Improve this report!