Skip to content

Commit

Permalink
[WIP] Make get-free-port reliable on Linux
Browse files Browse the repository at this point in the history
  • Loading branch information
mjameswh committed May 16, 2024
1 parent 0160df0 commit 571e079
Show file tree
Hide file tree
Showing 9 changed files with 274 additions and 97 deletions.
3 changes: 2 additions & 1 deletion temporalcli/commands.server.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/google/uuid"
"github.com/temporalio/cli/temporalcli/devserver"
"github.com/temporalio/cli/temporalcli/internal/freeport"
)

func (t *TemporalServerStartDevCommand) run(cctx *CommandContext, args []string) error {
Expand Down Expand Up @@ -87,7 +88,7 @@ func (t *TemporalServerStartDevCommand) run(cctx *CommandContext, args []string)
}
// Grab a free port for metrics ahead-of-time so we know what port is selected
if opts.MetricsPort == 0 {
opts.MetricsPort = devserver.MustGetFreePort()
opts.MetricsPort = freeport.MustGetFreePort(t.Ip)
}

// Start, wait for context complete, then stop
Expand Down
4 changes: 2 additions & 2 deletions temporalcli/commands.server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/temporalio/cli/temporalcli/devserver"
"github.com/temporalio/cli/temporalcli/internal/freeport"
"go.temporal.io/sdk/client"
)

Expand All @@ -21,7 +21,7 @@ func TestServer_StartDev_Simple(t *testing.T) {
defer h.Close()

// Start in background, then wait for client to be able to connect
port := strconv.Itoa(devserver.MustGetFreePort())
port := strconv.Itoa(freeport.MustGetFreePort("127.0.0.1"))
resCh := make(chan *CommandResult, 1)
// TODO(cretz): Remove --headless when
// https://github.com/temporalio/ui/issues/1773 fixed
Expand Down
3 changes: 2 additions & 1 deletion temporalcli/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/temporalio/cli/temporalcli"
"github.com/temporalio/cli/temporalcli/devserver"
"github.com/temporalio/cli/temporalcli/internal/freeport"

"google.golang.org/grpc"
)
Expand Down Expand Up @@ -273,7 +274,7 @@ func StartDevServer(t *testing.T, options DevServerOptions) *DevServer {
d.Options.FrontendIP = "127.0.0.1"
}
if d.Options.FrontendPort == 0 {
d.Options.FrontendPort = devserver.MustGetFreePort()
d.Options.FrontendPort = freeport.MustGetFreePort(d.Options.FrontendIP)
}
if len(d.Options.Namespaces) == 0 {
d.Options.Namespaces = []string{
Expand Down
84 changes: 0 additions & 84 deletions temporalcli/devserver/freeport.go

This file was deleted.

17 changes: 8 additions & 9 deletions temporalcli/devserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"strconv"
"time"

"github.com/temporalio/cli/temporalcli/internal/freeport"
uiserver "github.com/temporalio/ui-server/v2/server"
uiconfig "github.com/temporalio/ui-server/v2/server/config"
uiserveroptions "github.com/temporalio/ui-server/v2/server/server_options"
Expand Down Expand Up @@ -260,13 +261,11 @@ func (s *StartOptions) buildServerConfig() (*config.Config, error) {
}
}
conf.DCRedirectionPolicy.Policy = "noop"
portProvider := NewPortProvider()
defer portProvider.Close()
conf.Services = map[string]config.Service{
"frontend": s.buildServiceConfig(portProvider, true),
"history": s.buildServiceConfig(portProvider, false),
"matching": s.buildServiceConfig(portProvider, false),
"worker": s.buildServiceConfig(portProvider, false),
"frontend": s.buildServiceConfig(true),
"history": s.buildServiceConfig(false),
"matching": s.buildServiceConfig(false),
"worker": s.buildServiceConfig(false),
}
conf.Archival.History.State = "disabled"
conf.Archival.Visibility.State = "disabled"
Expand Down Expand Up @@ -319,7 +318,7 @@ func (s *StartOptions) buildSQLConfig() (*config.SQL, error) {
return &conf, nil
}

func (s *StartOptions) buildServiceConfig(p *PortProvider, frontend bool) config.Service {
func (s *StartOptions) buildServiceConfig(frontend bool) config.Service {
var conf config.Service
if frontend {
conf.RPC.GRPCPort = s.FrontendPort
Expand All @@ -328,9 +327,9 @@ func (s *StartOptions) buildServiceConfig(p *PortProvider, frontend bool) config
conf.RPC.HTTPPort = s.FrontendHTTPPort
}
} else {
conf.RPC.GRPCPort = p.MustGetFreePort()
conf.RPC.GRPCPort = freeport.MustGetFreePort("127.0.0.1")
conf.RPC.BindOnLocalHost = true
}
conf.RPC.MembershipPort = p.MustGetFreePort()
conf.RPC.MembershipPort = freeport.MustGetFreePort("127.0.0.1")
return conf
}
11 changes: 11 additions & 0 deletions temporalcli/internal/freeport/freeport.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package freeport

import "fmt"

func MustGetFreePort(host string) int {
port, err := GetFreePort(host)
if err != nil {
panic(fmt.Errorf("failed assigning ephemeral port: %w", err))
}
return port
}
157 changes: 157 additions & 0 deletions temporalcli/internal/freeport/freeport_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
package freeport_test

import (
"fmt"
"net"
"testing"

"github.com/temporalio/cli/temporalcli/internal/freeport"
)

func TestFreePort_NoDouble(t *testing.T) {
host := "127.0.0.1"
portSet := make(map[int]bool)

for i := 0; i < 2000; i++ {
p, err := freeport.GetFreePort(host)
if err != nil {
t.Fatalf("Error: %s", err)
break
}

if _, exists := portSet[p]; exists {
t.Fatalf("Port %d has been assigned more than once", p)
}

// Add port to the set
portSet[p] = true
}
}

func TestFreePort_CanBindImmediatelySameProcess(t *testing.T) {
host := "127.0.0.1"

for i := 0; i < 500; i++ {
p, err := freeport.GetFreePort(host)
if err != nil {
t.Fatalf("Error: %s", err)
break
}
err = tryListenAndDialOn(host, p)
if err != nil {
t.Fatalf("Error: %s", err)
break
}
}
}

// This function is used as part of unit tests, to ensure that the port
func tryListenAndDialOn(host string, port int) error {
l, err := net.Listen("tcp", fmt.Sprintf("%s:%d", host, port))
if err != nil {
return err
}
defer l.Close()

r, err := net.DialTCP("tcp", nil, l.Addr().(*net.TCPAddr))
if err != nil {
panic(err)
}
defer r.Close()

c, err := l.Accept()
defer c.Close()
if err != nil {
panic(err)
}

return nil
}

// func main() {
// if (len(os.Args) > 1) {
// port, err := strconv.Atoi(os.Args[1])
// if err != nil {
// panic(err)
// }
// tryListenAndDialOn("127.0.0.1", port)
// return
// }

// portSet := make(map[int]bool)

// for i := 0; i < 5000; i++ {
// p, err := GetFreePort("127.0.0.1")
// if err != nil {
// fmt.Printf("Error: %s\n", err)
// continue
// }
// fmt.Printf("... %d\n", p)

// if _, exists := portSet[p]; exists {
// fmt.Printf("Port %d has been assigned more than once\n", p)
// }

// tryListenAndDialOn(p)

// // cmd := exec.Command(os.Args[0], strconv.Itoa(p))
// // cmd.Stdout = os.Stdout
// // cmd.Stderr = os.Stderr
// // err = cmd.Start()
// // if err != nil {
// // panic(err)
// // }
// // err = cmd.Wait()
// // if err != nil {
// // panic(err)
// // }

// // Add port to the set
// portSet[p] = true
// }
// }

// func main() {
// if (len(os.Args) > 1) {
// port, err := strconv.Atoi(os.Args[1])
// if err != nil {
// panic(err)
// }
// listenAndDial(port)
// return
// }

// portSet := make(map[int]bool)

// for i := 0; i < 5000; i++ {
// p, err := GetFreePort()
// if err != nil {
// fmt.Printf("Error: %s\n", err)
// continue
// }
// fmt.Printf("... %d\n", p)

// if _, exists := portSet[p]; exists {
// fmt.Printf("Port %d has been assigned more than once\n", p)
// }

// // Test 2
// listenAndDial(p)

// // Test 3
// // cmd := exec.Command(os.Args[0], strconv.Itoa(p))
// // cmd.Stdout = os.Stdout
// // cmd.Stderr = os.Stderr
// // err = cmd.Start()
// // if err != nil {
// // panic(err)
// // }
// // err = cmd.Wait()
// // if err != nil {
// // panic(err)
// // }

// // Add port to the set
// portSet[p] = true
// }
// }
Loading

0 comments on commit 571e079

Please sign in to comment.