From 55b1cf78d301a4566ff57f507d1f3bc1b778f896 Mon Sep 17 00:00:00 2001
From: Vince Prignano <vincepri@vmware.com>
Date: Mon, 21 Jun 2021 20:11:08 -0700
Subject: [PATCH] :seedling: 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 | 84 ++++++++++++++++++++--------
 1 file changed, 62 insertions(+), 22 deletions(-)

diff --git a/pkg/internal/testing/addr/manager.go b/pkg/internal/testing/addr/manager.go
index 15f36fe2d0..e8c1a13f91 100644
--- a/pkg/internal/testing/addr/manager.go
+++ b/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)
 }