Skip to content

Commit 6c9e3d6

Browse files
authored
Configure __apply_policy_default_values_allow_list for task and job clusters (#3255)
## Changes Automatically reapply cluster policy defaults for task and job clusters when updating existing jobs. Note: if you relied on task and job cluster settings that originate from a cluster policy _not_ changing on deploy, even though the cluster policy defaults they were drawn from did change, then you'll need to explicitly include those values in your bundle configuration from now on. ## Why The latest version of the Terraform provider comes with a feature specifically for re-applying cluster policy defaults for task and job clusters when updating a job (databricks/terraform-provider-databricks#4834). Before this change, it was not possible to reapply cluster policy defaults for an existing job, as Terraform keeps server-side default values in its state. Users had to destroy and recreate jobs for new defaults to take effect. With the Terraform dependency in DABs, the same happened for DABs users, and cluster policy defaults were effectively "snapshotted" on the first deploy, and never updated thereafter. This is contrary to the behavior we expect to see. Unless a field is explicitly configured in the bundle configuration, cluster policy defaults should be applied on every deploy, regardless of the job being created or updated. This change passes the list of fields explicitly specified in the bundle configuration to Terraform via the `__allow_policy_default_values_allow_list` property introduced in the linked PR. ## Tests * Unit tests pass. * Manually confirmed that when the cluster policy for a job cluster is changed, the fields that aren't explicitly specified in the bundle configuration draw their default values from the new cluster policy. <!-- If your PR needs to be included in the release notes for next release, add a separate entry in NEXT_CHANGELOG.md as part of your PR. -->
1 parent d383899 commit 6c9e3d6

File tree

5 files changed

+247
-0
lines changed

5 files changed

+247
-0
lines changed

NEXT_CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,6 @@
1212

1313
### Bundles
1414

15+
* Jobs that use cluster policy default values for their cluster configuration now correctly update those defaults on every deployment ([#3255](https://github.com/databricks/cli/pull/3255)).
16+
1517
### API Changes

bundle/deploy/terraform/tfdyn/convert_job.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ package tfdyn
33
import (
44
"context"
55
"fmt"
6+
"slices"
67
"sort"
8+
"strings"
79

810
"github.com/databricks/cli/bundle/internal/tf/schema"
911
"github.com/databricks/cli/libs/dyn"
@@ -12,6 +14,58 @@ import (
1214
"github.com/databricks/databricks-sdk-go/service/jobs"
1315
)
1416

17+
func patchApplyPolicyDefaultValues(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
18+
// If the field "apply_policy_default_values" is not set, do nothing.
19+
if b, ok := v.Get("apply_policy_default_values").AsBool(); !ok || !b {
20+
return v, nil
21+
}
22+
23+
// If the field "policy_id" is not set, do nothing.
24+
if _, ok := v.Get("policy_id").AsString(); !ok {
25+
return v, nil
26+
}
27+
28+
// The field "apply_policy_default_values" is set.
29+
// We need to collect the list of fields that are set explicitly
30+
// and pass it to Terraform. This enables Terraform to clear
31+
// server-side defaults from the update request, which in turn
32+
// allows the backend to re-apply the policy defaults.
33+
//
34+
// For more details, see: https://github.com/databricks/terraform-provider-databricks/pull/4834
35+
//
36+
paths := dyn.CollectLeafPaths(v)
37+
38+
// If any of the map or sequence fields are set, always include them entirely instead of traversing the them.
39+
for _, field := range []string{
40+
"custom_tags",
41+
"init_scripts",
42+
"spark_conf",
43+
"spark_env_vars",
44+
"ssh_public_keys",
45+
} {
46+
if vv := v.Get(field); vv.IsValid() {
47+
// Remove all paths that start with the field.
48+
paths = slices.DeleteFunc(paths, func(p string) bool {
49+
return strings.HasPrefix(p, field+".") || strings.HasPrefix(p, field+"[")
50+
})
51+
// Add the field to the paths.
52+
paths = append(paths, field)
53+
}
54+
}
55+
56+
sort.Strings(paths)
57+
valList := make([]dyn.Value, len(paths))
58+
for i, s := range paths {
59+
valList[i] = dyn.V(s)
60+
}
61+
v, err := dyn.Set(v, "__apply_policy_default_values_allow_list", dyn.V(valList))
62+
if err != nil {
63+
return dyn.InvalidValue, err
64+
}
65+
66+
return v, nil
67+
}
68+
1569
func convertJobResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) {
1670
// Normalize the input value to the underlying job schema.
1771
// This removes superfluous keys and adapts the input to the expected schema.
@@ -101,6 +155,22 @@ func convertJobResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) {
101155
log.Debugf(ctx, "job normalization diagnostic: %s", diag.Summary)
102156
}
103157

158+
// Apply __apply_policy_default_values_allow_list for tasks
159+
vout, err = dyn.Map(vout, "task", dyn.Foreach(func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
160+
return dyn.Map(v, "new_cluster", patchApplyPolicyDefaultValues)
161+
}))
162+
if err != nil {
163+
return dyn.InvalidValue, err
164+
}
165+
166+
// Apply __apply_policy_default_values_allow_list for job clusters
167+
vout, err = dyn.Map(vout, "job_cluster", dyn.Foreach(func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
168+
return dyn.Map(v, "new_cluster", patchApplyPolicyDefaultValues)
169+
}))
170+
if err != nil {
171+
return dyn.InvalidValue, err
172+
}
173+
104174
return vout, err
105175
}
106176

bundle/deploy/terraform/tfdyn/convert_job_test.go

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,140 @@ func TestConvertJob(t *testing.T) {
149149
},
150150
}, out.Permissions["job_my_job"])
151151
}
152+
153+
func TestConvertJobApplyPolicyDefaultValues(t *testing.T) {
154+
src := resources.Job{
155+
JobSettings: jobs.JobSettings{
156+
Name: "my job",
157+
JobClusters: []jobs.JobCluster{
158+
{
159+
JobClusterKey: "key",
160+
NewCluster: compute.ClusterSpec{
161+
ApplyPolicyDefaultValues: true,
162+
PolicyId: "policy_id",
163+
GcpAttributes: &compute.GcpAttributes{
164+
Availability: "SPOT",
165+
LocalSsdCount: 2,
166+
},
167+
},
168+
},
169+
{
170+
JobClusterKey: "key2",
171+
NewCluster: compute.ClusterSpec{
172+
ApplyPolicyDefaultValues: true,
173+
PolicyId: "policy_id2",
174+
CustomTags: map[string]string{
175+
"key": "value",
176+
},
177+
InitScripts: []compute.InitScriptInfo{
178+
{
179+
Workspace: &compute.WorkspaceStorageInfo{
180+
Destination: "/Workspace/path/to/init_script1",
181+
},
182+
},
183+
{
184+
Workspace: &compute.WorkspaceStorageInfo{
185+
Destination: "/Workspace/path/to/init_script2",
186+
},
187+
},
188+
},
189+
SparkConf: map[string]string{
190+
"key": "value",
191+
},
192+
SparkEnvVars: map[string]string{
193+
"key": "value",
194+
},
195+
SshPublicKeys: []string{
196+
"ssh-rsa 1234",
197+
},
198+
},
199+
},
200+
{
201+
JobClusterKey: "key3",
202+
NewCluster: compute.ClusterSpec{
203+
ApplyPolicyDefaultValues: true,
204+
SparkVersion: "16.4.x-scala2.12",
205+
},
206+
},
207+
},
208+
},
209+
}
210+
211+
vin, err := convert.FromTyped(src, dyn.NilValue)
212+
require.NoError(t, err)
213+
214+
ctx := context.Background()
215+
out := schema.NewResources()
216+
err = jobConverter{}.Convert(ctx, "my_job", vin, out)
217+
require.NoError(t, err)
218+
219+
assert.Equal(t, map[string]any{
220+
"name": "my job",
221+
"job_cluster": []any{
222+
map[string]any{
223+
"job_cluster_key": "key",
224+
"new_cluster": map[string]any{
225+
"__apply_policy_default_values_allow_list": []any{
226+
"apply_policy_default_values",
227+
"gcp_attributes.availability",
228+
"gcp_attributes.local_ssd_count",
229+
"policy_id",
230+
},
231+
"apply_policy_default_values": true,
232+
"policy_id": "policy_id",
233+
"gcp_attributes": map[string]any{
234+
"availability": "SPOT",
235+
"local_ssd_count": int64(2),
236+
},
237+
},
238+
},
239+
map[string]any{
240+
"job_cluster_key": "key2",
241+
"new_cluster": map[string]any{
242+
"__apply_policy_default_values_allow_list": []any{
243+
"apply_policy_default_values",
244+
"custom_tags",
245+
"init_scripts",
246+
"policy_id",
247+
"spark_conf",
248+
"spark_env_vars",
249+
"ssh_public_keys",
250+
},
251+
"apply_policy_default_values": true,
252+
"policy_id": "policy_id2",
253+
"custom_tags": map[string]any{
254+
"key": "value",
255+
},
256+
"init_scripts": []any{
257+
map[string]any{
258+
"workspace": map[string]any{
259+
"destination": "/Workspace/path/to/init_script1",
260+
},
261+
},
262+
map[string]any{
263+
"workspace": map[string]any{
264+
"destination": "/Workspace/path/to/init_script2",
265+
},
266+
},
267+
},
268+
"spark_conf": map[string]any{
269+
"key": "value",
270+
},
271+
"spark_env_vars": map[string]any{
272+
"key": "value",
273+
},
274+
"ssh_public_keys": []any{
275+
"ssh-rsa 1234",
276+
},
277+
},
278+
},
279+
map[string]any{
280+
"job_cluster_key": "key3",
281+
"new_cluster": map[string]any{
282+
"apply_policy_default_values": true,
283+
"spark_version": "16.4.x-scala2.12",
284+
},
285+
},
286+
},
287+
}, out.Job["my_job"])
288+
}

libs/dyn/walk.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,26 @@ func walk(v Value, p Path, fn func(p Path, v Value) (Value, error)) (Value, erro
6666

6767
return v, nil
6868
}
69+
70+
// CollectLeafPaths traverses the value and returns all paths (as dot notation strings) to leaf nodes (non-map, non-sequence).
71+
// The return value is not ordered.
72+
func CollectLeafPaths(v Value) []string {
73+
var paths []string
74+
75+
Walk(v, func(p Path, v Value) (Value, error) { //nolint:errcheck
76+
if len(p) == 0 {
77+
return v, nil
78+
}
79+
80+
switch v.Kind() {
81+
case KindMap, KindSequence:
82+
// Ignore internal nodes.
83+
default:
84+
paths = append(paths, p.String())
85+
}
86+
87+
return v, nil
88+
})
89+
90+
return paths
91+
}

libs/dyn/walk_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,3 +252,18 @@ func TestWalkSequenceError(t *testing.T) {
252252
assert.Equal(t, MustPathFromString(".[1]"), tracker.calls[2].path)
253253
assert.Equal(t, V("bar"), tracker.calls[2].value)
254254
}
255+
256+
func TestCollectLeafPaths(t *testing.T) {
257+
v := V(map[string]Value{
258+
"a": V(1),
259+
"b": V(map[string]Value{
260+
"c": V(2),
261+
"d": V(map[string]Value{
262+
"e": V(3),
263+
}),
264+
}),
265+
"f": V([]Value{V(4), V(5)}),
266+
})
267+
paths := CollectLeafPaths(v)
268+
assert.ElementsMatch(t, []string{"a", "b.c", "b.d.e", "f[0]", "f[1]"}, paths)
269+
}

0 commit comments

Comments
 (0)