bundle: add --deploy-apps to push source code during bundle deploy#5055
Open
jamesbroadhead wants to merge 2 commits intodatabricks:mainfrom
Open
bundle: add --deploy-apps to push source code during bundle deploy#5055jamesbroadhead wants to merge 2 commits intodatabricks:mainfrom
jamesbroadhead 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
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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).