Skip to content

Commit 796b4f9

Browse files
denikdenik
authored
Resolve remote references during plan for "skip" targets (#3993)
## Changes If target action is "skip", resolve remote references for it using fetched remote state. ## Why Otherwise there is permanent drift when remote references are used. New behaviour also matches terraform. Originally we did resolve remote references but that was changed in #3636 ## Tests Existing tests.
1 parent ed84fe9 commit 796b4f9

File tree

9 files changed

+131
-67
lines changed

9 files changed

+131
-67
lines changed

‎acceptance/bundle/resource_deps/jobs_update_remote/output.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ Deployment complete!
5858
}
5959
}
6060

61+
>>> [CLI] bundle plan
62+
Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged
63+
6164
=== Update trigger.periodic.unit remotely and re-deploy; jobs.bar is unchanged
6265
>>> envsubst
6366

‎acceptance/bundle/resource_deps/jobs_update_remote/script‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ $CLI bundle plan -o json > out.plan_create.$DATABRICKS_BUNDLE_ENGINE.json
33
trace $CLI bundle deploy
44
trace print_requests.py //jobs
55

6+
trace $CLI bundle plan
7+
68
foo_id=`read_id.py jobs foo`
79
echo "$foo_id:FOO_ID" >> ACC_REPLS
810
export foo_id

‎acceptance/bundle/resource_deps/remote_field_storage_location/out.plan_after_deploy.direct.json‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
{
2+
"plan": {
3+
"resources.schemas.my": {
4+
"action": "skip",
5+
"remote_state": {
6+
"browse_only": false,
7+
"catalog_name": "main",
8+
"catalog_type": "MANAGED_CATALOG",
9+
"created_at": [UNIX_TIME_MILLIS][0],
10+
"created_by": "[USERNAME]",
11+
"full_name": "main.myschema-[UNIQUE_NAME]",
12+
"name": "myschema-[UNIQUE_NAME]",
13+
"owner": "[USERNAME]",
14+
"updated_at": [UNIX_TIME_MILLIS][0],
15+
"updated_by": "[USERNAME]"
16+
}
17+
},
18+
"resources.volumes.bar": {
19+
"depends_on": [
20+
{
21+
"node": "resources.schemas.my",
22+
"label": "${resources.schemas.my.name}"
23+
}
24+
],
25+
"action": "skip",
26+
"remote_state": {
27+
"catalog_name": "main",
28+
"created_at": [UNIX_TIME_MILLIS][1],
29+
"created_by": "[USERNAME]",
30+
"full_name": "main.myschema-[UNIQUE_NAME].volumebar-[UNIQUE_NAME]",
31+
"name": "volumebar-[UNIQUE_NAME]",
32+
"owner": "[USERNAME]",
33+
"schema_name": "myschema-[UNIQUE_NAME]",
34+
"storage_location": "s3://deco-uc-prod-isolated-aws-us-east-1/metastore/[UUID]/volumes/[UUID]",
35+
"updated_at": [UNIX_TIME_MILLIS][1],
36+
"volume_type": "MANAGED"
37+
},
38+
"changes": {
39+
"remote": {
40+
"storage_location": {
41+
"action": "skip",
42+
"reason": "server_side_default"
43+
}
44+
}
45+
}
46+
},
47+
"resources.volumes.foo": {
48+
"depends_on": [
49+
{
50+
"node": "resources.schemas.my",
51+
"label": "${resources.schemas.my.name}"
52+
},
53+
{
54+
"node": "resources.volumes.bar",
55+
"label": "${resources.volumes.bar.storage_location}"
56+
}
57+
],
58+
"action": "skip",
59+
"remote_state": {
60+
"catalog_name": "main",
61+
"comment": "s3://deco-uc-prod-isolated-aws-us-east-1/metastore/[UUID]/volumes/[UUID]",
62+
"created_at": [UNIX_TIME_MILLIS][2],
63+
"created_by": "[USERNAME]",
64+
"full_name": "main.myschema-[UNIQUE_NAME].volumefoo-[UNIQUE_NAME]",
65+
"name": "volumefoo-[UNIQUE_NAME]",
66+
"owner": "[USERNAME]",
67+
"schema_name": "myschema-[UNIQUE_NAME]",
68+
"storage_location": "s3://deco-uc-prod-isolated-aws-us-east-1/metastore/[UUID]/volumes/[UUID]",
69+
"updated_at": [UNIX_TIME_MILLIS][2],
70+
"volume_type": "MANAGED"
71+
},
72+
"changes": {
73+
"remote": {
74+
"storage_location": {
75+
"action": "skip",
76+
"reason": "server_side_default"
77+
}
78+
}
79+
}
80+
}
81+
}
82+
}

‎acceptance/bundle/resource_deps/remote_field_storage_location/out.plan_after_deploy.direct.txt‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1 @@
1-
update volumes.foo
2-
3-
Plan: 0 to add, 1 to change, 0 to delete, 2 unchanged
1+
Plan: 0 to add, 0 to change, 0 to delete, 3 unchanged

‎acceptance/bundle/resource_deps/remote_field_storage_location/out.plan_after_deploy.terraform.json‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"plan": {
3+
"resources.schemas.my": {
4+
"action": "skip"
5+
},
6+
"resources.volumes.bar": {
7+
"action": "skip"
8+
},
9+
"resources.volumes.foo": {
10+
"action": "create"
11+
}
12+
}
13+
}

‎acceptance/bundle/resource_deps/remote_field_storage_location/script‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ print_requests.py --get //unity &> out.deploy.requests.$DATABRICKS_BUNDLE_ENGINE
1313

1414
# Should show empty plan for direct, but shows update in volumes.foo (permanent drift)
1515
$CLI bundle plan &> out.plan_after_deploy.$DATABRICKS_BUNDLE_ENGINE.txt
16+
$CLI bundle plan -o json &> out.plan_after_deploy.$DATABRICKS_BUNDLE_ENGINE.json

‎acceptance/bundle/resource_deps/remote_pipeline/out.plan_skip.direct.json‎

Lines changed: 3 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,7 @@
3737
"label": "${resources.pipelines.foo1.creator_user_name}"
3838
}
3939
],
40-
"action": "update",
41-
"new_state": {
42-
"value": {
43-
"channel": "CURRENT",
44-
"deployment": {
45-
"kind": "BUNDLE",
46-
"metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
47-
},
48-
"edition": "ADVANCED",
49-
"name": "foo2 foo1.creator_user_name=${resources.pipelines.foo1.creator_user_name}"
50-
},
51-
"vars": {
52-
"name": "foo2 foo1.creator_user_name=${resources.pipelines.foo1.creator_user_name}"
53-
}
54-
},
40+
"action": "skip",
5541
"remote_state": {
5642
"creator_user_name": "[USERNAME]",
5743
"last_modified": [UNIX_TIME_MILLIS],
@@ -72,11 +58,6 @@
7258
"state": "IDLE"
7359
},
7460
"changes": {
75-
"local": {
76-
"name": {
77-
"action": "update"
78-
}
79-
},
8061
"remote": {
8162
"storage": {
8263
"action": "skip",
@@ -92,21 +73,7 @@
9273
"label": "${resources.pipelines.foo2.state}"
9374
}
9475
],
95-
"action": "update",
96-
"new_state": {
97-
"value": {
98-
"channel": "CURRENT",
99-
"deployment": {
100-
"kind": "BUNDLE",
101-
"metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
102-
},
103-
"edition": "ADVANCED",
104-
"name": "foo3 foo2.state=${resources.pipelines.foo2.state}"
105-
},
106-
"vars": {
107-
"name": "foo3 foo2.state=${resources.pipelines.foo2.state}"
108-
}
109-
},
76+
"action": "skip",
11077
"remote_state": {
11178
"creator_user_name": "[USERNAME]",
11279
"last_modified": [UNIX_TIME_MILLIS],
@@ -127,11 +94,6 @@
12794
"state": "IDLE"
12895
},
12996
"changes": {
130-
"local": {
131-
"name": {
132-
"action": "update"
133-
}
134-
},
13597
"remote": {
13698
"storage": {
13799
"action": "skip",
@@ -147,21 +109,7 @@
147109
"label": "${resources.pipelines.foo3.last_modified}"
148110
}
149111
],
150-
"action": "update",
151-
"new_state": {
152-
"value": {
153-
"channel": "CURRENT",
154-
"deployment": {
155-
"kind": "BUNDLE",
156-
"metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
157-
},
158-
"edition": "ADVANCED",
159-
"name": "foo4 foo3.last_modified=${resources.pipelines.foo3.last_modified}"
160-
},
161-
"vars": {
162-
"name": "foo4 foo3.last_modified=${resources.pipelines.foo3.last_modified}"
163-
}
164-
},
112+
"action": "skip",
165113
"remote_state": {
166114
"creator_user_name": "[USERNAME]",
167115
"last_modified": [UNIX_TIME_MILLIS],
@@ -182,11 +130,6 @@
182130
"state": "IDLE"
183131
},
184132
"changes": {
185-
"local": {
186-
"name": {
187-
"action": "update"
188-
}
189-
},
190133
"remote": {
191134
"storage": {
192135
"action": "skip",

‎bundle/direct/bundle_apply.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
2525
}
2626

2727
b.StateDB.AssertOpened()
28+
b.RemoteStateCache.Clear()
2829

2930
g, err := makeGraph(plan)
3031
if err != nil {

‎bundle/direct/bundle_plan.go‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,13 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
196196
remoteAction, remoteChangeMap = interpretOldStateVsRemoteState(ctx, adapter, remoteDiff, remoteState)
197197
}
198198

199-
entry.Action = max(localAction, remoteAction).String()
199+
action := max(localAction, remoteAction)
200+
if action == deployplan.ActionTypeSkip {
201+
// resource is not going to change, can use remoteState to resolve references
202+
b.RemoteStateCache.Store(resourceKey, remoteState)
203+
}
204+
205+
entry.Action = action.String()
200206

201207
if len(localChangeMap) > 0 || len(remoteChangeMap) > 0 {
202208
entry.Changes = &deployplan.Changes{
@@ -276,9 +282,11 @@ func interpretOldStateVsRemoteState(ctx context.Context, adapter *dresources.Ada
276282
return action, m
277283
}
278284

285+
// TODO: calling this "Local" is not right, it can resolve "id" and remote refrences for "skip" targets
279286
func (b *DeploymentBundle) LookupReferenceLocal(ctx context.Context, path *structpath.PathNode) (any, error) {
280287
// TODO: Prefix(3) assumes resources.jobs.foo but not resources.jobs.foo.permissions
281288
targetResourceKey := path.Prefix(3).String()
289+
282290
fieldPath := path.SkipPrefix(3)
283291
fieldPathS := fieldPath.String()
284292

@@ -349,19 +357,32 @@ func (b *DeploymentBundle) LookupReferenceLocal(ctx context.Context, path *struc
349357

350358
if configValidErr != nil && remoteValidErr == nil {
351359
// The field is only present in remote state schema.
352-
// TODO: If resource is unchanged in this plan, we can proceed with resolution.
353-
// If resource is going to change, we need to postpone this until deploy.
360+
if targetAction != deployplan.ActionTypeSkip {
361+
// The resource is going to be updated, so remoteState can change
362+
return nil, errDelayed
363+
}
364+
remoteState, ok := b.RemoteStateCache.Load(targetResourceKey)
365+
if ok {
366+
return structaccess.Get(remoteState, fieldPath)
367+
}
354368
return nil, errDelayed
355369
}
356370

357-
// Field is present in both: try local, fallback to delayed.
371+
// Field is present in both: try local, fallback to remote (if skip) then delayed.
358372

359373
value, err := structaccess.Get(localConfig, fieldPath)
360374

361375
if err == nil && value != nil {
362376
return value, nil
363377
}
364378

379+
if targetAction == deployplan.ActionTypeSkip {
380+
remoteState, ok := b.RemoteStateCache.Load(targetResourceKey)
381+
if ok {
382+
return structaccess.Get(remoteState, fieldPath)
383+
}
384+
}
385+
365386
return nil, errDelayed
366387
}
367388

0 commit comments

Comments
 (0)