Merge bfe4168 into 4d1e402

redhat-developer · Apr 24, 2023 · 54cada4 · 54cada4
2 parents 4d1e402 + bfe4168
commit 54cada4
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 23 deletions.
diff --git a/tests/helper/helper_http.go b/tests/helper/helper_http.go
@@ -5,9 +5,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"strconv"
 	"strings"
-	"sync/atomic"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -50,10 +48,11 @@ func HttpWaitForWithStatus(url string, match string, maxRetry int, interval int,
 	Fail(fmt.Sprintf("Failed after %d retries. Content in %s doesn't include '%s'.", maxRetry, url, match))
 }
 
-var startPort int64 = 30000
-
-// GetRandomFreePort increases the counter of global variable startPort, and returns.
-func GetRandomFreePort() string {
-	atomic.AddInt64(&startPort, 1)
-	return strconv.FormatInt(startPort, 10)
+// GetCustomStartPort returns a port that can be used as starting value for custom port mapping.
+// Because of the way Ginkgo runs specs in parallel (by isolating them in different processes),
+// this function needs to be called in a Before* node or test spec.
+// It returns a starting value that aims at minimizing the probability of collisions.
+// Callers can then safely increment the returned value in their specs if needed.
+func GetCustomStartPort() int {
+	return 30000 + 100*GinkgoParallelProcess()
 }
diff --git a/tests/integration/cmd_dev_debug_test.go b/tests/integration/cmd_dev_debug_test.go
@@ -3,6 +3,7 @@ package integration
 import (
 	"fmt"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	"k8s.io/utils/pointer"
@@ -47,16 +48,17 @@ var _ = Describe("odo dev debug command tests", func() {
 					ports      map[string]string
 				)
 				var (
-					LocalPort      = helper.GetRandomFreePort()
-					LocalDebugPort = helper.GetRandomFreePort()
+					LocalPort, LocalDebugPort      int
 				)
 				const (
 					ContainerPort      = "3000"
 					ContainerDebugPort = "5858"
 				)
 
 				BeforeEach(func() {
-					opts := []string{"--debug", fmt.Sprintf("--port-forward=%s:%s", LocalPort, ContainerPort), fmt.Sprintf("--port-forward=%s:%s", LocalDebugPort, ContainerDebugPort)}
+					LocalPort = helper.GetCustomStartPort()
+					LocalDebugPort = LocalPort + 1
+					opts := []string{"--debug", fmt.Sprintf("--port-forward=%d:%s", LocalPort, ContainerPort), fmt.Sprintf("--port-forward=%d:%s", LocalDebugPort, ContainerDebugPort)}
 					if podman {
 						opts = append(opts, "--forward-localhost")
 					}
@@ -82,7 +84,7 @@ var _ = Describe("odo dev debug command tests", func() {
 						// 400 response expected because the endpoint expects a websocket request and we are doing a HTTP GET
 						// We are just using this to validate if nodejs agent is listening on the other side
 						url := fmt.Sprintf("http://%s", ports[ContainerDebugPort])
-						Expect(url).To(ContainSubstring(LocalDebugPort))
+						Expect(url).To(ContainSubstring(strconv.Itoa(LocalDebugPort)))
 
 						helper.HttpWaitForWithStatus(url, "WebSockets request was expected", 12, 5, 400)
 					})

diff --git a/tests/integration/cmd_dev_test.go b/tests/integration/cmd_dev_test.go
@@ -984,12 +984,13 @@ ComponentSettings:
 					}
 					When("devfile has single endpoint", func() {
 						var (
-							LocalPort = helper.GetRandomFreePort()
+							LocalPort int
 						)
 						const (
 							ContainerPort = "3000"
 						)
 						BeforeEach(func() {
+							LocalPort = helper.GetCustomStartPort()
 							helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context)
 							helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile.yaml")).ShouldPass()
 						})
@@ -1001,7 +1002,7 @@ ComponentSettings:
 								var err error
 								opts := []string{}
 								if customPortForwarding {
-									opts = []string{fmt.Sprintf("--port-forward=%s:%s", LocalPort, ContainerPort)}
+									opts = []string{fmt.Sprintf("--port-forward=%d:%s", LocalPort, ContainerPort)}
 								}
 								if manual {
 									opts = append(opts, "--no-watch")
@@ -1022,7 +1023,7 @@ ComponentSettings:
 							It(fmt.Sprintf("should expose the endpoint on localhost (podman=%v, manual=%v, customPortForwarding=%v)", podman, manual, customPortForwarding), func() {
 								url := fmt.Sprintf("http://%s", ports[ContainerPort])
 								if customPortForwarding {
-									Expect(url).To(ContainSubstring(LocalPort))
+									Expect(url).To(ContainSubstring(strconv.Itoa(LocalPort)))
 								}
 								resp, err := http.Get(url)
 								Expect(err).ToNot(HaveOccurred())
@@ -1079,7 +1080,7 @@ ComponentSettings:
 											Eventually(func(g Gomega) {
 												url := fmt.Sprintf("http://%s", ports[ContainerPort])
 												if customPortForwarding {
-													Expect(url).To(ContainSubstring(LocalPort))
+													Expect(url).To(ContainSubstring(strconv.Itoa(LocalPort)))
 												}
 												resp, err := http.Get(url)
 												g.Expect(err).ToNot(HaveOccurred())
@@ -1100,9 +1101,7 @@ ComponentSettings:
 
 					When("devfile has multiple endpoints", func() {
 						var (
-							LocalPort1 = helper.GetRandomFreePort()
-							LocalPort2 = helper.GetRandomFreePort()
-							LocalPort3 = helper.GetRandomFreePort()
+							LocalPort1, LocalPort2, LocalPort3 int
 						)
 						const (
 							// ContainerPort<N> are hard-coded from devfile-with-multiple-endpoints.yaml
@@ -1114,6 +1113,9 @@ ComponentSettings:
 							ContainerPort3 = "7890"
 						)
 						BeforeEach(func() {
+							LocalPort1 = helper.GetCustomStartPort()
+							LocalPort2 = LocalPort1 + 1
+							LocalPort3 = LocalPort1 + 2
 							helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project-with-multiple-endpoints"), commonVar.Context)
 							helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile-with-multiple-endpoints.yaml")).ShouldPass()
 						})
@@ -1124,7 +1126,7 @@ ComponentSettings:
 							BeforeEach(func() {
 								opts := []string{}
 								if customPortForwarding {
-									opts = []string{fmt.Sprintf("--port-forward=%s:%s", LocalPort1, ContainerPort1), fmt.Sprintf("--port-forward=%s:%s", LocalPort2, ContainerPort2), fmt.Sprintf("--port-forward=%s:%s", LocalPort3, ContainerPort3)}
+									opts = []string{fmt.Sprintf("--port-forward=%d:%s", LocalPort1, ContainerPort1), fmt.Sprintf("--port-forward=%d:%s", LocalPort2, ContainerPort2), fmt.Sprintf("--port-forward=%d:%s", LocalPort3, ContainerPort3)}
 								}
 								if manual {
 									opts = append(opts, "--no-watch")
@@ -1166,13 +1168,13 @@ ComponentSettings:
 									return string(body), nil
 								}
 								containerPorts := []string{ContainerPort1, ContainerPort2, ContainerPort3}
-								localPorts := []string{LocalPort1, LocalPort2, LocalPort3}
+								localPorts := []int{LocalPort1, LocalPort2, LocalPort3}
 
 								for i := range containerPorts {
 									containerPort := containerPorts[i]
 									localPort := localPorts[i]
 									By(fmt.Sprintf("exposing a port targeting container port %s", containerPort), func() {
-										r, err := getServerResponse(containerPort, localPort)
+										r, err := getServerResponse(containerPort, strconv.Itoa(localPort))
 										Expect(err).ShouldNot(HaveOccurred())
 										helper.MatchAllInOutput(r, []string{"Hello from Node.js Starter Application!"})
 									})
@@ -1208,7 +1210,7 @@ ComponentSettings:
 									By(fmt.Sprintf("returning the right response when querying port forwarded for container port %s", containerPort),
 										func() {
 											Eventually(func(g Gomega) string {
-												r, err := getServerResponse(containerPort, localPort)
+												r, err := getServerResponse(containerPort, strconv.Itoa(localPort))
 												g.Expect(err).ShouldNot(HaveOccurred())
 												return r
 											}, 180, 10).Should(Equal("H3110 from Node.js Starter Application!"))