databricks / cli

Databricks CLI
Other
132 stars 50 forks source link

Sort tasks by `task_key` before generating the Terraform configuration #1776

Closed shreyas-goenka closed 4 days ago

shreyas-goenka commented 1 week ago

Changes

Sort the tasks in the resultant bundle.tf.json. This is important because configuration from one task can leak into another if the tasks are not sorted.

For more details see:

  1. https://github.com/databricks/terraform-provider-databricks/issues/3951
  2. https://github.com/databricks/terraform-provider-databricks/issues/4011

Tests

Unit test and manually.

For manual testing I used the following configuration:

resources:
  jobs:
    foo:
      tasks: 
        - task_key: task-Z
          notebook_task: 
            notebook_path: nb.py
            source: GIT
          existing_cluster_id: 0715-133738-ju0ma84z

        - task_key: task-1
          notebook_task: 
            notebook_path: ${workspace.file_path}/local.py
            source: WORKSPACE
          existing_cluster_id: 0715-133738-ju0ma84z
          depends_on: 
            - task_key: task-Z

      git_source: 
        git_provider: gitHub
        git_url: https://github.com/shreyas-goenka/job-source-tmp.git
        git_branch: main

Steps (1):

  1. Deploy this bundle.
  2. Comment out "source: GIT"
  3. Deploy again

Before: Deploying this bundle twice would fail. This is because the "source: GIT" would carry over to the next deployment.

After: There was no error on the subsequent deployment.

Steps (2):

  1. Deploy once
  2. Deploy again

Before: Works correctly but leads to a update API call every time.

After: No diff is detected by terraform.

shreyas-goenka commented 1 week ago

Note: This stabilises the plan because tasks are an ordered list in Terraform, and Terraform computes the diff for an ordered list based on the index.

Sorting works because the Databricks Terraform provider sorts the task keys when returned by the GET job API, resulting in a stable plan.

This is not a complete solution yet because a new task can cause displacement in the ordered task list, which might possibly cause configuration intermixing across tasks. However, solving this in Terraform requires a migration to the Terraform plugin for databricks_job.

pietern commented 1 week ago

@shreyas-goenka Do you know where the tasks are reordered?

They are an array in the configuration and, therefore, should have stable order all the way until they hit the backend.

shreyas-goenka commented 1 week ago

@pietern The tasks are reordered in the terraform provider itself. They do not seem to be altered by the API based on my personal testing.

The code reference for sorting it is three years old, so it might have been reordered by the API at some point and then fixed. We could very well fix this upstream as well by not sorting by task key anymore. I'll need to run this by @mgyucht and the Jobs team to confirm that an upstream patch could work.

ref: https://github.com/databricks/terraform-provider-databricks/blame/af46555600f4895d84419675ae445bfe61c4143c/jobs/resource_job.go#L341.

shreyas-goenka commented 1 week ago

Note: The order of the tasks is faithfully retained by both the API get body and the YAML file for the job in the UI.