roachtest: better failure message for c2c roachtests

[lidorcarmel]

In some failures cases the c2c roachtests may try to cutover even though the replication did not start successfully. This patch makes sure we fail with a more clear error in those cases.

Epic: none
Fixes: #107935

Release note: None

[cockroach-teamcity]

This change is 

[msbutler]

theres a bug in the roachtest. could this PR instead just return the error properly?

cockroach/pkg/cmd/roachtest/tests/cluster_to_cluster.go

Line 916 in dcaa348

return nil

[lidorcarmel]

theres a bug in the roachtest. could this PR instead just return the error properly?

I agree that it's good to fail that monitor but how will that help here? we'll still be in main(), right?

also, probably the right solution here is to implement this todo:

cockroach/pkg/cmd/roachtest/roachtestutil/health_checker.go

Line 76 in c5b6392

// TODO(tschottdorf): actually let this fail the test instead of logging complaints.

[msbutler]

Lol nice find with that todo.

My cursory read of the code is that if monitor.Go() returns with an error, the whole monitor will shut down.

And to be clear, i think adding a require around retainedTime<cutoverTIme seems fine to me.

[lidorcarmel]

My cursory read of the code is that if monitor.Go() returns with an error, the whole monitor will shut down.

right, but we defer the wait() to be done after main() completes: code

And to be clear, i think adding a require around retainedTime<cutoverTIme seems fine to me.

so... lgtm please?

BTW maybe I should have clarified better that this pr is not a fix, it is just improving the error message when the test fails.

[msbutler]

i see. We can clean up this goroutine management in a seperate pr

[lidorcarmel]

thanks!

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)