diff --git a/internal/command/deploy/deploy.go b/internal/command/deploy/deploy.go index 837a9ebfcc..e3d72ba305 100644 --- a/internal/command/deploy/deploy.go +++ b/internal/command/deploy/deploy.go @@ -602,9 +602,19 @@ func deployToMachines( maxConcurrent = immediateMaxConcurrent } + // img is nil for compose deploys that build no source image (every service + // uses a pre-built image); each container carries its own image reference. + var imageTag, imageBuilderID string + var imageBuildID int64 + if img != nil { + imageTag = img.Tag + imageBuildID = img.BuildID + imageBuilderID = img.BuilderID + } + status.AppName = app.Name status.OrgSlug = app.Organization.Slug - status.Image = img.Tag + status.Image = imageTag status.Strategy = cfg.DeployStrategy() if flag.GetString(ctx, "strategy") != "" { status.Strategy = flag.GetString(ctx, "strategy") @@ -650,7 +660,7 @@ func deployToMachines( args := MachineDeploymentArgs{ App: app, - DeploymentImage: img.Tag, + DeploymentImage: imageTag, Strategy: flag.GetString(ctx, "strategy"), EnvFromFlags: flag.GetStringArray(ctx, "env"), PrimaryRegionFlag: status.PrimaryRegion, @@ -677,8 +687,8 @@ func deployToMachines( VolumeInitialSize: flag.GetInt(ctx, "volume-initial-size"), ProcessGroups: processGroups, DeployRetries: deployRetries, - BuildID: img.BuildID, - BuilderID: img.BuilderID, + BuildID: imageBuildID, + BuilderID: imageBuilderID, } var path = flag.GetString(ctx, "export-manifest") diff --git a/internal/command/deploy/deploy_build.go b/internal/command/deploy/deploy_build.go index fd6605c476..02758ac216 100644 --- a/internal/command/deploy/deploy_build.go +++ b/internal/command/deploy/deploy_build.go @@ -13,6 +13,7 @@ import ( "github.com/superfly/flyctl/internal/appconfig" "github.com/superfly/flyctl/internal/build/imgsrc" "github.com/superfly/flyctl/internal/cmdutil" + "github.com/superfly/flyctl/internal/containerconfig" "github.com/superfly/flyctl/internal/env" "github.com/superfly/flyctl/internal/flag" "github.com/superfly/flyctl/internal/flyutil" @@ -167,6 +168,20 @@ func determineImage(ctx context.Context, app *flaps.App, appConfig *appconfig.Co return } + // Docker Compose: only build from source when a service declares `build:`. + usesCompose, composeBuild, err := composeBuildInfo(appConfig) + if err != nil { + tracing.RecordError(span, err, "failed to read compose build info") + + return + } + if usesCompose && composeBuild == nil { + span.AddEvent("compose deploy with no build service; skipping source build") + terminal.Debug("compose deploy uses only pre-built images; skipping source build") + + return nil, nil + } + build := appConfig.Build if build == nil { build = new(appconfig.Build) @@ -241,6 +256,16 @@ func determineImage(ctx context.Context, app *flaps.App, appConfig *appconfig.Co return } + // A compose `build:` directive takes precedence over an auto-detected + // Dockerfile: route its context/dockerfile into the build options. + if composeBuild != nil { + if err = applyComposeBuild(&opts, appConfig, composeBuild); err != nil { + tracing.RecordError(span, err, "failed to apply compose build directive") + + return + } + } + if opts.IgnorefilePath, err = resolveIgnorefilePath(ctx, appConfig); err != nil { tracing.RecordError(span, err, "failed to resolveIgnorefilePath") @@ -285,6 +310,51 @@ func determineImage(ctx context.Context, app *flaps.App, appConfig *appconfig.Co return } +func composeBuildInfo(appConfig *appconfig.Config) (usesCompose bool, cb *containerconfig.ComposeBuild, err error) { + if appConfig.Build == nil || appConfig.Build.Compose == nil { + return false, nil, nil + } + + composePath := appConfig.DetectComposeFile() + if composePath == "" { + return true, nil, nil + } + if !filepath.IsAbs(composePath) { + composePath = filepath.Join(filepath.Dir(appConfig.ConfigFilePath()), composePath) + } + + cb, err = containerconfig.ComposeBuildInfo(composePath) + if err != nil { + return true, nil, err + } + + return true, cb, nil +} + +func applyComposeBuild(opts *imgsrc.ImageOptions, appConfig *appconfig.Config, cb *containerconfig.ComposeBuild) error { + base := filepath.Dir(appConfig.ConfigFilePath()) + + ctxDir := opts.WorkingDir + if cb.Context != "" { + abs, err := filepath.Abs(filepath.Join(base, cb.Context)) + if err != nil { + return err + } + ctxDir = abs + opts.WorkingDir = abs + } + + if cb.Dockerfile != "" { + abs, err := filepath.Abs(filepath.Join(ctxDir, cb.Dockerfile)) + if err != nil { + return err + } + opts.DockerfilePath = abs + } + + return nil +} + // resolveDockerfilePath returns the absolute path to the Dockerfile // if one was specified in the app config or a command line argument func resolveDockerfilePath(ctx context.Context, appConfig *appconfig.Config) (path string, err error) { diff --git a/internal/command/deploy/deploy_build_test.go b/internal/command/deploy/deploy_build_test.go index 5a4aac4f51..c27a3cb53c 100644 --- a/internal/command/deploy/deploy_build_test.go +++ b/internal/command/deploy/deploy_build_test.go @@ -9,6 +9,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/superfly/flyctl/internal/appconfig" + "github.com/superfly/flyctl/internal/build/imgsrc" + "github.com/superfly/flyctl/internal/containerconfig" "github.com/superfly/flyctl/internal/state" ) @@ -37,3 +39,113 @@ func TestMultipleDockerfile(t *testing.T) { err = multipleDockerfile(ctx, cfg) assert.ErrorContains(t, err, "fly.production.toml") } + +// writeComposeApp writes a fly.toml with [build.compose] and a compose.yml into +// a temp dir, plus an optional stray Dockerfile, and returns the loaded config. +func writeComposeApp(t *testing.T, composeYML string, withDockerfile bool) *appconfig.Config { + t.Helper() + dir := t.TempDir() + + require.NoError(t, os.WriteFile(filepath.Join(dir, "compose.yml"), []byte(composeYML), 0644)) + if withDockerfile { + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM scratch\n"), 0644)) + } + + flyToml := filepath.Join(dir, "fly.toml") + require.NoError(t, os.WriteFile(flyToml, []byte(`app = "compose-test" + +[build.compose] + file = "compose.yml" +`), 0644)) + + cfg, err := appconfig.LoadConfig(flyToml) + require.NoError(t, err) + + return cfg +} + +func TestComposeBuildInfo_NoBuildService(t *testing.T) { + // Every service uses a pre-built image and a stray Dockerfile is present: + // the gate must report "uses compose, no build needed". + cfg := writeComposeApp(t, `services: + web: + image: nginx:latest + db: + image: postgres:14 +`, true /* stray Dockerfile present */) + + usesCompose, cb, err := composeBuildInfo(cfg) + require.NoError(t, err) + assert.True(t, usesCompose, "expected compose to be detected") + assert.Nil(t, cb, "expected no build directive when all services use images") +} + +func TestComposeBuildInfo_WithBuildService(t *testing.T) { + cfg := writeComposeApp(t, `services: + app: + build: + context: ./src + dockerfile: Dockerfile.custom + cache: + image: redis:alpine +`, false) + + usesCompose, cb, err := composeBuildInfo(cfg) + require.NoError(t, err) + assert.True(t, usesCompose) + require.NotNil(t, cb) + assert.Equal(t, "./src", cb.Context) + assert.Equal(t, "Dockerfile.custom", cb.Dockerfile) +} + +func TestComposeBuildInfo_NotCompose(t *testing.T) { + // A plain Dockerfile app must be reported as not using compose. + dir := t.TempDir() + flyToml := filepath.Join(dir, "fly.toml") + require.NoError(t, os.WriteFile(flyToml, []byte(`app = "plain"`+"\n"), 0644)) + cfg, err := appconfig.LoadConfig(flyToml) + require.NoError(t, err) + + usesCompose, cb, err := composeBuildInfo(cfg) + require.NoError(t, err) + assert.False(t, usesCompose) + assert.Nil(t, cb) +} + +func TestApplyComposeBuild_ContextAndDockerfile(t *testing.T) { + cfg := writeComposeApp(t, `services: + app: + build: + context: ./src + dockerfile: Dockerfile.custom +`, false) + base := filepath.Dir(cfg.ConfigFilePath()) + + opts := &imgsrc.ImageOptions{WorkingDir: base} + cb := &containerconfig.ComposeBuild{Context: "./src", Dockerfile: "Dockerfile.custom"} + + require.NoError(t, applyComposeBuild(opts, cfg, cb)) + + wantWorkDir, _ := filepath.Abs(filepath.Join(base, "src")) + wantDockerfile, _ := filepath.Abs(filepath.Join(base, "src", "Dockerfile.custom")) + assert.Equal(t, wantWorkDir, opts.WorkingDir) + assert.Equal(t, wantDockerfile, opts.DockerfilePath) +} + +func TestApplyComposeBuild_DockerfileOnly(t *testing.T) { + cfg := writeComposeApp(t, `services: + app: + build: + dockerfile: Dockerfile.custom +`, false) + base := filepath.Dir(cfg.ConfigFilePath()) + + opts := &imgsrc.ImageOptions{WorkingDir: base} + cb := &containerconfig.ComposeBuild{Dockerfile: "Dockerfile.custom"} + + require.NoError(t, applyComposeBuild(opts, cfg, cb)) + + wantDockerfile, _ := filepath.Abs(filepath.Join(base, "Dockerfile.custom")) + assert.Equal(t, base, opts.WorkingDir, "working dir unchanged when no context") + assert.Equal(t, wantDockerfile, opts.DockerfilePath) +} diff --git a/internal/command/deploy/machines.go b/internal/command/deploy/machines.go index f9ca40d913..f6330417d3 100644 --- a/internal/command/deploy/machines.go +++ b/internal/command/deploy/machines.go @@ -170,9 +170,6 @@ func NewMachineDeployment(ctx context.Context, args MachineDeploymentArgs) (_ Ma ctx, span := tracing.GetTracer().Start(ctx, "new_machines_deployment") defer span.End() - if !args.RestartOnly && args.DeploymentImage == "" { - return nil, fmt.Errorf("BUG: machines deployment created without specifying the image") - } if args.RestartOnly && args.DeploymentImage != "" { return nil, fmt.Errorf("BUG: restartOnly machines deployment created and specified an image") } @@ -183,6 +180,11 @@ func NewMachineDeployment(ctx context.Context, args MachineDeploymentArgs) (_ Ma return nil, err } + usesCompose := appConfig.Build != nil && appConfig.Build.Compose != nil + if !args.RestartOnly && args.DeploymentImage == "" && !usesCompose { + return nil, fmt.Errorf("BUG: machines deployment created without specifying the image") + } + // TODO: Blend extraInfo into ValidationError and remove this hack if err, extraInfo := appConfig.ValidateGroups(ctx, lo.Keys(args.ProcessGroups)); err != nil { fmt.Fprint(io.ErrOut, extraInfo) @@ -668,6 +670,11 @@ func (md *machineDeployment) setImg(ctx context.Context) error { if md.img != "" { return nil } + // A compose deploy where every service uses a pre-built image has no + // top-level image on purpose; each container carries its own reference. + if md.appConfig != nil && md.appConfig.Build != nil && md.appConfig.Build.Compose != nil { + return nil + } latestImg, err := md.apiClient.LatestImage(ctx, md.app.Name) if err == nil { md.img = latestImg diff --git a/internal/containerconfig/compose.go b/internal/containerconfig/compose.go index a2b5eaded5..4584e8966e 100644 --- a/internal/containerconfig/compose.go +++ b/internal/containerconfig/compose.go @@ -85,6 +85,48 @@ func parseComposeFile(composePath string) (*ComposeFile, error) { return &compose, nil } +type ComposeBuild struct { + Context string + Dockerfile string +} + +func ComposeBuildInfo(composePath string) (*ComposeBuild, error) { + compose, err := parseComposeFile(composePath) + if err != nil { + return nil, err + } + + for _, service := range compose.Services { + if service.Build == nil { + continue + } + + return parseComposeBuild(service.Build), nil + } + + return nil, nil +} + +// parseComposeBuild converts the two compose `build:` forms into a ComposeBuild +func parseComposeBuild(build any) *ComposeBuild { + switch b := build.(type) { + case string: + return &ComposeBuild{Context: b} + case map[string]any: + cb := &ComposeBuild{} + if ctx, ok := b["context"].(string); ok { + cb.Context = ctx + } + if df, ok := b["dockerfile"].(string); ok { + cb.Dockerfile = df + } + + return cb + default: + return &ComposeBuild{} + } +} + // parseDependsOn parses both short and long syntax depends_on func parseDependsOn(dependsOn any) (ServiceDependencies, error) { deps := ServiceDependencies{ diff --git a/internal/containerconfig/compose_test.go b/internal/containerconfig/compose_test.go index 773ab7993c..8630b92b2f 100644 --- a/internal/containerconfig/compose_test.go +++ b/internal/containerconfig/compose_test.go @@ -542,3 +542,90 @@ services: t.Error("Expected dependency on 'redis'") } } + +func writeComposeFile(t *testing.T, content string) string { + t.Helper() + tmpDir := t.TempDir() + composePath := filepath.Join(tmpDir, "compose.yml") + if err := os.WriteFile(composePath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write test compose file: %v", err) + } + + return composePath +} + +func TestComposeBuildInfo_NoBuildService(t *testing.T) { + // Every service uses a pre-built image: nothing to build. + path := writeComposeFile(t, `version: "3" +services: + web: + image: nginx:latest + db: + image: postgres:14 +`) + + cb, err := ComposeBuildInfo(path) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cb != nil { + t.Errorf("expected nil build info, got %+v", cb) + } +} + +func TestComposeBuildInfo_ShorthandString(t *testing.T) { + // build: ./app -> context only, default dockerfile. + path := writeComposeFile(t, `version: "3" +services: + app: + build: ./app + cache: + image: redis:alpine +`) + + cb, err := ComposeBuildInfo(path) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cb == nil { + t.Fatal("expected build info, got nil") + } + if cb.Context != "./app" { + t.Errorf("expected context './app', got %q", cb.Context) + } + if cb.Dockerfile != "" { + t.Errorf("expected empty dockerfile, got %q", cb.Dockerfile) + } +} + +func TestComposeBuildInfo_LongForm(t *testing.T) { + // build: { context:, dockerfile: } -> both honored. + path := writeComposeFile(t, `version: "3" +services: + app: + build: + context: ./src + dockerfile: Dockerfile.custom +`) + + cb, err := ComposeBuildInfo(path) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cb == nil { + t.Fatal("expected build info, got nil") + } + if cb.Context != "./src" { + t.Errorf("expected context './src', got %q", cb.Context) + } + if cb.Dockerfile != "Dockerfile.custom" { + t.Errorf("expected dockerfile 'Dockerfile.custom', got %q", cb.Dockerfile) + } +} + +func TestComposeBuildInfo_MissingFile(t *testing.T) { + _, err := ComposeBuildInfo(filepath.Join(t.TempDir(), "does-not-exist.yml")) + if err == nil { + t.Fatal("expected error for missing compose file, got nil") + } +} diff --git a/test/preflight/fly_deploy_test.go b/test/preflight/fly_deploy_test.go index 6df55babfb..0f205da964 100644 --- a/test/preflight/fly_deploy_test.go +++ b/test/preflight/fly_deploy_test.go @@ -448,3 +448,78 @@ func TestDeploy(t *testing.T) { testDeploy(t, filepath.Join(testlib.RepositoryRoot(), "test", "preflight", "fixtures", "example"), "--buildkit --remote-only") }) } + +func TestFlyDeployComposeNoBuild(t *testing.T) { + f := testlib.NewTestEnvFromEnv(t) + appName := f.CreateRandomAppMachines() + require.NotEmpty(t, appName) + + // An invalid Dockerfile that would fail the build if it were ever used. + f.WriteFile("Dockerfile", "FROM this-base-image-does-not-exist-4963:nope\n") + f.WriteFile("compose.yml", `services: + web: + image: nginx:latest +`) + f.WriteFlyToml(`app = "%s" +primary_region = "%s" + +[build.compose] + file = "compose.yml" + +[http_service] + internal_port = 80 + force_https = true +`, appName, f.PrimaryRegion()) + + // Must succeed without building the stray Dockerfile. + f.Fly("deploy --remote-only --ha=false") + + // The container should carry the pre-built nginx image, and no source image + // should have been substituted. + machines := f.MachinesList(appName) + require.NotEmpty(t, machines, "expected at least one machine") + var found bool + for _, m := range machines { + for _, c := range m.Config.Containers { + if strings.Contains(c.Image, "nginx") { + found = true + } + require.NotContains(t, c.Image, "registry.fly.io", + "compose container must use the pre-built image, not a source build") + } + } + require.True(t, found, "expected a container using the nginx image") +} + +func TestFlyDeployComposeWithBuild(t *testing.T) { + f := testlib.NewTestEnvFromEnv(t) + appName := f.CreateRandomAppMachines() + require.NotEmpty(t, appName) + + // Decoy: an invalid root Dockerfile that must NOT be selected. + f.WriteFile("Dockerfile", "FROM this-base-image-does-not-exist-4963:nope\n") + // The real build target referenced by the compose build directive. + f.WriteFile("Dockerfile.app", "FROM nginx\nENV PREFLIGHT_TEST=true\n") + f.WriteFile("compose.yml", `services: + app: + build: + context: . + dockerfile: Dockerfile.app +`) + f.WriteFlyToml(`app = "%s" +primary_region = "%s" + +[build.compose] + file = "compose.yml" + +[http_service] + internal_port = 80 + force_https = true +`, appName, f.PrimaryRegion()) + + // Must succeed by building Dockerfile.app, not the invalid root Dockerfile. + f.Fly("deploy --buildkit --remote-only --ha=false") + + machines := f.MachinesList(appName) + require.NotEmpty(t, machines, "expected at least one machine") +}