bundle: add --deploy-apps to push source code during bundle deploy#5055
bundle: add --deploy-apps to push source code during bundle deploy#5055jamesbroadhead wants to merge 2 commits intodatabricks:mainfrom
Conversation
Addresses the DEPLOY-01 "bundle deploy does not actually deploy apps" gap. Today
`bundle deploy` reconciles resources via terraform/direct, but the app source
code is pushed by a separate `bundle run <app>` step that users routinely miss
— they run the documented deploy command, see their app unchanged, and
conclude the platform is broken.
With `--deploy-apps`, the Deploy phase calls `w.Apps.Deploy()` for every app in
the bundle after the resource CRUD path completes. The flag defaults to false
to preserve existing behavior for users and CI pipelines relying on the
two-step flow; once the flag proves out in the field we can flip the default.
Changes
-------
* bundle/appdeploy/config.go: factor the ${resources.*} reference resolution out
of appRunner.resolvedConfig() into a package-level ResolveAppConfig. Takes
*config.Root (not *bundle.Bundle) to avoid the bundle -> direct ->
dresources -> appdeploy import cycle.
* bundle/phases/deploy_apps.go: new DeployApps phase. Iterates b.Config.Resources.Apps,
resolves config, calls appdeploy.Deploy per app. Accumulates per-app errors
via errors.Join so one failed app does not mask the others.
* bundle/phases/deploy.go: call DeployApps at the end of the deploy phase (after
the core mutator, before ScriptPostDeploy), gated on b.DeployApps.
* bundle/bundle.go: new DeployApps bool alongside AutoApprove.
* cmd/bundle/deploy.go: wire the --deploy-apps flag through ProcessOptions.InitFunc.
* bundle/run/app.go: replace the in-line resolvedConfig method with a call to
the new package-level helper so both code paths share the same resolution
behavior.
* bundle/appdeploy/config_test.go: unit coverage for the nil / no-config cases.
A ${resources.*} interpolation test is deferred to follow-up (needs a
populated config.Root via the standard bundle load path).
Follow-ups
----------
* Integration test that deploys a bundle with an app and asserts the source
code ends up at the workspace path.
* Decide default flip (likely tied to a v0.N release where the behavior is
documented as the new expectation).
* A deeper dyn.Value-driven unit test for ResolveAppConfig to cover interpolation.
Co-authored-by: Isaac
If the bundle has `apps:` resources but the user did not pass `--deploy-apps`, the resource phase succeeds and apps appear "deployed" — yet their source code was never pushed. This is the exact first-five- minutes confusion the flag was introduced to fix; silent skipping made the original problem slightly worse for anyone who didn't read the release notes. Print a line per app pointing at `databricks bundle run <app>`, which remains the current way to push source after a `bundle deploy`. Co-authored-by: Isaac
|
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
Approval status: pending
|
| // new AppDeployment for every app in the bundle. When false (the default), | ||
| // `bundle deploy` only reconciles resources and the user must still run | ||
| // `bundle run <app>` to push source. | ||
| DeployApps bool |
There was a problem hiding this comment.
We have recently added lifecycle.started field, can you check if it works? Unlike pure config option, it is actually tracked in resource state so it handles transitions between started/stopped/unspecified better.
|
Please see this change #4672 Note: it's only available for direct deployment mode |
|
@andrewnester that's great news - thanks for letting me know! |
Summary
Addresses the DEPLOY-01 "
bundle deploydoes not actually deploy apps" gap surfaced in the internal EMEA Apps gaps doc (number 1 developer complaint from — the first-five-minutes experience).Today
bundle deployonly reconciles resources via the terraform/direct engine; the app source code is pushed by a separatebundle run <app>step. Users routinely miss step 2 and conclude the platform is broken when their "deployed" app does nothing new.With
--deploy-apps, the Deploy phase callsw.Apps.Deploy()for every app in the bundle after the resource CRUD completes. The flag defaults to false to preserve existing behavior for users and CI pipelines that rely on the two-step flow; once the flag proves out in the field we can flip the default.Changes
bundle/appdeploy/config.go— factor the${resources.*}reference resolution out ofappRunner.resolvedConfig()into a package-levelResolveAppConfig. Takes*config.Root(not*bundle.Bundle) to avoid thebundle → direct → dresources → appdeployimport cycle.bundle/phases/deploy_apps.go— newDeployAppsphase. Iteratesb.Config.Resources.Apps, resolves config, callsappdeploy.Deployper app. Accumulates per-app errors viaerrors.Joinso one failed app does not mask the others.bundle/phases/deploy.go— callDeployAppsat the end of the deploy phase (after the core mutator, beforeScriptPostDeploy), gated onb.DeployApps.bundle/bundle.go— newDeployApps boolalongsideAutoApprove.cmd/bundle/deploy.go— wire the--deploy-appsflag throughProcessOptions.InitFunc.bundle/run/app.go— replace the in-lineresolvedConfigmethod with a call to the new package-level helper so both code paths share the same resolution behavior.bundle/appdeploy/config_test.go— unit coverage for the nil / no-config cases.Test plan
go build ./...passes (including the no-import-cycle check that initially flagged my first draft)go test ./bundle/appdeploy/... ./bundle/phases/... ./bundle/run/... ./cmd/bundle/...passesmake fmtcleanapps:resource (follow-up)Follow-ups explicitly out of scope
dyn.Value-driven unit test forResolveAppConfig(requires setting up a populatedconfig.Rootvia the standard bundle load path).appdeploy.Deployretries on in-progress deployments; there may be additional pre-conditions worth surfacing here.This pull request and its description were written by Claude (claude.ai).