Replace os.IsTimeout and string matching with errors.As for SSH timeout checks

[guettli]

Summary

pkg/services/baremetal/host/host.go checks for SSH timeouts in three places like this:

case os.IsTimeout(out.Err) || sshclient.IsTimeoutError(out.Err):

• host.go:891 (analyzeSSHErrorRegistering)
• host.go:1939 (analyzeSSHOutputInstallImage)
• host.go:2234 (analyzeSSHOutputProvisioned)

Neither half of that condition is a good check on its own.

os.IsTimeout does not walk %w wraps. It only unwraps *os.PathError, *os.LinkError and *os.SyscallError, and the Go source says so explicitly in os/error.go: "Note that this function is not errors.Is: underlyingError only unwraps the specific error-wrapping types that it historically did". The SSH client wraps its dial error once, in ssh_client.go:642, which is already enough to make os.IsTimeout return false.

sshclient.IsTimeoutError is strings.Contains(err.Error(), "i/o timeout") (ssh_client.go:611). It happens to catch those wrapped errors, because the wrapped message still contains the inner text. So today the string match is what actually does the work, and os.IsTimeout only contributes one case: a bare context.DeadlineExceeded, whose text is "context deadline exceeded" and so does not match the string. That is why ssh_client.go:637-639 returns ctx.Err() unwrapped, with the comment "Return ctx.Err() unwrapped so os.IsTimeout detects it".

Matching on error text is fragile, and the pair only works because of that coincidence.

Suggested change

One structural check covers everything both halves cover today, because context.DeadlineExceeded also satisfies net.Error with Timeout() == true:

// IsTimeoutError checks whether the error is a network or context timeout.
func IsTimeoutError(err error) bool {
	var netErr net.Error
	return errors.As(err, &netErr) && netErr.Timeout()
}

Then the three cases in host.go become case sshclient.IsTimeoutError(out.Err):, and the os import there can probably go. The new version is also nil-safe, while the current one would panic in err.Error() if a caller ever stopped checking for nil first.

Things to decide while doing it

The two code paths want different things from a context deadline, and the follow-up should keep that difference and write it down instead of letting errors.As decide by itself:

• pkg/services/hcloud/server/server.go excludes context.DeadlineExceeded on purpose (see 🌱 Treat an SSH dial timeout as a retry while booting into the rescue system #2232). There it means our own reconcile ran out of time, not that the server is still booting, so it should keep being logged.
• host.go currently treats a context deadline as a timeout, so the bare metal state machine retries. That behavior should stay.

One small behavior change to expect: a wrapped context.DeadlineExceeded currently matches neither half and ends up in the default: branch as "unhandled ssh error". After the change it is recognized as a timeout. That looks like a fix. In practice the SSH client returns ctx.Err() unwrapped anyway.

Once sshclient.IsTimeoutError uses errors.As, the workaround in ssh_client.go:637-639 is no longer needed for detection. Returning the error unwrapped is harmless, but the comment should be updated or removed.

Note on ordering

Wait until #2232 is merged before starting this, to avoid conflicts in the same functions.