Merge pull request #83 from imup-io/phanyzewski/global-seed-windows

fix: do not retry failed speed test, use deprecated random seeder due to issue on windows

config/config.go

@@ -5,13 +5,15 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"math/rand"
 	"net"
 	"net/http"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/imup-io/client/util"
 	log "golang.org/x/exp/slog"
@@ -21,6 +23,7 @@ import (
 var ImUpAPIHost = "https://api.imup.io"
 
 var (
+	seedRandom sync.Once
 	setupFlags sync.Once
 
 	allowlistedIPs               *string
@@ -162,6 +165,9 @@ func New() (Reloadable, error) {
 	defer mu.Unlock()
 	// do not instantiate a new copy of config, use the package level global
 	cfg = &config{}
+	seedRandom.Do(func() {
+		rand.Seed(time.Now().UTC().UnixNano())
+	})
 
 	setupFlags.Do(func() {
 		allowlistedIPs = flag.String("allowlisted-ips", "", "comma separated list of CIDR strings to match against host IP that determines whether speed and connectivity testing will be run, default is allow all")

helpers.go

@@ -9,7 +9,6 @@ import (
 
 // https://github.com/m-lab/ndt-server/blob/master/spec/ndt7-protocol.md#requirements-for-non-interactive-clients
 func sleepTime() time.Duration {
-	rand := rand.New(rand.NewSource(int64(time.Now().Nanosecond())))
 	t := rand.ExpFloat64() * 21600
 	if t < 2160 {
 		t = 2160

http_client.go

@@ -27,9 +27,6 @@ func exactJitterBackoff(min, max time.Duration, attemptNum int, resp *nethttp.Re
 		return min
 	}
 
-	// Seed rand; doing this every time is fine
-	rand := rand.New(rand.NewSource(int64(time.Now().Nanosecond())))
-
 	// Pick a random number that lies somewhere between the min and max and
 	// multiply by the attemptNum. attemptNum starts at zero so we always
 	// increment here. We first get a random percentage, then apply that to the

speedtesting/speedtest.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"math/rand"
 	"net/url"
 	"runtime"
 	"sync"
@@ -47,7 +46,7 @@ type Options struct {
 	// if set takes precedence over ndt7 locate API as well as the serviceURL
 	Server string
 
-	// is set takes precendece over ndt7 locate API
+	// if set takes precedence over ndt7 locate API
 	ServiceURL *url.URL
 }
 
@@ -57,7 +56,8 @@ func Run(ctx context.Context, opts Options) (*SpeedTestResult, error) {
 	defer mu.Unlock()
 
 	startTime := time.Now().UnixNano()
-	result, err := opts.speedTest(ctx, 0)
+	// TODO: fix error handling for windows speed testing
+	result, err := RunSpeedTest(ctx, &opts)
 	endTime := time.Now().UnixNano()
 
 	if err != nil && !errors.Is(err, context.Canceled) {
@@ -73,32 +73,3 @@ func Run(ctx context.Context, opts Options) (*SpeedTestResult, error) {
 
 	return result, nil
 }
-
-// speed test recursively attempts to get a speed test result utilizing the ndt7 back-off spec and a max retry
-func (opts *Options) speedTest(ctx context.Context, retries int) (*SpeedTestResult, error) {
-	s, err := RunSpeedTest(ctx, opts)
-	if err != nil && !errors.Is(err, context.Canceled) && retries < 10 {
-		retries += 1
-		// https://github.com/m-lab/ndt-server/blob/master/spec/ndt7-protocol.md#requirements-for-non-interactive-clients
-		for mean := 60.0; mean <= 960.0; mean *= 2.0 {
-			stdev := 0.05 * mean
-			seconds := rand.NormFloat64()*stdev + mean
-
-			wg := sync.WaitGroup{}
-			wg.Add(1)
-			go func() {
-				defer wg.Done()
-				select {
-				case <-ctx.Done():
-				case <-time.After(time.Duration(seconds * float64(time.Second))):
-				}
-			}()
-
-			wg.Wait()
-			return opts.speedTest(ctx, retries)
-
-		}
-	}
-
-	return s, err
-}

speedtesting/speedtest_test.go

@@ -22,6 +22,9 @@ import (
 
 func TestSpeedTest(t *testing.T) {
 	h, srv := NewNDT7Server(t)
+	// https://github.com/Microsoft/hcsshim/issues/108
+	time.Sleep(2 * time.Second)
+
 	defer os.RemoveAll(h.DataDir)
 	defer srv.Close()