fix(testing): check if iperf server is up/reachable #482
Conversation
edipascale
force-pushed
the
ema/fix-310
branch
2 times, most recently
from
2acb6fd to
496b452
Compare
pkg/hhfab/testing.go
Outdated
| out, err := fromSSH.RunContext(ctx, cmd) | ||
| if err != nil { | ||
| return fmt.Errorf("running iperf client: %w: %s", err, string(out)) | ||
| maxRetries := 10 |
There was a problem hiding this comment.
10 retries feel like really a lot... Aren't we going to hide some other issues this way?
There was a problem hiding this comment.
I'm not too concerned about hiding other issues as we will only retry on an ssh error (or, to be more precise, an error with the "ssh:" string in it), but I can lower it to something more reasonable. Does 3 sound better?
There was a problem hiding this comment.
Correct me if I'm wrong but the retry logic wouldn't be masking Fabric issues as SSH goes through a separate network. So the retries are addressing the reliability in the test infrastructure itself
We should investigate anyhow why the server isn't ready when the client connects
There was a problem hiding this comment.
If the retries work, I don't think there's anything to investigate - we are giving the server 1 second of head start, but under load in a virtualized environment there is no guarantee that it will be enough, that's why I thought retries would be sensible
Fixes: #310 Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>
and retry up to 10 times if not.
Fixes: #310