🌱 addr.Suggest should lock a file instead of memory

Envtest is often running in parallel when using go test, which spins up
multiple indipendent go test processes that cannot talk to each other.
The address suggestion code, mostly used to find an open port, can
cause port collisions and a race condition between different envtests
running at the same time.

This change switches the internal memory to use a file based system that
creates a file.

Signed-off-by: Vince Prignano <vincepri@vmware.com>

pkg/internal/testing/addr/manager.go

@@ -18,7 +18,11 @@ package addr
 
 import (
 	"fmt"
+	"io/fs"
 	"net"
+	"os"
+	"path/filepath"
+	"strings"
 	"sync"
 	"time"
 )
@@ -28,33 +32,68 @@ import (
 const (
 	portReserveTime   = 1 * time.Minute
 	portConflictRetry = 100
+	portFilePrefix    = "port-"
 )
 
+var (
+	cacheDir string
+)
+
+func init() {
+	baseDir, err := os.UserCacheDir()
+	if err != nil {
+		baseDir = os.TempDir()
+	}
+	cacheDir = filepath.Join(baseDir, "kubebuilder-envtest")
+	_ = os.MkdirAll(cacheDir, 0750)
+}
+
 type portCache struct {
-	lock  sync.Mutex
-	ports map[int]time.Time
+	lock sync.Mutex
 }
 
-func (c *portCache) add(port int) bool {
+func (c *portCache) add(port int) (bool, error) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
-	// remove outdated port
-	for p, t := range c.ports {
-		if time.Since(t) > portReserveTime {
-			delete(c.ports, p)
+	// Remove outdated ports.
+	if err := fs.WalkDir(os.DirFS(cacheDir), ".", func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
 		}
+		if d.IsDir() || !d.Type().IsRegular() || !strings.HasPrefix(path, portFilePrefix) {
+			return nil
+		}
+		info, err := d.Info()
+		if err != nil {
+			return err
+		}
+		if time.Since(info.ModTime()) > portReserveTime {
+			if err := os.Remove(filepath.Join(cacheDir, path)); err != nil {
+				panic(err)
+			}
+		}
+		return nil
+	}); err != nil {
+		return false, err
 	}
-	// try allocating new port
-	if _, ok := c.ports[port]; ok {
-		return false
+	// Try allocating new port, by creating a file.
+	f, err := os.OpenFile(
+		fmt.Sprintf("%s/%s%d", cacheDir, portFilePrefix, port),
+		os.O_RDWR|os.O_CREATE|os.O_EXCL,
+		0666,
+	)
+	if os.IsExist(err) {
+		return false, nil
+	} else if err != nil {
+		return false, err
 	}
-	c.ports[port] = time.Now()
-	return true
+	if err := f.Close(); err != nil {
+		return false, err
+	}
+	return true, nil
 }
 
-var cache = &portCache{
-	ports: make(map[int]time.Time),
-}
+var cache = &portCache{}
 
 func suggest(listenHost string) (port int, resolvedHost string, err error) {
 	if listenHost == "" {
@@ -80,16 +119,17 @@ func suggest(listenHost string) (port int, resolvedHost string, err error) {
 // a tuple consisting of a free port and the hostname resolved to its IP.
 // It makes sure that new port allocated does not conflict with old ports
 // allocated within 1 minute.
-func Suggest(listenHost string) (port int, resolvedHost string, err error) {
+func Suggest(listenHost string) (int, string, error) {
 	for i := 0; i < portConflictRetry; i++ {
-		port, resolvedHost, err = suggest(listenHost)
+		port, resolvedHost, err := suggest(listenHost)
 		if err != nil {
-			return
+			return -1, "", err
 		}
-		if cache.add(port) {
-			return
+		if ok, err := cache.add(port); ok {
+			return port, resolvedHost, nil
+		} else if err != nil {
+			return -1, "", err
 		}
 	}
-	err = fmt.Errorf("no free ports found after %d retries", portConflictRetry)
-	return
+	return -1, "", fmt.Errorf("no free ports found after %d retries", portConflictRetry)
 }