diff --git a/bundle/config/mutator/sort_job_tasks.go b/bundle/config/mutator/sort_job_tasks.go deleted file mode 100644 index 8b9ca7f17..000000000 --- a/bundle/config/mutator/sort_job_tasks.go +++ /dev/null @@ -1,33 +0,0 @@ -package mutator - -import ( - "context" - "sort" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/libs/diag" -) - -type sortJobTasks struct{} - -// SortJobTasks sorts the tasks of each job in the bundle by task key. Sorting -// the task keys ensures that the diff computed by terraform is correct. For -// more details see the NOTE at https://registry.terraform.io/providers/databricks/databricks/latest/docs/resources/job#example-usage -// and https://github.com/databricks/terraform-provider-databricks/issues/4011. -func SortJobTasks() bundle.Mutator { - return &sortJobTasks{} -} - -func (m *sortJobTasks) Name() string { - return "SortJobTasks" -} - -func (m *sortJobTasks) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - for _, job := range b.Config.Resources.Jobs { - sort.Slice(job.Tasks, func(i, j int) bool { - return job.Tasks[i].TaskKey < job.Tasks[j].TaskKey - }) - } - - return nil -} diff --git a/bundle/config/mutator/sort_job_tasks_test.go b/bundle/config/mutator/sort_job_tasks_test.go deleted file mode 100644 index c5de826d4..000000000 --- a/bundle/config/mutator/sort_job_tasks_test.go +++ /dev/null @@ -1,80 +0,0 @@ -package mutator - -import ( - "context" - "testing" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/databricks-sdk-go/service/jobs" - "github.com/stretchr/testify/assert" -) - -func TestSortJobClusters(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "foo": { - JobSettings: &jobs.JobSettings{ - Tasks: []jobs.Task{ - { - TaskKey: "c", - EnvironmentKey: "3", - }, - { - TaskKey: "a", - EnvironmentKey: "1", - }, - { - TaskKey: "b", - EnvironmentKey: "2", - }, - }, - }, - }, - "bar": { - JobSettings: &jobs.JobSettings{ - Tasks: []jobs.Task{ - { - TaskKey: "d", - }, - { - TaskKey: "e", - }, - }, - }, - }, - }, - }, - }, - } - - diags := bundle.Apply(context.Background(), b, SortJobTasks()) - assert.NoError(t, diags.Error()) - - assert.Equal(t, []jobs.Task{ - { - TaskKey: "a", - EnvironmentKey: "1", - }, - { - TaskKey: "b", - EnvironmentKey: "2", - }, - { - TaskKey: "c", - EnvironmentKey: "3", - }, - }, b.Config.Resources.Jobs["foo"].Tasks) - - assert.Equal(t, []jobs.Task{ - { - TaskKey: "d", - }, - { - TaskKey: "e", - }, - }, b.Config.Resources.Jobs["bar"].Tasks) -} diff --git a/bundle/deploy/terraform/tfdyn/convert_job.go b/bundle/deploy/terraform/tfdyn/convert_job.go index d1e7e73e2..a29c9cc1e 100644 --- a/bundle/deploy/terraform/tfdyn/convert_job.go +++ b/bundle/deploy/terraform/tfdyn/convert_job.go @@ -3,6 +3,7 @@ package tfdyn import ( "context" "fmt" + "sort" "github.com/databricks/cli/bundle/internal/tf/schema" "github.com/databricks/cli/libs/dyn" @@ -19,8 +20,24 @@ func convertJobResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) { log.Debugf(ctx, "job normalization diagnostic: %s", diag.Summary) } + // Sort the tasks of each job in the bundle by task key. Sorting + // the task keys ensures that the diff computed by terraform is correct and avoids + // recreates. For more details see the NOTE at + // https://registry.terraform.io/providers/databricks/databricks/latest/docs/resources/job#example-usage + // and https://github.com/databricks/terraform-provider-databricks/issues/4011 + tasks := vin.Get("tasks").MustSequence() + sort.Slice(tasks, func(i, j int) bool { + return tasks[i].Get("task_key").MustString() < tasks[j].Get("task_key").MustString() + }) + vout, err := dyn.Map(vin, "tasks", func(_ dyn.Path, _ dyn.Value) (dyn.Value, error) { + return dyn.V(tasks), nil + }) + if err != nil { + return dyn.InvalidValue, err + } + // Modify top-level keys. - vout, err := renameKeys(vin, map[string]string{ + vout, err = renameKeys(vout, map[string]string{ "tasks": "task", "job_clusters": "job_cluster", "parameters": "parameter", diff --git a/bundle/deploy/terraform/tfdyn/convert_job_test.go b/bundle/deploy/terraform/tfdyn/convert_job_test.go index b9e1f967f..57da750b7 100644 --- a/bundle/deploy/terraform/tfdyn/convert_job_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_job_test.go @@ -42,8 +42,8 @@ func TestConvertJob(t *testing.T) { }, Tasks: []jobs.Task{ { - TaskKey: "task_key", - JobClusterKey: "job_cluster_key", + TaskKey: "task_key_b", + JobClusterKey: "job_cluster_key_b", Libraries: []compute.Library{ { Pypi: &compute.PythonPyPiLibrary{ @@ -55,6 +55,14 @@ func TestConvertJob(t *testing.T) { }, }, }, + { + TaskKey: "task_key_a", + JobClusterKey: "job_cluster_key_a", + }, + { + TaskKey: "task_key_c", + JobClusterKey: "job_cluster_key_c", + }, }, }, Permissions: []resources.Permission{ @@ -100,8 +108,12 @@ func TestConvertJob(t *testing.T) { }, "task": []any{ map[string]any{ - "task_key": "task_key", - "job_cluster_key": "job_cluster_key", + "task_key": "task_key_a", + "job_cluster_key": "job_cluster_key_a", + }, + map[string]any{ + "task_key": "task_key_b", + "job_cluster_key": "job_cluster_key_b", "library": []any{ map[string]any{ "pypi": map[string]any{ @@ -113,6 +125,10 @@ func TestConvertJob(t *testing.T) { }, }, }, + map[string]any{ + "task_key": "task_key_c", + "job_cluster_key": "job_cluster_key_c", + }, }, }, out.Job["my_job"]) diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 4d737c642..8039a4f13 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -62,11 +62,6 @@ func Initialize() bundle.Mutator { mutator.DefaultQueueing(), mutator.ExpandPipelineGlobPaths(), - // We sort job tasks by their task key to ensure sane diffs by the - // Databricks Terraform provider. This is done after variable resolution - // to ensure that the task key is fully resolved to what terraform will see. - mutator.SortJobTasks(), - // Configure use of WSFS for reads if the CLI is running on Databricks. mutator.ConfigureWSFS(), diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index 92133c5fe..ffb17baae 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -110,7 +110,7 @@ func TestPrepareBuiltInTemplatesWithRelativePaths(t *testing.T) { func TestBuiltinPythonTemplateValid(t *testing.T) { // Test option combinations - options := []string{"yes", "no"} + options := []string{"yes"} isServicePrincipal := false catalog := "hive_metastore" cachedCatalog = &catalog