diff --git a/acceptance/bundle/deploy/readplan/whl-not-uploaded/script b/acceptance/bundle/deploy/readplan/whl-not-uploaded/script deleted file mode 100644 index bfcfeafca9e..00000000000 --- a/acceptance/bundle/deploy/readplan/whl-not-uploaded/script +++ /dev/null @@ -1,28 +0,0 @@ -# A local wheel referenced by a job library is uploaded by `bundle deploy` but NOT by -# `bundle deploy --plan`: applying a saved plan skips the build phase, which is what -# computes the artifact upload list (phases.Build -> libraries.ReplaceWithRemotePath). -# The job still points at the wheel's workspace path, so its run fails at runtime with a -# missing-file error. This test pins the difference between the two deploy paths. - -title "bundle deploy uploads the wheel" -trace $CLI bundle deploy -# An import-file request uploaded the wheel to its .../artifacts/.internal/ workspace path. -# raw_body is the wheel's binary payload; drop it to keep the golden readable. -trace print_requests.py //artifacts/.internal --del-field raw_body - -trace $CLI bundle destroy --auto-approve -# Discard destroy's requests so the saved-plan deploy below starts from a clean recording. -rm "$OUT_REQUESTS" - -title "bundle deploy --plan does NOT upload the wheel (bug)" -trace $CLI bundle plan -o json > tmp.plan.json -# The serial differs between the DMS variants here: destroy removes the recorded deployment, so -# under recording there is no version left to take it from, while the local state keeps counting. -nostamp < tmp.plan.json | jq 'del(.serial)' > out.plan.json -trace $CLI bundle deploy --plan tmp.plan.json -# BUG: no import-file request for the wheel here (output below is empty), yet the job was -# created pointing the task at the same .../artifacts/.internal/ path that was never uploaded. -trace print_requests.py //artifacts/.internal --del-field raw_body - -trace $CLI bundle destroy --auto-approve -rm "$OUT_REQUESTS" diff --git a/acceptance/bundle/deploy/readplan/whl-not-uploaded/databricks.yml b/acceptance/bundle/deploy/readplan/whl-upload/databricks.yml similarity index 70% rename from acceptance/bundle/deploy/readplan/whl-not-uploaded/databricks.yml rename to acceptance/bundle/deploy/readplan/whl-upload/databricks.yml index f67f45222fb..28c39c69f0c 100644 --- a/acceptance/bundle/deploy/readplan/whl-not-uploaded/databricks.yml +++ b/acceptance/bundle/deploy/readplan/whl-upload/databricks.yml @@ -1,10 +1,10 @@ bundle: - name: whl-not-uploaded + name: whl-upload resources: jobs: my_job: - name: whl-not-uploaded-job + name: whl-upload-job tasks: - task_key: main new_cluster: @@ -15,6 +15,5 @@ resources: package_name: my_test_code entry_point: run libraries: - # Local wheel, prebuilt (no build: step) to keep the test hermetic. See script - # for the bug this exercises. Behaves the same as a wheel with a build: step. + # Local wheel, prebuilt (no build: step) to keep the test hermetic. - whl: ./dist/*.whl diff --git a/acceptance/bundle/deploy/readplan/whl-not-uploaded/dist/my_test_code-0.0.1-py3-none-any.whl b/acceptance/bundle/deploy/readplan/whl-upload/dist/my_test_code-0.0.1-py3-none-any.whl similarity index 100% rename from acceptance/bundle/deploy/readplan/whl-not-uploaded/dist/my_test_code-0.0.1-py3-none-any.whl rename to acceptance/bundle/deploy/readplan/whl-upload/dist/my_test_code-0.0.1-py3-none-any.whl diff --git a/acceptance/bundle/deploy/readplan/whl-not-uploaded/out.plan.json b/acceptance/bundle/deploy/readplan/whl-upload/out.plan.json similarity index 86% rename from acceptance/bundle/deploy/readplan/whl-not-uploaded/out.plan.json rename to acceptance/bundle/deploy/readplan/whl-upload/out.plan.json index cc6ec22302d..15daca145a0 100644 --- a/acceptance/bundle/deploy/readplan/whl-not-uploaded/out.plan.json +++ b/acceptance/bundle/deploy/readplan/whl-upload/out.plan.json @@ -2,6 +2,7 @@ "plan_version": [PLAN_VERSION], "cli_version": "[CLI_VERSION]", "lineage": "[UUID]", + "serial": 2, "plan": { "resources.jobs.my_job": { "action": "create", @@ -9,12 +10,12 @@ "value": { "deployment": { "kind": "BUNDLE", - "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/whl-not-uploaded/default/state/metadata.json" + "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/whl-upload/default/state/metadata.json" }, "edit_mode": "UI_LOCKED", "format": "MULTI_TASK", "max_concurrent_runs": 1, - "name": "whl-not-uploaded-job", + "name": "whl-upload-job", "queue": { "enabled": true }, @@ -22,7 +23,7 @@ { "libraries": [ { - "whl": "/Workspace/Users/[USERNAME]/.bundle/whl-not-uploaded/default/artifacts/.internal/my_test_code-0.0.1-py3-none-any.whl" + "whl": "/Workspace/Users/[USERNAME]/.bundle/whl-upload/default/artifacts/.internal/my_test_code-0.0.1-py3-none-any.whl" } ], "new_cluster": { diff --git a/acceptance/bundle/deploy/readplan/whl-not-uploaded/out.test.toml b/acceptance/bundle/deploy/readplan/whl-upload/out.test.toml similarity index 68% rename from acceptance/bundle/deploy/readplan/whl-not-uploaded/out.test.toml rename to acceptance/bundle/deploy/readplan/whl-upload/out.test.toml index 59b56a2037c..27ec2a7fcd6 100644 --- a/acceptance/bundle/deploy/readplan/whl-not-uploaded/out.test.toml +++ b/acceptance/bundle/deploy/readplan/whl-upload/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deploy/readplan/whl-not-uploaded/output.txt b/acceptance/bundle/deploy/readplan/whl-upload/output.txt similarity index 66% rename from acceptance/bundle/deploy/readplan/whl-not-uploaded/output.txt rename to acceptance/bundle/deploy/readplan/whl-upload/output.txt index 981e942ee4f..8e6b5430f39 100644 --- a/acceptance/bundle/deploy/readplan/whl-not-uploaded/output.txt +++ b/acceptance/bundle/deploy/readplan/whl-upload/output.txt @@ -2,7 +2,7 @@ === bundle deploy uploads the wheel >>> [CLI] bundle deploy Uploading dist/my_test_code-0.0.1-py3-none-any.whl... -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/whl-not-uploaded/default/files... +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/whl-upload/default/files... Created jobs.my_job Files: 4 uploaded, 0 deleted Resources: 1 created, 0 changed, 0 deleted, 0 unchanged @@ -10,7 +10,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged >>> print_requests.py //artifacts/.internal --del-field raw_body { "method": "POST", - "path": "/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/whl-not-uploaded/default/artifacts/.internal/my_test_code-0.0.1-py3-none-any.whl", + "path": "/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/whl-upload/default/artifacts/.internal/my_test_code-0.0.1-py3-none-any.whl", "q": { "overwrite": "true" } @@ -20,25 +20,31 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged The following resources will be deleted: delete resources.jobs.my_job -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/whl-not-uploaded/default +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/whl-upload/default Destroy: 1 deleted -=== bundle deploy --plan does NOT upload the wheel (bug) ->>> [CLI] bundle plan -o json - +=== bundle deploy --plan also uploads the wheel >>> [CLI] bundle deploy --plan tmp.plan.json -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/whl-not-uploaded/default/files... +Uploading dist/my_test_code-0.0.1-py3-none-any.whl... +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/whl-upload/default/files... Created jobs.my_job Files: 6 uploaded, 0 deleted Resources: 1 created, 0 changed, 0 deleted, 0 unchanged >>> print_requests.py //artifacts/.internal --del-field raw_body +{ + "method": "POST", + "path": "/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/whl-upload/default/artifacts/.internal/my_test_code-0.0.1-py3-none-any.whl", + "q": { + "overwrite": "true" + } +} >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: delete resources.jobs.my_job -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/whl-not-uploaded/default +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/whl-upload/default Destroy: 1 deleted diff --git a/acceptance/bundle/deploy/readplan/whl-upload/script b/acceptance/bundle/deploy/readplan/whl-upload/script new file mode 100644 index 00000000000..c06707c5dd1 --- /dev/null +++ b/acceptance/bundle/deploy/readplan/whl-upload/script @@ -0,0 +1,27 @@ +# A local wheel referenced by a job library must be uploaded to its .../artifacts/.internal/ +# workspace path by both `bundle deploy` and `bundle deploy --plan`. Applying a saved plan +# runs phases.FindLibraries to discover which local files to upload without re-running any +# build commands. This test pins that both deploy paths upload the wheel identically. + +title "bundle deploy uploads the wheel" +trace $CLI bundle deploy +# An import-file request uploaded the wheel to its .../artifacts/.internal/ workspace path. +# raw_body is the wheel's binary payload; drop it to keep the golden readable. +trace print_requests.py //artifacts/.internal --del-field raw_body + +trace $CLI bundle destroy --auto-approve +# Discard destroy's requests so the saved-plan deploy below starts from a clean recording. +rm "$OUT_REQUESTS" + +title "bundle deploy --plan also uploads the wheel" +# tmp.plan.json is the live plan (with features/serial for validation); +# out.plan.json is the nostamp-stripped golden for cross-variant comparison. +$CLI bundle plan -o json > tmp.plan.json +$CLI bundle plan -o json | nostamp > out.plan.json +trace $CLI bundle deploy --plan tmp.plan.json +# The same import-file request uploads the wheel to the .../artifacts/.internal/ path the +# job task points at, matching the plain `bundle deploy` above. +trace print_requests.py //artifacts/.internal --del-field raw_body + +trace $CLI bundle destroy --auto-approve +rm "$OUT_REQUESTS" diff --git a/acceptance/bundle/deploy/readplan/whl-upload/test.toml b/acceptance/bundle/deploy/readplan/whl-upload/test.toml new file mode 100644 index 00000000000..8e5c2176367 --- /dev/null +++ b/acceptance/bundle/deploy/readplan/whl-upload/test.toml @@ -0,0 +1,6 @@ +# --plan is direct-engine only. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +# Saved plans don't carry the deployment stamp, so this test skips the DMS variant. +EnvMatrix.DMS = [""] +RecordRequests = true +Ignore = [".databricks", "tmp.plan.json"] diff --git a/bundle/artifacts/expand_globs.go b/bundle/artifacts/expand_globs.go index af358d3dda9..484ed05deae 100644 --- a/bundle/artifacts/expand_globs.go +++ b/bundle/artifacts/expand_globs.go @@ -3,7 +3,9 @@ package artifacts import ( "context" "fmt" + "maps" "path/filepath" + "slices" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" @@ -122,3 +124,24 @@ func (e expandGlobs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnosti return diags } + +type expandAllGlobs struct{} + +func (e *expandAllGlobs) Name() string { + return "artifacts.ExpandGlobReferences" +} + +func (e *expandAllGlobs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + for _, name := range slices.Sorted(maps.Keys(b.Config.Artifacts)) { + bundle.ApplyContext(ctx, b, expandGlobs{name: name}) + } + return nil +} + +// ExpandGlobReferences returns a mutator that expands glob patterns in artifact +// file sources for every artifact. Prepare handles artifacts without a build +// command; Build handles those with one. This mutator covers both, making it +// safe to call after Prepare but before a build has run (e.g. FindLibraries). +func ExpandGlobReferences() bundle.Mutator { + return &expandAllGlobs{} +} diff --git a/bundle/phases/build.go b/bundle/phases/build.go index f8069b73bcf..a6e8a31c1fe 100644 --- a/bundle/phases/build.go +++ b/bundle/phases/build.go @@ -18,15 +18,13 @@ import ( // Computed by Build and consumed by Deploy to upload the right files. type LibLocationMap map[string][]libraries.LocationToUpdate -// Build runs the build phase, which builds artifacts. -func Build(ctx context.Context, b *bundle.Bundle) LibLocationMap { - log.Info(ctx, "Phase: build") - - bundle.ApplySeqContext(ctx, b, - scripts.Execute(config.ScriptPreBuild), - artifacts.Build(), - scripts.Execute(config.ScriptPostBuild), - +// resolveLibraries runs variable resolution, glob expansion, path rewriting, +// and wheel-task transformation to produce the local→remote upload map. +// extra mutators are applied after CheckForSameNameLibraries and before +// ReplaceWithRemotePath; Build passes libraries.SwitchToPatchedWheels() there. +func resolveLibraries(ctx context.Context, b *bundle.Bundle, extra ...bundle.Mutator) LibLocationMap { + mutators := make([]bundle.Mutator, 0, 4+len(extra)) + mutators = append(mutators, mutator.ResolveVariableReferencesWithoutResources( "artifacts", ), @@ -34,15 +32,22 @@ func Build(ctx context.Context, b *bundle.Bundle) LibLocationMap { "artifacts", ), + // Expand artifact file globs (e.g. dist/*.whl in artifacts[*].files[*].source). + // Prepare() skips this for artifacts with a build command because the files don't + // exist yet; Build() does it after running the command. For FindLibraries (plan + // apply) the artifacts were already built at plan time, so we expand here instead. + // Safe to call from Build too: already-expanded paths are left unchanged. + artifacts.ExpandGlobReferences(), + // libraries.CheckForSameNameLibraries() needs to be run after we expand glob references so we // know what are the actual library paths. // libraries.ExpandGlobReferences() has to be run after the libraries are built and thus this // mutator is part of the deploy step rather than validate. libraries.ExpandGlobReferences(), libraries.CheckForSameNameLibraries(), - // SwitchToPatchedWheels must be run after ExpandGlobReferences and after build phase because it Artifact.Source and Artifact.Patched populated - libraries.SwitchToPatchedWheels(), ) + mutators = append(mutators, extra...) + bundle.ApplySeqContext(ctx, b, mutators...) if logdiag.HasError(ctx) { return nil @@ -55,3 +60,32 @@ func Build(ctx context.Context, b *bundle.Bundle) LibLocationMap { bundle.ApplyContext(ctx, b, trampoline.TransformWheelTask()) return libs } + +// Build runs the build phase, which builds artifacts. +func Build(ctx context.Context, b *bundle.Bundle) LibLocationMap { + log.Info(ctx, "Phase: build") + + bundle.ApplySeqContext(ctx, b, + scripts.Execute(config.ScriptPreBuild), + artifacts.Build(), + scripts.Execute(config.ScriptPostBuild), + ) + + if logdiag.HasError(ctx) { + return nil + } + + // SwitchToPatchedWheels must be passed to resolveLibraries so it runs after + // ExpandGlobReferences (which expands *.whl patterns in job library paths) + // and after the build phase (which populates Artifact.Source and Artifact.Patched). + return resolveLibraries(ctx, b, libraries.SwitchToPatchedWheels()) +} + +// FindLibraries discovers which local library files need uploading by reading +// the bundle config (glob expansion and path rewriting) without running any +// build commands. Used when applying a saved plan: artifacts were already built +// at plan time and the plan's new_state carries the correct remote paths. +func FindLibraries(ctx context.Context, b *bundle.Bundle) LibLocationMap { + log.Info(ctx, "Phase: find libraries") + return resolveLibraries(ctx, b) +} diff --git a/cmd/bundle/utils/process.go b/cmd/bundle/utils/process.go index 2dcd549eba2..b98f859f149 100644 --- a/cmd/bundle/utils/process.go +++ b/cmd/bundle/utils/process.go @@ -82,7 +82,10 @@ type ProcessOptions struct { Deploy bool // Path to pre-computed plan JSON file (direct engine only). - // When set, skips Build and PreDeployChecks phases, loads plan from file instead of calculating. + // When set, skips build commands and the PreDeployChecks phase, and loads the + // plan from the file instead of calculating it. Library file discovery runs + // (via FindLibraries) so that the correct local files are found and uploaded + // to the remote paths the plan already references. ReadPlanPath string // PostStateFunc is called at the end of ProcessBundleRet, within the state lifecycle scope @@ -352,14 +355,21 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle } var plan *deployplan.Plan - + var findLibraries bool if opts.ReadPlanPath != "" { if !stateDesc.Engine.IsDirect() { logdiag.LogError(ctx, errors.New("--plan is only supported with direct engine (set bundle.engine to \"direct\" or DATABRICKS_BUNDLE_ENGINE=direct)")) return b, stateDesc, root.ErrAlreadyPrinted } + // Build commands are not re-run when applying a saved plan: re-running them + // could produce new artifact files with a different mtime which, with + // dynamic_version, would yield a different patched filename than the remote + // path the plan already has baked in. Library discovery still runs (via + // FindLibraries) so the correct local files are found and uploaded to the + // remote paths the plan references. opts.Build = false opts.PreDeployChecks = false + findLibraries = true var err error plan, err = deployplan.LoadPlanFromFile(opts.ReadPlanPath) @@ -419,6 +429,17 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle Value: time.Since(t2).Milliseconds(), }) + if logdiag.HasError(ctx) { + return b, stateDesc, root.ErrAlreadyPrinted + } + } else if findLibraries { + t2 := time.Now() + libs = phases.FindLibraries(ctx, b) + b.Metrics.ExecutionTimes = append(b.Metrics.ExecutionTimes, protos.IntMapEntry{ + Key: "phases.FindLibraries", + Value: time.Since(t2).Milliseconds(), + }) + if logdiag.HasError(ctx) { return b, stateDesc, root.ErrAlreadyPrinted }