roachtest: report health status during teardown

[smg260]

roachtest: report health status of all nodes in teardown

In #102622, we changed how to determine if a node was suitable to connect to for post test validations, to first check a node's health?ready=1 endpoint.

This extends that PR by extracting the http://node:port/health?ready=1 functionality, and using it to report on each node during teardown by logging the status and/or body or error. Finding a live node piggy backs of this information to create a connection to any node with 200 status.

This could be useful to add to test_interface

Epic: none
Release note: none

[cockroach-teamcity]

This change is 

[herkolategan]

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

---

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

		var validationNode int
		for _, s := range statuses {
			if s.Err != nil {

Nit: use of a switch or two continues might be cleaner / more go idiomatic like.

---

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

		// not seem to be enough.
		//
		// TODO(testinfra): figure out why this can still get stuck despite the

Nit: Should we update this comment or is the issue mentioned still likely to happen (leave the comment for future ref)?

[smg260]

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

---

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

Previously, herkolategan (Herko Lategan) wrote…

Nit: use of a switch or two continues might be cleaner / more go idiomatic like.

Thanks. Updated to be more in line with style guide

---

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

Previously, herkolategan (Herko Lategan) wrote…

Nit: Should we update this comment or is the issue mentioned still likely to happen (leave the comment for future ref)?

It is possible that the changes in this PR help, but I'd prefer to keep the comment there for now.

[smg260]

pkg/cmd/roachtest/cluster.go line 1427 at r2 (raw file):

Previously, renatolabs (Renato Costa) wrote…

Can't this entire block just be results[node-1] = getStatus(ctx, node)? The main blocking call in that function is httputil.Get which gets passed the same context.

If there is a reason for not trusting context cancelation, we should document it.

Yes it can, thanks. An early version used http.Get which did not accept a context.

[renatolabs]

Might be good to run a few roachtests on this branch before merging.

[smg260]

TC job for this branch running a subset of roachtests

TC result: Tests passed: 45, ignored: 6; number of tests 51

[smg260]

TFTR

bors r=herkolategan,renatolabs

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)