Add retries to SSH iperf on server #342
Conversation
pkg/hhfab/testing.go
Outdated
| out, err := toSSH.RunContext(ctx, fmt.Sprintf("toolbox -q timeout -v %d iperf3 -s -1", opts.IPerfsSeconds+25)) | ||
| if err != nil { | ||
| return fmt.Errorf("running iperf server: %w: %s", err, string(out)) | ||
| maxRetries := 3 |
There was a problem hiding this comment.
the context.WithTimeout is built around iperfsseconds so there is no "time" to retry and if we're restarting the server we most probably need to restart the client as well. It maybe makes sense to retry the whole checkIPerf function
pau-hedgehog
force-pushed
the
iperf_ssh_retries
branch
2 times, most recently
from
70278c4 to
7877d91
Compare
pkg/hhfab/testing.go
Outdated
| if err := iperfs.Acquire(ctx, 1); err != nil { | ||
| return fmt.Errorf("acquiring iperf semaphore: %w", err) | ||
| } | ||
| defer iperfs.Release(1) |
There was a problem hiding this comment.
Defer in the loop will be called when the parent function containing the loop finished, not when one iteration of the loop is finished so it'll stuck forever after a first attempt
pau-hedgehog
force-pushed
the
iperf_ssh_retries
branch
from
7877d91 to
c090a91
Compare
Extend show tech logs gathered on server VMs Fixes #310 Signed-off-by: Pau Capdevila <[email protected]>
Add iperf server readiness check Signed-off-by: Pau Capdevila <[email protected]>
Signed-off-by: Pau Capdevila <[email protected]>
pau-hedgehog
force-pushed
the
iperf_ssh_retries
branch
from
ade7be7 to
19b6815
Compare
|
|
||
| slog.Debug("Running iperf", "from", from, "to", to) | ||
| g.Go(func() error { |
There was a problem hiding this comment.
There is something I don't understand... If I read the docs on golang's errgroup correctly, each of these functions will be started immediately in a separate goroutine (as we are not setting any limit on the errgroup), but the operations should actually be sequential, or at least partly so - i.e. starting the server and checking for it in the client could be done in parallel, although I'm not quite sure what the benefits are, but you definitely do not want to start the actual client until the server is ready, especially since that operation does not have a retry. Did I misinterpret something here?
Extend show tech logs gathered on server VMs
Fixes #310