Is your feature request related to a problem? Please describe.

There's a steady trickle of nightly failures across teams that are caused by flaky SSH connections (which generally are thought to be caused by cloud networking hiccups and the like). For example, #82123 (comment)

This is a drag on the eng teams, who would rather only see more meaningful failures.

Describe the solution you'd like

We already have a way to post failures to an infra-failure test issue instead when a cluster creation operation fails:

https://github.com/cockroachdb/cockroach/pull/73706/files#diff-1e3ec1fc5bade2df4b8681c44959be2a390f703839f30a979da98f963769c9fcR589-R592

We should use a similar mechanism to redirect (to test-eng) failures in which we see an indication of failed SSH connections, to shield the eng teams from such noise.

Ideally we do so in a way that isn't too hard-coded to SSH failures, so we can reuse it for other failures that are similarly "not the owning team's fault". For example, an apt-get flake should get similar special casing, though this may not necessarily be done as part of this issue. A good mechanism of action is to rely on cockroachdb/errors to return semantics with the error.

For example, we could have a marker error ErrSSHProblem, and could have code around here

that does this:

if isExitCode255Err(err) {
  err = errors.Mark(err, something.ErrSSHProblem)
}

and then the issue posting code could do

if errors.Is(err, something.ErrSSHProblem) {
  // Override what issue to post
}

in maybePostGithubIssue, whose main caller is here:

One minor change that needs to happen here is that the issue poster only sees a string, but it should see the full error. In particular, Fatalf should internally translate to t.mu.err = errors.Errorf(format, args...) instead of what these two methods do now:

and we should stringify only when actually posting the issue. This would make for a good standalone, first, PR.

Describe alternatives you've considered

Retries might be able to alleviate some of the pain, but not all operations are idempotent and we'd have to do full passes and folks would have to "do it right" for all new code. I consider that an orthogonal approach and we might want to do both.

Additional context

the roachprod logic to test SSH flakes is here:

ClassifyCmdError basically checks if err is an *exec.ExitError with code 255, which is then presumed to come from ssh. This assumption isn't right in entirety, if the remote command exits with 255 then we get 255 as well:

tobias@gceworker-tobias:~/go/src/github.com/cockroachdb/cockroach$ roachprod ssh $c:4 -- exit 12
Error: COMMAND_PROBLEM: exit status 12
(1) COMMAND_PROBLEM
Wraps: (2) Node 4. Command with error:
  | ```
  | exit 12
  | ```
Wraps: (3) exit status 12
Error types: (1) errors.Cmd (2) *hintdetail.withDetail (3) *exec.ExitError
tobias@gceworker-tobias:~/go/src/github.com/cockroachdb/cockroach$ roachprod ssh $c:4 -- exit 255
Error: SSH_PROBLEM: exit status 255
(1) SSH_PROBLEM
Wraps: (2) Node 4. Command with error:
  | ```
  | exit 255
  | ```
Wraps: (3) exit status 255
Error types: (1) errors.SSH (2) *hintdetail.withDetail (3) *exec.ExitError

but apart from that it's mostly right (from man ssh):

EXIT STATUS
     ssh exits with the exit status of the remote command or with 255 if an
     error occurred.

Btw, we also do this thing where we try to get the remote exit status from a string that is printed at the end:

I'm not sure why this is needed, as the roachprod ssh $c:4 -- exit N experiment shows that we get the exit code directly from the returned error and this should be reliable.

Jira issue: CRDB-16353