cockroachdb / cockroachdb/cockroach
roachtest: re-evaluate SSH flake detection and prevention
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
As of a few months ago, we started using ssh's exit code to identify "SSH flakes": failures that we believe are due to SSH connection as opposed to an actual error in the command itself. As time went on, I believe we started to see how this mechanism is less than ideal, at least the way it's implemented today: there's no way to differentiate a 255 exit code coming from `ssh` itself (arguably an SSH flake, see below) from one where the command being run returned 255 for its own reasons (which should translate to a legitimate test failure).
The following documents a few recent instances where test failures were marked as SSH flakes when they should have been legitimate test failures:
**Jepsen test failures**
We recently had to perform an update in our Jepsen fork [1] to fix something that caused the failure of every Jepsen roachtest. The Jepsen failures were marked as SSH flakes [2] because Jepsen will exit with code 255 when an unhandled exception is caught [3].
**ruby-install failures**
We have seen `ruby-install` failures being marked as SSH flakes for months as well [4]. Turns out `ruby-install` will exit with code `-1` when it fails [5], which translates to an actual exit code of 255.
**GetInternalIP failures**
`GetInternalIP` will return the output of `hostname --all-ip-addresses`. However, `hostname` itself will return 255 in some (maybe all?) error conditions (e.g., try `hostname --invalid; echo $?` on Linux).
More generally, there are a lot of commands that return 255 in error conditions [6], and in those cases we do want the test to fail.
**How about when `ssh` itself returns 255 -- that's gotta be an SSH flake, right?**
I'd say depends on how we define "flake". In [7], for example, we see that an authentication failure due to misconfigured SSH keys led to a 255 exit code and therefore (unwanted) retries. In this case, it's not a "flake" (in the sense that the command will fail deterministically), but it _would_ be a failure that, were it to happen on a test, we wouldn't want to notify teams about.
As another example, take this `GetInternalIP` failure [8]. If we look at the associated SSH logs, we see that the actual reason for the failure is:
```
debug1: kex_exchange_identification: banner line 0: Exceeded MaxStartups
kex_exchange_identification: Connection closed by remote host
```
I won't get too much into the details of the semantics of `MaxStartups`; it's documented in [9]. Suffice it to say that it can cause ssh to randomly drop connections when there are too many unauthenticated SSH connections. It has previously been a source of flakes and back in 2019 (#37001) we bumped that limit. However, no amount of bumping can catch up with port scanners. Indeed, shortly before the failure in [8], we see a port scanner in action:
```
sshd[18234]: Invalid user ts3 from 171.244.62.120 port 60474
sshd[18235]: Invalid user minecraft from 171.244.62.120 port 60504
sshd[18232]: Invalid user pi from 171.244.62.120 port 60544
sshd[18227]: Invalid user testuser from 171.244.62.120 port 60340
sshd[18242]: Invalid user deploy from 171.244.62.120 port 60624
sshd[18241]: Invalid user test from 171.244.62.120 port 60566
sshd[18244]: Invalid user ansible from 171.244.62.120 port 60594
sshd[18239]: Invalid user bob from 171.244.62.120 port 60450
sshd[18238]: Invalid user test from 171.244.62.120 port 60664
sshd[18257]: Invalid user test from 171.244.62.120 port 60604
sshd[18258]: Invalid user testftp from 171.244.62.120 port 60546
sshd[18259]: Invalid user raspberry from 171.244.62.120 port 60432
sshd[18260]: Invalid user test from 171.244.62.120 port 60390
sshd[18262]: Invalid user hadoop from 171.244.62.120 port 60470
sshd[18261]: Invalid user ubnt from 171.244.62.120 port 60540
```
Which brings us to _prevention_; we shouldn't let random port scanners cause a test to fail.
This rambly issue can be summarized in a few points:
* checking exit status from `ssh` to determine whether something was an SSH flake is too brittle. We want to check the exit status of the command being run at the very least.
* ideally, we would also not retry commands that fail due to authentication failures
* we can probably do a better job at preventing SSH flakes, especially in cases where a port scanner causes sshd to drop connections. We can investigate to what extent we can re-enable `sshguard` [10], or a similar solution. Even better, the more long term approach would be to get rid of public IPs entirely, as proposed in #77644.
[1] https://github.com/cockroachdb/jepsen/pull/34
[2] Example: https://github.com/cockroachdb/cockroach/issues/90695#issuecomment-1490493958
[3] https://github.com/cockroachdb/jepsen/blob/3d7c345d6958f067edb097f9b82ab8e7a4a752c7/jepsen/src/jepsen/cli.clj#L278-L280
[4] Example: https://github.com/cockroachdb/cockroach/issues/99828#issue-1644364645
[5] https://github.com/postmodern/ruby-install/blob/f59dd9cb31a073c2e2c94eb679a348f7ec3ca1b1/share/ruby-install/logging.sh#L71-L75
[6] Others have expressed their confusion on the topic as well, see for example.
[7] https://cockroachlabs.slack.com/archives/CJ0H8Q97C/p1679342613439679 (internal)
[8] https://github.com/cockroachdb/cockroach/issues/99828#issuecomment-1493005569
[9] https://man7.org/linux/man-pages/man5/sshd_config.5.html
[10] https://github.com/cockroachdb/cockroach/blob/master/pkg/roachprod/vm/gce/utils.go#L134-L136
Jira issue: CRDB-26665
Contributor guide
Assessment
This issue has not been assessed yet.