Skip to content

Commit

Permalink
Fix flaky test using custom port mapping when running specs in parallel
Browse files Browse the repository at this point in the history 
Using a global counter currently does not work
because Ginkgo runs parallel specs by isolating them in
completely different processes [1].
This relies on the parallel process index returned by Ginkgo
to get a value that can be used as starting port.
Callers can then safely increment this value if needed in the
specs to get a different custom value.

[1] https://onsi.github.io/ginkgo/#mental-model-how-ginkgo-runs-parallel-specs
  • Loading branch information
@rm3l
rm3l committed Apr 21, 2023
1 parent 32f92d1 commit 05cdfc4
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 23 deletions.
15 changes: 7 additions & 8 deletions tests/helper/helper_http.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ import (
"fmt"
"io"
"net/http"
"strconv"
"strings"
"sync/atomic"
"time"

. "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -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()
}
11 changes: 7 additions & 4 deletions tests/integration/cmd_dev_debug_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package integration
import (
"fmt"
"path/filepath"
"strconv"
"strings"

"k8s.io/utils/pointer"
Expand Down Expand Up @@ -47,16 +48,18 @@ var _ = Describe("odo dev debug command tests", func() {
ports map[string]string
)
var (
LocalPort = helper.GetRandomFreePort()
LocalDebugPort = helper.GetRandomFreePort()
LocalPort int
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")
}
Expand All @@ -82,7 +85,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)
})
Expand Down
26 changes: 15 additions & 11 deletions tests/integration/cmd_dev_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -915,12 +915,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()
})
Expand All @@ -932,7 +933,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")
Expand All @@ -953,7 +954,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())
Expand Down Expand Up @@ -1010,7 +1011,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())
Expand All @@ -1031,9 +1032,9 @@ ComponentSettings:

When("devfile has multiple endpoints", func() {
var (
LocalPort1 = helper.GetRandomFreePort()
LocalPort2 = helper.GetRandomFreePort()
LocalPort3 = helper.GetRandomFreePort()
LocalPort1 int
LocalPort2 int
LocalPort3 int
)
const (
// ContainerPort<N> are hard-coded from devfile-with-multiple-endpoints.yaml
Expand All @@ -1045,6 +1046,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()
})
Expand All @@ -1055,7 +1059,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")
Expand Down Expand Up @@ -1097,13 +1101,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!"})
})
Expand Down Expand Up @@ -1139,7 +1143,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!"))
Expand Down

0 comments on commit 05cdfc4

Please sign in to comment.