From d01410a0ff5632e240cc735d230f94eb630458b4 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 8 Nov 2024 16:28:42 +1100 Subject: [PATCH] no longer need bind allocator everywhere --- frontend/cli/bind.go | 46 ----------------------------- frontend/cli/bind_test.go | 26 ---------------- frontend/cli/cmd_box.go | 9 +----- frontend/cli/cmd_box_run.go | 2 +- frontend/cli/cmd_build.go | 7 +---- frontend/cli/cmd_deploy.go | 7 +---- frontend/cli/cmd_dev.go | 2 +- internal/buildengine/engine.go | 5 +--- internal/buildengine/engine_test.go | 10 +------ 9 files changed, 7 insertions(+), 107 deletions(-) delete mode 100644 frontend/cli/bind.go delete mode 100644 frontend/cli/bind_test.go diff --git a/frontend/cli/bind.go b/frontend/cli/bind.go deleted file mode 100644 index ccc84f7810..0000000000 --- a/frontend/cli/bind.go +++ /dev/null @@ -1,46 +0,0 @@ -package main - -import ( - "fmt" - "net" - "net/url" - - "github.com/TBD54566975/ftl/internal/bind" -) - -// Create a bind allocator that skips the reserved port for the controller. -// -// The bind allocator will use cli.Endpoint if it is a local URL with a port. Otherwise a default port is used. -func bindAllocatorWithoutController() (*bind.BindAllocator, error) { - var url *url.URL - var err error - // use cli.Endpoint if it is a local URL with a port - if cli.Endpoint != nil && cli.Endpoint.Port() != "" { - h := cli.Endpoint.Hostname() - ips, err := net.LookupIP(h) - if err != nil { - return nil, fmt.Errorf("failed to look up IP: %w", err) - } - for _, netip := range ips { - if netip.IsLoopback() { - url = cli.Endpoint - break - } - } - } - // fallback to default - if url == nil { - url, err = url.Parse("http://127.0.0.1:8892") - if err != nil { - return nil, fmt.Errorf("failed to parse default URL: %w", err) - } - } - bindAllocator, err := bind.NewBindAllocator(url, 0) - if err != nil { - return nil, fmt.Errorf("could not create bind allocator: %w", err) - } - // Skip initial ports as it is reserved for the controller and provisioner - _, _ = bindAllocator.Next() //nolint:errcheck - _, _ = bindAllocator.Next() //nolint:errcheck - return bindAllocator, nil -} diff --git a/frontend/cli/bind_test.go b/frontend/cli/bind_test.go deleted file mode 100644 index 70bab7363f..0000000000 --- a/frontend/cli/bind_test.go +++ /dev/null @@ -1,26 +0,0 @@ -package main - -import ( - "net/url" - "strings" - "testing" - - "github.com/alecthomas/assert/v2" -) - -func TestBindLocalWithRemoteEndpoint(t *testing.T) { - var err error - cli.Endpoint, err = url.Parse("http://block.xyz:80") - assert.NoError(t, err) - - bindAllocator, err := bindAllocatorWithoutController() - assert.NoError(t, err) - - nextURL, err := bindAllocator.Next() - assert.NoError(t, err) - - assert.True(t, strings.HasPrefix(nextURL.String(), "http://127.0.0.1:"), nextURL.String()) - - _, err = bindAllocatorWithoutController() - assert.NoError(t, err) -} diff --git a/frontend/cli/cmd_box.go b/frontend/cli/cmd_box.go index 28052c5573..6fd2159d54 100644 --- a/frontend/cli/cmd_box.go +++ b/frontend/cli/cmd_box.go @@ -123,14 +123,7 @@ func (b *boxCmd) Run(ctx context.Context, client ftlv1connect.ControllerServiceC if len(b.Build.Dirs) == 0 { return errors.New("no directories specified") } - - // use the cli endpoint to create the bind allocator, but leave the first port unused as it is meant to be reserved by a controller - bindAllocator, err := bindAllocatorWithoutController() - if err != nil { - return err - } - - engine, err := buildengine.New(ctx, client, projConfig, b.Build.Dirs, bindAllocator, buildengine.BuildEnv(b.Build.BuildEnv), buildengine.Parallelism(b.Build.Parallelism)) + engine, err := buildengine.New(ctx, client, projConfig, b.Build.Dirs, buildengine.BuildEnv(b.Build.BuildEnv), buildengine.Parallelism(b.Build.Parallelism)) if err != nil { return err } diff --git a/frontend/cli/cmd_box_run.go b/frontend/cli/cmd_box_run.go index a184c94476..b330d6a6c8 100644 --- a/frontend/cli/cmd_box_run.go +++ b/frontend/cli/cmd_box_run.go @@ -80,7 +80,7 @@ func (b *boxRunCmd) Run( return fmt.Errorf("controller failed to start: %w", err) } - engine, err := buildengine.New(ctx, client, projConfig, []string{b.Dir}, bindAllocator) + engine, err := buildengine.New(ctx, client, projConfig, []string{b.Dir}) if err != nil { return fmt.Errorf("failed to create build engine: %w", err) } diff --git a/frontend/cli/cmd_build.go b/frontend/cli/cmd_build.go index 175560845d..aa9e8c02d4 100644 --- a/frontend/cli/cmd_build.go +++ b/frontend/cli/cmd_build.go @@ -23,16 +23,11 @@ func (b *buildCmd) Run(ctx context.Context, client ftlv1connect.ControllerServic if len(b.Dirs) == 0 { return errors.New("no directories specified") } - // use the cli endpoint to create the bind allocator, but leave the first port unused as it is meant to be reserved by a controller - bindAllocator, err := bindAllocatorWithoutController() - if err != nil { - return err - } // Cancel build engine context to ensure all language plugins are killed. ctx, cancel := context.WithCancel(ctx) defer cancel() - engine, err := buildengine.New(ctx, client, projConfig, b.Dirs, bindAllocator, buildengine.BuildEnv(b.BuildEnv), buildengine.Parallelism(b.Parallelism)) + engine, err := buildengine.New(ctx, client, projConfig, b.Dirs, buildengine.BuildEnv(b.BuildEnv), buildengine.Parallelism(b.Parallelism)) if err != nil { return err } diff --git a/frontend/cli/cmd_deploy.go b/frontend/cli/cmd_deploy.go index fc32fbca44..e8e1b4e700 100644 --- a/frontend/cli/cmd_deploy.go +++ b/frontend/cli/cmd_deploy.go @@ -25,15 +25,10 @@ func (d *deployCmd) Run(ctx context.Context, projConfig projectconfig.Config) er client = rpc.ClientFromContext[ftlv1connect.ControllerServiceClient](ctx) } - bindAllocator, err := bindAllocatorWithoutController() - if err != nil { - return err - } - // Cancel build engine context to ensure all language plugins are killed. ctx, cancel := context.WithCancel(ctx) defer cancel() - engine, err := buildengine.New(ctx, client, projConfig, d.Build.Dirs, bindAllocator, buildengine.BuildEnv(d.Build.BuildEnv), buildengine.Parallelism(d.Build.Parallelism)) + engine, err := buildengine.New(ctx, client, projConfig, d.Build.Dirs, buildengine.BuildEnv(d.Build.BuildEnv), buildengine.Parallelism(d.Build.Parallelism)) if err != nil { return err } diff --git a/frontend/cli/cmd_dev.go b/frontend/cli/cmd_dev.go index cc89e690a8..ca21b8c2f9 100644 --- a/frontend/cli/cmd_dev.go +++ b/frontend/cli/cmd_dev.go @@ -114,7 +114,7 @@ func (d *devCmd) Run( }) } - engine, err := buildengine.New(ctx, client, projConfig, d.Build.Dirs, bindAllocator, opts...) + engine, err := buildengine.New(ctx, client, projConfig, d.Build.Dirs, opts...) if err != nil { return err } diff --git a/internal/buildengine/engine.go b/internal/buildengine/engine.go index 9c913f06d8..6bae3133f0 100644 --- a/internal/buildengine/engine.go +++ b/internal/buildengine/engine.go @@ -19,7 +19,6 @@ import ( "golang.org/x/sync/errgroup" ftlv1 "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1" - "github.com/TBD54566975/ftl/internal/bind" "github.com/TBD54566975/ftl/internal/buildengine/languageplugin" "github.com/TBD54566975/ftl/internal/log" "github.com/TBD54566975/ftl/internal/moduleconfig" @@ -176,7 +175,6 @@ type rebuildRequest struct { // Engine for building a set of modules. type Engine struct { client DeployClient - bindAllocator *bind.BindAllocator moduleMetas *xsync.MapOf[string, moduleMeta] projectConfig projectconfig.Config moduleDirs []string @@ -238,7 +236,7 @@ func WithStartTime(startTime time.Time) Option { // pull in missing schemas. // // "dirs" are directories to scan for local modules. -func New(ctx context.Context, client DeployClient, projectConfig projectconfig.Config, moduleDirs []string, bindAllocator *bind.BindAllocator, options ...Option) (*Engine, error) { +func New(ctx context.Context, client DeployClient, projectConfig projectconfig.Config, moduleDirs []string, options ...Option) (*Engine, error) { ctx = rpc.ContextWithClient(ctx, client) e := &Engine{ client: client, @@ -254,7 +252,6 @@ func New(ctx context.Context, client DeployClient, projectConfig projectconfig.C rebuildRequests: make(chan rebuildRequest, 128), rawEngineUpdates: make(chan rawEngineEvent, 128), EngineUpdates: pubsub.New[EngineEvent](), - bindAllocator: bindAllocator, } for _, option := range options { option(e) diff --git a/internal/buildengine/engine_test.go b/internal/buildengine/engine_test.go index 3a41593ea5..cc23202e04 100644 --- a/internal/buildengine/engine_test.go +++ b/internal/buildengine/engine_test.go @@ -2,13 +2,11 @@ package buildengine_test import ( "context" - "net/url" "path/filepath" "testing" "github.com/alecthomas/assert/v2" - "github.com/TBD54566975/ftl/internal/bind" "github.com/TBD54566975/ftl/internal/buildengine" "github.com/TBD54566975/ftl/internal/log" "github.com/TBD54566975/ftl/internal/projectconfig" @@ -18,17 +16,11 @@ import ( func TestGraph(t *testing.T) { ctx, cancel := context.WithCancel(log.ContextWithNewDefaultLogger(context.Background())) t.Cleanup(cancel) - - bindURL, err := url.Parse("http://127.0.0.1:8893") - assert.NoError(t, err) - bindAllocator, err := bind.NewBindAllocator(bindURL, 0) - assert.NoError(t, err) - projConfig := projectconfig.Config{ Path: filepath.Join(t.TempDir(), "ftl-project.toml"), Name: "test", } - engine, err := buildengine.New(ctx, nil, projConfig, []string{"testdata/alpha", "testdata/other", "testdata/another"}, bindAllocator) + engine, err := buildengine.New(ctx, nil, projConfig, []string{"testdata/alpha", "testdata/other", "testdata/another"}) assert.NoError(t, err) defer engine.Close()