feat: use buildengine for deploy cmd #1002

wesbillman · 2024-02-28T18:49:10Z

Not fully tested yet, but starting to flesh out the subscription flow for build -> deploy

ftl deploy ../ftl-examples/online-boutique/backend/services examples/go

info:time: Building module
info:ad: Building module
info:currency: Building module
info:cart: Building module
info:cart: Deploying module
info:ad: Deploying module
info:time: Deploying module
info:currency: Deploying module
info:shipping: Building module
info:productcatalog: Building module
info:payment: Building module
info:echo: Building module
info:shipping: Deploying module
info:payment: Deploying module
info:echo: Deploying module
info:productcatalog: Deploying module
info:recommendation: Building module
info:checkout: Building module
info:recommendation: Deploying module
info:checkout: Deploying module

alecthomas · 2024-02-28T21:23:04Z

buildengine/deploy.go

+	ctx = log.ContextWithLogger(ctx, logger)
+	logger.Infof("Deploying module")
+
+	deployDir := filepath.Join(module.Dir, module.DeployDir)


Unrelated but I keep having to do this, maybe we should add helper methods that do the joining for us, eg. module.AbsDeployDir()

Or maybe we should make DeployDir et al absolute when we load the config. Not sure.

Yeah same! I'll add the module.AbsDeployDir for now and maybe we can clean this up with the DeployDir stuff if that makes sense

alecthomas · 2024-02-28T21:26:41Z

buildengine/engine.go

@@ -201,6 +206,56 @@ func (e *Engine) Build(ctx context.Context, modules ...string) error {
 	return nil
 }

+func (e *Engine) Deploy(ctx context.Context, replicas int32, waitForDeployOnline bool, modules ...string) error {


This needs a comment.

alecthomas · 2024-02-28T21:30:01Z

buildengine/engine.go

+		modules = maps.Keys(e.modules)
+	}
+
+	buildSubscription := e.builds.SubscribeSync(nil)


I wouldn't bother with pubsub for an internal implementation. It's useful for decoupling components, but if all your code is in a single type then it's just overhead. I'd probably refactor the exist build code into a function that can accept a callback that is executed after each module is built.

Oh I like that! I'll give it a go

alecthomas · 2024-02-28T21:30:48Z

cmd/ftl/cmd_deploy.go

-	}
-
-	_, err = client.ReplaceDeploy(ctx, connect.NewRequest(&ftlv1.ReplaceDeployRequest{DeploymentName: resp.Msg.GetDeploymentName(), MinReplicas: d.Replicas}))
+func (d *deployCmd) Run(ctx context.Context) error {


FWIW both these approaches work - clients in context and clients passed directly.

alecthomas · 2024-02-29T21:09:03Z

buildengine/deploy.go

+	localPath string
+}
+
+func Deploy(ctx context.Context, module Module, replicas int32, waitForDeployOnline bool, client ftlv1connect.ControllerServiceClient) error {


Needs a comment.

alecthomas · 2024-02-29T21:12:53Z

buildengine/engine.go

 // Build attempts to build the specified modules, or all local modules if none are provided.
-func (e *Engine) Build(ctx context.Context, modules ...string) error {
+// The callback is invoked for each module after it is built.
+func (e *Engine) Build(ctx context.Context, callback BuildCallback, modules ...string) error {


The callback is (currently) an internal implementation detail only used by Deploy, just make its use private.

alecthomas · 2024-03-01T21:35:03Z

buildengine/engine.go

+}
+
+// Deploy attempts to build and deploy the specified modules, or all local modules if none are provided.
+func (e *Engine) Deploy(ctx context.Context, replicas int32, waitForDeployOnline bool, modules ...string) error {


I don't have a better idea really, but passing the same number of replicas for all deployments probably isn't what we'll want longer term.

Yeah same. I wasn't 100% sure what to do here either, but open to improvements. Maybe we give this a go and if we have some use cases in the future that need something else we can update.

alecthomas · 2024-03-01T21:37:51Z

buildengine/engine.go

+	}
+
+	return e.buildWithCallback(ctx, func(ctx context.Context, module Module) error {
+		return Deploy(ctx, module, replicas, waitForDeployOnline, e.client)


Having this wait for each deploy in turn will significantly slow down the deploy, as the build won't be able to continue until each group has fully deployed.

OTOH this will mean the deployed services will always have the correct dependencies up, so I'm a bit conflicted.

Probably okay for now, but we'll see how it goes. I think what would ideally happen is that deployments that don't have all the correct dependencies available wouldn't go live.

Word! I was thinking, this is nice and reliable for now, but probably can be quicker in the future. I had it deploying without a wait, but def ran into issues with complex dependencies. Maybe something we can chat through to see if we can make this fast and reliable.

alecthomas reviewed Feb 28, 2024

View reviewed changes

wesbillman force-pushed the create-engine-deploy branch from 7d4822e to c1ca97a Compare February 29, 2024 17:22

alecthomas reviewed Feb 29, 2024

View reviewed changes

alecthomas reviewed Mar 1, 2024

View reviewed changes

wesbillman force-pushed the create-engine-deploy branch from 24c37ef to 4df9870 Compare March 1, 2024 17:38

wesbillman marked this pull request as ready for review March 1, 2024 17:49

wesbillman requested review from dhanji and worstell as code owners March 1, 2024 17:49

wesbillman force-pushed the create-engine-deploy branch from 7806df1 to 00a755e Compare March 1, 2024 17:49

alecthomas approved these changes Mar 1, 2024

View reviewed changes

wesbillman added 4 commits March 5, 2024 08:53

feat: use buildengine for deploy cmd

58855a7

fix: use callback instead of pubsub

e95710c

fix: clean up Build interface

d93fbcc

fix: add AbsDeployDir helper func

574087a

wesbillman force-pushed the create-engine-deploy branch from 4a9a9c1 to 574087a Compare March 5, 2024 17:03

chore(autofmt): Automated formatting

c9050e8

wesbillman merged commit c995fa4 into main Mar 5, 2024

wesbillman deleted the create-engine-deploy branch March 5, 2024 17:07

alecthomas mentioned this pull request Mar 5, 2024

Dashboard #728

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use buildengine for deploy cmd #1002

feat: use buildengine for deploy cmd #1002

wesbillman commented Feb 28, 2024 •

edited

Loading

alecthomas Feb 28, 2024

wesbillman Mar 5, 2024

alecthomas Feb 28, 2024

alecthomas Feb 28, 2024

wesbillman Feb 29, 2024

alecthomas Feb 28, 2024

alecthomas Feb 29, 2024

alecthomas Feb 29, 2024

alecthomas Mar 1, 2024

wesbillman Mar 5, 2024

alecthomas Mar 1, 2024

wesbillman Mar 5, 2024

feat: use buildengine for deploy cmd #1002

feat: use buildengine for deploy cmd #1002

Conversation

wesbillman commented Feb 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wesbillman commented Feb 28, 2024 •

edited

Loading