Skip to content

Commit 0840a20

Browse files
authored
direct: resolve immutable fields during plan (#4417)
## Changes If field is in recreate_on_changes resource setting then we assume it does not change unless the resource is recreated and can resolve it before deploying the resource (like we do with "id" field). ## Why More precise plan. In some cases this can downgrade 'recreate' to 'update' or 'update' to 'skip'. ## Tests New regression test.
1 parent aaab9a1 commit 0840a20

File tree

8 files changed

+67
-19
lines changed

8 files changed

+67
-19
lines changed
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
bundle:
2+
name: test-bundle
3+
4+
resources:
5+
schemas:
6+
schema:
7+
catalog_name: main
8+
name: schema
9+
volumes:
10+
foo:
11+
catalog_name: main
12+
schema_name: schema
13+
name: volume
14+
comment: comment1
15+
jobs:
16+
bar:
17+
name: job bar
18+
description: storage_location is ${resources.volumes.foo.storage_location}, catalog_name is ${resources.volumes.foo.catalog_name}

acceptance/bundle/resource_deps/immutable_field_ref/out.test.toml

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
2+
>>> [CLI] bundle deploy
3+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
4+
Deploying resources...
5+
Updating deployment state...
6+
Deployment complete!
7+
8+
>>> update_file.py databricks.yml comment1 comment2
9+
10+
>>> [CLI] bundle plan
11+
update volumes.foo
12+
13+
Plan: 0 to add, 1 to change, 0 to delete, 2 unchanged
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
trace $CLI bundle deploy
2+
trace update_file.py databricks.yml "comment1" "comment2"
3+
# Even though we have remote reference in jobs.bar (to volumes.foo.storage_location), since that field is immutable
4+
# we skip updating the job.
5+
trace $CLI bundle plan | contains.py '1 to change, 0 to delete, 2 unchanged' 'update volumes.foo' '!update jobs' '!update schemas'
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
RecordRequests = false
2+
EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]

bundle/direct/bundle_apply.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
160160
}
161161
}
162162

163-
func (b *DeploymentBundle) LookupReferenceRemote(ctx context.Context, path *structpath.PathNode) (any, error) {
163+
func (b *DeploymentBundle) LookupReferencePostDeploy(ctx context.Context, path *structpath.PathNode) (any, error) {
164164
// TODO: Prefix(3) assumes resources.jobs.foo but not resources.jobs.foo.permissions
165165
targetResourceKey := path.Prefix(3).String()
166166
fieldPath := path.SkipPrefix(3)

bundle/direct/bundle_plan.go

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -281,10 +281,9 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
281281
action = getMaxAction(entry.Changes)
282282
}
283283

284-
if action == deployplan.Skip {
285-
// resource is not going to change, can use remoteState to resolve references
286-
b.RemoteStateCache.Store(resourceKey, remoteState)
287-
}
284+
// Note, this unconditionally stores remoteState. However, it may updated post-deploy, so whether
285+
// it can be used for variable resolution depends on several factors, see canReadRemoteCache in LookupReferencePreDeploy
286+
b.RemoteStateCache.Store(resourceKey, remoteState)
288287

289288
// Validate that resources without DoUpdate don't have update actions
290289
if action == deployplan.Update && !adapter.HasDoUpdate() {
@@ -519,8 +518,7 @@ func isEmptyStruct(values ...any) bool {
519518
return true
520519
}
521520

522-
// TODO: calling this "Local" is not right, it can resolve "id" and remote refrences for "skip" targets
523-
func (b *DeploymentBundle) LookupReferenceLocal(ctx context.Context, path *structpath.PathNode) (any, error) {
521+
func (b *DeploymentBundle) LookupReferencePreDeploy(ctx context.Context, path *structpath.PathNode) (any, error) {
524522
// TODO: Prefix(3) assumes resources.jobs.foo but not resources.jobs.foo.permissions
525523
targetResourceKey := path.Prefix(3).String()
526524

@@ -598,15 +596,15 @@ func (b *DeploymentBundle) LookupReferenceLocal(ctx context.Context, path *struc
598596
return value, nil
599597
}
600598

599+
canReadRemoteCache := targetAction == deployplan.Skip || (targetAction.KeepsID() && adapter.IsFieldInRecreateOnChanges(fieldPath))
600+
601601
if configValidErr != nil && remoteValidErr == nil {
602602
// The field is only present in remote state schema.
603-
if targetAction != deployplan.Skip {
604-
// The resource is going to be updated, so remoteState can change
605-
return nil, errDelayed
606-
}
607-
remoteState, ok := b.RemoteStateCache.Load(targetResourceKey)
608-
if ok {
609-
return structaccess.Get(remoteState, fieldPath)
603+
if canReadRemoteCache {
604+
remoteState, ok := b.RemoteStateCache.Load(targetResourceKey)
605+
if ok {
606+
return structaccess.Get(remoteState, fieldPath)
607+
}
610608
}
611609
return nil, errDelayed
612610
}
@@ -619,7 +617,7 @@ func (b *DeploymentBundle) LookupReferenceLocal(ctx context.Context, path *struc
619617
return value, nil
620618
}
621619

622-
if targetAction == deployplan.Skip {
620+
if canReadRemoteCache {
623621
remoteState, ok := b.RemoteStateCache.Load(targetResourceKey)
624622
if ok {
625623
return structaccess.Get(remoteState, fieldPath)
@@ -640,8 +638,6 @@ func (b *DeploymentBundle) getStructVar(resourceKey string) (*structvar.StructVa
640638
}
641639

642640
// resolveReferences processes all references in entry.NewState.Refs.
643-
// If isLocal is true, uses LookupReferenceLocal (for planning phase).
644-
// If isLocal is false, uses LookupReferenceRemote (for apply phase).
645641
func (b *DeploymentBundle) resolveReferences(ctx context.Context, resourceKey string, entry *deployplan.PlanEntry, errorPrefix string, isLocal bool) bool {
646642
sv, err := b.getStructVar(resourceKey)
647643
if err != nil {
@@ -667,7 +663,7 @@ func (b *DeploymentBundle) resolveReferences(ctx context.Context, resourceKey st
667663

668664
var value any
669665
if isLocal {
670-
value, err = b.LookupReferenceLocal(ctx, targetPath)
666+
value, err = b.LookupReferencePreDeploy(ctx, targetPath)
671667
if err != nil {
672668
if errors.Is(err, errDelayed) {
673669
continue
@@ -676,7 +672,7 @@ func (b *DeploymentBundle) resolveReferences(ctx context.Context, resourceKey st
676672
return false
677673
}
678674
} else {
679-
value, err = b.LookupReferenceRemote(ctx, targetPath)
675+
value, err = b.LookupReferencePostDeploy(ctx, targetPath)
680676
if err != nil {
681677
logdiag.LogError(ctx, fmt.Errorf("%s: cannot resolve %q: %w", errorPrefix, ref, err))
682678
return false

bundle/direct/dresources/adapter.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,15 @@ func (a *Adapter) ResourceConfig() *ResourceLifecycleConfig {
355355
return a.resourceConfig
356356
}
357357

358+
func (a *Adapter) IsFieldInRecreateOnChanges(path *structpath.PathNode) bool {
359+
for _, p := range a.resourceConfig.RecreateOnChanges {
360+
if path.HasPrefix(p) {
361+
return true
362+
}
363+
}
364+
return false
365+
}
366+
358367
func (a *Adapter) PrepareState(input any) (any, error) {
359368
outs, err := a.prepareState.Call(input)
360369
if err != nil {

0 commit comments

Comments
 (0)