roachtest: add invalid descriptors post test validation

[herkolategan]

Add a validation check for the invalid descriptors virtual table against the cockroach cluster at the end of each roachtest. Assert that the table crdb_internal.invalid_descriptors is empty.

Resolves: #85330

Release note: None

[cockroach-teamcity]

This change is 

[renatolabs]

Minor comments, LGTM!

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @herkolategan and @smg260)

---

pkg/cmd/roachtest/cluster.go line 1390 at r1 (raw file):

// crdb_internal.check_consistency(true, ”, ”) indicates that any ranges'
// replicas are inconsistent with each other. It uses the first node that
// is up to run the query.

This first sentence no longer applies here now that the logic was extracted to its own function.

---

pkg/cmd/roachtest/test_runner.go line 1078 at r1 (raw file):

		//
		// TODO(testinfra): figure out why this can still get stuck despite the
		// above.

I've often wondered about the comment above: I don't remember ever seeing these calls time out. Perhaps we should remove this comment/TODO. cc @tbg, if you have thoughts (as writer of the comment).

Doesn't have to be on this PR, I'm mostly thinking out loud here.

---

pkg/cmd/roachtest/test_runner.go line 1081 at r1 (raw file):

		db, node := c.ConnectToLiveNode(ctx, t)
		if db != nil {
			defer func() { _ = db.Close() }()

Nit: this reads slightly better as defer db.Close()

---

pkg/cmd/roachtest/test_runner.go line 1082 at r1 (raw file):

		if db != nil {
			defer func() { _ = db.Close() }()
			t.L().Printf("running (fast) validation checks on node %d", node)

I know this comment was just moved, but "fast" is relative. Suggestion: running validation checks on node %d (<10m)

[renatolabs]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @herkolategan and @smg260)

---

pkg/cmd/roachtest/cluster.go line 1390 at r1 (raw file):

Previously, renatolabs (Renato Costa) wrote…

This first sentence no longer applies here now that the logic was extracted to its own function.

Ops, I meant last sentence.

[herkolategan]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @renatolabs, @smg260, and @tbg)

---

pkg/cmd/roachtest/cluster.go line 1390 at r1 (raw file):

Previously, renatolabs (Renato Costa) wrote…

Ops, I meant last sentence.

Done.

---

pkg/cmd/roachtest/test_runner.go line 1078 at r1 (raw file):

Previously, renatolabs (Renato Costa) wrote…

I've often wondered about the comment above: I don't remember ever seeing these calls time out. Perhaps we should remove this comment/TODO. cc @tbg, if you have thoughts (as writer of the comment).

Doesn't have to be on this PR, I'm mostly thinking out loud here.

Done.

---

pkg/cmd/roachtest/test_runner.go line 1082 at r1 (raw file):

Previously, renatolabs (Renato Costa) wrote…

I know this comment was just moved, but "fast" is relative. Suggestion: running validation checks on node %d (<10m)

Done.

[herkolategan]

bors r=renatolabs

[craig]

Build failed:

• Bazel Essential CI (Cockroach)

[herkolategan]

bors retry

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)

[tbg]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

---

pkg/cmd/roachtest/test_runner.go line 1078 at r1 (raw file):

Previously, herkolategan (Herko Lategan) wrote…

Done.

I'm fine removing this. I haven't seen it in a while either (and also in the meantime rewrote the server-side impl of the consistency checks, so they are more likely to be cancelable now), though I also haven't been paying attention.