From de49419036ff59865dc7fa4f48b8e1d9e8c5ce22 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 | 67 ++++++++++++++++++++--------
 1 file changed, 49 insertions(+), 18 deletions(-)

diff --git a/pkg/internal/testing/addr/manager.go b/pkg/internal/testing/addr/manager.go
index 15f36fe2d0..e16430ab87 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,61 @@ import (
 const (
 	portReserveTime   = 1 * time.Minute
 	portConflictRetry = 100
+	portFilePrefix    = "port-"
 )
 
+var (
+	tempDir = fmt.Sprintf("%skubebuilder-envtest", os.TempDir())
+)
+
+func init() {
+	_ = os.MkdirAll(tempDir, 0750)
+}
+
 type portCache struct {
-	lock  sync.Mutex
-	ports map[int]time.Time
+	lock sync.Mutex
 }
 
 func (c *portCache) add(port int) bool {
 	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(tempDir), ".", 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(tempDir, path)); err != nil {
+				panic(err)
+			}
+		}
+		return nil
+	}); err != nil {
+		panic(err)
 	}
-	// try allocating new port
-	if _, ok := c.ports[port]; ok {
+	// Try allocating new port, by creating a file.
+	f, err := os.OpenFile(
+		fmt.Sprintf("%s/%s%d", tempDir, portFilePrefix, port),
+		os.O_RDWR|os.O_CREATE|os.O_EXCL,
+		0666,
+	)
+	if os.IsExist(err) {
 		return false
+	} else if err != nil {
+		panic(err)
 	}
-	c.ports[port] = time.Now()
+	_ = f.Close()
 	return true
 }
 
-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 +112,15 @@ 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
+			return port, resolvedHost, nil
 		}
 	}
-	err = fmt.Errorf("no free ports found after %d retries", portConflictRetry)
-	return
+	return -1, "", fmt.Errorf("no free ports found after %d retries", portConflictRetry)
 }