From 7342ec09da74bc2862b89281c04e94b486a1bfec Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 4 Apr 2025 16:48:04 +0200 Subject: [PATCH 01/41] WIP - deploying without terraform --- .github/workflows/push.yml | 13 + acceptance/acceptance_test.go | 2 + acceptance/bin/read_id.py | 17 +- acceptance/bin/read_state.py | 26 +- .../bundle/artifacts/whl_dynamic/test.toml | 3 + .../whl_prebuilt_outside_dynamic/test.toml | 1 + acceptance/bundle/debug/test.toml | 3 + .../bundle/deploy/dashboard/simple/test.toml | 3 + .../deploy/fail-on-active-runs/test.toml | 3 + .../jobs/double-underscore-keys/test.toml | 2 + .../deploy/jobs/fail-on-active-runs/test.toml | 2 + .../deploy/pipeline/auto-approve/test.toml | 2 + .../bundle/deploy/pipeline/recreate/test.toml | 2 + .../deploy/schema/auto-approve/test.toml | 2 + .../bundle/deploy/secret-scope/test.toml | 2 + acceptance/bundle/deployment/bind/test.toml | 2 + .../python/restricted-execution/test.toml | 2 + acceptance/bundle/resources/apps/output.txt | 15 +- acceptance/bundle/resources/apps/script | 3 +- .../bundle/resources/pipelines/output.txt | 25 -- acceptance/bundle/resources/pipelines/script | 5 +- acceptance/bundle/state/test.toml | 2 + acceptance/bundle/telemetry/test.toml | 3 + .../integration_classic/test.toml | 3 + .../cmd/workspace/apps/run-local/test.toml | 3 + acceptance/test.toml | 7 + bundle/bundle.go | 61 +++++ bundle/config/resources.go | 17 ++ bundle/phases/deploy.go | 57 +++- bundle/phases/initialize.go | 38 ++- bundle/terranova/deploy_mutator.go | 251 ++++++++++++++++++ bundle/terranova/terranova_resources/app.go | 77 ++++++ bundle/terranova/terranova_resources/job.go | 93 +++++++ .../terranova/terranova_resources/pipeline.go | 72 +++++ .../terranova/terranova_resources/resource.go | 90 +++++++ .../terranova/terranova_resources/schema.go | 76 ++++++ .../terranova_resources/sdk_error.go | 43 +++ bundle/terranova/terranova_resources/util.go | 22 ++ bundle/terranova/terranova_state/state.go | 129 +++++++++ libs/dag/dag.go | 200 ++++++++++++++ libs/dag/dag_test.go | 113 ++++++++ libs/dag/stack.go | 8 + libs/testserver/server.go | 32 +++ 43 files changed, 1474 insertions(+), 58 deletions(-) create mode 100644 acceptance/bundle/python/restricted-execution/test.toml create mode 100644 bundle/terranova/deploy_mutator.go create mode 100644 bundle/terranova/terranova_resources/app.go create mode 100644 bundle/terranova/terranova_resources/job.go create mode 100644 bundle/terranova/terranova_resources/pipeline.go create mode 100644 bundle/terranova/terranova_resources/resource.go create mode 100644 bundle/terranova/terranova_resources/schema.go create mode 100644 bundle/terranova/terranova_resources/sdk_error.go create mode 100644 bundle/terranova/terranova_resources/util.go create mode 100644 bundle/terranova/terranova_state/state.go create mode 100644 libs/dag/dag.go create mode 100644 libs/dag/dag_test.go create mode 100644 libs/dag/stack.go diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 945c7367e6..bbad4fb517 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -42,15 +42,24 @@ jobs: - macos-latest - ubuntu-latest - windows-latest + deployment: + - "terraform" + - "direct" steps: - name: Checkout repository and submodules uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - name: Create deployment-specific cache identifier + run: echo "${{ matrix.deployment }}" > deployment-type.txt + - name: Setup Go uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: go-version-file: go.mod + cache-dependency-path: | + go.sum + deployment-type.txt - name: Setup Python uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 @@ -72,11 +81,15 @@ jobs: # and would like to run the tests as fast as possible. We run it on schedule as well, because that is what # populates the cache and cache may include test results. if: ${{ github.event_name == 'pull_request' || github.event_name == 'schedule' }} + env: + DATABRICKS_CLI_DEPLOYMENT: ${{ matrix.deployment }} run: make test - name: Run tests with coverage # Still run 'make cover' on push to main and merge checks to make sure it does not get broken. if: ${{ github.event_name != 'pull_request' && github.event_name != 'schedule' }} + env: + DATABRICKS_CLI_DEPLOYMENT: ${{ matrix.deployment }} run: make cover - name: Analyze slow tests diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index e8448ee8ed..73da7e6125 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -35,6 +35,8 @@ import ( "github.com/stretchr/testify/require" ) +const deploymentEnvVar = "DATABRICKS_CLI_DEPLOYMENT" + var ( KeepTmp bool NoRepl bool diff --git a/acceptance/bin/read_id.py b/acceptance/bin/read_id.py index e7fb681d45..47ad0667e3 100755 --- a/acceptance/bin/read_id.py +++ b/acceptance/bin/read_id.py @@ -29,4 +29,19 @@ def print_resource_terraform(section, name): return -print_resource_terraform(*sys.argv[1:]) +def print_resource_terranova(section, name): + filename = ".databricks/bundle/default/resources.json" + raw = open(filename).read() + data = json.loads(raw) + resources = data["resources"].get(section, {}) + result = resources.get(name) + if result is None: + print(f"Resource {section=} {name=} not found. Available: {raw}") + return + print(result.get("__id__")) + + +if os.environ.get("DATABRICKS_CLI_DEPLOYMENT") == "direct": + print_resource_terranova(*sys.argv[1:]) +else: + print_resource_terraform(*sys.argv[1:]) diff --git a/acceptance/bin/read_state.py b/acceptance/bin/read_state.py index bcceae3887..61944cda0a 100755 --- a/acceptance/bin/read_state.py +++ b/acceptance/bin/read_state.py @@ -13,17 +13,15 @@ def print_resource_terraform(section, name, *attrs): resource_type = "databricks_" + section[:-1] filename = ".databricks/bundle/default/terraform/terraform.tfstate" - data = json.load(open(filename)) - available = [] + raw = open(filename).read() + data = json.loads(raw) found = 0 for r in data["resources"]: r_type = r["type"] r_name = r["name"] if r_type != resource_type: - available.append((r_type, r_name)) continue if r_name != name: - available.append((r_type, r_name)) continue for inst in r["instances"]: attribute_values = inst.get("attributes") @@ -35,4 +33,22 @@ def print_resource_terraform(section, name, *attrs): print(f"State not found for {section}.{name}") -print_resource_terraform(*sys.argv[1:]) +def print_resource_terranova(section, name, *attrs): + filename = ".databricks/bundle/default/resources.json" + raw = open(filename).read() + data = json.loads(raw) + resources = data["resources"].get(section, {}) + result = resources.get(name) + if result is None: + print(f"State not found for {section}.{name}") + return + state = result["state"] + state.setdefault("id", result.get("__id__")) + values = [f"{x}={state.get(x)!r}" for x in attrs] + print(section, name, " ".join(values)) + + +if os.environ.get("DATABRICKS_CLI_DEPLOYMENT") == "direct": + print_resource_terranova(*sys.argv[1:]) +else: + print_resource_terraform(*sys.argv[1:]) diff --git a/acceptance/bundle/artifacts/whl_dynamic/test.toml b/acceptance/bundle/artifacts/whl_dynamic/test.toml index 571512c6ab..f7cb985494 100644 --- a/acceptance/bundle/artifacts/whl_dynamic/test.toml +++ b/acceptance/bundle/artifacts/whl_dynamic/test.toml @@ -1,3 +1,6 @@ +# Terraform sorts tasks +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + [[Repls]] Old = '\\\\' New = '/' diff --git a/acceptance/bundle/artifacts/whl_prebuilt_outside_dynamic/test.toml b/acceptance/bundle/artifacts/whl_prebuilt_outside_dynamic/test.toml index 53db9644c8..3a3837a6a3 100644 --- a/acceptance/bundle/artifacts/whl_prebuilt_outside_dynamic/test.toml +++ b/acceptance/bundle/artifacts/whl_prebuilt_outside_dynamic/test.toml @@ -1,4 +1,5 @@ BundleConfig.default_name = "" +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # need to sort tasks by key [[Repls]] Old = '\\' diff --git a/acceptance/bundle/debug/test.toml b/acceptance/bundle/debug/test.toml index 4761ce71d6..aaf02810c0 100644 --- a/acceptance/bundle/debug/test.toml +++ b/acceptance/bundle/debug/test.toml @@ -1,3 +1,6 @@ +# Debug output is naturally different. TODO: split debug tests in two: terraform and terranova +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + [[Repls]] # The keys are unsorted and also vary per OS Old = 'Environment variables for Terraform: ([A-Z_ ,]+) ' diff --git a/acceptance/bundle/deploy/dashboard/simple/test.toml b/acceptance/bundle/deploy/dashboard/simple/test.toml index 26b6edd4c7..3c0b6562ae 100644 --- a/acceptance/bundle/deploy/dashboard/simple/test.toml +++ b/acceptance/bundle/deploy/dashboard/simple/test.toml @@ -2,6 +2,9 @@ Local = true Cloud = true RequiresWarehouse = true +# dashboards not implemented yet +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deploy/fail-on-active-runs/test.toml b/acceptance/bundle/deploy/fail-on-active-runs/test.toml index 0fd427e3a3..1619b25e22 100644 --- a/acceptance/bundle/deploy/fail-on-active-runs/test.toml +++ b/acceptance/bundle/deploy/fail-on-active-runs/test.toml @@ -1,5 +1,8 @@ RecordRequests = true +# --fail-on-active-runs not implemented yet +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + [[Server]] Pattern = "GET /api/2.2/jobs/runs/list" Response.Body = ''' diff --git a/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml b/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml index a07b3c091e..72ef883dc8 100644 --- a/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml +++ b/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml @@ -1,6 +1,8 @@ Local = true Cloud = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # "bundle destroy" + Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deploy/jobs/fail-on-active-runs/test.toml b/acceptance/bundle/deploy/jobs/fail-on-active-runs/test.toml index a07b3c091e..8136c306bf 100644 --- a/acceptance/bundle/deploy/jobs/fail-on-active-runs/test.toml +++ b/acceptance/bundle/deploy/jobs/fail-on-active-runs/test.toml @@ -1,6 +1,8 @@ Local = true Cloud = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deploy/pipeline/auto-approve/test.toml b/acceptance/bundle/deploy/pipeline/auto-approve/test.toml index e7b8c4f1f0..b74d4288c9 100644 --- a/acceptance/bundle/deploy/pipeline/auto-approve/test.toml +++ b/acceptance/bundle/deploy/pipeline/auto-approve/test.toml @@ -1,6 +1,8 @@ Local = true Cloud = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + Ignore = [ "databricks.yml" ] diff --git a/acceptance/bundle/deploy/pipeline/recreate/test.toml b/acceptance/bundle/deploy/pipeline/recreate/test.toml index d8d85f487e..f80626809a 100644 --- a/acceptance/bundle/deploy/pipeline/recreate/test.toml +++ b/acceptance/bundle/deploy/pipeline/recreate/test.toml @@ -2,6 +2,8 @@ Local = true Cloud = true RequiresUnityCatalog = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + Ignore = [ "databricks.yml" ] diff --git a/acceptance/bundle/deploy/schema/auto-approve/test.toml b/acceptance/bundle/deploy/schema/auto-approve/test.toml index 1d77334643..dba69ddb9b 100644 --- a/acceptance/bundle/deploy/schema/auto-approve/test.toml +++ b/acceptance/bundle/deploy/schema/auto-approve/test.toml @@ -2,6 +2,8 @@ Local = true Cloud = true RequiresUnityCatalog = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + Ignore = [ "databricks.yml", "test-file-*.txt", diff --git a/acceptance/bundle/deploy/secret-scope/test.toml b/acceptance/bundle/deploy/secret-scope/test.toml index 4d7598a227..6f73417839 100644 --- a/acceptance/bundle/deploy/secret-scope/test.toml +++ b/acceptance/bundle/deploy/secret-scope/test.toml @@ -1,6 +1,8 @@ Cloud = true Local = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deployment/bind/test.toml b/acceptance/bundle/deployment/bind/test.toml index 5f99440260..395dfe849b 100644 --- a/acceptance/bundle/deployment/bind/test.toml +++ b/acceptance/bundle/deployment/bind/test.toml @@ -1,2 +1,4 @@ BundleConfig.default_name.bundle.name = "test-bundle-$UNIQUE_NAME" BundleConfigTarget = "databricks.yml.tmpl" + +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] diff --git a/acceptance/bundle/python/restricted-execution/test.toml b/acceptance/bundle/python/restricted-execution/test.toml new file mode 100644 index 0000000000..fa4a6e83fd --- /dev/null +++ b/acceptance/bundle/python/restricted-execution/test.toml @@ -0,0 +1,2 @@ +# "bundle summary" is not implemented +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] diff --git a/acceptance/bundle/resources/apps/output.txt b/acceptance/bundle/resources/apps/output.txt index 02bdeaebe3..6f05523d51 100644 --- a/acceptance/bundle/resources/apps/output.txt +++ b/acceptance/bundle/resources/apps/output.txt @@ -7,12 +7,12 @@ Deployment complete! >>> print_requests { - "method": "POST", - "path": "/api/2.0/apps", "body": { "description": "my_app_description", "name": "myapp" - } + }, + "method": "POST", + "path": "/api/2.0/apps" } apps myapp name='myapp' description='my_app_description' @@ -27,12 +27,11 @@ Deployment complete! >>> print_requests { - "method": "PATCH", - "path": "/api/2.0/apps/myapp", "body": { "description": "MY_APP_DESCRIPTION", - "name": "myapp", - "url": "myapp-123.cloud.databricksapps.com" - } + "name": "myapp" + }, + "method": "PATCH", + "path": "/api/2.0/apps/myapp" } apps myapp name='myapp' description='MY_APP_DESCRIPTION' diff --git a/acceptance/bundle/resources/apps/script b/acceptance/bundle/resources/apps/script index 01824bc88f..818b9c6400 100644 --- a/acceptance/bundle/resources/apps/script +++ b/acceptance/bundle/resources/apps/script @@ -1,5 +1,6 @@ print_requests() { - jq 'select(.method != "GET" and (.path | contains("/apps")))' < out.requests.txt + # url is output-only field that terraform adds but that is ignored by the backend + jq --sort-keys 'select(.method != "GET" and (.path | contains("/apps"))) | (.body.url = null | del(.body.url))' < out.requests.txt rm out.requests.txt } diff --git a/acceptance/bundle/resources/pipelines/output.txt b/acceptance/bundle/resources/pipelines/output.txt index dd448efaed..252fbd1720 100644 --- a/acceptance/bundle/resources/pipelines/output.txt +++ b/acceptance/bundle/resources/pipelines/output.txt @@ -36,31 +36,6 @@ Deploying resources... Updating deployment state... Deployment complete! ->>> print_requests -{ - "body": { - "channel": "CURRENT", - "deployment": { - "kind": "BUNDLE", - "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/acc-[UNIQUE_NAME]/default/state/metadata.json" - }, - "edition": "ADVANCED", - "id": "[UUID]", - "libraries": [ - { - "file": { - "path": "/Workspace/Users/[USERNAME]/.bundle/acc-[UNIQUE_NAME]/default/files/bar.py" - } - } - ], - "name": "test-pipeline-[UNIQUE_NAME]", - "storage": "dbfs:/pipelines/[UUID]" - }, - "method": "PUT", - "path": "/api/2.0/pipelines/[UUID]" -} -pipelines my id='[UUID]' name='test-pipeline-[UNIQUE_NAME]' - >>> [CLI] pipelines get [UUID] { "creator_user_name":"[USERNAME]", diff --git a/acceptance/bundle/resources/pipelines/script b/acceptance/bundle/resources/pipelines/script index 3e7e50a488..8a7b2533c7 100644 --- a/acceptance/bundle/resources/pipelines/script +++ b/acceptance/bundle/resources/pipelines/script @@ -13,7 +13,10 @@ trace print_requests trace update_file.py databricks.yml foo.py bar.py trace $CLI bundle deploy -trace print_requests +# Update requests are different between terraform and direct, because terraform also re-sends +# fields that it previously recevied from the backend. +# The get call below verifies that it does not matter -- the pipeline is the same in the end. +#trace print_requests ppid=`read_id.py pipelines my` trace $CLI pipelines get $ppid diff --git a/acceptance/bundle/state/test.toml b/acceptance/bundle/state/test.toml index 159efe0269..31623b255a 100644 --- a/acceptance/bundle/state/test.toml +++ b/acceptance/bundle/state/test.toml @@ -1 +1,3 @@ +# Test needs splitting or updating to handle terranova state file +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] RecordRequests = true diff --git a/acceptance/bundle/telemetry/test.toml b/acceptance/bundle/telemetry/test.toml index d583d9d69b..0b7e564019 100644 --- a/acceptance/bundle/telemetry/test.toml +++ b/acceptance/bundle/telemetry/test.toml @@ -1,6 +1,9 @@ RecordRequests = true IncludeRequestHeaders = ["User-Agent"] +# Retrieves job/pipelines IDs, not populated yet +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + [[Repls]] Old = '"execution_time_ms": \d{1,5},' New = '"execution_time_ms": SMALL_INT,' diff --git a/acceptance/bundle/templates/default-python/integration_classic/test.toml b/acceptance/bundle/templates/default-python/integration_classic/test.toml index 826f6ced08..248a6c1c85 100644 --- a/acceptance/bundle/templates/default-python/integration_classic/test.toml +++ b/acceptance/bundle/templates/default-python/integration_classic/test.toml @@ -1,6 +1,9 @@ Cloud = true Timeout = '1m' +# "bundle summary" is not supported yet +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + Ignore = [ '.venv', ] diff --git a/acceptance/cmd/workspace/apps/run-local/test.toml b/acceptance/cmd/workspace/apps/run-local/test.toml index ed349f948c..c7120a5ba6 100644 --- a/acceptance/cmd/workspace/apps/run-local/test.toml +++ b/acceptance/cmd/workspace/apps/run-local/test.toml @@ -2,6 +2,9 @@ RecordRequests = false Timeout = '2m' TimeoutWindows = '10m' +# Cannot run 2 instances of this test because it uses fixed ports +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] + Ignore = [ '.venv', '__pycache__' diff --git a/acceptance/test.toml b/acceptance/test.toml index 500f3067c1..0833cd2b24 100644 --- a/acceptance/test.toml +++ b/acceptance/test.toml @@ -69,3 +69,10 @@ Order = 10 Old = '2\d\d\d-\d\d-\d\d(T| )\d\d:\d\d:\d\d' New = "[TIMESTAMP]" Order = 10 + +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform", "direct"] +EnvRepl.DATABRICKS_CLI_DEPLOYMENT = false + +[[Repls]] +Old = '"exec_path": " "' +New = '"exec_path": "[TERRAFORM]"' diff --git a/bundle/bundle.go b/bundle/bundle.go index 137e2404dd..4cb018b019 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -8,6 +8,7 @@ package bundle import ( "context" + "encoding/json" "errors" "fmt" "os" @@ -15,9 +16,12 @@ import ( "sync" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/env" "github.com/databricks/cli/bundle/metadata" + "github.com/databricks/cli/bundle/terranova/terranova_state" "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/fileset" "github.com/databricks/cli/libs/locker" "github.com/databricks/cli/libs/log" @@ -112,6 +116,11 @@ type Bundle struct { Tagging tags.Cloud Metrics Metrics + + // If true, don't use terraform. Set by DATABRICKS_CLI_DEPLOYMENT=direct + DirectDeployment bool + + ResourceDatabase terranova_state.TerranovaState } func Load(ctx context.Context, path string) (*Bundle, error) { @@ -268,3 +277,55 @@ func (b *Bundle) AuthEnv() (map[string]string, error) { cfg := b.client.Config return auth.Env(cfg), nil } + +func GetResourceConfigT[T any](b *Bundle, section, name string) (*T, bool) { + v, err := dyn.GetByPath(b.Config.Value(), dyn.NewPath(dyn.Key("resources"), dyn.Key(section), dyn.Key(name))) + if err != nil { + return nil, false + } + + // Note, we cannot read b.Config.Resources.Jobs[name] because we don't populate ForceSendFields properly on those + bytes, err := json.Marshal(v.AsAny()) + if err != nil { + return nil, false + } + + var result T + err = json.Unmarshal(bytes, &result) + if err != nil { + return nil, false + } + + return &result, true +} + +func (b *Bundle) GetResourceConfig(section, name string) (any, bool) { + // TODO: validate that the config is fully resolved + switch section { + case "jobs": + return GetResourceConfigT[resources.Job](b, section, name) + case "pipelines": + return GetResourceConfigT[resources.Pipeline](b, section, name) + case "models": + return GetResourceConfigT[resources.MlflowModel](b, section, name) + case "experiments": + return GetResourceConfigT[resources.MlflowExperiment](b, section, name) + case "model_serving_endpoints": + return GetResourceConfigT[resources.ModelServingEndpoint](b, section, name) + case "registered_models": + return GetResourceConfigT[resources.RegisteredModel](b, section, name) + case "quality_monitors": + return GetResourceConfigT[resources.QualityMonitor](b, section, name) + case "schemas": + return GetResourceConfigT[resources.Schema](b, section, name) + case "volumes": + return GetResourceConfigT[resources.Volume](b, section, name) + case "clusters": + return GetResourceConfigT[resources.Cluster](b, section, name) + case "dashboards": + return GetResourceConfigT[resources.Dashboard](b, section, name) + case "apps": + return GetResourceConfigT[resources.App](b, section, name) + } + return nil, false +} diff --git a/bundle/config/resources.go b/bundle/config/resources.go index 117955002a..02cbd4763c 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -27,6 +27,23 @@ type Resources struct { SecretScopes map[string]*resources.SecretScope `json:"secret_scopes,omitempty"` } +func (r *Resources) GetResourceConfig(section, name string) (any, bool) { + // TODO: validate that the config is fully resolved + switch section { + case "jobs": + cfg, ok := r.Jobs[name] + return cfg, ok + case "schemas": + cfg, ok := r.Schemas[name] + return cfg, ok + case "apps": + cfg, ok := r.Schemas[name] + return cfg, ok + default: + return nil, false + } +} + type ConfigResource interface { // Exists returns true if the resource exists in the workspace configured in // the input workspace client. diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index d196019bc4..11135dc119 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -18,6 +18,7 @@ import ( "github.com/databricks/cli/bundle/metrics" "github.com/databricks/cli/bundle/permissions" "github.com/databricks/cli/bundle/scripts" + "github.com/databricks/cli/bundle/terranova" "github.com/databricks/cli/bundle/trampoline" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/diag" @@ -56,6 +57,10 @@ func filterDeleteOrRecreateActions(changes []*tfjson.ResourceChange, resourceTyp } func approvalForDeploy(ctx context.Context, b *bundle.Bundle) (bool, error) { + if b.DirectDeployment { + return true, nil + } + tf := b.Terraform if tf == nil { return false, errors.New("terraform not initialized") @@ -143,13 +148,25 @@ func deployCore(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // Core mutators that CRUD resources and modify deployment state. These // mutators need informed consent if they are potentially destructive. cmdio.LogString(ctx, "Deploying resources...") - diags := bundle.Apply(ctx, b, terraform.Apply()) - // following original logic, continuing with sequence below even if terraform had errors + var diags diag.Diagnostics + + if b.DirectDeployment { + diags = bundle.Apply(ctx, b, terranova.TerranovaDeploy()) + } else { + diags = bundle.Apply(ctx, b, terraform.Apply()) + // following original logic, continuing with sequence below even if terraform had errors + newDiags := bundle.ApplySeq(ctx, b, + terraform.StatePush(), + terraform.Load(), + ) + diags = diags.Extend(newDiags) + if newDiags.HasError() { + return diags + } + } diags = diags.Extend(bundle.ApplySeq(ctx, b, - terraform.StatePush(), - terraform.Load(), apps.InterpolateVariables(), apps.UploadConfig(), metadata.Compute(), @@ -184,9 +201,18 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand diags = diags.Extend(bundle.Apply(ctx, b, lock.Release(lock.GoalDeploy))) }() - diags = bundle.ApplySeq(ctx, b, - terraform.StatePull(), - terraform.CheckDashboardsModifiedRemotely(), + if !b.DirectDeployment { + diags = diags.Extend(bundle.ApplySeq(ctx, b, + terraform.StatePull(), + terraform.CheckDashboardsModifiedRemotely(), + )) + + if diags.HasError() { + return diags + } + } + + diags = diags.Extend(bundle.ApplySeq(ctx, b, deploy.StatePull(), mutator.ValidateGitDetails(), terraform.CheckRunningResource(), @@ -205,11 +231,20 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand deploy.StateUpdate(), deploy.StatePush(), permissions.ApplyWorkspaceRootPermissions(), - terraform.Interpolate(), - terraform.Write(), - terraform.Plan(terraform.PlanGoal("deploy")), metrics.TrackUsedCompute(), - ) + )) + + if diags.HasError() { + return diags + } + + if !b.DirectDeployment { + diags = diags.Extend(bundle.ApplySeq(ctx, b, + terraform.Interpolate(), + terraform.Write(), + terraform.Plan(terraform.PlanGoal("deploy")), + )) + } if diags.HasError() { return diags diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index c3d3d27656..218b2e2ebe 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -2,6 +2,8 @@ package phases import ( "context" + "fmt" + "os" "github.com/databricks/cli/bundle/config/mutator/resourcemutator" @@ -27,7 +29,17 @@ import ( func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { log.Info(ctx, "Phase: initialize") - return bundle.ApplySeq(ctx, b, + deployment := os.Getenv("DATABRICKS_CLI_DEPLOYMENT") + if deployment == "direct" { + b.DirectDeployment = true + } else if deployment == "terraform" || deployment == "" { + b.DirectDeployment = false + } else { + err := fmt.Errorf("Unexpected setting for DATABRICKS_CLI_DEPLOYMENT=%#v (expected 'terraform' or 'direct' or absent/empty which means 'terraform')", deployment) + return diag.FromErr(err) + } + + diags := bundle.ApplySeq(ctx, b, // Reads (dynamic): resource.*.* // Checks that none of resources.. is nil. Raises error otherwise. validate.AllResourcesHaveValues(), @@ -182,15 +194,29 @@ func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // Updates (typed): b.Config.Resources.Pipelines[].CreatePipeline.Deployment (sets deployment metadata for bundle deployments) // Annotates pipelines with bundle deployment metadata metadata.AnnotatePipelines(), + ) + if diags.HasError() { + return diags + } + + if b.DirectDeployment { + // For acceptance tests, initialize terraform map + bundle.ApplyFunc(ctx, b, func(_ context.Context, b *bundle.Bundle) diag.Diagnostics { + b.Config.Bundle.Terraform = &config.Terraform{ExecPath: " "} + return nil + }) + } else { // Reads (typed): b.Config.Bundle.Terraform (checks terraform configuration) // Updates (typed): b.Config.Bundle.Terraform (sets default values if not already set) // Updates (typed): b.Terraform (initializes Terraform executor with proper environment variables and paths) // Initializes Terraform with the correct binary, working directory, and environment variables for authentication - terraform.Initialize(), - // Reads (typed): b.Config.Experimental.Scripts["post_init"] (checks if script is defined) - // Executes the post_init script hook defined in the bundle configuration - scripts.Execute(config.ScriptPostInit), - ) + diags = diags.Extend(bundle.Apply(ctx, b, terraform.Initialize())) + } + + // Reads (typed): b.Config.Experimental.Scripts["post_init"] (checks if script is defined) + // Executes the post_init script hook defined in the bundle configuration + diags = diags.Extend(bundle.Apply(ctx, b, scripts.Execute(config.ScriptPostInit))) + return diags } diff --git a/bundle/terranova/deploy_mutator.go b/bundle/terranova/deploy_mutator.go new file mode 100644 index 0000000000..c59bcc9d45 --- /dev/null +++ b/bundle/terranova/deploy_mutator.go @@ -0,0 +1,251 @@ +package terranova + +import ( + "context" + "fmt" + "path/filepath" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/terranova/terranova_resources" + "github.com/databricks/cli/bundle/terranova/terranova_state" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/dag" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/structdiff" + "github.com/databricks/databricks-sdk-go" +) + +const maxPoolSize = 10 + +type terranovaDeployMutator struct{} + +func TerranovaDeploy() bundle.Mutator { + return &terranovaDeployMutator{} +} + +func (m *terranovaDeployMutator) Name() string { + return "TerranovaDeploy" +} + +func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.SafeDiagnostics + + client := b.WorkspaceClient() + + cacheDir, err := b.CacheDir(ctx) + if err != nil { + return diag.FromErr(err) + } + + databasePath := filepath.Join(cacheDir, "resources.json") + err = b.ResourceDatabase.Open(databasePath) + if err != nil { + return diag.FromErr(err) + } + + g := dag.NewGraph() + + _, err = dyn.MapByPattern( + b.Config.Value(), + dyn.NewPattern(dyn.Key("resources"), dyn.AnyKey(), dyn.AnyKey()), + func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + section := p[1].Key() + resourceName := p[2].Key() + node := section + "." + resourceName + // log.Warnf(ctx, "Adding node=%s", node) + g.AddNode(node) + + // TODO: Scan v for references and use g.AddDirectedEdge to add dependency + return v, nil + }, + ) + + countDeployed := 0 + + err = g.Run(maxPoolSize, func(node string) { + // TODO func(node string) bool + // If function returns false, downstream callers are not called + // g.Run() should return list of not executed nodes + // log.Warnf(ctx, "Processing node=%s", node) + + items := strings.SplitN(node, ".", 2) + if len(items) != 2 { + diags.AppendErrorf("internal error: unexpected DAG node %#v", node) + return + } + + section := items[0] + name := items[1] + + // TODO: resolve all resource references inside this resource. It should be possible, if graph was constructed correctly. + // If it is not possible, return error (and fail this and dependent resources) + + config, ok := b.GetResourceConfig(section, name) + if !ok { + diags.AppendErrorf("internal error: cannot get config for %s", node) + return + } + + d := Deployer{ + client: client, + db: &b.ResourceDatabase, + section: section, + resourceName: name, + } + + err = d.Deploy(ctx, config) + if err != nil { + diags.AppendError(err) + return + } + // TODO handle error; count stats + + countDeployed = countDeployed + 1 + }) + if err != nil { + diags.AppendError(err) + } + + // Not uploading at the moment, just logging to match the output. + if countDeployed > 0 { + cmdio.LogString(ctx, "Updating deployment state...") + } + + _ = b.ResourceDatabase.Finalize() + // TODO: handle errors + + return diags.Diags +} + +type Deployer struct { + client *databricks.WorkspaceClient + db *terranova_state.TerranovaState + section string + resourceName string +} + +func (d *Deployer) Deploy(ctx context.Context, inputConfig any) error { + err := d.deploy(ctx, inputConfig) + if err != nil { + return fmt.Errorf("deploying %s.%s: %w", d.section, d.resourceName, err) + } + return nil +} + +func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { + oldID, err := d.db.GetResourceID(d.section, d.resourceName) + if err != nil { + return err + } + + resource, err := terranova_resources.New(d.client, d.section, d.resourceName, inputConfig) + if err != nil { + return err + } + + config := resource.Config() + + // presence of id in the state file implies that the resource was created by us + + if oldID == "" { + newID, err := resource.DoCreate(ctx) + if err != nil { + return err + } + + log.Infof(ctx, "Created %s.%s id=%#v", d.section, d.resourceName, newID) + + err = d.db.SaveState(d.section, d.resourceName, newID, config) + if err != nil { + return err + } + + return resource.WaitAfterCreate(ctx) + } + + savedState, err := d.db.GetSavedState(d.section, d.resourceName, resource.GetType()) + if err != nil { + return fmt.Errorf("reading state: %w", err) + } + localDiff, err := structdiff.GetStructDiff(savedState, config) + if err != nil { + return fmt.Errorf("state error: %w", err) + } + // log.Warnf(ctx, "localDiff: %#v", localDiff) + + localDiffType := terranova_resources.ChangeTypeNone + if len(localDiff) > 0 { + localDiffType = resource.ClassifyChanges(localDiff) + } + + if localDiffType.IsRecreate() { + return d.Recreate(ctx, resource, oldID, config) + } + + if localDiffType.IsUpdate() { + return d.Update(ctx, resource, oldID, config) + } + + // localDiffType is either None or Partial: we should proceed to remote diff + + // remoteState := DoRead() + // compare config and remoteState + // OR compare config and state + config and remoteState separately + // decide what to do for this: config=X state=Y remoteState=X; should this trigger deploy? probably not. + + log.Warnf(ctx, "Unchanged %s.%s id=%#v", d.section, d.resourceName, oldID) + return nil +} + +func (d *Deployer) Recreate(ctx context.Context, oldResource terranova_resources.IResource, oldID string, config any) error { + err := oldResource.DoDelete(ctx, oldID) + if err != nil { + return fmt.Errorf("deleting old id=%s: %w", oldID, err) + } + + err = d.db.SaveState(d.section, d.resourceName, "", nil) + if err != nil { + return fmt.Errorf("deleting state: %w", err) + } + + newResource, err := terranova_resources.New(d.client, d.section, d.resourceName, config) + if err != nil { + return fmt.Errorf("initializing: %w", err) + } + + newID, err := newResource.DoCreate(ctx) + if err != nil { + return fmt.Errorf("re-creating: %w", err) + } + + log.Warnf(ctx, "Re-created %s.%s id=%#v (previously %#v)", d.section, d.resourceName, newID, oldID) + err = d.db.SaveState(d.section, d.resourceName, newID, config) + if err != nil { + return fmt.Errorf("saving state: %w", err) + } + + return newResource.WaitAfterCreate(ctx) +} + +func (d *Deployer) Update(ctx context.Context, resource terranova_resources.IResource, oldID string, config any) error { + newID, err := resource.DoUpdate(ctx, oldID) + if err != nil { + return fmt.Errorf("updating id=%s: %w", oldID, err) + } + + if oldID != newID { + log.Infof(ctx, "Updated %s.%s id=%#v (previously %#v)", d.section, d.resourceName, newID, oldID) + } else { + log.Infof(ctx, "Updated %s.%s id=%#v", d.section, d.resourceName, newID) + } + + err = d.db.SaveState(d.section, d.resourceName, newID, config) + if err != nil { + return fmt.Errorf("saving state id=%s: %w", oldID, err) + } + + return resource.WaitAfterUpdate(ctx) +} diff --git a/bundle/terranova/terranova_resources/app.go b/bundle/terranova/terranova_resources/app.go new file mode 100644 index 0000000000..724584ab65 --- /dev/null +++ b/bundle/terranova/terranova_resources/app.go @@ -0,0 +1,77 @@ +package terranova_resources + +import ( + "context" + "reflect" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/structdiff" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/service/apps" +) + +type ResourceApp struct { + client *databricks.WorkspaceClient + config apps.App +} + +func NewResourceApp(client *databricks.WorkspaceClient, config resources.App) (ResourceApp, error) { + return ResourceApp{ + client: client, + config: config.App, + }, nil +} + +func (r *ResourceApp) Config() any { + return r.config +} + +func (r *ResourceApp) DoCreate(ctx context.Context) (string, error) { + request := apps.CreateAppRequest{ + App: r.config, + NoCompute: true, + } + waiter, err := r.client.Apps.Create(ctx, request) + if err != nil { + return "", SDKError{Method: "Apps.Create", Err: err} + } + + // TODO: Store waiter for Wait method + + return waiter.Response.Name, nil +} + +func (r *ResourceApp) DoUpdate(ctx context.Context, id string) (string, error) { + request := apps.UpdateAppRequest{ + App: r.config, + Name: id, + } + response, err := r.client.Apps.Update(ctx, request) + if err != nil { + return "", SDKError{Method: "Apps.Update", Err: err} + } + + return response.Name, nil +} + +func (r *ResourceApp) DoDelete(ctx context.Context, id string) error { + return nil +} + +func (r *ResourceApp) WaitAfterCreate(ctx context.Context) error { + return nil +} + +func (r *ResourceApp) WaitAfterUpdate(ctx context.Context) error { + return nil +} + +func (r *ResourceApp) ClassifyChanges(changes []structdiff.Change) ChangeType { + return ChangeTypeUpdate +} + +var appType = reflect.TypeOf(ResourceApp{}.config) + +func (r *ResourceApp) GetType() reflect.Type { + return appType +} diff --git a/bundle/terranova/terranova_resources/job.go b/bundle/terranova/terranova_resources/job.go new file mode 100644 index 0000000000..77af670826 --- /dev/null +++ b/bundle/terranova/terranova_resources/job.go @@ -0,0 +1,93 @@ +package terranova_resources + +import ( + "context" + "reflect" + "strconv" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/structdiff" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/service/jobs" +) + +type ResourceJob struct { + client *databricks.WorkspaceClient + config jobs.JobSettings +} + +func NewResourceJob(client *databricks.WorkspaceClient, job resources.Job) (ResourceJob, error) { + return ResourceJob{ + client: client, + // TODO Use Processor with explicit field mapping + config: job.JobSettings, + }, nil +} + +func (r *ResourceJob) Config() any { + return r.config +} + +func (r *ResourceJob) DoCreate(ctx context.Context) (string, error) { + request, err := makeCreateJob(r.config) + if err != nil { + return "", err + } + response, err := r.client.Jobs.Create(ctx, request) + if err != nil { + return "", SDKError{Method: "Jobs.Create", Err: err} + } + return strconv.FormatInt(response.JobId, 10), nil +} + +func (r *ResourceJob) DoUpdate(ctx context.Context, id string) (string, error) { + request, err := makeResetJob(r.config, id) + if err != nil { + return "", err + } + err = r.client.Jobs.Reset(ctx, request) + if err != nil { + return "", SDKError{Method: "Jobs.Reset", Err: err} + } + return id, nil +} + +func (r *ResourceJob) DoDelete(ctx context.Context, id string) error { + return nil +} + +func (r *ResourceJob) WaitAfterCreate(ctx context.Context) error { + return nil +} + +func (r *ResourceJob) WaitAfterUpdate(ctx context.Context) error { + return nil +} + +func (r *ResourceJob) ClassifyChanges(changes []structdiff.Change) ChangeType { + return ChangeTypeUpdate +} + +func makeCreateJob(config jobs.JobSettings) (jobs.CreateJob, error) { + result := jobs.CreateJob{} + // TODO: Validate copy - all fields must be initialized or explicitly allowed to be empty + // Unset AccessControlList + err := copyViaJSON(&result, config) + return result, err +} + +var jobSettingsType = reflect.TypeOf(jobs.JobSettings{}) + +func (r *ResourceJob) GetType() reflect.Type { + return jobSettingsType +} + +func makeResetJob(config jobs.JobSettings, id string) (jobs.ResetJob, error) { + idInt, err := strconv.ParseInt(id, 10, 64) + if err != nil { + return jobs.ResetJob{}, err + } + result := jobs.ResetJob{JobId: idInt, NewSettings: config} + // TODO: Validate copy - all fields must be initialized or explicitly allowed to be empty + return result, err +} diff --git a/bundle/terranova/terranova_resources/pipeline.go b/bundle/terranova/terranova_resources/pipeline.go new file mode 100644 index 0000000000..8898a43183 --- /dev/null +++ b/bundle/terranova/terranova_resources/pipeline.go @@ -0,0 +1,72 @@ +package terranova_resources + +import ( + "context" + "reflect" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/structdiff" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/service/pipelines" +) + +type ResourcePipeline struct { + client *databricks.WorkspaceClient + config pipelines.CreatePipeline +} + +func NewResourcePipeline(client *databricks.WorkspaceClient, resource resources.Pipeline) (ResourcePipeline, error) { + return ResourcePipeline{ + client: client, + config: resource.CreatePipeline, + }, nil +} + +func (r *ResourcePipeline) Config() any { + return r.config +} + +func (r *ResourcePipeline) DoCreate(ctx context.Context) (string, error) { + response, err := r.client.Pipelines.Create(ctx, r.config) + if err != nil { + return "", SDKError{Method: "Pipelines.Create", Err: err} + } + return response.PipelineId, nil +} + +func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string) (string, error) { + request := pipelines.EditPipeline{} + err := copyViaJSON(&request, r.config) + if err != nil { + return "", err + } + request.PipelineId = id + + err = r.client.Pipelines.Update(ctx, request) + if err != nil { + return "", SDKError{Method: "Pipelines.Updater", Err: err} + } + return id, nil +} + +func (r *ResourcePipeline) DoDelete(ctx context.Context, id string) error { + return nil +} + +func (r *ResourcePipeline) WaitAfterCreate(ctx context.Context) error { + return nil +} + +func (r *ResourcePipeline) WaitAfterUpdate(ctx context.Context) error { + return nil +} + +func (r *ResourcePipeline) ClassifyChanges(changes []structdiff.Change) ChangeType { + return ChangeTypeUpdate +} + +var pipelineType = reflect.TypeOf(ResourcePipeline{}.config) + +func (r *ResourcePipeline) GetType() reflect.Type { + return pipelineType +} diff --git a/bundle/terranova/terranova_resources/resource.go b/bundle/terranova/terranova_resources/resource.go new file mode 100644 index 0000000000..2c17a9f9d8 --- /dev/null +++ b/bundle/terranova/terranova_resources/resource.go @@ -0,0 +1,90 @@ +package terranova_resources + +import ( + "context" + "fmt" + "reflect" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/structdiff" + "github.com/databricks/databricks-sdk-go" +) + +type IResource interface { + Config() any + + // Create the resource. Returns id of the resource. + DoCreate(ctx context.Context) (string, error) + + // Update the resource. Returns id of the resource (might be updated). + DoUpdate(ctx context.Context, oldId string) (string, error) + + DoDelete(ctx context.Context, oldId string) error + + WaitAfterCreate(ctx context.Context) error + WaitAfterUpdate(ctx context.Context) error + + // Get type of the struct that stores the state + GetType() reflect.Type + + ClassifyChanges(changes []structdiff.Change) ChangeType +} + +type ChangeType int + +func (c ChangeType) IsRecreate() bool { return c == ChangeTypeRecreate } +func (c ChangeType) IsUpdate() bool { return c == ChangeTypeUpdate } + +const ( + ChangeTypeNone ChangeType = 0 + ChangeTypeUpdate ChangeType = 1 + ChangeTypeRecreate ChangeType = -1 +) + +func New(client *databricks.WorkspaceClient, section, name string, config any) (IResource, error) { + switch section { + case "jobs": + typedConfig, ok := config.(*resources.Job) + if !ok { + return nil, fmt.Errorf("unexpected config type for jobs: %T", config) + } + if typedConfig == nil { + return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) + } + r, err := NewResourceJob(client, *typedConfig) + return &r, err + case "pipelines": + typedConfig, ok := config.(*resources.Pipeline) + if !ok { + return nil, fmt.Errorf("unexpected config type for pipelines: %T", config) + } + if typedConfig == nil { + return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) + } + r, err := NewResourcePipeline(client, *typedConfig) + return &r, err + case "schemas": + typedConfig, ok := config.(*resources.Schema) + if !ok { + return nil, fmt.Errorf("unexpected config type for schemas: %T", config) + } + if typedConfig == nil { + return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) + } + r, err := NewResourceSchema(client, *typedConfig) + return &r, err + case "apps": + typedConfig, ok := config.(*resources.App) + if !ok { + return nil, fmt.Errorf("unexpected config type for apps: %T", config) + } + if typedConfig == nil { + return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) + } + r, err := NewResourceApp(client, *typedConfig) + return &r, err + + default: + return nil, fmt.Errorf("unsupported resource type: %s", section) + } +} diff --git a/bundle/terranova/terranova_resources/schema.go b/bundle/terranova/terranova_resources/schema.go new file mode 100644 index 0000000000..8f6b257b49 --- /dev/null +++ b/bundle/terranova/terranova_resources/schema.go @@ -0,0 +1,76 @@ +package terranova_resources + +import ( + "context" + "reflect" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/structdiff" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/service/catalog" +) + +type ResourceSchema struct { + client *databricks.WorkspaceClient + config catalog.CreateSchema +} + +func NewResourceSchema(client *databricks.WorkspaceClient, schema resources.Schema) (ResourceSchema, error) { + return ResourceSchema{ + client: client, + config: schema.CreateSchema, + }, nil +} + +func (r *ResourceSchema) Config() any { + return r.config +} + +func (r *ResourceSchema) DoCreate(ctx context.Context) (string, error) { + response, err := r.client.Schemas.Create(ctx, r.config) + if err != nil { + return "", SDKError{Method: "Schemas.Create", Err: err} + } + return response.FullName, nil +} + +func (r *ResourceSchema) DoUpdate(ctx context.Context, id string) (string, error) { + updateRequest := catalog.UpdateSchema{} + err := copyViaJSON(&updateRequest, r.config) + if err != nil { + return "", err + } + + // TODO: properly populate these instead + updateRequest.ForceSendFields = nil + updateRequest.FullName = id + + response, err := r.client.Schemas.Update(ctx, updateRequest) + if err != nil { + return "", SDKError{Method: "Schemas.Update", Err: err} + } + + return response.FullName, nil +} + +func (r *ResourceSchema) DoDelete(ctx context.Context, id string) error { + return nil +} + +func (r *ResourceSchema) WaitAfterCreate(ctx context.Context) error { + return nil +} + +func (r *ResourceSchema) WaitAfterUpdate(ctx context.Context) error { + return nil +} + +func (r *ResourceSchema) ClassifyChanges(changes []structdiff.Change) ChangeType { + return ChangeTypeUpdate +} + +var schemaType = reflect.TypeOf(ResourceSchema{}.config) + +func (r *ResourceSchema) GetType() reflect.Type { + return schemaType +} diff --git a/bundle/terranova/terranova_resources/sdk_error.go b/bundle/terranova/terranova_resources/sdk_error.go new file mode 100644 index 0000000000..9497ec5761 --- /dev/null +++ b/bundle/terranova/terranova_resources/sdk_error.go @@ -0,0 +1,43 @@ +package terranova_resources + +import ( + "fmt" + + "github.com/databricks/databricks-sdk-go/apierr" + "github.com/databricks/databricks-sdk-go/retries" +) + +// Default String() for APIError only returns Message. +// This wrapper adds other info from the response such as StatusCode, ErrorCode and type information. +// This is motivated by working with testserver which might have incomplete or broken error messages (empty Message). +// However it would not hurt to have this info when debugging real endpoints as well. +// It also adds Method which refers to API method that was called (e.g. Jobs.Reset). + +type SDKError struct { + Method string + Err error +} + +func (e SDKError) Error() string { + if e.Method != "" { + return fmt.Sprintf("Method=%s %s", e.Method, formatSDKError(e.Err)) + } else { + return formatSDKError(e.Err) + } +} + +func formatSDKError(e error) string { + retriesErr, ok := e.(*retries.Err) + if ok { + return fmt.Sprintf("%T %s", retriesErr, formatAPIError(retriesErr.Err)) + } + return formatAPIError(e) +} + +func formatAPIError(e error) string { + apiErr, ok := e.(*apierr.APIError) + if ok { + return fmt.Sprintf("%T StatusCode=%d ErrorCode=%#v Message=%#v", apiErr, apiErr.StatusCode, apiErr.ErrorCode, apiErr.Message) + } + return e.Error() +} diff --git a/bundle/terranova/terranova_resources/util.go b/bundle/terranova/terranova_resources/util.go new file mode 100644 index 0000000000..be6a1e0af3 --- /dev/null +++ b/bundle/terranova/terranova_resources/util.go @@ -0,0 +1,22 @@ +package terranova_resources + +import ( + "encoding/json" + "errors" + "fmt" +) + +func copyViaJSON[T1, T2 any](dest *T1, src T2) error { + if dest == nil { + return errors.New("internal error: unexpected nil") + } + data, err := json.Marshal(src) + if err != nil { + return fmt.Errorf("Failed to serialize %T: %w", src, err) + } + err = json.Unmarshal(data, dest) + if err != nil { + return fmt.Errorf("Failed JSON roundtrip from %T to %T: %w", src, dest, err) + } + return nil +} diff --git a/bundle/terranova/terranova_state/state.go b/bundle/terranova/terranova_state/state.go new file mode 100644 index 0000000000..d0f45c6db7 --- /dev/null +++ b/bundle/terranova/terranova_state/state.go @@ -0,0 +1,129 @@ +package terranova_state + +import ( + "bytes" + "encoding/json" + "os" + "reflect" + "sync" + + "github.com/google/uuid" +) + +type TerranovaState struct { + Path string + data Database + mu sync.Mutex +} + +type Database struct { + Lineage string `json:"lineage"` + Serial int `json:"serial"` + Resources map[string]map[string]ResourceEntry `json:"resources"` +} + +type ResourceEntry struct { + ID string `json:"__id__"` + State any `json:"state"` +} + +func (db *TerranovaState) SaveState(section, resourceName, newID string, state any) error { + db.mu.Lock() + defer db.mu.Unlock() + + sectionData, ok := db.data.Resources[section] + if !ok { + sectionData = make(map[string]ResourceEntry) + db.data.Resources[section] = sectionData + } + + sectionData[resourceName] = ResourceEntry{ + ID: newID, + State: state, + } + + return nil +} + +func jsonRoundTrip(src, dest any) error { + raw, err := json.Marshal(src) + if err != nil { + return err + } + + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + dec.DisallowUnknownFields() + + return dec.Decode(dest) +} + +func (db *TerranovaState) GetSavedState(section, resourceName string, stateType reflect.Type) (any, error) { + sectionData, ok := db.data.Resources[section] + if !ok { + return nil, nil + } + + entry, ok := sectionData[resourceName] + if !ok { + return nil, nil + } + + destPtr := reflect.New(stateType).Interface() + err := jsonRoundTrip(entry.State, destPtr) + if err != nil { + return nil, err + } + + return reflect.ValueOf(destPtr).Elem().Interface(), nil +} + +func (db *TerranovaState) GetResourceID(section, resourceName string) (string, error) { + db.mu.Lock() + defer db.mu.Unlock() + + sectionData, ok := db.data.Resources[section] + if !ok { + return "", nil + } + + return sectionData[resourceName].ID, nil +} + +func (db *TerranovaState) Open(path string) error { + db.mu.Lock() + defer db.mu.Unlock() + + db.Path = path + + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + db.data = Database{ + Serial: 0, + Lineage: uuid.New().String(), + Resources: make(map[string]map[string]ResourceEntry), + } + return db.unlockedSave() + } + return err + } + + return json.Unmarshal(data, &db.data) +} + +func (db *TerranovaState) Finalize() error { + db.mu.Lock() + defer db.mu.Unlock() + + return db.unlockedSave() +} + +func (db *TerranovaState) unlockedSave() error { + data, err := json.MarshalIndent(db.data, "", " ") + if err != nil { + return err + } + + return os.WriteFile(db.Path, data, 0o600) +} diff --git a/libs/dag/dag.go b/libs/dag/dag.go new file mode 100644 index 0000000000..1f47a2fe9d --- /dev/null +++ b/libs/dag/dag.go @@ -0,0 +1,200 @@ +package dag + +import ( + "fmt" + "sort" + "strings" + "sync" +) + +type adjEdge struct { + to string + label string +} + +type Graph struct { + adj map[string][]adjEdge +} + +func NewGraph() *Graph { return &Graph{adj: make(map[string][]adjEdge)} } + +func (g *Graph) AddNode(name string) { + if _, ok := g.adj[name]; !ok { + g.adj[name] = nil + } +} + +// AddDirectedEdge inserts from → to with label. +func (g *Graph) AddDirectedEdge(from, to, label string) error { + if from == to { + return fmt.Errorf("self-loop %q", from) + } + g.AddNode(to) + g.adj[from] = append(g.adj[from], adjEdge{to: to, label: label}) + return nil +} + +type CycleError struct { + Nodes []string + Edges []string +} + +func (e *CycleError) Error() string { + if len(e.Nodes) == 0 { + return "cycle detected" + } + var b strings.Builder + b.WriteString("cycle detected: ") + b.WriteString(e.Nodes[0]) + for i := 1; i < len(e.Nodes); i++ { + b.WriteString(" refers to ") + b.WriteString(e.Nodes[i]) + b.WriteString(" via ") + b.WriteString(e.Edges[i-1]) + } + b.WriteString(" which refers to ") + b.WriteString(e.Nodes[0]) + b.WriteString(" via ") + b.WriteString(e.Edges[len(e.Edges)-1]) + b.WriteString(".") + return b.String() +} + +func (g *Graph) indegrees() map[string]int { + in := make(map[string]int, len(g.adj)) + for v := range g.adj { + in[v] = 0 + } + for _, outs := range g.adj { + for _, e := range outs { + in[e.to]++ + } + } + return in +} + +/* non-recursive DFS cycle check */ + +func (g *Graph) DetectCycle() error { + for _, outs := range g.adj { + sort.Slice(outs, func(i, j int) bool { return outs[i].to < outs[j].to }) + } + roots := make([]string, 0, len(g.adj)) + for v := range g.adj { + roots = append(roots, v) + } + sort.Strings(roots) + + const ( + white = 0 + grey = 1 + black = 2 + ) + color := make(map[string]int, len(g.adj)) + + type frame struct { + node string + inLbl string // edge label via which we entered this node + next int // next neighbour index to explore + } + var st stack[frame] + + for _, r := range roots { + if color[r] != white { + continue + } + color[r] = grey + st.push(frame{node: r}) + + for st.len() > 0 { + f := st.peek() + outs := g.adj[f.node] + + if f.next < len(outs) { + edge := outs[f.next] + st.peek().next++ + switch color[edge.to] { + case white: + color[edge.to] = grey + st.push(frame{node: edge.to, inLbl: edge.label}) + case grey: // back-edge → cycle + closeLbl := edge.label + var nodes, edges []string + for i := st.len() - 1; i >= 0; i-- { + nodes = append(nodes, st.data[i].node) + if lbl := st.data[i].inLbl; lbl != "" { + edges = append(edges, lbl) + } + if st.data[i].node == edge.to { + break + } + } + for i, j := 0, len(nodes)-1; i < j; i, j = i+1, j-1 { + nodes[i], nodes[j] = nodes[j], nodes[i] + } + for i, j := 0, len(edges)-1; i < j; i, j = i+1, j-1 { + edges[i], edges[j] = edges[j], edges[i] + } + edges = append(edges, closeLbl) + return &CycleError{Nodes: nodes, Edges: edges} + } + } else { + color[f.node] = black + st.pop() + } + } + } + return nil +} + +/* Run with fixed worker pool */ + +func (g *Graph) Run(pool int, runUnit func(string)) error { + if err := g.DetectCycle(); err != nil { + return err + } + + if pool > len(g.adj) { + pool = len(g.adj) + } + + in := g.indegrees() + ready := make(chan string, len(in)) + done := make(chan string, len(in)) + + var wg sync.WaitGroup + wg.Add(pool) + for range pool { + go func() { + defer wg.Done() + for n := range ready { + runUnit(n) + done <- n + } + }() + } + + keys := make([]string, 0, len(in)) + for k := range in { + keys = append(keys, k) + } + sort.Strings(keys) + for _, n := range keys { + if in[n] == 0 { + ready <- n + } + } + + for remaining := len(in); remaining > 0; { + n := <-done + remaining-- + for _, e := range g.adj[n] { + if in[e.to]--; in[e.to] == 0 { + ready <- e.to + } + } + } + close(ready) + wg.Wait() + return nil +} diff --git a/libs/dag/dag_test.go b/libs/dag/dag_test.go new file mode 100644 index 0000000000..4a98c6f447 --- /dev/null +++ b/libs/dag/dag_test.go @@ -0,0 +1,113 @@ +package dag + +import ( + "fmt" + "sort" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type edge struct{ from, to, name string } + +func TestRun_VariousGraphsAndPools(t *testing.T) { + pools := []int{1, 2, 3, 4} + + tests := []struct { + name string + nodes []string + seen []string + seen_sorted []string + edges []edge + cycle bool + msg string + }{ + // disconnected graphs + { + name: "one node", + nodes: []string{"A"}, + seen: []string{"A"}, + }, + { + name: "two nodes", + nodes: []string{"A", "B"}, + seen_sorted: []string{"A", "B"}, + }, + { + name: "three nodes", + nodes: []string{"A", "B", "C"}, + seen_sorted: []string{"A", "B", "C"}, + }, + { + name: "simple DAG", + edges: []edge{ + {"A", "B", "A->B"}, + {"B", "C", "B->C"}, + }, + seen: []string{"A", "B", "C"}, + }, + { + name: "two-node cycle", + edges: []edge{ + {"A", "B", "${A.id}"}, + {"B", "A", "${B.id}"}, + }, + cycle: true, + msg: "cycle detected: A refers to B via ${A.id} which refers to A via ${B.id}.", + }, + { + name: "three-node cycle", + edges: []edge{ + {"X", "Y", "e1"}, + {"Y", "Z", "e2"}, + {"Z", "X", "e3"}, + }, + cycle: true, + }, + } + + for _, tc := range tests { + tc := tc + for _, p := range pools { + t.Run(tc.name+fmt.Sprintf(" pool=%d", p), func(t *testing.T) { + g := NewGraph() + for _, n := range tc.nodes { + g.AddNode(n) + } + for _, e := range tc.edges { + _ = g.AddDirectedEdge(e.from, e.to, e.name) + } + + var mu sync.Mutex + var seen []string + err := g.Run(p, func(n string) { + mu.Lock() + seen = append(seen, n) + mu.Unlock() + }) + + if tc.cycle { + if err == nil { + t.Fatalf("expected cycle, got none") + } + if tc.msg != "" && err.Error() != tc.msg { + t.Fatalf("wrong msg:\n got %q\nwant %q", err, tc.msg) + } + } else { + require.NoError(t, err) + } + + if tc.seen != nil { + assert.Equal(t, tc.seen, seen) + } else if tc.seen_sorted != nil { + sort.Strings(seen) + assert.Equal(t, tc.seen_sorted, seen) + } else { + assert.Empty(t, seen) + } + }) + } + } +} diff --git a/libs/dag/stack.go b/libs/dag/stack.go new file mode 100644 index 0000000000..58b45b3a53 --- /dev/null +++ b/libs/dag/stack.go @@ -0,0 +1,8 @@ +package dag + +type stack[T any] struct{ data []T } + +func (s *stack[T]) push(v T) { s.data = append(s.data, v) } +func (s *stack[T]) pop() T { v := s.data[len(s.data)-1]; s.data = s.data[:len(s.data)-1]; return v } +func (s *stack[T]) peek() *T { return &s.data[len(s.data)-1] } +func (s *stack[T]) len() int { return len(s.data) } diff --git a/libs/testserver/server.go b/libs/testserver/server.go index b9f64a2bf6..f2521d22e4 100644 --- a/libs/testserver/server.go +++ b/libs/testserver/server.go @@ -305,3 +305,35 @@ func isNil(i any) bool { return false } } + +func CallHandler[T any](handler func(req Request, parsed T) Response, req Request) Response { + var parsed T + + err := json.Unmarshal(req.Body, &parsed) + if err != nil { + return Response{ + Body: fmt.Sprintf("cannot parse request: %s", err), + StatusCode: 400, + } + } + + defer req.Workspace.LockUnlock()() + + return handler(req, parsed) +} + +func CallHandler1[T any](handler func(req Request, parsed T, param1 string) Response, req Request, param1 string) Response { + var parsed T + + err := json.Unmarshal(req.Body, &parsed) + if err != nil { + return Response{ + Body: fmt.Sprintf("cannot parse request: %s", err), + StatusCode: 400, + } + } + + defer req.Workspace.LockUnlock()() + + return handler(req, parsed, param1) +} From fcc9d025e147e6f5f2986854b585a665f8ddcdb1 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 26 May 2025 11:11:59 +0200 Subject: [PATCH 02/41] shortenv env var names to avoid "The directory name is invalid" on windows >>> ../check_output.py [CLI] bundle deploy -t dev +STDOUT: 124 chars +Error: terraform init: fork/exec [TERRAFORM]: The directory name is invalid. --- .../default-python/combinations/check_output.py | 7 +++++-- .../default-python/combinations/input.json.tmpl | 8 ++++---- .../default-python/combinations/test.toml | 16 ++++++++++++---- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/acceptance/bundle/templates/default-python/combinations/check_output.py b/acceptance/bundle/templates/default-python/combinations/check_output.py index 2df337f4d7..f3367c20e7 100755 --- a/acceptance/bundle/templates/default-python/combinations/check_output.py +++ b/acceptance/bundle/templates/default-python/combinations/check_output.py @@ -3,8 +3,11 @@ import os import subprocess +SERVERLESS = os.environ["S"] == "yes" +INCLUDE_PYTHON = os.environ["P"] == "yes" + CLOUD_ENV = os.environ.get("CLOUD_ENV") -if CLOUD_ENV and os.environ["SERVERLESS"] == "yes" and not os.environ.get("TEST_METASTORE_ID"): +if CLOUD_ENV and SERVERLESS and not os.environ.get("TEST_METASTORE_ID"): sys.exit(f"SKIP_TEST SERVERLESS=yes but TEST_METASTORE_ID is empty in this env {CLOUD_ENV=}") BUILDING = "Building python_artifact" @@ -36,7 +39,7 @@ def is_printable_line(line): if is_printable_line(line): print(line.strip()) - if os.environ["INCLUDE_PYTHON"] == "yes": + if INCLUDE_PYTHON: assert BUILDING in p.stderr assert UPLOADING in p.stderr else: diff --git a/acceptance/bundle/templates/default-python/combinations/input.json.tmpl b/acceptance/bundle/templates/default-python/combinations/input.json.tmpl index 123fa25825..7177495fb3 100644 --- a/acceptance/bundle/templates/default-python/combinations/input.json.tmpl +++ b/acceptance/bundle/templates/default-python/combinations/input.json.tmpl @@ -1,7 +1,7 @@ { "project_name": "X$UNIQUE_NAME", - "include_notebook": "$INCLUDE_NOTEBOOK", - "include_dlt": "$INCLUDE_DLT", - "include_python": "$INCLUDE_PYTHON", - "serverless": "$SERVERLESS" + "include_notebook": "$N", + "include_dlt": "$D", + "include_python": "$P", + "serverless": "$S" } diff --git a/acceptance/bundle/templates/default-python/combinations/test.toml b/acceptance/bundle/templates/default-python/combinations/test.toml index e4c34dee79..4b670a9c86 100644 --- a/acceptance/bundle/templates/default-python/combinations/test.toml +++ b/acceptance/bundle/templates/default-python/combinations/test.toml @@ -1,10 +1,18 @@ Cloud = true Ignore = ["input.json"] -EnvMatrix.INCLUDE_NOTEBOOK = ["yes", "no"] -EnvMatrix.INCLUDE_DLT = ["yes", "no"] -EnvMatrix.INCLUDE_PYTHON = ["yes", "no"] -EnvMatrix.SERVERLESS = ["yes", "no"] + +# INCLUDE_NOTEBOOK +EnvMatrix.N = ["yes", "no"] + +# INCLUDE_DLT +EnvMatrix.D = ["yes", "no"] + +# INCLUDE_PYTHON +EnvMatrix.P = ["yes", "no"] + +# SERVERLESS +EnvMatrix.S = ["yes", "no"] [[Repls]] Old = '202\d{5}.\d{5,}' From 3f289c92f391de02df19e05d1f29cdbec54a6c44 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 26 May 2025 11:19:01 +0200 Subject: [PATCH 03/41] disable acc/cloud tests that don't work yet --- acceptance/bundle/deploy/dashboard/detect-change/test.toml | 2 ++ acceptance/bundle/deploy/files/no-snapshot-sync/test.toml | 2 ++ acceptance/bundle/deploy/jobs/check-metadata/test.toml | 2 ++ acceptance/bundle/deploy/jobs/shared-root-path/test.toml | 2 ++ acceptance/bundle/deploy/mlops-stacks/test.toml | 1 + acceptance/bundle/destroy/jobs-and-pipeline/test.toml | 2 ++ 6 files changed, 11 insertions(+) diff --git a/acceptance/bundle/deploy/dashboard/detect-change/test.toml b/acceptance/bundle/deploy/dashboard/detect-change/test.toml index f60aa89c84..53f74e8864 100644 --- a/acceptance/bundle/deploy/dashboard/detect-change/test.toml +++ b/acceptance/bundle/deploy/dashboard/detect-change/test.toml @@ -1,6 +1,8 @@ Local = false Cloud = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] + Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deploy/files/no-snapshot-sync/test.toml b/acceptance/bundle/deploy/files/no-snapshot-sync/test.toml index 7653e8c819..4ee7ce41f7 100644 --- a/acceptance/bundle/deploy/files/no-snapshot-sync/test.toml +++ b/acceptance/bundle/deploy/files/no-snapshot-sync/test.toml @@ -1,6 +1,8 @@ Local = false Cloud = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] + Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deploy/jobs/check-metadata/test.toml b/acceptance/bundle/deploy/jobs/check-metadata/test.toml index 844dc98bdd..1e4fef560d 100644 --- a/acceptance/bundle/deploy/jobs/check-metadata/test.toml +++ b/acceptance/bundle/deploy/jobs/check-metadata/test.toml @@ -1,6 +1,8 @@ Local = false Cloud = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] + Ignore = [ "databricks.yml", "a/b/resources.yml", diff --git a/acceptance/bundle/deploy/jobs/shared-root-path/test.toml b/acceptance/bundle/deploy/jobs/shared-root-path/test.toml index 0dfc598d6d..8685288a59 100644 --- a/acceptance/bundle/deploy/jobs/shared-root-path/test.toml +++ b/acceptance/bundle/deploy/jobs/shared-root-path/test.toml @@ -1,6 +1,8 @@ Local = false Cloud = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] + Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deploy/mlops-stacks/test.toml b/acceptance/bundle/deploy/mlops-stacks/test.toml index f652b1cfe4..07b07d6580 100644 --- a/acceptance/bundle/deploy/mlops-stacks/test.toml +++ b/acceptance/bundle/deploy/mlops-stacks/test.toml @@ -2,6 +2,7 @@ Cloud=true Local=false Badness = "the newly initialized bundle from the 'mlops-stacks' template contains two validation warnings in the configuration" +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] Ignore = [ "config.json" diff --git a/acceptance/bundle/destroy/jobs-and-pipeline/test.toml b/acceptance/bundle/destroy/jobs-and-pipeline/test.toml index f9455dae87..67e0370d74 100644 --- a/acceptance/bundle/destroy/jobs-and-pipeline/test.toml +++ b/acceptance/bundle/destroy/jobs-and-pipeline/test.toml @@ -1,6 +1,8 @@ Local = false Cloud = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] + Ignore = [ "databricks.yml", "resources.yml", From 2e92a33293ff8f00e56da8191241745117c35620 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 26 May 2025 11:29:43 +0200 Subject: [PATCH 04/41] clean up --- bundle/config/resources.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/bundle/config/resources.go b/bundle/config/resources.go index 02cbd4763c..117955002a 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -27,23 +27,6 @@ type Resources struct { SecretScopes map[string]*resources.SecretScope `json:"secret_scopes,omitempty"` } -func (r *Resources) GetResourceConfig(section, name string) (any, bool) { - // TODO: validate that the config is fully resolved - switch section { - case "jobs": - cfg, ok := r.Jobs[name] - return cfg, ok - case "schemas": - cfg, ok := r.Schemas[name] - return cfg, ok - case "apps": - cfg, ok := r.Schemas[name] - return cfg, ok - default: - return nil, false - } -} - type ConfigResource interface { // Exists returns true if the resource exists in the workspace configured in // the input workspace client. From da7c29179c42e1614f5aadc48665b0f8f47083c3 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 26 May 2025 14:27:55 +0200 Subject: [PATCH 05/41] WIP: destroy mutator + test (pipelines only); Graph is now generic --- .../bundle/resources/pipelines/output.txt | 1 + acceptance/bundle/resources/pipelines/script | 13 ++- bundle/phases/destroy.go | 62 ++++++++--- bundle/phases/initialize.go | 12 ++ bundle/terranova/deploy_mutator.go | 42 ++----- bundle/terranova/destroy_mutator.go | 57 ++++++++++ bundle/terranova/terranova_resources/app.go | 4 +- bundle/terranova/terranova_resources/job.go | 4 +- .../terranova/terranova_resources/pipeline.go | 10 +- .../terranova/terranova_resources/resource.go | 39 +++++-- .../terranova/terranova_resources/schema.go | 4 +- bundle/terranova/terranova_state/state.go | 52 +++++++++ libs/dag/dag.go | 104 ++++++++++-------- libs/dag/dag_test.go | 22 +++- 14 files changed, 308 insertions(+), 118 deletions(-) create mode 100644 bundle/terranova/destroy_mutator.go diff --git a/acceptance/bundle/resources/pipelines/output.txt b/acceptance/bundle/resources/pipelines/output.txt index 252fbd1720..adc9d4c098 100644 --- a/acceptance/bundle/resources/pipelines/output.txt +++ b/acceptance/bundle/resources/pipelines/output.txt @@ -36,6 +36,7 @@ Deploying resources... Updating deployment state... Deployment complete! +=== Fetch pipeline ID and verify remote state >>> [CLI] pipelines get [UUID] { "creator_user_name":"[USERNAME]", diff --git a/acceptance/bundle/resources/pipelines/script b/acceptance/bundle/resources/pipelines/script index 8a7b2533c7..40c8e20cb8 100644 --- a/acceptance/bundle/resources/pipelines/script +++ b/acceptance/bundle/resources/pipelines/script @@ -5,8 +5,8 @@ trace $CLI bundle deploy print_requests() { jq --sort-keys 'select(.method != "GET" and (.path | contains("/pipelines")))' < out.requests.txt - rm out.requests.txt - read_state.py pipelines my id name + errcode rm out.requests.txt + errcode read_state.py pipelines my id name } trace print_requests @@ -19,12 +19,19 @@ trace $CLI bundle deploy #trace print_requests ppid=`read_id.py pipelines my` +rm out.requests.txt + +title "Fetch pipeline ID and verify remote state" + +ppid=`read_id.py pipelines my` + +>>>>>>> 06684a8e5 (WIP: destroy mutator + test (pipelines only); Graph is now generic) trace $CLI pipelines get $ppid rm out.requests.txt title "Destroy the pipeline and verify that it's removed from the state and from remote" trace $CLI bundle destroy --auto-approve - trace print_requests + trace musterr $CLI pipelines get $ppid rm out.requests.txt diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go index b2c01343b5..055825db43 100644 --- a/bundle/phases/destroy.go +++ b/bundle/phases/destroy.go @@ -4,11 +4,13 @@ import ( "context" "errors" "net/http" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/files" "github.com/databricks/cli/bundle/deploy/lock" "github.com/databricks/cli/bundle/deploy/terraform" + "github.com/databricks/cli/bundle/terranova" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/diag" @@ -31,16 +33,31 @@ func assertRootPathExists(ctx context.Context, b *bundle.Bundle) (bool, error) { return true, err } -func approvalForDestroy(ctx context.Context, b *bundle.Bundle) (bool, error) { +func getDeleteActions(ctx context.Context, b *bundle.Bundle) ([]terraformlib.Action, error) { + if b.DirectDeployment { + allResources := b.ResourceDatabase.GetAllResources() + var deleteActions []terraformlib.Action + for _, node := range allResources { + rType, _ := strings.CutSuffix(node.Section, "s") + deleteActions = append(deleteActions, terraformlib.Action{ + Action: terraformlib.ActionTypeDelete, + ResourceType: rType, + ResourceName: node.Name, + }) + } + return deleteActions, nil + } + tf := b.Terraform + if tf == nil { - return false, errors.New("terraform not initialized") + return nil, errors.New("terraform not initialized") } // read plan file plan, err := tf.ShowPlanFile(ctx, b.Plan.Path) if err != nil { - return false, err + return nil, err } var deleteActions []terraformlib.Action @@ -54,6 +71,12 @@ func approvalForDestroy(ctx context.Context, b *bundle.Bundle) (bool, error) { } } + return deleteActions, nil +} + +func approvalForDestroy(ctx context.Context, b *bundle.Bundle) (bool, error) { + deleteActions, err := getDeleteActions(ctx, b) + if len(deleteActions) > 0 { cmdio.LogString(ctx, "The following resources will be deleted:") for _, a := range deleteActions { @@ -79,11 +102,20 @@ func approvalForDestroy(ctx context.Context, b *bundle.Bundle) (bool, error) { } func destroyCore(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - // Core destructive mutators for destroy. These require informed user consent. - diags := bundle.ApplySeq(ctx, b, - terraform.Apply(), - files.Delete(), - ) + var diags diag.Diagnostics + + if b.DirectDeployment { + diags = bundle.Apply(ctx, b, terranova.TerranovaDestroy()) + } else { + // Core destructive mutators for destroy. These require informed user consent. + diags = bundle.Apply(ctx, b, terraform.Apply()) + } + + if diags.HasError() { + return diags + } + + diags = diags.Extend(bundle.Apply(ctx, b, files.Delete())) if !diags.HasError() { cmdio.LogString(ctx, "Destroy complete!") @@ -115,12 +147,14 @@ func Destroy(ctx context.Context, b *bundle.Bundle) (diags diag.Diagnostics) { diags = diags.Extend(bundle.Apply(ctx, b, lock.Release(lock.GoalDestroy))) }() - diags = diags.Extend(bundle.ApplySeq(ctx, b, - terraform.StatePull(), - terraform.Interpolate(), - terraform.Write(), - terraform.Plan(terraform.PlanGoal("destroy")), - )) + if !b.DirectDeployment { + diags = diags.Extend(bundle.ApplySeq(ctx, b, + terraform.StatePull(), + terraform.Interpolate(), + terraform.Write(), + terraform.Plan(terraform.PlanGoal("destroy")), + )) + } if diags.HasError() { return diags diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 218b2e2ebe..94dd3823a7 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "path/filepath" "github.com/databricks/cli/bundle/config/mutator/resourcemutator" @@ -206,6 +207,17 @@ func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { b.Config.Bundle.Terraform = &config.Terraform{ExecPath: " "} return nil }) + + cacheDir, err := b.CacheDir(ctx) + if err != nil { + diags = diags.Extend(diag.FromErr(err)) + } + + databasePath := filepath.Join(cacheDir, "resources.json") + err = b.ResourceDatabase.Open(databasePath) + if err != nil { + diags = diags.Extend(diag.FromErr(err)) + } } else { // Reads (typed): b.Config.Bundle.Terraform (checks terraform configuration) // Updates (typed): b.Config.Bundle.Terraform (sets default values if not already set) diff --git a/bundle/terranova/deploy_mutator.go b/bundle/terranova/deploy_mutator.go index c59bcc9d45..60da6a58ce 100644 --- a/bundle/terranova/deploy_mutator.go +++ b/bundle/terranova/deploy_mutator.go @@ -3,8 +3,6 @@ package terranova import ( "context" "fmt" - "path/filepath" - "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/terranova/terranova_resources" @@ -35,28 +33,19 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di client := b.WorkspaceClient() - cacheDir, err := b.CacheDir(ctx) - if err != nil { - return diag.FromErr(err) - } - - databasePath := filepath.Join(cacheDir, "resources.json") - err = b.ResourceDatabase.Open(databasePath) - if err != nil { - return diag.FromErr(err) - } + g := dag.NewGraph[terranova_state.ResourceNode]() - g := dag.NewGraph() - - _, err = dyn.MapByPattern( + _, err := dyn.MapByPattern( b.Config.Value(), dyn.NewPattern(dyn.Key("resources"), dyn.AnyKey(), dyn.AnyKey()), func(p dyn.Path, v dyn.Value) (dyn.Value, error) { section := p[1].Key() - resourceName := p[2].Key() - node := section + "." + resourceName + name := p[2].Key() // log.Warnf(ctx, "Adding node=%s", node) - g.AddNode(node) + g.AddNode(terranova_state.ResourceNode{ + Section: section, + Name: name, + }) // TODO: Scan v for references and use g.AddDirectedEdge to add dependency return v, nil @@ -65,25 +54,16 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di countDeployed := 0 - err = g.Run(maxPoolSize, func(node string) { + err = g.Run(maxPoolSize, func(node terranova_state.ResourceNode) { // TODO func(node string) bool // If function returns false, downstream callers are not called // g.Run() should return list of not executed nodes // log.Warnf(ctx, "Processing node=%s", node) - items := strings.SplitN(node, ".", 2) - if len(items) != 2 { - diags.AppendErrorf("internal error: unexpected DAG node %#v", node) - return - } - - section := items[0] - name := items[1] - // TODO: resolve all resource references inside this resource. It should be possible, if graph was constructed correctly. // If it is not possible, return error (and fail this and dependent resources) - config, ok := b.GetResourceConfig(section, name) + config, ok := b.GetResourceConfig(node.Section, node.Name) if !ok { diags.AppendErrorf("internal error: cannot get config for %s", node) return @@ -92,8 +72,8 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di d := Deployer{ client: client, db: &b.ResourceDatabase, - section: section, - resourceName: name, + section: node.Section, + resourceName: node.Name, } err = d.Deploy(ctx, config) diff --git a/bundle/terranova/destroy_mutator.go b/bundle/terranova/destroy_mutator.go new file mode 100644 index 0000000000..5356d7349c --- /dev/null +++ b/bundle/terranova/destroy_mutator.go @@ -0,0 +1,57 @@ +package terranova + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/terranova/terranova_resources" + "github.com/databricks/cli/bundle/terranova/terranova_state" + "github.com/databricks/cli/libs/dag" + "github.com/databricks/cli/libs/diag" +) + +type terranovaDestroyMutator struct{} + +func TerranovaDestroy() bundle.Mutator { + return &terranovaDestroyMutator{} +} + +func (m *terranovaDestroyMutator) Name() string { + return "TerranovaDestroy" +} + +func (m *terranovaDestroyMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.SafeDiagnostics + client := b.WorkspaceClient() + + allResources := b.ResourceDatabase.GetAllResources() + g := dag.NewGraph[terranova_state.ResourceNode]() + + for _, node := range allResources { + g.AddNode(node) + } + + // TODO: respect dependencies; dependencies need to be part of state, not config. + + err := g.Run(maxPoolSize, func(node terranova_state.ResourceNode) { + err := terranova_resources.DestroyResource(ctx, client, node.Section, node.ID) + if err != nil { + diags.AppendErrorf("destroying %s: %s", node, err) + return + } + // TODO: did DestroyResource fail because it did not exist? we can clean it up from the state as well + + err = b.ResourceDatabase.DeleteState(node.Section, node.Name) + if err != nil { + diags.AppendErrorf("deleting from the state %s: %s", node, err) + return + } + }) + if err != nil { + diags.AppendError(err) + } + + _ = b.ResourceDatabase.Finalize() + + return diags.Diags +} diff --git a/bundle/terranova/terranova_resources/app.go b/bundle/terranova/terranova_resources/app.go index 724584ab65..b2c00fae9a 100644 --- a/bundle/terranova/terranova_resources/app.go +++ b/bundle/terranova/terranova_resources/app.go @@ -15,8 +15,8 @@ type ResourceApp struct { config apps.App } -func NewResourceApp(client *databricks.WorkspaceClient, config resources.App) (ResourceApp, error) { - return ResourceApp{ +func NewResourceApp(client *databricks.WorkspaceClient, config resources.App) (*ResourceApp, error) { + return &ResourceApp{ client: client, config: config.App, }, nil diff --git a/bundle/terranova/terranova_resources/job.go b/bundle/terranova/terranova_resources/job.go index 77af670826..1eb6c1e7b7 100644 --- a/bundle/terranova/terranova_resources/job.go +++ b/bundle/terranova/terranova_resources/job.go @@ -16,8 +16,8 @@ type ResourceJob struct { config jobs.JobSettings } -func NewResourceJob(client *databricks.WorkspaceClient, job resources.Job) (ResourceJob, error) { - return ResourceJob{ +func NewResourceJob(client *databricks.WorkspaceClient, job resources.Job) (*ResourceJob, error) { + return &ResourceJob{ client: client, // TODO Use Processor with explicit field mapping config: job.JobSettings, diff --git a/bundle/terranova/terranova_resources/pipeline.go b/bundle/terranova/terranova_resources/pipeline.go index 8898a43183..f4419696a8 100644 --- a/bundle/terranova/terranova_resources/pipeline.go +++ b/bundle/terranova/terranova_resources/pipeline.go @@ -15,8 +15,8 @@ type ResourcePipeline struct { config pipelines.CreatePipeline } -func NewResourcePipeline(client *databricks.WorkspaceClient, resource resources.Pipeline) (ResourcePipeline, error) { - return ResourcePipeline{ +func NewResourcePipeline(client *databricks.WorkspaceClient, resource resources.Pipeline) (*ResourcePipeline, error) { + return &ResourcePipeline{ client: client, config: resource.CreatePipeline, }, nil @@ -44,12 +44,16 @@ func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string) (string, err err = r.client.Pipelines.Update(ctx, request) if err != nil { - return "", SDKError{Method: "Pipelines.Updater", Err: err} + return "", SDKError{Method: "Pipelines.Update", Err: err} } return id, nil } func (r *ResourcePipeline) DoDelete(ctx context.Context, id string) error { + err := r.client.Pipelines.DeleteByPipelineId(ctx, id) + if err != nil { + return SDKError{Method: "Pipelines.DeleteByPipelineId", Err: err} + } return nil } diff --git a/bundle/terranova/terranova_resources/resource.go b/bundle/terranova/terranova_resources/resource.go index 2c17a9f9d8..89f397d1de 100644 --- a/bundle/terranova/terranova_resources/resource.go +++ b/bundle/terranova/terranova_resources/resource.go @@ -51,8 +51,8 @@ func New(client *databricks.WorkspaceClient, section, name string, config any) ( if typedConfig == nil { return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) } - r, err := NewResourceJob(client, *typedConfig) - return &r, err + return NewResourceJob(client, *typedConfig) + case "pipelines": typedConfig, ok := config.(*resources.Pipeline) if !ok { @@ -61,8 +61,8 @@ func New(client *databricks.WorkspaceClient, section, name string, config any) ( if typedConfig == nil { return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) } - r, err := NewResourcePipeline(client, *typedConfig) - return &r, err + return NewResourcePipeline(client, *typedConfig) + case "schemas": typedConfig, ok := config.(*resources.Schema) if !ok { @@ -71,8 +71,8 @@ func New(client *databricks.WorkspaceClient, section, name string, config any) ( if typedConfig == nil { return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) } - r, err := NewResourceSchema(client, *typedConfig) - return &r, err + return NewResourceSchema(client, *typedConfig) + case "apps": typedConfig, ok := config.(*resources.App) if !ok { @@ -81,10 +81,33 @@ func New(client *databricks.WorkspaceClient, section, name string, config any) ( if typedConfig == nil { return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) } - r, err := NewResourceApp(client, *typedConfig) - return &r, err + return NewResourceApp(client, *typedConfig) default: return nil, fmt.Errorf("unsupported resource type: %s", section) } } + +func DestroyResource(ctx context.Context, client *databricks.WorkspaceClient, section, id string) error { + var err error + var r IResource + + switch section { + case "jobs": + r, err = NewResourceJob(client, resources.Job{}) + case "pipelines": + r, err = NewResourcePipeline(client, resources.Pipeline{}) + case "schemas": + r, err = NewResourceSchema(client, resources.Schema{}) + case "apps": + r, err = NewResourceApp(client, resources.App{}) + default: + return fmt.Errorf("unsupported resource type: %s", section) + } + + if err != nil { + return err + } + + return r.DoDelete(ctx, id) +} diff --git a/bundle/terranova/terranova_resources/schema.go b/bundle/terranova/terranova_resources/schema.go index 8f6b257b49..89607c5d0b 100644 --- a/bundle/terranova/terranova_resources/schema.go +++ b/bundle/terranova/terranova_resources/schema.go @@ -15,8 +15,8 @@ type ResourceSchema struct { config catalog.CreateSchema } -func NewResourceSchema(client *databricks.WorkspaceClient, schema resources.Schema) (ResourceSchema, error) { - return ResourceSchema{ +func NewResourceSchema(client *databricks.WorkspaceClient, schema resources.Schema) (*ResourceSchema, error) { + return &ResourceSchema{ client: client, config: schema.CreateSchema, }, nil diff --git a/bundle/terranova/terranova_state/state.go b/bundle/terranova/terranova_state/state.go index d0f45c6db7..16ff471434 100644 --- a/bundle/terranova/terranova_state/state.go +++ b/bundle/terranova/terranova_state/state.go @@ -7,6 +7,7 @@ import ( "reflect" "sync" + "github.com/databricks/cli/libs/utils" "github.com/google/uuid" ) @@ -27,6 +28,12 @@ type ResourceEntry struct { State any `json:"state"` } +type ResourceNode struct { + Section string + Name string + ID string +} + func (db *TerranovaState) SaveState(section, resourceName, newID string, state any) error { db.mu.Lock() defer db.mu.Unlock() @@ -45,6 +52,20 @@ func (db *TerranovaState) SaveState(section, resourceName, newID string, state a return nil } +func (db *TerranovaState) DeleteState(section, resourceName string) error { + db.mu.Lock() + defer db.mu.Unlock() + + sectionData, ok := db.data.Resources[section] + if !ok { + return nil + } + + delete(sectionData, resourceName) + + return nil +} + func jsonRoundTrip(src, dest any) error { raw, err := json.Marshal(src) if err != nil { @@ -90,6 +111,23 @@ func (db *TerranovaState) GetResourceID(section, resourceName string) (string, e return sectionData[resourceName].ID, nil } +func (db *TerranovaState) GetAllResources() []ResourceNode { + nodes := make([]ResourceNode, 0, len(db.data.Resources)*4) + + for _, section := range utils.SortedKeys(db.data.Resources) { + sectionData := db.data.Resources[section] + for _, name := range utils.SortedKeys(sectionData) { + nodes = append(nodes, ResourceNode{ + Section: section, + Name: name, + ID: sectionData[name].ID, + }) + } + } + + return nodes +} + func (db *TerranovaState) Open(path string) error { db.mu.Lock() defer db.mu.Unlock() @@ -127,3 +165,17 @@ func (db *TerranovaState) unlockedSave() error { return os.WriteFile(db.Path, data, 0o600) } + +func (r ResourceNode) Less(other ResourceNode) bool { + if r.Section < other.Section { + return true + } + if r.Section > other.Section { + return false + } + return r.Name < other.Name +} + +func (r ResourceNode) String() string { + return r.Section + "." + r.Name + "#" + r.ID +} diff --git a/libs/dag/dag.go b/libs/dag/dag.go index 1f47a2fe9d..3be6924da3 100644 --- a/libs/dag/dag.go +++ b/libs/dag/dag.go @@ -7,61 +7,68 @@ import ( "sync" ) -type adjEdge struct { - to string +type Lessable[T comparable] interface { + comparable + Less(T) bool +} + +type adjEdge[N Lessable[N]] struct { + to N label string } -type Graph struct { - adj map[string][]adjEdge +type Graph[N Lessable[N]] struct { + adj map[N][]adjEdge[N] } -func NewGraph() *Graph { return &Graph{adj: make(map[string][]adjEdge)} } +func NewGraph[N Lessable[N]]() *Graph[N] { + return &Graph[N]{adj: make(map[N][]adjEdge[N])} +} -func (g *Graph) AddNode(name string) { - if _, ok := g.adj[name]; !ok { - g.adj[name] = nil +func (g *Graph[N]) AddNode(n N) { + if _, ok := g.adj[n]; !ok { + g.adj[n] = nil } } -// AddDirectedEdge inserts from → to with label. -func (g *Graph) AddDirectedEdge(from, to, label string) error { +func (g *Graph[N]) AddDirectedEdge(from, to N, label string) error { if from == to { - return fmt.Errorf("self-loop %q", from) + return fmt.Errorf("self-loop %v", from) } + g.AddNode(from) g.AddNode(to) - g.adj[from] = append(g.adj[from], adjEdge{to: to, label: label}) + g.adj[from] = append(g.adj[from], adjEdge[N]{to: to, label: label}) return nil } -type CycleError struct { - Nodes []string +type CycleError[N Lessable[N]] struct { + Nodes []N Edges []string } -func (e *CycleError) Error() string { +func (e *CycleError[N]) Error() string { if len(e.Nodes) == 0 { return "cycle detected" } var b strings.Builder b.WriteString("cycle detected: ") - b.WriteString(e.Nodes[0]) + fmt.Fprint(&b, e.Nodes[0]) for i := 1; i < len(e.Nodes); i++ { b.WriteString(" refers to ") - b.WriteString(e.Nodes[i]) + fmt.Fprint(&b, e.Nodes[i]) b.WriteString(" via ") b.WriteString(e.Edges[i-1]) } b.WriteString(" which refers to ") - b.WriteString(e.Nodes[0]) + fmt.Fprint(&b, e.Nodes[0]) b.WriteString(" via ") b.WriteString(e.Edges[len(e.Edges)-1]) b.WriteString(".") return b.String() } -func (g *Graph) indegrees() map[string]int { - in := make(map[string]int, len(g.adj)) +func (g *Graph[N]) indegrees() map[N]int { + in := make(map[N]int, len(g.adj)) for v := range g.adj { in[v] = 0 } @@ -73,38 +80,40 @@ func (g *Graph) indegrees() map[string]int { return in } -/* non-recursive DFS cycle check */ - -func (g *Graph) DetectCycle() error { - for _, outs := range g.adj { - sort.Slice(outs, func(i, j int) bool { return outs[i].to < outs[j].to }) +func (g *Graph[N]) DetectCycle() error { + // 1. sort every adjacency list once + for k, outs := range g.adj { + sort.Slice(outs, func(i, j int) bool { return outs[i].to.Less(outs[j].to) }) + g.adj[k] = outs } - roots := make([]string, 0, len(g.adj)) + + // 2. sorted list of roots + roots := make([]N, 0, len(g.adj)) for v := range g.adj { roots = append(roots, v) } - sort.Strings(roots) + sort.Slice(roots, func(i, j int) bool { return roots[i].Less(roots[j]) }) const ( white = 0 grey = 1 black = 2 ) - color := make(map[string]int, len(g.adj)) + color := make(map[N]int, len(g.adj)) type frame struct { - node string - inLbl string // edge label via which we entered this node - next int // next neighbour index to explore + node N + inLbl string + next int } var st stack[frame] - for _, r := range roots { - if color[r] != white { + for _, root := range roots { + if color[root] != white { continue } - color[r] = grey - st.push(frame{node: r}) + color[root] = grey + st.push(frame{node: root}) for st.len() > 0 { f := st.peek() @@ -117,9 +126,10 @@ func (g *Graph) DetectCycle() error { case white: color[edge.to] = grey st.push(frame{node: edge.to, inLbl: edge.label}) - case grey: // back-edge → cycle + case grey: closeLbl := edge.label - var nodes, edges []string + var nodes []N + var edges []string for i := st.len() - 1; i >= 0; i-- { nodes = append(nodes, st.data[i].node) if lbl := st.data[i].inLbl; lbl != "" { @@ -136,7 +146,7 @@ func (g *Graph) DetectCycle() error { edges[i], edges[j] = edges[j], edges[i] } edges = append(edges, closeLbl) - return &CycleError{Nodes: nodes, Edges: edges} + return &CycleError[N]{Nodes: nodes, Edges: edges} } } else { color[f.node] = black @@ -147,24 +157,21 @@ func (g *Graph) DetectCycle() error { return nil } -/* Run with fixed worker pool */ - -func (g *Graph) Run(pool int, runUnit func(string)) error { +func (g *Graph[N]) Run(pool int, runUnit func(N)) error { if err := g.DetectCycle(); err != nil { return err } - - if pool > len(g.adj) { + if pool <= 0 || pool > len(g.adj) { pool = len(g.adj) } in := g.indegrees() - ready := make(chan string, len(in)) - done := make(chan string, len(in)) + ready := make(chan N, len(in)) + done := make(chan N, len(in)) var wg sync.WaitGroup wg.Add(pool) - for range pool { + for i := 0; i < pool; i++ { go func() { defer wg.Done() for n := range ready { @@ -174,11 +181,12 @@ func (g *Graph) Run(pool int, runUnit func(string)) error { }() } - keys := make([]string, 0, len(in)) + // stable initial-ready order + keys := make([]N, 0, len(in)) for k := range in { keys = append(keys, k) } - sort.Strings(keys) + sort.Slice(keys, func(i, j int) bool { return keys[i].Less(keys[j]) }) for _, n := range keys { if in[n] == 0 { ready <- n diff --git a/libs/dag/dag_test.go b/libs/dag/dag_test.go index 4a98c6f447..389ab6161e 100644 --- a/libs/dag/dag_test.go +++ b/libs/dag/dag_test.go @@ -12,6 +12,18 @@ import ( type edge struct{ from, to, name string } +type stringWrapper struct { + Value string +} + +func (s stringWrapper) Less(other stringWrapper) bool { + return s.Value < other.Value +} + +func (s stringWrapper) String() string { + return s.Value +} + func TestRun_VariousGraphsAndPools(t *testing.T) { pools := []int{1, 2, 3, 4} @@ -72,19 +84,19 @@ func TestRun_VariousGraphsAndPools(t *testing.T) { tc := tc for _, p := range pools { t.Run(tc.name+fmt.Sprintf(" pool=%d", p), func(t *testing.T) { - g := NewGraph() + g := NewGraph[stringWrapper]() for _, n := range tc.nodes { - g.AddNode(n) + g.AddNode(stringWrapper{n}) } for _, e := range tc.edges { - _ = g.AddDirectedEdge(e.from, e.to, e.name) + _ = g.AddDirectedEdge(stringWrapper{e.from}, stringWrapper{e.to}, e.name) } var mu sync.Mutex var seen []string - err := g.Run(p, func(n string) { + err := g.Run(p, func(n stringWrapper) { mu.Lock() - seen = append(seen, n) + seen = append(seen, n.Value) mu.Unlock() }) From 985ccf90c414ff9426e1c432474086ddfc7fe234 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 26 May 2025 14:34:00 +0200 Subject: [PATCH 06/41] destroy for jobs + test --- acceptance/bundle/resources/jobs/output.txt | 63 ++++++++++++++++++++- acceptance/bundle/resources/jobs/script | 14 +++++ bundle/terranova/terranova_resources/job.go | 8 +++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/acceptance/bundle/resources/jobs/output.txt b/acceptance/bundle/resources/jobs/output.txt index b28e1bd1af..ecbca8fa2a 100644 --- a/acceptance/bundle/resources/jobs/output.txt +++ b/acceptance/bundle/resources/jobs/output.txt @@ -1,4 +1,3 @@ - >>> [CLI] bundle deploy Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... Deploying resources... @@ -87,4 +86,64 @@ Deployment complete! "method": "POST", "path": "/api/2.2/jobs/reset" } -jobs foo id='[NUMID]' name='foo' +jobs foo id='[TEST_JOB_ID+0]' name='foo' + +=== Fetch job ID and verify remote state +>>> [CLI] jobs get [TEST_JOB_ID+0] +{ + "job_id":[TEST_JOB_ID+0], + "settings": { + "deployment": { + "kind":"BUNDLE", + "metadata_file_path":"/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json" + }, + "edit_mode":"UI_LOCKED", + "format":"MULTI_TASK", + "job_clusters": [ + { + "job_cluster_key":"key", + "new_cluster": { + "num_workers":0, + "spark_version":"13.3.x-scala2.12" + } + } + ], + "max_concurrent_runs":1, + "name":"foo", + "queue": { + "enabled":true + }, + "trigger": { + "pause_status":"UNPAUSED", + "periodic": { + "interval":1, + "unit":"HOURS" + } + } + } +} + +=== Destroy the job and verify that it's removed from the state and from remote +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete job foo + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Deleting files... +Destroy complete! + +>>> print_requests +{ + "body": { + "job_id": [TEST_JOB_ID+0] + }, + "method": "POST", + "path": "/api/2.2/jobs/delete" +} +State not found for jobs.foo + +>>> musterr [CLI] jobs get [TEST_JOB_ID+0] +Error: Not Found + +Exit code (musterr): 1 diff --git a/acceptance/bundle/resources/jobs/script b/acceptance/bundle/resources/jobs/script index 2ac136e3a3..dc76550165 100644 --- a/acceptance/bundle/resources/jobs/script +++ b/acceptance/bundle/resources/jobs/script @@ -13,3 +13,17 @@ title "Update trigger.periodic.unit and re-deploy" trace update_file.py databricks.yml DAYS HOURS trace $CLI bundle deploy trace print_requests + +title "Fetch job ID and verify remote state" + +ppid=`read_id.py jobs foo` + +trace $CLI jobs get $ppid +rm out.requests.txt + +title "Destroy the job and verify that it's removed from the state and from remote" +trace $CLI bundle destroy --auto-approve +trace print_requests + +trace musterr $CLI jobs get $ppid +rm out.requests.txt diff --git a/bundle/terranova/terranova_resources/job.go b/bundle/terranova/terranova_resources/job.go index 1eb6c1e7b7..589eb43a35 100644 --- a/bundle/terranova/terranova_resources/job.go +++ b/bundle/terranova/terranova_resources/job.go @@ -53,6 +53,14 @@ func (r *ResourceJob) DoUpdate(ctx context.Context, id string) (string, error) { } func (r *ResourceJob) DoDelete(ctx context.Context, id string) error { + idInt, err := strconv.ParseInt(id, 10, 64) + if err != nil { + return err + } + r.client.Jobs.DeleteByJobId(ctx, idInt) + if err != nil { + return SDKError{Method: "Jobs.DeleteByJobId", Err: err} + } return nil } From 0b56d8992045374e10b38778b300d461c74e3199 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 27 May 2025 11:26:18 +0200 Subject: [PATCH 07/41] update github action to use ENVFILTER --- .github/workflows/push.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index bbad4fb517..1f739e70fa 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -82,14 +82,14 @@ jobs: # populates the cache and cache may include test results. if: ${{ github.event_name == 'pull_request' || github.event_name == 'schedule' }} env: - DATABRICKS_CLI_DEPLOYMENT: ${{ matrix.deployment }} + ENVFILTER: DATABRICKS_CLI_DEPLOYMENT=${{ matrix.deployment }} run: make test - name: Run tests with coverage # Still run 'make cover' on push to main and merge checks to make sure it does not get broken. if: ${{ github.event_name != 'pull_request' && github.event_name != 'schedule' }} env: - DATABRICKS_CLI_DEPLOYMENT: ${{ matrix.deployment }} + ENVFILTER: DATABRICKS_CLI_DEPLOYMENT=${{ matrix.deployment }} run: make cover - name: Analyze slow tests From 8e39d69fd5d26ff856b115e6b4d3c30f277ab60e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 27 May 2025 11:36:53 +0200 Subject: [PATCH 08/41] lint fix --- acceptance/acceptance_test.go | 2 -- bundle/phases/destroy.go | 3 +++ bundle/terranova/terranova_resources/job.go | 2 +- libs/dag/dag.go | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 73da7e6125..e8448ee8ed 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -35,8 +35,6 @@ import ( "github.com/stretchr/testify/require" ) -const deploymentEnvVar = "DATABRICKS_CLI_DEPLOYMENT" - var ( KeepTmp bool NoRepl bool diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go index 055825db43..8d07949a1b 100644 --- a/bundle/phases/destroy.go +++ b/bundle/phases/destroy.go @@ -76,6 +76,9 @@ func getDeleteActions(ctx context.Context, b *bundle.Bundle) ([]terraformlib.Act func approvalForDestroy(ctx context.Context, b *bundle.Bundle) (bool, error) { deleteActions, err := getDeleteActions(ctx, b) + if err != nil { + return false, err + } if len(deleteActions) > 0 { cmdio.LogString(ctx, "The following resources will be deleted:") diff --git a/bundle/terranova/terranova_resources/job.go b/bundle/terranova/terranova_resources/job.go index 589eb43a35..12a024eb9b 100644 --- a/bundle/terranova/terranova_resources/job.go +++ b/bundle/terranova/terranova_resources/job.go @@ -57,7 +57,7 @@ func (r *ResourceJob) DoDelete(ctx context.Context, id string) error { if err != nil { return err } - r.client.Jobs.DeleteByJobId(ctx, idInt) + err = r.client.Jobs.DeleteByJobId(ctx, idInt) if err != nil { return SDKError{Method: "Jobs.DeleteByJobId", Err: err} } diff --git a/libs/dag/dag.go b/libs/dag/dag.go index 3be6924da3..9b039d0608 100644 --- a/libs/dag/dag.go +++ b/libs/dag/dag.go @@ -171,7 +171,7 @@ func (g *Graph[N]) Run(pool int, runUnit func(N)) error { var wg sync.WaitGroup wg.Add(pool) - for i := 0; i < pool; i++ { + for range pool { go func() { defer wg.Done() for n := range ready { From 756253ec389d0d2cf7b3f1f56fdc969748f0b79e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 27 May 2025 11:41:50 +0200 Subject: [PATCH 09/41] do not create empty resources.json --- bundle/terranova/terranova_state/state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/terranova/terranova_state/state.go b/bundle/terranova/terranova_state/state.go index 16ff471434..a6a6635d1b 100644 --- a/bundle/terranova/terranova_state/state.go +++ b/bundle/terranova/terranova_state/state.go @@ -142,7 +142,7 @@ func (db *TerranovaState) Open(path string) error { Lineage: uuid.New().String(), Resources: make(map[string]map[string]ResourceEntry), } - return db.unlockedSave() + return nil } return err } From 9dbf7778c727f0b96b2cd3430ff1485e1bb5fd3e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 27 May 2025 16:37:06 +0200 Subject: [PATCH 10/41] clean up env var from tests --- acceptance/bundle/deploy/dashboard/detect-change/test.toml | 2 -- acceptance/bundle/deploy/files/no-snapshot-sync/test.toml | 2 -- acceptance/bundle/deploy/jobs/check-metadata/test.toml | 2 -- acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml | 2 -- acceptance/bundle/deploy/jobs/shared-root-path/test.toml | 2 -- acceptance/bundle/deploy/mlops-stacks/test.toml | 1 - acceptance/bundle/destroy/jobs-and-pipeline/test.toml | 2 -- 7 files changed, 13 deletions(-) diff --git a/acceptance/bundle/deploy/dashboard/detect-change/test.toml b/acceptance/bundle/deploy/dashboard/detect-change/test.toml index 53f74e8864..f60aa89c84 100644 --- a/acceptance/bundle/deploy/dashboard/detect-change/test.toml +++ b/acceptance/bundle/deploy/dashboard/detect-change/test.toml @@ -1,8 +1,6 @@ Local = false Cloud = true -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] - Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deploy/files/no-snapshot-sync/test.toml b/acceptance/bundle/deploy/files/no-snapshot-sync/test.toml index 4ee7ce41f7..7653e8c819 100644 --- a/acceptance/bundle/deploy/files/no-snapshot-sync/test.toml +++ b/acceptance/bundle/deploy/files/no-snapshot-sync/test.toml @@ -1,8 +1,6 @@ Local = false Cloud = true -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] - Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deploy/jobs/check-metadata/test.toml b/acceptance/bundle/deploy/jobs/check-metadata/test.toml index 1e4fef560d..844dc98bdd 100644 --- a/acceptance/bundle/deploy/jobs/check-metadata/test.toml +++ b/acceptance/bundle/deploy/jobs/check-metadata/test.toml @@ -1,8 +1,6 @@ Local = false Cloud = true -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] - Ignore = [ "databricks.yml", "a/b/resources.yml", diff --git a/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml b/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml index 72ef883dc8..a07b3c091e 100644 --- a/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml +++ b/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml @@ -1,8 +1,6 @@ Local = true Cloud = true -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # "bundle destroy" - Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deploy/jobs/shared-root-path/test.toml b/acceptance/bundle/deploy/jobs/shared-root-path/test.toml index 8685288a59..0dfc598d6d 100644 --- a/acceptance/bundle/deploy/jobs/shared-root-path/test.toml +++ b/acceptance/bundle/deploy/jobs/shared-root-path/test.toml @@ -1,8 +1,6 @@ Local = false Cloud = true -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] - Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deploy/mlops-stacks/test.toml b/acceptance/bundle/deploy/mlops-stacks/test.toml index 07b07d6580..f652b1cfe4 100644 --- a/acceptance/bundle/deploy/mlops-stacks/test.toml +++ b/acceptance/bundle/deploy/mlops-stacks/test.toml @@ -2,7 +2,6 @@ Cloud=true Local=false Badness = "the newly initialized bundle from the 'mlops-stacks' template contains two validation warnings in the configuration" -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] Ignore = [ "config.json" diff --git a/acceptance/bundle/destroy/jobs-and-pipeline/test.toml b/acceptance/bundle/destroy/jobs-and-pipeline/test.toml index 67e0370d74..f9455dae87 100644 --- a/acceptance/bundle/destroy/jobs-and-pipeline/test.toml +++ b/acceptance/bundle/destroy/jobs-and-pipeline/test.toml @@ -1,8 +1,6 @@ Local = false Cloud = true -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["direct"] - Ignore = [ "databricks.yml", "resources.yml", From 72a8bf0ee8de14dcd315249c1dd6ffd53c0c95f4 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 27 May 2025 16:38:14 +0200 Subject: [PATCH 11/41] enable test --- acceptance/bundle/deploy/jobs/fail-on-active-runs/test.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/acceptance/bundle/deploy/jobs/fail-on-active-runs/test.toml b/acceptance/bundle/deploy/jobs/fail-on-active-runs/test.toml index 8136c306bf..a07b3c091e 100644 --- a/acceptance/bundle/deploy/jobs/fail-on-active-runs/test.toml +++ b/acceptance/bundle/deploy/jobs/fail-on-active-runs/test.toml @@ -1,8 +1,6 @@ Local = true Cloud = true -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] - Ignore = [ "databricks.yml", ] From d5743b3d462eed198730e1703103e870a4f3f2b5 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 27 May 2025 16:54:21 +0200 Subject: [PATCH 12/41] disable dashboard tests --- acceptance/bundle/deploy/dashboard/simple/test.toml | 3 --- acceptance/bundle/deploy/dashboard/test.toml | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) create mode 100644 acceptance/bundle/deploy/dashboard/test.toml diff --git a/acceptance/bundle/deploy/dashboard/simple/test.toml b/acceptance/bundle/deploy/dashboard/simple/test.toml index 3c0b6562ae..26b6edd4c7 100644 --- a/acceptance/bundle/deploy/dashboard/simple/test.toml +++ b/acceptance/bundle/deploy/dashboard/simple/test.toml @@ -2,9 +2,6 @@ Local = true Cloud = true RequiresWarehouse = true -# dashboards not implemented yet -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] - Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/deploy/dashboard/test.toml b/acceptance/bundle/deploy/dashboard/test.toml new file mode 100644 index 0000000000..05705fa83e --- /dev/null +++ b/acceptance/bundle/deploy/dashboard/test.toml @@ -0,0 +1 @@ +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # dashboard not supported yet From 81f322ea591dcaf9d7a91e32df664ee097a08f9a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 28 May 2025 16:13:31 +0200 Subject: [PATCH 13/41] fix script --- acceptance/bundle/resources/pipelines/script | 1 - 1 file changed, 1 deletion(-) diff --git a/acceptance/bundle/resources/pipelines/script b/acceptance/bundle/resources/pipelines/script index 40c8e20cb8..c1353f79a3 100644 --- a/acceptance/bundle/resources/pipelines/script +++ b/acceptance/bundle/resources/pipelines/script @@ -25,7 +25,6 @@ title "Fetch pipeline ID and verify remote state" ppid=`read_id.py pipelines my` ->>>>>>> 06684a8e5 (WIP: destroy mutator + test (pipelines only); Graph is now generic) trace $CLI pipelines get $ppid rm out.requests.txt From bba263d8e10f91a93fd6d59b1402722a34032630 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 28 May 2025 17:08:02 +0200 Subject: [PATCH 14/41] enable run-local test --- acceptance/cmd/workspace/apps/run-local/test.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/acceptance/cmd/workspace/apps/run-local/test.toml b/acceptance/cmd/workspace/apps/run-local/test.toml index c7120a5ba6..ed349f948c 100644 --- a/acceptance/cmd/workspace/apps/run-local/test.toml +++ b/acceptance/cmd/workspace/apps/run-local/test.toml @@ -2,9 +2,6 @@ RecordRequests = false Timeout = '2m' TimeoutWindows = '10m' -# Cannot run 2 instances of this test because it uses fixed ports -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] - Ignore = [ '.venv', '__pycache__' From f6b5f2d8741f187e6e74f5f5615ee76a1164d86b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 13:50:28 +0200 Subject: [PATCH 15/41] disable jobs/check-metadata, mlops-stacks --- acceptance/bundle/deploy/jobs/check-metadata/test.toml | 2 ++ acceptance/bundle/deploy/mlops-stacks/test.toml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/acceptance/bundle/deploy/jobs/check-metadata/test.toml b/acceptance/bundle/deploy/jobs/check-metadata/test.toml index 844dc98bdd..a8b9cdaaec 100644 --- a/acceptance/bundle/deploy/jobs/check-metadata/test.toml +++ b/acceptance/bundle/deploy/jobs/check-metadata/test.toml @@ -1,6 +1,8 @@ Local = false Cloud = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # require "bundle summary" + Ignore = [ "databricks.yml", "a/b/resources.yml", diff --git a/acceptance/bundle/deploy/mlops-stacks/test.toml b/acceptance/bundle/deploy/mlops-stacks/test.toml index f652b1cfe4..c321fcaae0 100644 --- a/acceptance/bundle/deploy/mlops-stacks/test.toml +++ b/acceptance/bundle/deploy/mlops-stacks/test.toml @@ -3,6 +3,8 @@ Local=false Badness = "the newly initialized bundle from the 'mlops-stacks' template contains two validation warnings in the configuration" +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # requires "bundle summary" + Ignore = [ "config.json" ] From 40c447f18aba050a79413bae4850402158efb93d Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 13:53:17 +0200 Subject: [PATCH 16/41] disable 2 more integration tests --- acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml | 2 ++ acceptance/bundle/destroy/jobs-and-pipeline/test.toml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml b/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml index a07b3c091e..1fd53e7af2 100644 --- a/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml +++ b/acceptance/bundle/deploy/jobs/double-underscore-keys/test.toml @@ -1,6 +1,8 @@ Local = true Cloud = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # needs investigation Error: deploying jobs.foo: Method=Jobs.Create *retries.Err *apierr.APIError StatusCode=400 ErrorCode="INVALID_PARAMETER_VALUE" Message="Missing required field: settings.tasks.task_key." + Ignore = [ "databricks.yml", ] diff --git a/acceptance/bundle/destroy/jobs-and-pipeline/test.toml b/acceptance/bundle/destroy/jobs-and-pipeline/test.toml index f9455dae87..a8eed8bbaf 100644 --- a/acceptance/bundle/destroy/jobs-and-pipeline/test.toml +++ b/acceptance/bundle/destroy/jobs-and-pipeline/test.toml @@ -1,6 +1,8 @@ Local = false Cloud = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # requires "bundle summary" + Ignore = [ "databricks.yml", "resources.yml", From 99d0a4403235dcfdada1f53fa98f945c747bd110 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 14:35:05 +0200 Subject: [PATCH 17/41] disable volumes --- acceptance/bundle/deploy/volume/recreate/test.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/acceptance/bundle/deploy/volume/recreate/test.toml b/acceptance/bundle/deploy/volume/recreate/test.toml index 3111e59565..4898cfa352 100644 --- a/acceptance/bundle/deploy/volume/recreate/test.toml +++ b/acceptance/bundle/deploy/volume/recreate/test.toml @@ -2,6 +2,8 @@ Local = false Cloud = true RequiresUnityCatalog = true +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # volumes are not supported + Ignore = [ "databricks.yml", ] From 3cbc07a4b1bbb7f8ea3a4089f6236191eadcd2fd Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 14:38:19 +0200 Subject: [PATCH 18/41] comment why test is disabled --- acceptance/bundle/deploy/pipeline/auto-approve/test.toml | 2 +- acceptance/bundle/deploy/schema/auto-approve/test.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/bundle/deploy/pipeline/auto-approve/test.toml b/acceptance/bundle/deploy/pipeline/auto-approve/test.toml index b74d4288c9..a894897d0e 100644 --- a/acceptance/bundle/deploy/pipeline/auto-approve/test.toml +++ b/acceptance/bundle/deploy/pipeline/auto-approve/test.toml @@ -1,7 +1,7 @@ Local = true Cloud = true -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # requires "bundle summary" Ignore = [ "databricks.yml" diff --git a/acceptance/bundle/deploy/schema/auto-approve/test.toml b/acceptance/bundle/deploy/schema/auto-approve/test.toml index dba69ddb9b..64e3371a74 100644 --- a/acceptance/bundle/deploy/schema/auto-approve/test.toml +++ b/acceptance/bundle/deploy/schema/auto-approve/test.toml @@ -2,7 +2,7 @@ Local = true Cloud = true RequiresUnityCatalog = true -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # requires "bundle summary" Ignore = [ "databricks.yml", From f77eff94b9c58af7a4ab87c51feb3fa753a18be8 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 15:08:33 +0200 Subject: [PATCH 19/41] Add resource_types.go + test --- bundle/config/resources_types.go | 41 +++++++++++++++++++++++++++ bundle/config/resources_types_test.go | 18 ++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 bundle/config/resources_types.go create mode 100644 bundle/config/resources_types_test.go diff --git a/bundle/config/resources_types.go b/bundle/config/resources_types.go new file mode 100644 index 0000000000..b1d06020e0 --- /dev/null +++ b/bundle/config/resources_types.go @@ -0,0 +1,41 @@ +package config + +import ( + "reflect" + + "github.com/databricks/cli/libs/structdiff/jsontag" +) + +// ResourcesTypes maps the configuration key of each Databricks resource section (for example +// "jobs" or "pipelines") to the Go type that represents a single resource instance inside +// that section (for example `resources.Job`). +// +// The map is populated at package‐initialisation time by inspecting the `Resources` struct with +// reflection, so it automatically stays up-to-date when new resource kinds are added. +var ResourcesTypes = func() map[string]reflect.Type { + var r Resources + rt := reflect.TypeOf(r) + res := make(map[string]reflect.Type, rt.NumField()) + + for _, field := range reflect.VisibleFields(rt) { + tag := jsontag.JSONTag(field.Tag.Get("json")) + name := tag.Name() + if name == "" || name == "-" { + continue + } + + // The type stored in Resources fields is expected to be: + // map[string]*resources.SomeType + if field.Type.Kind() != reflect.Map { + continue + } + elemType := field.Type.Elem() + if elemType.Kind() == reflect.Ptr { + elemType = elemType.Elem() + } + + res[name] = elemType + } + + return res +}() diff --git a/bundle/config/resources_types_test.go b/bundle/config/resources_types_test.go new file mode 100644 index 0000000000..72495f854e --- /dev/null +++ b/bundle/config/resources_types_test.go @@ -0,0 +1,18 @@ +package config + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/databricks/cli/bundle/config/resources" +) + +func TestResourcesTypesMap(t *testing.T) { + assert.Greater(t, len(ResourcesTypes), 10, "expected ResourcesTypes to have more than 10 entries") + + typ, ok := ResourcesTypes["jobs"] + assert.True(t, ok, "resources type for 'jobs' not found in ResourcesTypes map") + assert.Equal(t, reflect.TypeOf(resources.Job{}), typ, "resources type for 'jobs' mismatch") +} From 168721f4235d0d7162b19c53b2668d7c0460aa5b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 15:13:05 +0200 Subject: [PATCH 20/41] remove switch from GetResourceConfig --- bundle/bundle.go | 59 ++++++++++++++++-------------------------------- 1 file changed, 20 insertions(+), 39 deletions(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index 4cb018b019..edff528112 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -13,10 +13,10 @@ import ( "fmt" "os" "path/filepath" + "reflect" "sync" "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/env" "github.com/databricks/cli/bundle/metadata" "github.com/databricks/cli/bundle/terranova/terranova_state" @@ -278,54 +278,35 @@ func (b *Bundle) AuthEnv() (map[string]string, error) { return auth.Env(cfg), nil } -func GetResourceConfigT[T any](b *Bundle, section, name string) (*T, bool) { - v, err := dyn.GetByPath(b.Config.Value(), dyn.NewPath(dyn.Key("resources"), dyn.Key(section), dyn.Key(name))) - if err != nil { +// GetResourceConfig returns the configuration object for a given resource section/name pair. +// The returned value is a pointer to the concrete struct that represents that resource type. +// When the section or name is not found the second return value is false. +func (b *Bundle) GetResourceConfig(section, name string) (any, bool) { + // Resolve the Go type that represents a single resource in this section. + typ, ok := config.ResourcesTypes[section] + if !ok { return nil, false } - // Note, we cannot read b.Config.Resources.Jobs[name] because we don't populate ForceSendFields properly on those - bytes, err := json.Marshal(v.AsAny()) + // Fetch the raw value from the dynamic representation of the bundle config. + v, err := dyn.GetByPath( + b.Config.Value(), + dyn.NewPath(dyn.Key("resources"), dyn.Key(section), dyn.Key(name)), + ) if err != nil { return nil, false } - var result T - err = json.Unmarshal(bytes, &result) + // json-round-trip into a value of the concrete resource type to ensure proper handling of ForceSendFields + bytes, err := json.Marshal(v.AsAny()) if err != nil { return nil, false } - return &result, true -} - -func (b *Bundle) GetResourceConfig(section, name string) (any, bool) { - // TODO: validate that the config is fully resolved - switch section { - case "jobs": - return GetResourceConfigT[resources.Job](b, section, name) - case "pipelines": - return GetResourceConfigT[resources.Pipeline](b, section, name) - case "models": - return GetResourceConfigT[resources.MlflowModel](b, section, name) - case "experiments": - return GetResourceConfigT[resources.MlflowExperiment](b, section, name) - case "model_serving_endpoints": - return GetResourceConfigT[resources.ModelServingEndpoint](b, section, name) - case "registered_models": - return GetResourceConfigT[resources.RegisteredModel](b, section, name) - case "quality_monitors": - return GetResourceConfigT[resources.QualityMonitor](b, section, name) - case "schemas": - return GetResourceConfigT[resources.Schema](b, section, name) - case "volumes": - return GetResourceConfigT[resources.Volume](b, section, name) - case "clusters": - return GetResourceConfigT[resources.Cluster](b, section, name) - case "dashboards": - return GetResourceConfigT[resources.Dashboard](b, section, name) - case "apps": - return GetResourceConfigT[resources.App](b, section, name) + ptr := reflect.New(typ) // *T + if err := json.Unmarshal(bytes, ptr.Interface()); err != nil { + return nil, false } - return nil, false + + return ptr.Interface(), true } From 6740c7fa0a615e0b0777dea21904d5bfe10cd7aa Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 15:18:52 +0200 Subject: [PATCH 21/41] rename terranova_{state,resources} to tn{state,resources} --- bundle/bundle.go | 4 +- bundle/terranova/deploy_mutator.go | 22 +++--- bundle/terranova/destroy_mutator.go | 10 +-- .../app.go | 2 +- bundle/terranova/tnresources/create_p.txt | 74 ++++++++++++++++++ bundle/terranova/tnresources/edit_p.txt | 77 +++++++++++++++++++ .../job.go | 2 +- bundle/terranova/tnresources/p_spec.txt | 61 +++++++++++++++ .../pipeline.go | 2 +- .../resource.go | 2 +- .../schema.go | 2 +- .../sdk_error.go | 2 +- .../util.go | 2 +- .../{terranova_state => tnstate}/state.go | 2 +- 14 files changed, 238 insertions(+), 26 deletions(-) rename bundle/terranova/{terranova_resources => tnresources}/app.go (98%) create mode 100644 bundle/terranova/tnresources/create_p.txt create mode 100644 bundle/terranova/tnresources/edit_p.txt rename bundle/terranova/{terranova_resources => tnresources}/job.go (98%) create mode 100644 bundle/terranova/tnresources/p_spec.txt rename bundle/terranova/{terranova_resources => tnresources}/pipeline.go (98%) rename bundle/terranova/{terranova_resources => tnresources}/resource.go (99%) rename bundle/terranova/{terranova_resources => tnresources}/schema.go (98%) rename bundle/terranova/{terranova_resources => tnresources}/sdk_error.go (97%) rename bundle/terranova/{terranova_resources => tnresources}/util.go (93%) rename bundle/terranova/{terranova_state => tnstate}/state.go (99%) diff --git a/bundle/bundle.go b/bundle/bundle.go index edff528112..e504a3cf79 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -19,7 +19,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/env" "github.com/databricks/cli/bundle/metadata" - "github.com/databricks/cli/bundle/terranova/terranova_state" + "github.com/databricks/cli/bundle/terranova/tnstate" "github.com/databricks/cli/libs/auth" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/fileset" @@ -120,7 +120,7 @@ type Bundle struct { // If true, don't use terraform. Set by DATABRICKS_CLI_DEPLOYMENT=direct DirectDeployment bool - ResourceDatabase terranova_state.TerranovaState + ResourceDatabase tnstate.TerranovaState } func Load(ctx context.Context, path string) (*Bundle, error) { diff --git a/bundle/terranova/deploy_mutator.go b/bundle/terranova/deploy_mutator.go index 60da6a58ce..1e634ac579 100644 --- a/bundle/terranova/deploy_mutator.go +++ b/bundle/terranova/deploy_mutator.go @@ -5,8 +5,8 @@ import ( "fmt" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/terranova/terranova_resources" - "github.com/databricks/cli/bundle/terranova/terranova_state" + "github.com/databricks/cli/bundle/terranova/tnresources" + "github.com/databricks/cli/bundle/terranova/tnstate" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/dag" "github.com/databricks/cli/libs/diag" @@ -33,7 +33,7 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di client := b.WorkspaceClient() - g := dag.NewGraph[terranova_state.ResourceNode]() + g := dag.NewGraph[tnstate.ResourceNode]() _, err := dyn.MapByPattern( b.Config.Value(), @@ -42,7 +42,7 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di section := p[1].Key() name := p[2].Key() // log.Warnf(ctx, "Adding node=%s", node) - g.AddNode(terranova_state.ResourceNode{ + g.AddNode(tnstate.ResourceNode{ Section: section, Name: name, }) @@ -54,7 +54,7 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di countDeployed := 0 - err = g.Run(maxPoolSize, func(node terranova_state.ResourceNode) { + err = g.Run(maxPoolSize, func(node tnstate.ResourceNode) { // TODO func(node string) bool // If function returns false, downstream callers are not called // g.Run() should return list of not executed nodes @@ -102,7 +102,7 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di type Deployer struct { client *databricks.WorkspaceClient - db *terranova_state.TerranovaState + db *tnstate.TerranovaState section string resourceName string } @@ -121,7 +121,7 @@ func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { return err } - resource, err := terranova_resources.New(d.client, d.section, d.resourceName, inputConfig) + resource, err := tnresources.New(d.client, d.section, d.resourceName, inputConfig) if err != nil { return err } @@ -156,7 +156,7 @@ func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { } // log.Warnf(ctx, "localDiff: %#v", localDiff) - localDiffType := terranova_resources.ChangeTypeNone + localDiffType := tnresources.ChangeTypeNone if len(localDiff) > 0 { localDiffType = resource.ClassifyChanges(localDiff) } @@ -180,7 +180,7 @@ func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { return nil } -func (d *Deployer) Recreate(ctx context.Context, oldResource terranova_resources.IResource, oldID string, config any) error { +func (d *Deployer) Recreate(ctx context.Context, oldResource tnresources.IResource, oldID string, config any) error { err := oldResource.DoDelete(ctx, oldID) if err != nil { return fmt.Errorf("deleting old id=%s: %w", oldID, err) @@ -191,7 +191,7 @@ func (d *Deployer) Recreate(ctx context.Context, oldResource terranova_resources return fmt.Errorf("deleting state: %w", err) } - newResource, err := terranova_resources.New(d.client, d.section, d.resourceName, config) + newResource, err := tnresources.New(d.client, d.section, d.resourceName, config) if err != nil { return fmt.Errorf("initializing: %w", err) } @@ -210,7 +210,7 @@ func (d *Deployer) Recreate(ctx context.Context, oldResource terranova_resources return newResource.WaitAfterCreate(ctx) } -func (d *Deployer) Update(ctx context.Context, resource terranova_resources.IResource, oldID string, config any) error { +func (d *Deployer) Update(ctx context.Context, resource tnresources.IResource, oldID string, config any) error { newID, err := resource.DoUpdate(ctx, oldID) if err != nil { return fmt.Errorf("updating id=%s: %w", oldID, err) diff --git a/bundle/terranova/destroy_mutator.go b/bundle/terranova/destroy_mutator.go index 5356d7349c..a4fc8ed9fa 100644 --- a/bundle/terranova/destroy_mutator.go +++ b/bundle/terranova/destroy_mutator.go @@ -4,8 +4,8 @@ import ( "context" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/terranova/terranova_resources" - "github.com/databricks/cli/bundle/terranova/terranova_state" + "github.com/databricks/cli/bundle/terranova/tnresources" + "github.com/databricks/cli/bundle/terranova/tnstate" "github.com/databricks/cli/libs/dag" "github.com/databricks/cli/libs/diag" ) @@ -25,7 +25,7 @@ func (m *terranovaDestroyMutator) Apply(ctx context.Context, b *bundle.Bundle) d client := b.WorkspaceClient() allResources := b.ResourceDatabase.GetAllResources() - g := dag.NewGraph[terranova_state.ResourceNode]() + g := dag.NewGraph[tnstate.ResourceNode]() for _, node := range allResources { g.AddNode(node) @@ -33,8 +33,8 @@ func (m *terranovaDestroyMutator) Apply(ctx context.Context, b *bundle.Bundle) d // TODO: respect dependencies; dependencies need to be part of state, not config. - err := g.Run(maxPoolSize, func(node terranova_state.ResourceNode) { - err := terranova_resources.DestroyResource(ctx, client, node.Section, node.ID) + err := g.Run(maxPoolSize, func(node tnstate.ResourceNode) { + err := tnresources.DestroyResource(ctx, client, node.Section, node.ID) if err != nil { diags.AppendErrorf("destroying %s: %s", node, err) return diff --git a/bundle/terranova/terranova_resources/app.go b/bundle/terranova/tnresources/app.go similarity index 98% rename from bundle/terranova/terranova_resources/app.go rename to bundle/terranova/tnresources/app.go index b2c00fae9a..89ed9fa1ad 100644 --- a/bundle/terranova/terranova_resources/app.go +++ b/bundle/terranova/tnresources/app.go @@ -1,4 +1,4 @@ -package terranova_resources +package tnresources import ( "context" diff --git a/bundle/terranova/tnresources/create_p.txt b/bundle/terranova/tnresources/create_p.txt new file mode 100644 index 0000000000..c79652be19 --- /dev/null +++ b/bundle/terranova/tnresources/create_p.txt @@ -0,0 +1,74 @@ +type CreatePipeline struct { + // If false, deployment will fail if name conflicts with that of another + // pipeline. + AllowDuplicateNames bool `json:"allow_duplicate_names,omitempty"` + // Budget policy of this pipeline. + BudgetPolicyId string `json:"budget_policy_id,omitempty"` + // A catalog in Unity Catalog to publish data from this pipeline to. If + // `target` is specified, tables in this pipeline are published to a + // `target` schema inside `catalog` (for example, + // `catalog`.`target`.`table`). If `target` is not specified, no data is + // published to Unity Catalog. + Catalog string `json:"catalog,omitempty"` + // DLT Release Channel that specifies which version to use. + Channel string `json:"channel,omitempty"` + // Cluster settings for this pipeline deployment. + Clusters []PipelineCluster `json:"clusters,omitempty"` + // String-String configuration for this pipeline execution. + Configuration map[string]string `json:"configuration,omitempty"` + // Whether the pipeline is continuous or triggered. This replaces `trigger`. + Continuous bool `json:"continuous,omitempty"` + // Deployment type of this pipeline. + Deployment *PipelineDeployment `json:"deployment,omitempty"` + // Whether the pipeline is in Development mode. Defaults to false. + Development bool `json:"development,omitempty"` + + DryRun bool `json:"dry_run,omitempty"` + // Pipeline product edition. + Edition string `json:"edition,omitempty"` + // Event log configuration for this pipeline + EventLog *EventLogSpec `json:"event_log,omitempty"` + // Filters on which Pipeline packages to include in the deployed graph. + Filters *Filters `json:"filters,omitempty"` + // The definition of a gateway pipeline to support change data capture. + GatewayDefinition *IngestionGatewayPipelineDefinition `json:"gateway_definition,omitempty"` + // Unique identifier for this pipeline. + Id string `json:"id,omitempty"` + // The configuration for a managed ingestion pipeline. These settings cannot + // be used with the 'libraries', 'schema', 'target', or 'catalog' settings. + IngestionDefinition *IngestionPipelineDefinition `json:"ingestion_definition,omitempty"` + // Libraries or code needed by this deployment. + Libraries []PipelineLibrary `json:"libraries,omitempty"` + // Friendly identifier for this pipeline. + Name string `json:"name,omitempty"` + // List of notification settings for this pipeline. + Notifications []Notifications `json:"notifications,omitempty"` + // Whether Photon is enabled for this pipeline. + Photon bool `json:"photon,omitempty"` + // Restart window of this pipeline. + RestartWindow *RestartWindow `json:"restart_window,omitempty"` + // Write-only setting, available only in Create/Update calls. Specifies the + // user or service principal that the pipeline runs as. If not specified, + // the pipeline runs as the user who created the pipeline. + // + // Only `user_name` or `service_principal_name` can be specified. If both + // are specified, an error is thrown. + RunAs *RunAs `json:"run_as,omitempty"` + // The default schema (database) where tables are read from or published to. + Schema string `json:"schema,omitempty"` + // Whether serverless compute is enabled for this pipeline. + Serverless bool `json:"serverless,omitempty"` + // DBFS root directory for storing checkpoints and tables. + Storage string `json:"storage,omitempty"` + // Target schema (database) to add tables in this pipeline to. Exactly one + // of `schema` or `target` must be specified. To publish to Unity Catalog, + // also specify `catalog`. This legacy field is deprecated for pipeline + // creation in favor of the `schema` field. + Target string `json:"target,omitempty"` + // Which pipeline trigger to use. Deprecated: Use `continuous` instead. + Trigger *PipelineTrigger `json:"trigger,omitempty"` + + ForceSendFields []string `json:"-" url:"-"` +} + + diff --git a/bundle/terranova/tnresources/edit_p.txt b/bundle/terranova/tnresources/edit_p.txt new file mode 100644 index 0000000000..45aaf83cbe --- /dev/null +++ b/bundle/terranova/tnresources/edit_p.txt @@ -0,0 +1,77 @@ +type EditPipeline struct { + // If false, deployment will fail if name has changed and conflicts the name + // of another pipeline. + AllowDuplicateNames bool `json:"allow_duplicate_names,omitempty"` + // Budget policy of this pipeline. + BudgetPolicyId string `json:"budget_policy_id,omitempty"` + // A catalog in Unity Catalog to publish data from this pipeline to. If + // `target` is specified, tables in this pipeline are published to a + // `target` schema inside `catalog` (for example, + // `catalog`.`target`.`table`). If `target` is not specified, no data is + // published to Unity Catalog. + Catalog string `json:"catalog,omitempty"` + // DLT Release Channel that specifies which version to use. + Channel string `json:"channel,omitempty"` + // Cluster settings for this pipeline deployment. + Clusters []PipelineCluster `json:"clusters,omitempty"` + // String-String configuration for this pipeline execution. + Configuration map[string]string `json:"configuration,omitempty"` + // Whether the pipeline is continuous or triggered. This replaces `trigger`. + Continuous bool `json:"continuous,omitempty"` + // Deployment type of this pipeline. + Deployment *PipelineDeployment `json:"deployment,omitempty"` + // Whether the pipeline is in Development mode. Defaults to false. + Development bool `json:"development,omitempty"` + // Pipeline product edition. + Edition string `json:"edition,omitempty"` + // Event log configuration for this pipeline + EventLog *EventLogSpec `json:"event_log,omitempty"` + // If present, the last-modified time of the pipeline settings before the + // edit. If the settings were modified after that time, then the request + // will fail with a conflict. + ExpectedLastModified int64 `json:"expected_last_modified,omitempty"` + // Filters on which Pipeline packages to include in the deployed graph. + Filters *Filters `json:"filters,omitempty"` + // The definition of a gateway pipeline to support change data capture. + GatewayDefinition *IngestionGatewayPipelineDefinition `json:"gateway_definition,omitempty"` + // Unique identifier for this pipeline. + Id string `json:"id,omitempty"` + // The configuration for a managed ingestion pipeline. These settings cannot + // be used with the 'libraries', 'schema', 'target', or 'catalog' settings. + IngestionDefinition *IngestionPipelineDefinition `json:"ingestion_definition,omitempty"` + // Libraries or code needed by this deployment. + Libraries []PipelineLibrary `json:"libraries,omitempty"` + // Friendly identifier for this pipeline. + Name string `json:"name,omitempty"` + // List of notification settings for this pipeline. + Notifications []Notifications `json:"notifications,omitempty"` + // Whether Photon is enabled for this pipeline. + Photon bool `json:"photon,omitempty"` + // Unique identifier for this pipeline. + PipelineId string `json:"-" url:"-"` + // Restart window of this pipeline. + RestartWindow *RestartWindow `json:"restart_window,omitempty"` + // Write-only setting, available only in Create/Update calls. Specifies the + // user or service principal that the pipeline runs as. If not specified, + // the pipeline runs as the user who created the pipeline. + // + // Only `user_name` or `service_principal_name` can be specified. If both + // are specified, an error is thrown. + RunAs *RunAs `json:"run_as,omitempty"` + // The default schema (database) where tables are read from or published to. + Schema string `json:"schema,omitempty"` + // Whether serverless compute is enabled for this pipeline. + Serverless bool `json:"serverless,omitempty"` + // DBFS root directory for storing checkpoints and tables. + Storage string `json:"storage,omitempty"` + // Target schema (database) to add tables in this pipeline to. Exactly one + // of `schema` or `target` must be specified. To publish to Unity Catalog, + // also specify `catalog`. This legacy field is deprecated for pipeline + // creation in favor of the `schema` field. + Target string `json:"target,omitempty"` + // Which pipeline trigger to use. Deprecated: Use `continuous` instead. + Trigger *PipelineTrigger `json:"trigger,omitempty"` + + ForceSendFields []string `json:"-" url:"-"` +} + diff --git a/bundle/terranova/terranova_resources/job.go b/bundle/terranova/tnresources/job.go similarity index 98% rename from bundle/terranova/terranova_resources/job.go rename to bundle/terranova/tnresources/job.go index 12a024eb9b..2377e26e1e 100644 --- a/bundle/terranova/terranova_resources/job.go +++ b/bundle/terranova/tnresources/job.go @@ -1,4 +1,4 @@ -package terranova_resources +package tnresources import ( "context" diff --git a/bundle/terranova/tnresources/p_spec.txt b/bundle/terranova/tnresources/p_spec.txt new file mode 100644 index 0000000000..4aaafeb8e5 --- /dev/null +++ b/bundle/terranova/tnresources/p_spec.txt @@ -0,0 +1,61 @@ +type PipelineSpec struct { + // Budget policy of this pipeline. + BudgetPolicyId string `json:"budget_policy_id,omitempty"` + // A catalog in Unity Catalog to publish data from this pipeline to. If + // `target` is specified, tables in this pipeline are published to a + // `target` schema inside `catalog` (for example, + // `catalog`.`target`.`table`). If `target` is not specified, no data is + // published to Unity Catalog. + Catalog string `json:"catalog,omitempty"` + // DLT Release Channel that specifies which version to use. + Channel string `json:"channel,omitempty"` + // Cluster settings for this pipeline deployment. + Clusters []PipelineCluster `json:"clusters,omitempty"` + // String-String configuration for this pipeline execution. + Configuration map[string]string `json:"configuration,omitempty"` + // Whether the pipeline is continuous or triggered. This replaces `trigger`. + Continuous bool `json:"continuous,omitempty"` + // Deployment type of this pipeline. + Deployment *PipelineDeployment `json:"deployment,omitempty"` + // Whether the pipeline is in Development mode. Defaults to false. + Development bool `json:"development,omitempty"` + // Pipeline product edition. + Edition string `json:"edition,omitempty"` + // Event log configuration for this pipeline + EventLog *EventLogSpec `json:"event_log,omitempty"` + // Filters on which Pipeline packages to include in the deployed graph. + Filters *Filters `json:"filters,omitempty"` + // The definition of a gateway pipeline to support change data capture. + GatewayDefinition *IngestionGatewayPipelineDefinition `json:"gateway_definition,omitempty"` + // Unique identifier for this pipeline. + Id string `json:"id,omitempty"` + // The configuration for a managed ingestion pipeline. These settings cannot + // be used with the 'libraries', 'schema', 'target', or 'catalog' settings. + IngestionDefinition *IngestionPipelineDefinition `json:"ingestion_definition,omitempty"` + // Libraries or code needed by this deployment. + Libraries []PipelineLibrary `json:"libraries,omitempty"` + // Friendly identifier for this pipeline. + Name string `json:"name,omitempty"` + // List of notification settings for this pipeline. + Notifications []Notifications `json:"notifications,omitempty"` + // Whether Photon is enabled for this pipeline. + Photon bool `json:"photon,omitempty"` + // Restart window of this pipeline. + RestartWindow *RestartWindow `json:"restart_window,omitempty"` + // The default schema (database) where tables are read from or published to. + Schema string `json:"schema,omitempty"` + // Whether serverless compute is enabled for this pipeline. + Serverless bool `json:"serverless,omitempty"` + // DBFS root directory for storing checkpoints and tables. + Storage string `json:"storage,omitempty"` + // Target schema (database) to add tables in this pipeline to. Exactly one + // of `schema` or `target` must be specified. To publish to Unity Catalog, + // also specify `catalog`. This legacy field is deprecated for pipeline + // creation in favor of the `schema` field. + Target string `json:"target,omitempty"` + // Which pipeline trigger to use. Deprecated: Use `continuous` instead. + Trigger *PipelineTrigger `json:"trigger,omitempty"` + + ForceSendFields []string `json:"-" url:"-"` +} + diff --git a/bundle/terranova/terranova_resources/pipeline.go b/bundle/terranova/tnresources/pipeline.go similarity index 98% rename from bundle/terranova/terranova_resources/pipeline.go rename to bundle/terranova/tnresources/pipeline.go index f4419696a8..e4883ef3eb 100644 --- a/bundle/terranova/terranova_resources/pipeline.go +++ b/bundle/terranova/tnresources/pipeline.go @@ -1,4 +1,4 @@ -package terranova_resources +package tnresources import ( "context" diff --git a/bundle/terranova/terranova_resources/resource.go b/bundle/terranova/tnresources/resource.go similarity index 99% rename from bundle/terranova/terranova_resources/resource.go rename to bundle/terranova/tnresources/resource.go index 89f397d1de..4c3dca9564 100644 --- a/bundle/terranova/terranova_resources/resource.go +++ b/bundle/terranova/tnresources/resource.go @@ -1,4 +1,4 @@ -package terranova_resources +package tnresources import ( "context" diff --git a/bundle/terranova/terranova_resources/schema.go b/bundle/terranova/tnresources/schema.go similarity index 98% rename from bundle/terranova/terranova_resources/schema.go rename to bundle/terranova/tnresources/schema.go index 89607c5d0b..bb20982502 100644 --- a/bundle/terranova/terranova_resources/schema.go +++ b/bundle/terranova/tnresources/schema.go @@ -1,4 +1,4 @@ -package terranova_resources +package tnresources import ( "context" diff --git a/bundle/terranova/terranova_resources/sdk_error.go b/bundle/terranova/tnresources/sdk_error.go similarity index 97% rename from bundle/terranova/terranova_resources/sdk_error.go rename to bundle/terranova/tnresources/sdk_error.go index 9497ec5761..82035de522 100644 --- a/bundle/terranova/terranova_resources/sdk_error.go +++ b/bundle/terranova/tnresources/sdk_error.go @@ -1,4 +1,4 @@ -package terranova_resources +package tnresources import ( "fmt" diff --git a/bundle/terranova/terranova_resources/util.go b/bundle/terranova/tnresources/util.go similarity index 93% rename from bundle/terranova/terranova_resources/util.go rename to bundle/terranova/tnresources/util.go index be6a1e0af3..4702862d95 100644 --- a/bundle/terranova/terranova_resources/util.go +++ b/bundle/terranova/tnresources/util.go @@ -1,4 +1,4 @@ -package terranova_resources +package tnresources import ( "encoding/json" diff --git a/bundle/terranova/terranova_state/state.go b/bundle/terranova/tnstate/state.go similarity index 99% rename from bundle/terranova/terranova_state/state.go rename to bundle/terranova/tnstate/state.go index a6a6635d1b..596e4b86cb 100644 --- a/bundle/terranova/terranova_state/state.go +++ b/bundle/terranova/tnstate/state.go @@ -1,4 +1,4 @@ -package terranova_state +package tnstate import ( "bytes" From 3c93c3ae428fc355eb407cb5a33356c507be0183 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 15:32:42 +0200 Subject: [PATCH 22/41] clean up unused functions from testserver --- libs/testserver/server.go | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/libs/testserver/server.go b/libs/testserver/server.go index f2521d22e4..b9f64a2bf6 100644 --- a/libs/testserver/server.go +++ b/libs/testserver/server.go @@ -305,35 +305,3 @@ func isNil(i any) bool { return false } } - -func CallHandler[T any](handler func(req Request, parsed T) Response, req Request) Response { - var parsed T - - err := json.Unmarshal(req.Body, &parsed) - if err != nil { - return Response{ - Body: fmt.Sprintf("cannot parse request: %s", err), - StatusCode: 400, - } - } - - defer req.Workspace.LockUnlock()() - - return handler(req, parsed) -} - -func CallHandler1[T any](handler func(req Request, parsed T, param1 string) Response, req Request, param1 string) Response { - var parsed T - - err := json.Unmarshal(req.Body, &parsed) - if err != nil { - return Response{ - Body: fmt.Sprintf("cannot parse request: %s", err), - StatusCode: 400, - } - } - - defer req.Workspace.LockUnlock()() - - return handler(req, parsed, param1) -} From babfbb6377f59af3ac8dc7aacee977429e56162b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 15:45:01 +0200 Subject: [PATCH 23/41] add map with New functions that records resource types --- bundle/terranova/tnresources/resource.go | 117 +++++++++++++---------- 1 file changed, 65 insertions(+), 52 deletions(-) diff --git a/bundle/terranova/tnresources/resource.go b/bundle/terranova/tnresources/resource.go index 4c3dca9564..beb2dcf3e7 100644 --- a/bundle/terranova/tnresources/resource.go +++ b/bundle/terranova/tnresources/resource.go @@ -5,7 +5,6 @@ import ( "fmt" "reflect" - "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/structdiff" "github.com/databricks/databricks-sdk-go" ) @@ -41,70 +40,84 @@ const ( ChangeTypeRecreate ChangeType = -1 ) -func New(client *databricks.WorkspaceClient, section, name string, config any) (IResource, error) { - switch section { - case "jobs": - typedConfig, ok := config.(*resources.Job) - if !ok { - return nil, fmt.Errorf("unexpected config type for jobs: %T", config) - } - if typedConfig == nil { - return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) - } - return NewResourceJob(client, *typedConfig) +var resourceConstructors = map[string]reflect.Value{ + "jobs": reflect.ValueOf(NewResourceJob), + "pipelines": reflect.ValueOf(NewResourcePipeline), + "schemas": reflect.ValueOf(NewResourceSchema), + "apps": reflect.ValueOf(NewResourceApp), +} - case "pipelines": - typedConfig, ok := config.(*resources.Pipeline) - if !ok { - return nil, fmt.Errorf("unexpected config type for pipelines: %T", config) - } - if typedConfig == nil { - return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) - } - return NewResourcePipeline(client, *typedConfig) +// invokeConstructor converts cfg to the parameter type expected by ctor and +// executes the call, returning the IResource instance or error. +func invokeConstructor(ctor reflect.Value, client *databricks.WorkspaceClient, cfg any) (IResource, error) { + ft := ctor.Type() - case "schemas": - typedConfig, ok := config.(*resources.Schema) - if !ok { - return nil, fmt.Errorf("unexpected config type for schemas: %T", config) - } - if typedConfig == nil { - return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) - } - return NewResourceSchema(client, *typedConfig) + // Sanity check – every registered function must have two inputs and two outputs. + if ft.NumIn() != 2 || ft.NumOut() != 2 { + return nil, fmt.Errorf("invalid constructor signature: want func(*WorkspaceClient, T) (IResource, error)") + } - case "apps": - typedConfig, ok := config.(*resources.App) - if !ok { - return nil, fmt.Errorf("unexpected config type for apps: %T", config) + expectedCfgType := ft.In(1) // T + + // Prepare the config value matching the expected type. + var cfgVal reflect.Value + if cfg == nil { + // Treat nil as a request for the zero value of the expected config type. This + // is useful for actions (like deletion) where the config is irrelevant. + cfgVal = reflect.Zero(expectedCfgType) + } else { + suppliedVal := reflect.ValueOf(cfg) + if suppliedVal.Type() != expectedCfgType { + return nil, fmt.Errorf("unexpected config type: expected %s, got %T", expectedCfgType, cfg) } - if typedConfig == nil { - return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) - } - return NewResourceApp(client, *typedConfig) + cfgVal = suppliedVal + } - default: + results := ctor.Call([]reflect.Value{reflect.ValueOf(client), cfgVal}) + + if errIface := results[1].Interface(); errIface != nil { + return nil, errIface.(error) + } + + res, ok := results[0].Interface().(IResource) + if !ok { + return nil, fmt.Errorf("constructor did not return IResource") + } + return res, nil +} + +func New(client *databricks.WorkspaceClient, section, name string, config any) (IResource, error) { + ctor, ok := resourceConstructors[section] + if !ok { return nil, fmt.Errorf("unsupported resource type: %s", section) } + + // Disallow nil configs (including typed nil pointers). + if config == nil { + return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) + } + + // If the supplied config is a pointer value, dereference it so that we pass + // the underlying struct value to the constructor. Typed nil pointers were + // handled above. + v := reflect.ValueOf(config) + if v.Kind() == reflect.Ptr { + if v.IsNil() { + return nil, fmt.Errorf("unexpected nil in config: %s.%s", section, name) + } + config = v.Elem().Interface() + } + + return invokeConstructor(ctor, client, config) } func DestroyResource(ctx context.Context, client *databricks.WorkspaceClient, section, id string) error { - var err error - var r IResource - - switch section { - case "jobs": - r, err = NewResourceJob(client, resources.Job{}) - case "pipelines": - r, err = NewResourcePipeline(client, resources.Pipeline{}) - case "schemas": - r, err = NewResourceSchema(client, resources.Schema{}) - case "apps": - r, err = NewResourceApp(client, resources.App{}) - default: + ctor, ok := resourceConstructors[section] + if !ok { return fmt.Errorf("unsupported resource type: %s", section) } + r, err := invokeConstructor(ctor, client, nil) // zero config is enough for deletion if err != nil { return err } From 048ea861f2214e50b9b365fcc409236a7ac9557b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 15:49:30 +0200 Subject: [PATCH 24/41] update comments --- bundle/terranova/destroy_mutator.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bundle/terranova/destroy_mutator.go b/bundle/terranova/destroy_mutator.go index a4fc8ed9fa..f384ee5e32 100644 --- a/bundle/terranova/destroy_mutator.go +++ b/bundle/terranova/destroy_mutator.go @@ -31,7 +31,8 @@ func (m *terranovaDestroyMutator) Apply(ctx context.Context, b *bundle.Bundle) d g.AddNode(node) } - // TODO: respect dependencies; dependencies need to be part of state, not config. + // TODO: do we need to respect dependencies here? Should we store them in state in that case? + // Alternatively, do a few rounds of delete to make we capture cases where A needs to be deleted before B. err := g.Run(maxPoolSize, func(node tnstate.ResourceNode) { err := tnresources.DestroyResource(ctx, client, node.Section, node.ID) @@ -39,7 +40,7 @@ func (m *terranovaDestroyMutator) Apply(ctx context.Context, b *bundle.Bundle) d diags.AppendErrorf("destroying %s: %s", node, err) return } - // TODO: did DestroyResource fail because it did not exist? we can clean it up from the state as well + // TODO: handle situation where resources is already gone for whatever reason. err = b.ResourceDatabase.DeleteState(node.Section, node.Name) if err != nil { From 0a6c687e4a02adb50fee81356377572ad36bd365 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 15:50:23 +0200 Subject: [PATCH 25/41] clean up --- bundle/terranova/tnresources/create_p.txt | 74 ---------------------- bundle/terranova/tnresources/edit_p.txt | 77 ----------------------- bundle/terranova/tnresources/p_spec.txt | 61 ------------------ 3 files changed, 212 deletions(-) delete mode 100644 bundle/terranova/tnresources/create_p.txt delete mode 100644 bundle/terranova/tnresources/edit_p.txt delete mode 100644 bundle/terranova/tnresources/p_spec.txt diff --git a/bundle/terranova/tnresources/create_p.txt b/bundle/terranova/tnresources/create_p.txt deleted file mode 100644 index c79652be19..0000000000 --- a/bundle/terranova/tnresources/create_p.txt +++ /dev/null @@ -1,74 +0,0 @@ -type CreatePipeline struct { - // If false, deployment will fail if name conflicts with that of another - // pipeline. - AllowDuplicateNames bool `json:"allow_duplicate_names,omitempty"` - // Budget policy of this pipeline. - BudgetPolicyId string `json:"budget_policy_id,omitempty"` - // A catalog in Unity Catalog to publish data from this pipeline to. If - // `target` is specified, tables in this pipeline are published to a - // `target` schema inside `catalog` (for example, - // `catalog`.`target`.`table`). If `target` is not specified, no data is - // published to Unity Catalog. - Catalog string `json:"catalog,omitempty"` - // DLT Release Channel that specifies which version to use. - Channel string `json:"channel,omitempty"` - // Cluster settings for this pipeline deployment. - Clusters []PipelineCluster `json:"clusters,omitempty"` - // String-String configuration for this pipeline execution. - Configuration map[string]string `json:"configuration,omitempty"` - // Whether the pipeline is continuous or triggered. This replaces `trigger`. - Continuous bool `json:"continuous,omitempty"` - // Deployment type of this pipeline. - Deployment *PipelineDeployment `json:"deployment,omitempty"` - // Whether the pipeline is in Development mode. Defaults to false. - Development bool `json:"development,omitempty"` - - DryRun bool `json:"dry_run,omitempty"` - // Pipeline product edition. - Edition string `json:"edition,omitempty"` - // Event log configuration for this pipeline - EventLog *EventLogSpec `json:"event_log,omitempty"` - // Filters on which Pipeline packages to include in the deployed graph. - Filters *Filters `json:"filters,omitempty"` - // The definition of a gateway pipeline to support change data capture. - GatewayDefinition *IngestionGatewayPipelineDefinition `json:"gateway_definition,omitempty"` - // Unique identifier for this pipeline. - Id string `json:"id,omitempty"` - // The configuration for a managed ingestion pipeline. These settings cannot - // be used with the 'libraries', 'schema', 'target', or 'catalog' settings. - IngestionDefinition *IngestionPipelineDefinition `json:"ingestion_definition,omitempty"` - // Libraries or code needed by this deployment. - Libraries []PipelineLibrary `json:"libraries,omitempty"` - // Friendly identifier for this pipeline. - Name string `json:"name,omitempty"` - // List of notification settings for this pipeline. - Notifications []Notifications `json:"notifications,omitempty"` - // Whether Photon is enabled for this pipeline. - Photon bool `json:"photon,omitempty"` - // Restart window of this pipeline. - RestartWindow *RestartWindow `json:"restart_window,omitempty"` - // Write-only setting, available only in Create/Update calls. Specifies the - // user or service principal that the pipeline runs as. If not specified, - // the pipeline runs as the user who created the pipeline. - // - // Only `user_name` or `service_principal_name` can be specified. If both - // are specified, an error is thrown. - RunAs *RunAs `json:"run_as,omitempty"` - // The default schema (database) where tables are read from or published to. - Schema string `json:"schema,omitempty"` - // Whether serverless compute is enabled for this pipeline. - Serverless bool `json:"serverless,omitempty"` - // DBFS root directory for storing checkpoints and tables. - Storage string `json:"storage,omitempty"` - // Target schema (database) to add tables in this pipeline to. Exactly one - // of `schema` or `target` must be specified. To publish to Unity Catalog, - // also specify `catalog`. This legacy field is deprecated for pipeline - // creation in favor of the `schema` field. - Target string `json:"target,omitempty"` - // Which pipeline trigger to use. Deprecated: Use `continuous` instead. - Trigger *PipelineTrigger `json:"trigger,omitempty"` - - ForceSendFields []string `json:"-" url:"-"` -} - - diff --git a/bundle/terranova/tnresources/edit_p.txt b/bundle/terranova/tnresources/edit_p.txt deleted file mode 100644 index 45aaf83cbe..0000000000 --- a/bundle/terranova/tnresources/edit_p.txt +++ /dev/null @@ -1,77 +0,0 @@ -type EditPipeline struct { - // If false, deployment will fail if name has changed and conflicts the name - // of another pipeline. - AllowDuplicateNames bool `json:"allow_duplicate_names,omitempty"` - // Budget policy of this pipeline. - BudgetPolicyId string `json:"budget_policy_id,omitempty"` - // A catalog in Unity Catalog to publish data from this pipeline to. If - // `target` is specified, tables in this pipeline are published to a - // `target` schema inside `catalog` (for example, - // `catalog`.`target`.`table`). If `target` is not specified, no data is - // published to Unity Catalog. - Catalog string `json:"catalog,omitempty"` - // DLT Release Channel that specifies which version to use. - Channel string `json:"channel,omitempty"` - // Cluster settings for this pipeline deployment. - Clusters []PipelineCluster `json:"clusters,omitempty"` - // String-String configuration for this pipeline execution. - Configuration map[string]string `json:"configuration,omitempty"` - // Whether the pipeline is continuous or triggered. This replaces `trigger`. - Continuous bool `json:"continuous,omitempty"` - // Deployment type of this pipeline. - Deployment *PipelineDeployment `json:"deployment,omitempty"` - // Whether the pipeline is in Development mode. Defaults to false. - Development bool `json:"development,omitempty"` - // Pipeline product edition. - Edition string `json:"edition,omitempty"` - // Event log configuration for this pipeline - EventLog *EventLogSpec `json:"event_log,omitempty"` - // If present, the last-modified time of the pipeline settings before the - // edit. If the settings were modified after that time, then the request - // will fail with a conflict. - ExpectedLastModified int64 `json:"expected_last_modified,omitempty"` - // Filters on which Pipeline packages to include in the deployed graph. - Filters *Filters `json:"filters,omitempty"` - // The definition of a gateway pipeline to support change data capture. - GatewayDefinition *IngestionGatewayPipelineDefinition `json:"gateway_definition,omitempty"` - // Unique identifier for this pipeline. - Id string `json:"id,omitempty"` - // The configuration for a managed ingestion pipeline. These settings cannot - // be used with the 'libraries', 'schema', 'target', or 'catalog' settings. - IngestionDefinition *IngestionPipelineDefinition `json:"ingestion_definition,omitempty"` - // Libraries or code needed by this deployment. - Libraries []PipelineLibrary `json:"libraries,omitempty"` - // Friendly identifier for this pipeline. - Name string `json:"name,omitempty"` - // List of notification settings for this pipeline. - Notifications []Notifications `json:"notifications,omitempty"` - // Whether Photon is enabled for this pipeline. - Photon bool `json:"photon,omitempty"` - // Unique identifier for this pipeline. - PipelineId string `json:"-" url:"-"` - // Restart window of this pipeline. - RestartWindow *RestartWindow `json:"restart_window,omitempty"` - // Write-only setting, available only in Create/Update calls. Specifies the - // user or service principal that the pipeline runs as. If not specified, - // the pipeline runs as the user who created the pipeline. - // - // Only `user_name` or `service_principal_name` can be specified. If both - // are specified, an error is thrown. - RunAs *RunAs `json:"run_as,omitempty"` - // The default schema (database) where tables are read from or published to. - Schema string `json:"schema,omitempty"` - // Whether serverless compute is enabled for this pipeline. - Serverless bool `json:"serverless,omitempty"` - // DBFS root directory for storing checkpoints and tables. - Storage string `json:"storage,omitempty"` - // Target schema (database) to add tables in this pipeline to. Exactly one - // of `schema` or `target` must be specified. To publish to Unity Catalog, - // also specify `catalog`. This legacy field is deprecated for pipeline - // creation in favor of the `schema` field. - Target string `json:"target,omitempty"` - // Which pipeline trigger to use. Deprecated: Use `continuous` instead. - Trigger *PipelineTrigger `json:"trigger,omitempty"` - - ForceSendFields []string `json:"-" url:"-"` -} - diff --git a/bundle/terranova/tnresources/p_spec.txt b/bundle/terranova/tnresources/p_spec.txt deleted file mode 100644 index 4aaafeb8e5..0000000000 --- a/bundle/terranova/tnresources/p_spec.txt +++ /dev/null @@ -1,61 +0,0 @@ -type PipelineSpec struct { - // Budget policy of this pipeline. - BudgetPolicyId string `json:"budget_policy_id,omitempty"` - // A catalog in Unity Catalog to publish data from this pipeline to. If - // `target` is specified, tables in this pipeline are published to a - // `target` schema inside `catalog` (for example, - // `catalog`.`target`.`table`). If `target` is not specified, no data is - // published to Unity Catalog. - Catalog string `json:"catalog,omitempty"` - // DLT Release Channel that specifies which version to use. - Channel string `json:"channel,omitempty"` - // Cluster settings for this pipeline deployment. - Clusters []PipelineCluster `json:"clusters,omitempty"` - // String-String configuration for this pipeline execution. - Configuration map[string]string `json:"configuration,omitempty"` - // Whether the pipeline is continuous or triggered. This replaces `trigger`. - Continuous bool `json:"continuous,omitempty"` - // Deployment type of this pipeline. - Deployment *PipelineDeployment `json:"deployment,omitempty"` - // Whether the pipeline is in Development mode. Defaults to false. - Development bool `json:"development,omitempty"` - // Pipeline product edition. - Edition string `json:"edition,omitempty"` - // Event log configuration for this pipeline - EventLog *EventLogSpec `json:"event_log,omitempty"` - // Filters on which Pipeline packages to include in the deployed graph. - Filters *Filters `json:"filters,omitempty"` - // The definition of a gateway pipeline to support change data capture. - GatewayDefinition *IngestionGatewayPipelineDefinition `json:"gateway_definition,omitempty"` - // Unique identifier for this pipeline. - Id string `json:"id,omitempty"` - // The configuration for a managed ingestion pipeline. These settings cannot - // be used with the 'libraries', 'schema', 'target', or 'catalog' settings. - IngestionDefinition *IngestionPipelineDefinition `json:"ingestion_definition,omitempty"` - // Libraries or code needed by this deployment. - Libraries []PipelineLibrary `json:"libraries,omitempty"` - // Friendly identifier for this pipeline. - Name string `json:"name,omitempty"` - // List of notification settings for this pipeline. - Notifications []Notifications `json:"notifications,omitempty"` - // Whether Photon is enabled for this pipeline. - Photon bool `json:"photon,omitempty"` - // Restart window of this pipeline. - RestartWindow *RestartWindow `json:"restart_window,omitempty"` - // The default schema (database) where tables are read from or published to. - Schema string `json:"schema,omitempty"` - // Whether serverless compute is enabled for this pipeline. - Serverless bool `json:"serverless,omitempty"` - // DBFS root directory for storing checkpoints and tables. - Storage string `json:"storage,omitempty"` - // Target schema (database) to add tables in this pipeline to. Exactly one - // of `schema` or `target` must be specified. To publish to Unity Catalog, - // also specify `catalog`. This legacy field is deprecated for pipeline - // creation in favor of the `schema` field. - Target string `json:"target,omitempty"` - // Which pipeline trigger to use. Deprecated: Use `continuous` instead. - Trigger *PipelineTrigger `json:"trigger,omitempty"` - - ForceSendFields []string `json:"-" url:"-"` -} - From d57627b34585db0f142311d6455a53b180b3fb3a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 15:52:14 +0200 Subject: [PATCH 26/41] clean up --- bundle/terranova/tnresources/schema.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/bundle/terranova/tnresources/schema.go b/bundle/terranova/tnresources/schema.go index bb20982502..07c2836fc9 100644 --- a/bundle/terranova/tnresources/schema.go +++ b/bundle/terranova/tnresources/schema.go @@ -41,8 +41,6 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string) (string, error return "", err } - // TODO: properly populate these instead - updateRequest.ForceSendFields = nil updateRequest.FullName = id response, err := r.client.Schemas.Update(ctx, updateRequest) From 296bcf44be28b141087acf9eef71fb70a977cc6b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 15:55:49 +0200 Subject: [PATCH 27/41] lint fix --- bundle/terranova/tnresources/resource.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bundle/terranova/tnresources/resource.go b/bundle/terranova/tnresources/resource.go index beb2dcf3e7..c75897feb3 100644 --- a/bundle/terranova/tnresources/resource.go +++ b/bundle/terranova/tnresources/resource.go @@ -2,6 +2,7 @@ package tnresources import ( "context" + "errors" "fmt" "reflect" @@ -54,7 +55,7 @@ func invokeConstructor(ctor reflect.Value, client *databricks.WorkspaceClient, c // Sanity check – every registered function must have two inputs and two outputs. if ft.NumIn() != 2 || ft.NumOut() != 2 { - return nil, fmt.Errorf("invalid constructor signature: want func(*WorkspaceClient, T) (IResource, error)") + return nil, errors.New("invalid constructor signature: want func(*WorkspaceClient, T) (IResource, error)") } expectedCfgType := ft.In(1) // T @@ -81,7 +82,7 @@ func invokeConstructor(ctor reflect.Value, client *databricks.WorkspaceClient, c res, ok := results[0].Interface().(IResource) if !ok { - return nil, fmt.Errorf("constructor did not return IResource") + return nil, errors.New("constructor did not return IResource") } return res, nil } From 2e941b5ec4431c80eb0ed49c12878a9c8a80c513 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 30 May 2025 16:03:06 +0200 Subject: [PATCH 28/41] resourceConstructors -> supportedResources --- bundle/terranova/tnresources/resource.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/bundle/terranova/tnresources/resource.go b/bundle/terranova/tnresources/resource.go index c75897feb3..52378edc0c 100644 --- a/bundle/terranova/tnresources/resource.go +++ b/bundle/terranova/tnresources/resource.go @@ -10,6 +10,13 @@ import ( "github.com/databricks/databricks-sdk-go" ) +var supportedResources = map[string]reflect.Value{ + "jobs": reflect.ValueOf(NewResourceJob), + "pipelines": reflect.ValueOf(NewResourcePipeline), + "schemas": reflect.ValueOf(NewResourceSchema), + "apps": reflect.ValueOf(NewResourceApp), +} + type IResource interface { Config() any @@ -41,13 +48,6 @@ const ( ChangeTypeRecreate ChangeType = -1 ) -var resourceConstructors = map[string]reflect.Value{ - "jobs": reflect.ValueOf(NewResourceJob), - "pipelines": reflect.ValueOf(NewResourcePipeline), - "schemas": reflect.ValueOf(NewResourceSchema), - "apps": reflect.ValueOf(NewResourceApp), -} - // invokeConstructor converts cfg to the parameter type expected by ctor and // executes the call, returning the IResource instance or error. func invokeConstructor(ctor reflect.Value, client *databricks.WorkspaceClient, cfg any) (IResource, error) { @@ -88,7 +88,7 @@ func invokeConstructor(ctor reflect.Value, client *databricks.WorkspaceClient, c } func New(client *databricks.WorkspaceClient, section, name string, config any) (IResource, error) { - ctor, ok := resourceConstructors[section] + ctor, ok := supportedResources[section] if !ok { return nil, fmt.Errorf("unsupported resource type: %s", section) } @@ -113,7 +113,7 @@ func New(client *databricks.WorkspaceClient, section, name string, config any) ( } func DestroyResource(ctx context.Context, client *databricks.WorkspaceClient, section, id string) error { - ctor, ok := resourceConstructors[section] + ctor, ok := supportedResources[section] if !ok { return fmt.Errorf("unsupported resource type: %s", section) } From 6c254c89ea0b13e719252bdff02bc632aefd056e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 2 Jun 2025 09:45:53 +0200 Subject: [PATCH 29/41] extract IsDirectDeployment function --- bundle/phases/initialize.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 94dd3823a7..3c3a633410 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -28,15 +28,12 @@ import ( // Interpolation of fields referring to the "bundle" and "workspace" keys // happens upon completion of this phase. func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var err error + log.Info(ctx, "Phase: initialize") - deployment := os.Getenv("DATABRICKS_CLI_DEPLOYMENT") - if deployment == "direct" { - b.DirectDeployment = true - } else if deployment == "terraform" || deployment == "" { - b.DirectDeployment = false - } else { - err := fmt.Errorf("Unexpected setting for DATABRICKS_CLI_DEPLOYMENT=%#v (expected 'terraform' or 'direct' or absent/empty which means 'terraform')", deployment) + b.DirectDeployment, err = IsDirectDeployment() + if err != nil { return diag.FromErr(err) } @@ -232,3 +229,14 @@ func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { diags = diags.Extend(bundle.Apply(ctx, b, scripts.Execute(config.ScriptPostInit))) return diags } + +func IsDirectDeployment() (bool, error) { + deployment := os.Getenv("DATABRICKS_CLI_DEPLOYMENT") + if deployment == "direct" { + return true, nil + } else if deployment == "terraform" || deployment == "" { + return false, nil + } else { + return false, fmt.Errorf("Unexpected setting for DATABRICKS_CLI_DEPLOYMENT=%#v (expected 'terraform' or 'direct' or absent/empty which means 'terraform')", deployment) + } +} From 7b5b00d38de5afaa6dc4785495e32d2c58f83b92 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 2 Jun 2025 10:02:50 +0200 Subject: [PATCH 30/41] dag: maintain insertion order, do not sort and do not require Less() --- bundle/terranova/tnstate/state.go | 10 -------- libs/dag/dag.go | 41 +++++++++---------------------- libs/dag/dag_test.go | 4 --- 3 files changed, 11 insertions(+), 44 deletions(-) diff --git a/bundle/terranova/tnstate/state.go b/bundle/terranova/tnstate/state.go index 596e4b86cb..e9881b4db0 100644 --- a/bundle/terranova/tnstate/state.go +++ b/bundle/terranova/tnstate/state.go @@ -166,16 +166,6 @@ func (db *TerranovaState) unlockedSave() error { return os.WriteFile(db.Path, data, 0o600) } -func (r ResourceNode) Less(other ResourceNode) bool { - if r.Section < other.Section { - return true - } - if r.Section > other.Section { - return false - } - return r.Name < other.Name -} - func (r ResourceNode) String() string { return r.Section + "." + r.Name + "#" + r.ID } diff --git a/libs/dag/dag.go b/libs/dag/dag.go index 9b039d0608..f615f51a23 100644 --- a/libs/dag/dag.go +++ b/libs/dag/dag.go @@ -2,32 +2,28 @@ package dag import ( "fmt" - "sort" "strings" "sync" ) -type Lessable[T comparable] interface { - comparable - Less(T) bool -} - -type adjEdge[N Lessable[N]] struct { +type adjEdge[N comparable] struct { to N label string } -type Graph[N Lessable[N]] struct { - adj map[N][]adjEdge[N] +type Graph[N comparable] struct { + adj map[N][]adjEdge[N] + nodes []N // maintains insertion order of added nodes } -func NewGraph[N Lessable[N]]() *Graph[N] { +func NewGraph[N comparable]() *Graph[N] { return &Graph[N]{adj: make(map[N][]adjEdge[N])} } func (g *Graph[N]) AddNode(n N) { if _, ok := g.adj[n]; !ok { g.adj[n] = nil + g.nodes = append(g.nodes, n) } } @@ -41,7 +37,7 @@ func (g *Graph[N]) AddDirectedEdge(from, to N, label string) error { return nil } -type CycleError[N Lessable[N]] struct { +type CycleError[N comparable] struct { Nodes []N Edges []string } @@ -81,18 +77,8 @@ func (g *Graph[N]) indegrees() map[N]int { } func (g *Graph[N]) DetectCycle() error { - // 1. sort every adjacency list once - for k, outs := range g.adj { - sort.Slice(outs, func(i, j int) bool { return outs[i].to.Less(outs[j].to) }) - g.adj[k] = outs - } - - // 2. sorted list of roots - roots := make([]N, 0, len(g.adj)) - for v := range g.adj { - roots = append(roots, v) - } - sort.Slice(roots, func(i, j int) bool { return roots[i].Less(roots[j]) }) + // Build list of roots in insertion order + roots := g.nodes const ( white = 0 @@ -181,13 +167,8 @@ func (g *Graph[N]) Run(pool int, runUnit func(N)) error { }() } - // stable initial-ready order - keys := make([]N, 0, len(in)) - for k := range in { - keys = append(keys, k) - } - sort.Slice(keys, func(i, j int) bool { return keys[i].Less(keys[j]) }) - for _, n := range keys { + // stable initial-ready order based on insertion order + for _, n := range g.nodes { if in[n] == 0 { ready <- n } diff --git a/libs/dag/dag_test.go b/libs/dag/dag_test.go index 389ab6161e..e714af1ac9 100644 --- a/libs/dag/dag_test.go +++ b/libs/dag/dag_test.go @@ -16,10 +16,6 @@ type stringWrapper struct { Value string } -func (s stringWrapper) Less(other stringWrapper) bool { - return s.Value < other.Value -} - func (s stringWrapper) String() string { return s.Value } From 748eeabca35643cb6d24622c6290a316d3ed46f2 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 2 Jun 2025 10:06:50 +0200 Subject: [PATCH 31/41] simplify cycle message --- libs/dag/dag.go | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/libs/dag/dag.go b/libs/dag/dag.go index f615f51a23..a2ce1d785e 100644 --- a/libs/dag/dag.go +++ b/libs/dag/dag.go @@ -46,21 +46,19 @@ func (e *CycleError[N]) Error() string { if len(e.Nodes) == 0 { return "cycle detected" } - var b strings.Builder - b.WriteString("cycle detected: ") - fmt.Fprint(&b, e.Nodes[0]) + + // Build "A refers to B via E1" pieces for every edge except the closing one. + var parts []string for i := 1; i < len(e.Nodes); i++ { - b.WriteString(" refers to ") - fmt.Fprint(&b, e.Nodes[i]) - b.WriteString(" via ") - b.WriteString(e.Edges[i-1]) + parts = append(parts, fmt.Sprintf("%v refers to %v via %s", e.Nodes[i-1], e.Nodes[i], e.Edges[i-1])) } - b.WriteString(" which refers to ") - fmt.Fprint(&b, e.Nodes[0]) - b.WriteString(" via ") - b.WriteString(e.Edges[len(e.Edges)-1]) - b.WriteString(".") - return b.String() + + return fmt.Sprintf( + "cycle detected: %s which refers to %v via %s.", + strings.Join(parts, " "), + e.Nodes[0], + e.Edges[len(e.Edges)-1], + ) } func (g *Graph[N]) indegrees() map[N]int { From a1b9580a7aa71c4959540121608d8009977890a8 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 2 Jun 2025 10:38:41 +0200 Subject: [PATCH 32/41] rename libs/dag to libs/dagrun --- bundle/terranova/deploy_mutator.go | 4 ++-- bundle/terranova/destroy_mutator.go | 4 ++-- libs/{dag/dag.go => dagrun/dagrun.go} | 2 +- libs/{dag/dag_test.go => dagrun/dagrun_test.go} | 2 +- libs/{dag => dagrun}/stack.go | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) rename libs/{dag/dag.go => dagrun/dagrun.go} (99%) rename libs/{dag/dag_test.go => dagrun/dagrun_test.go} (99%) rename libs/{dag => dagrun}/stack.go (95%) diff --git a/bundle/terranova/deploy_mutator.go b/bundle/terranova/deploy_mutator.go index 1e634ac579..bbd05c119e 100644 --- a/bundle/terranova/deploy_mutator.go +++ b/bundle/terranova/deploy_mutator.go @@ -8,7 +8,7 @@ import ( "github.com/databricks/cli/bundle/terranova/tnresources" "github.com/databricks/cli/bundle/terranova/tnstate" "github.com/databricks/cli/libs/cmdio" - "github.com/databricks/cli/libs/dag" + "github.com/databricks/cli/libs/dagrun" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/log" @@ -33,7 +33,7 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di client := b.WorkspaceClient() - g := dag.NewGraph[tnstate.ResourceNode]() + g := dagrun.NewGraph[tnstate.ResourceNode]() _, err := dyn.MapByPattern( b.Config.Value(), diff --git a/bundle/terranova/destroy_mutator.go b/bundle/terranova/destroy_mutator.go index f384ee5e32..4830d2312b 100644 --- a/bundle/terranova/destroy_mutator.go +++ b/bundle/terranova/destroy_mutator.go @@ -6,7 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/terranova/tnresources" "github.com/databricks/cli/bundle/terranova/tnstate" - "github.com/databricks/cli/libs/dag" + "github.com/databricks/cli/libs/dagrun" "github.com/databricks/cli/libs/diag" ) @@ -25,7 +25,7 @@ func (m *terranovaDestroyMutator) Apply(ctx context.Context, b *bundle.Bundle) d client := b.WorkspaceClient() allResources := b.ResourceDatabase.GetAllResources() - g := dag.NewGraph[tnstate.ResourceNode]() + g := dagrun.NewGraph[tnstate.ResourceNode]() for _, node := range allResources { g.AddNode(node) diff --git a/libs/dag/dag.go b/libs/dagrun/dagrun.go similarity index 99% rename from libs/dag/dag.go rename to libs/dagrun/dagrun.go index a2ce1d785e..2140a11a0e 100644 --- a/libs/dag/dag.go +++ b/libs/dagrun/dagrun.go @@ -1,4 +1,4 @@ -package dag +package dagrun import ( "fmt" diff --git a/libs/dag/dag_test.go b/libs/dagrun/dagrun_test.go similarity index 99% rename from libs/dag/dag_test.go rename to libs/dagrun/dagrun_test.go index e714af1ac9..e7dc338b6a 100644 --- a/libs/dag/dag_test.go +++ b/libs/dagrun/dagrun_test.go @@ -1,4 +1,4 @@ -package dag +package dagrun import ( "fmt" diff --git a/libs/dag/stack.go b/libs/dagrun/stack.go similarity index 95% rename from libs/dag/stack.go rename to libs/dagrun/stack.go index 58b45b3a53..ff9340413c 100644 --- a/libs/dag/stack.go +++ b/libs/dagrun/stack.go @@ -1,4 +1,4 @@ -package dag +package dagrun type stack[T any] struct{ data []T } From a29be26b666a4b8fb603417ee1432ef73fefbf2d Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 2 Jun 2025 10:54:18 +0200 Subject: [PATCH 33/41] clean up; do not ignore errors --- bundle/config/resources_types.go | 3 --- bundle/terranova/deploy_mutator.go | 27 +++++++++------------------ bundle/terranova/destroy_mutator.go | 5 ++++- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/bundle/config/resources_types.go b/bundle/config/resources_types.go index b1d06020e0..230603fb88 100644 --- a/bundle/config/resources_types.go +++ b/bundle/config/resources_types.go @@ -9,9 +9,6 @@ import ( // ResourcesTypes maps the configuration key of each Databricks resource section (for example // "jobs" or "pipelines") to the Go type that represents a single resource instance inside // that section (for example `resources.Job`). -// -// The map is populated at package‐initialisation time by inspecting the `Resources` struct with -// reflection, so it automatically stays up-to-date when new resource kinds are added. var ResourcesTypes = func() map[string]reflect.Type { var r Resources rt := reflect.TypeOf(r) diff --git a/bundle/terranova/deploy_mutator.go b/bundle/terranova/deploy_mutator.go index bbd05c119e..f83168992b 100644 --- a/bundle/terranova/deploy_mutator.go +++ b/bundle/terranova/deploy_mutator.go @@ -55,13 +55,8 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di countDeployed := 0 err = g.Run(maxPoolSize, func(node tnstate.ResourceNode) { - // TODO func(node string) bool - // If function returns false, downstream callers are not called - // g.Run() should return list of not executed nodes - // log.Warnf(ctx, "Processing node=%s", node) - - // TODO: resolve all resource references inside this resource. It should be possible, if graph was constructed correctly. - // If it is not possible, return error (and fail this and dependent resources) + // TODO: if a given node fails, all downstream nodes should not be run. We should report those nodes. + // TODO: ensure that config for this node is fully resolved at this point. config, ok := b.GetResourceConfig(node.Section, node.Name) if !ok { @@ -81,7 +76,6 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di diags.AppendError(err) return } - // TODO handle error; count stats countDeployed = countDeployed + 1 }) @@ -94,8 +88,10 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di cmdio.LogString(ctx, "Updating deployment state...") } - _ = b.ResourceDatabase.Finalize() - // TODO: handle errors + err = b.ResourceDatabase.Finalize() + if err != nil { + diags.AppendError(err) + } return diags.Diags } @@ -128,7 +124,7 @@ func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { config := resource.Config() - // presence of id in the state file implies that the resource was created by us + // Presence of id in the state file implies that the resource was created by us if oldID == "" { newID, err := resource.DoCreate(ctx) @@ -150,11 +146,11 @@ func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { if err != nil { return fmt.Errorf("reading state: %w", err) } + localDiff, err := structdiff.GetStructDiff(savedState, config) if err != nil { return fmt.Errorf("state error: %w", err) } - // log.Warnf(ctx, "localDiff: %#v", localDiff) localDiffType := tnresources.ChangeTypeNone if len(localDiff) > 0 { @@ -169,12 +165,7 @@ func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { return d.Update(ctx, resource, oldID, config) } - // localDiffType is either None or Partial: we should proceed to remote diff - - // remoteState := DoRead() - // compare config and remoteState - // OR compare config and state + config and remoteState separately - // decide what to do for this: config=X state=Y remoteState=X; should this trigger deploy? probably not. + // localDiffType is either None or Partial: we should proceed to fetching remote state and calculate local+remote diff log.Warnf(ctx, "Unchanged %s.%s id=%#v", d.section, d.resourceName, oldID) return nil diff --git a/bundle/terranova/destroy_mutator.go b/bundle/terranova/destroy_mutator.go index 4830d2312b..7defa09036 100644 --- a/bundle/terranova/destroy_mutator.go +++ b/bundle/terranova/destroy_mutator.go @@ -52,7 +52,10 @@ func (m *terranovaDestroyMutator) Apply(ctx context.Context, b *bundle.Bundle) d diags.AppendError(err) } - _ = b.ResourceDatabase.Finalize() + err = b.ResourceDatabase.Finalize() + if err != nil { + diags.AppendError(err) + } return diags.Diags } From 75aed056cebec04e8707b967de49756b329371a7 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 3 Jun 2025 15:44:21 +0200 Subject: [PATCH 34/41] post-rebase fix of test.toml + update outputs --- acceptance/bundle/resources/jobs/output.txt | 11 ++++++----- acceptance/test.toml | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/acceptance/bundle/resources/jobs/output.txt b/acceptance/bundle/resources/jobs/output.txt index ecbca8fa2a..98bafff913 100644 --- a/acceptance/bundle/resources/jobs/output.txt +++ b/acceptance/bundle/resources/jobs/output.txt @@ -1,3 +1,4 @@ + >>> [CLI] bundle deploy Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... Deploying resources... @@ -86,12 +87,12 @@ Deployment complete! "method": "POST", "path": "/api/2.2/jobs/reset" } -jobs foo id='[TEST_JOB_ID+0]' name='foo' +jobs foo id='[NUMID]' name='foo' === Fetch job ID and verify remote state ->>> [CLI] jobs get [TEST_JOB_ID+0] +>>> [CLI] jobs get [NUMID] { - "job_id":[TEST_JOB_ID+0], + "job_id":[NUMID], "settings": { "deployment": { "kind":"BUNDLE", @@ -136,14 +137,14 @@ Destroy complete! >>> print_requests { "body": { - "job_id": [TEST_JOB_ID+0] + "job_id": [NUMID] }, "method": "POST", "path": "/api/2.2/jobs/delete" } State not found for jobs.foo ->>> musterr [CLI] jobs get [TEST_JOB_ID+0] +>>> musterr [CLI] jobs get [NUMID] Error: Not Found Exit code (musterr): 1 diff --git a/acceptance/test.toml b/acceptance/test.toml index 0833cd2b24..0d18f512bb 100644 --- a/acceptance/test.toml +++ b/acceptance/test.toml @@ -14,6 +14,9 @@ TimeoutCloud = '25m' Env.PYTHONDONTWRITEBYTECODE = "1" Env.PYTHONUNBUFFERED = "1" +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform", "direct"] +EnvRepl.DATABRICKS_CLI_DEPLOYMENT = false + # >>> datetime.datetime.fromtimestamp(18000000000) # datetime.datetime(2027, 1, 15, 9, 0) # >>> datetime.datetime.fromtimestamp(1900000000) @@ -70,9 +73,6 @@ Old = '2\d\d\d-\d\d-\d\d(T| )\d\d:\d\d:\d\d' New = "[TIMESTAMP]" Order = 10 -EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform", "direct"] -EnvRepl.DATABRICKS_CLI_DEPLOYMENT = false - [[Repls]] Old = '"exec_path": " "' New = '"exec_path": "[TERRAFORM]"' From 872ac3fca9f3c70850daa744e926f5af54b8570f Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 3 Jun 2025 15:59:57 +0200 Subject: [PATCH 35/41] post rebase - restore acceptance/bundle/run/basic/test.toml --- acceptance/bundle/run/basic/test.toml | 1 + 1 file changed, 1 insertion(+) create mode 100644 acceptance/bundle/run/basic/test.toml diff --git a/acceptance/bundle/run/basic/test.toml b/acceptance/bundle/run/basic/test.toml new file mode 100644 index 0000000000..3332757cde --- /dev/null +++ b/acceptance/bundle/run/basic/test.toml @@ -0,0 +1 @@ +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # "bundle run" not implemented yet From b1d6005df9e2f09b560cb1e397084ab32ac682d6 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 5 Jun 2025 12:31:22 +0200 Subject: [PATCH 36/41] clean up warning --- bundle/terranova/deploy_mutator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/terranova/deploy_mutator.go b/bundle/terranova/deploy_mutator.go index f83168992b..703ffaa78f 100644 --- a/bundle/terranova/deploy_mutator.go +++ b/bundle/terranova/deploy_mutator.go @@ -167,7 +167,7 @@ func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { // localDiffType is either None or Partial: we should proceed to fetching remote state and calculate local+remote diff - log.Warnf(ctx, "Unchanged %s.%s id=%#v", d.section, d.resourceName, oldID) + log.Debugf(ctx, "Unchanged %s.%s id=%#v", d.section, d.resourceName, oldID) return nil } From 94cb7735c37aa4090c42ef2173c2afcd60da2c7a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 6 Jun 2025 10:26:57 +0200 Subject: [PATCH 37/41] clean up; add comments; add error wrapping; use atomic --- acceptance/bundle/resources/pipelines/script | 4 +- .../default-python/combinations/test.toml | 1 + bundle/bundle.go | 1 + bundle/phases/deploy.go | 1 + bundle/phases/destroy.go | 3 + bundle/phases/initialize.go | 19 +++--- bundle/terranova/deploy_mutator.go | 64 ++++++++++++------- bundle/terranova/destroy_mutator.go | 2 +- bundle/terranova/tnstate/state.go | 1 + 9 files changed, 63 insertions(+), 33 deletions(-) diff --git a/acceptance/bundle/resources/pipelines/script b/acceptance/bundle/resources/pipelines/script index c1353f79a3..db7ee11a4f 100644 --- a/acceptance/bundle/resources/pipelines/script +++ b/acceptance/bundle/resources/pipelines/script @@ -5,8 +5,8 @@ trace $CLI bundle deploy print_requests() { jq --sort-keys 'select(.method != "GET" and (.path | contains("/pipelines")))' < out.requests.txt - errcode rm out.requests.txt - errcode read_state.py pipelines my id name + rm out.requests.txt + read_state.py pipelines my id name } trace print_requests diff --git a/acceptance/bundle/templates/default-python/combinations/test.toml b/acceptance/bundle/templates/default-python/combinations/test.toml index 4b670a9c86..42988258ea 100644 --- a/acceptance/bundle/templates/default-python/combinations/test.toml +++ b/acceptance/bundle/templates/default-python/combinations/test.toml @@ -2,6 +2,7 @@ Cloud = true Ignore = ["input.json"] +# Using shortcuts, because max path component name on Windows is 256 chars # INCLUDE_NOTEBOOK EnvMatrix.N = ["yes", "no"] diff --git a/bundle/bundle.go b/bundle/bundle.go index e504a3cf79..1908bd5770 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -120,6 +120,7 @@ type Bundle struct { // If true, don't use terraform. Set by DATABRICKS_CLI_DEPLOYMENT=direct DirectDeployment bool + // State file access for direct deployment (only initialized if DirectDeployment = true) ResourceDatabase tnstate.TerranovaState } diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 11135dc119..288f11f1f5 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -58,6 +58,7 @@ func filterDeleteOrRecreateActions(changes []*tfjson.ResourceChange, resourceTyp func approvalForDeploy(ctx context.Context, b *bundle.Bundle) (bool, error) { if b.DirectDeployment { + // TODO: implement this for DirectDeployment return true, nil } diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go index 8d07949a1b..79f641f3f6 100644 --- a/bundle/phases/destroy.go +++ b/bundle/phases/destroy.go @@ -38,6 +38,9 @@ func getDeleteActions(ctx context.Context, b *bundle.Bundle) ([]terraformlib.Act allResources := b.ResourceDatabase.GetAllResources() var deleteActions []terraformlib.Action for _, node := range allResources { + // TODO: + // Note, rType is only used for stringification so it's not important to get it right (although cutting "s" is always correct) + // Also, terraformlib.Action will create terraformish resources names. We should instead switch to method-neutral format. rType, _ := strings.CutSuffix(node.Section, "s") deleteActions = append(deleteActions, terraformlib.Action{ Action: terraformlib.ActionTypeDelete, diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 3c3a633410..94fe578030 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -3,7 +3,6 @@ package phases import ( "context" "fmt" - "os" "path/filepath" "github.com/databricks/cli/bundle/config/mutator/resourcemutator" @@ -21,6 +20,7 @@ import ( "github.com/databricks/cli/bundle/scripts" "github.com/databricks/cli/bundle/trampoline" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/log" ) @@ -32,7 +32,7 @@ func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { log.Info(ctx, "Phase: initialize") - b.DirectDeployment, err = IsDirectDeployment() + b.DirectDeployment, err = IsDirectDeployment(ctx) if err != nil { return diag.FromErr(err) } @@ -207,13 +207,13 @@ func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { cacheDir, err := b.CacheDir(ctx) if err != nil { - diags = diags.Extend(diag.FromErr(err)) + return diags.Extend(diag.FromErr(err)) } databasePath := filepath.Join(cacheDir, "resources.json") err = b.ResourceDatabase.Open(databasePath) if err != nil { - diags = diags.Extend(diag.FromErr(err)) + return diags.Extend(diag.FromErr(err)) } } else { // Reads (typed): b.Config.Bundle.Terraform (checks terraform configuration) @@ -224,14 +224,17 @@ func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { diags = diags.Extend(bundle.Apply(ctx, b, terraform.Initialize())) } + if diags.HasError() { + return diags + } + // Reads (typed): b.Config.Experimental.Scripts["post_init"] (checks if script is defined) // Executes the post_init script hook defined in the bundle configuration - diags = diags.Extend(bundle.Apply(ctx, b, scripts.Execute(config.ScriptPostInit))) - return diags + return diags.Extend(bundle.Apply(ctx, b, scripts.Execute(config.ScriptPostInit))) } -func IsDirectDeployment() (bool, error) { - deployment := os.Getenv("DATABRICKS_CLI_DEPLOYMENT") +func IsDirectDeployment(ctx context.Context) (bool, error) { + deployment := env.Get(ctx, "DATABRICKS_CLI_DEPLOYMENT") if deployment == "direct" { return true, nil } else if deployment == "terraform" || deployment == "" { diff --git a/bundle/terranova/deploy_mutator.go b/bundle/terranova/deploy_mutator.go index 703ffaa78f..8bd4a4419f 100644 --- a/bundle/terranova/deploy_mutator.go +++ b/bundle/terranova/deploy_mutator.go @@ -3,6 +3,7 @@ package terranova import ( "context" "fmt" + "sync/atomic" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/terranova/tnresources" @@ -16,7 +17,8 @@ import ( "github.com/databricks/databricks-sdk-go" ) -const maxPoolSize = 10 +// How many parallel operations (API calls) are allowed +const defaultParallelism = 10 type terranovaDeployMutator struct{} @@ -41,7 +43,6 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di func(p dyn.Path, v dyn.Value) (dyn.Value, error) { section := p[1].Key() name := p[2].Key() - // log.Warnf(ctx, "Adding node=%s", node) g.AddNode(tnstate.ResourceNode{ Section: section, Name: name, @@ -52,9 +53,9 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di }, ) - countDeployed := 0 + var countDeployed atomic.Uint64 - err = g.Run(maxPoolSize, func(node tnstate.ResourceNode) { + err = g.Run(defaultParallelism, func(node tnstate.ResourceNode) { // TODO: if a given node fails, all downstream nodes should not be run. We should report those nodes. // TODO: ensure that config for this node is fully resolved at this point. @@ -77,14 +78,14 @@ func (m *terranovaDeployMutator) Apply(ctx context.Context, b *bundle.Bundle) di return } - countDeployed = countDeployed + 1 + countDeployed.Add(1) }) if err != nil { diags.AppendError(err) } // Not uploading at the moment, just logging to match the output. - if countDeployed > 0 { + if countDeployed.Load() > 0 { cmdio.LogString(ctx, "Updating deployment state...") } @@ -112,6 +113,7 @@ func (d *Deployer) Deploy(ctx context.Context, inputConfig any) error { } func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { + // Note, oldID might be empty if resource is new oldID, err := d.db.GetResourceID(d.section, d.resourceName) if err != nil { return err @@ -127,19 +129,7 @@ func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { // Presence of id in the state file implies that the resource was created by us if oldID == "" { - newID, err := resource.DoCreate(ctx) - if err != nil { - return err - } - - log.Infof(ctx, "Created %s.%s id=%#v", d.section, d.resourceName, newID) - - err = d.db.SaveState(d.section, d.resourceName, newID, config) - if err != nil { - return err - } - - return resource.WaitAfterCreate(ctx) + return d.Create(ctx, resource, config) } savedState, err := d.db.GetSavedState(d.section, d.resourceName, resource.GetType()) @@ -171,6 +161,27 @@ func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { return nil } +func (d *Deployer) Create(ctx context.Context, resource tnresources.IResource, config any) error { + newID, err := resource.DoCreate(ctx) + if err != nil { + return fmt.Errorf("creating: %w", err) + } + + log.Infof(ctx, "Created %s.%s id=%#v", d.section, d.resourceName, newID) + + err = d.db.SaveState(d.section, d.resourceName, newID, config) + if err != nil { + return fmt.Errorf("saving state after creating id=%s: %w", newID, err) + } + + err = resource.WaitAfterCreate(ctx) + if err != nil { + return fmt.Errorf("waiting after creating id=%s: %w", newID, err) + } + + return nil +} + func (d *Deployer) Recreate(ctx context.Context, oldResource tnresources.IResource, oldID string, config any) error { err := oldResource.DoDelete(ctx, oldID) if err != nil { @@ -195,10 +206,15 @@ func (d *Deployer) Recreate(ctx context.Context, oldResource tnresources.IResour log.Warnf(ctx, "Re-created %s.%s id=%#v (previously %#v)", d.section, d.resourceName, newID, oldID) err = d.db.SaveState(d.section, d.resourceName, newID, config) if err != nil { - return fmt.Errorf("saving state: %w", err) + return fmt.Errorf("saving state for id=%s: %w", newID, err) } - return newResource.WaitAfterCreate(ctx) + err = newResource.WaitAfterCreate(ctx) + if err != nil { + return fmt.Errorf("waiting after re-creating id=%s: %w", newID, err) + } + + return nil } func (d *Deployer) Update(ctx context.Context, resource tnresources.IResource, oldID string, config any) error { @@ -218,5 +234,9 @@ func (d *Deployer) Update(ctx context.Context, resource tnresources.IResource, o return fmt.Errorf("saving state id=%s: %w", oldID, err) } - return resource.WaitAfterUpdate(ctx) + err = resource.WaitAfterUpdate(ctx) + if err != nil { + return fmt.Errorf("waiting after updating id=%s: %w", newID, err) + } + return nil } diff --git a/bundle/terranova/destroy_mutator.go b/bundle/terranova/destroy_mutator.go index 7defa09036..46ddf5eb7b 100644 --- a/bundle/terranova/destroy_mutator.go +++ b/bundle/terranova/destroy_mutator.go @@ -34,7 +34,7 @@ func (m *terranovaDestroyMutator) Apply(ctx context.Context, b *bundle.Bundle) d // TODO: do we need to respect dependencies here? Should we store them in state in that case? // Alternatively, do a few rounds of delete to make we capture cases where A needs to be deleted before B. - err := g.Run(maxPoolSize, func(node tnstate.ResourceNode) { + err := g.Run(defaultParallelism, func(node tnstate.ResourceNode) { err := tnresources.DestroyResource(ctx, client, node.Section, node.ID) if err != nil { diags.AppendErrorf("destroying %s: %s", node, err) diff --git a/bundle/terranova/tnstate/state.go b/bundle/terranova/tnstate/state.go index e9881b4db0..a249fe9866 100644 --- a/bundle/terranova/tnstate/state.go +++ b/bundle/terranova/tnstate/state.go @@ -99,6 +99,7 @@ func (db *TerranovaState) GetSavedState(section, resourceName string, stateType return reflect.ValueOf(destPtr).Elem().Interface(), nil } +// Return resource id if the resource is present in the state or "" if it's not func (db *TerranovaState) GetResourceID(section, resourceName string) (string, error) { db.mu.Lock() defer db.mu.Unlock() From 2864e1fa416daaa174a5c3661be40b7bed163577 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 6 Jun 2025 10:36:58 +0200 Subject: [PATCH 38/41] add comments --- bundle/terranova/tnresources/app.go | 1 + bundle/terranova/tnresources/resource.go | 3 ++- bundle/terranova/tnresources/util.go | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/bundle/terranova/tnresources/app.go b/bundle/terranova/tnresources/app.go index 89ed9fa1ad..c2c9b614cb 100644 --- a/bundle/terranova/tnresources/app.go +++ b/bundle/terranova/tnresources/app.go @@ -67,6 +67,7 @@ func (r *ResourceApp) WaitAfterUpdate(ctx context.Context) error { } func (r *ResourceApp) ClassifyChanges(changes []structdiff.Change) ChangeType { + // TODO: changing name is recreation return ChangeTypeUpdate } diff --git a/bundle/terranova/tnresources/resource.go b/bundle/terranova/tnresources/resource.go index 52378edc0c..12c15467b6 100644 --- a/bundle/terranova/tnresources/resource.go +++ b/bundle/terranova/tnresources/resource.go @@ -23,7 +23,8 @@ type IResource interface { // Create the resource. Returns id of the resource. DoCreate(ctx context.Context) (string, error) - // Update the resource. Returns id of the resource (might be updated). + // Update the resource. Returns id of the resource. + // Usually returns the same id as oldId but can also return a different one (e.g. schemas and volumes when certain fields are changed) DoUpdate(ctx context.Context, oldId string) (string, error) DoDelete(ctx context.Context, oldId string) error diff --git a/bundle/terranova/tnresources/util.go b/bundle/terranova/tnresources/util.go index 4702862d95..5099c137e7 100644 --- a/bundle/terranova/tnresources/util.go +++ b/bundle/terranova/tnresources/util.go @@ -6,6 +6,8 @@ import ( "fmt" ) +// Utility to copy from one type to another based on intermediate JSON transformation +// (e.g. to copy from JobSettings to CreateJob) func copyViaJSON[T1, T2 any](dest *T1, src T2) error { if dest == nil { return errors.New("internal error: unexpected nil") From 9eefe8e8f42bc7ef60911bd27f7ec4f14e29da03 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 6 Jun 2025 10:43:51 +0200 Subject: [PATCH 39/41] rm dec.DisallowUnknownFields --- bundle/terranova/tnstate/state.go | 1 - 1 file changed, 1 deletion(-) diff --git a/bundle/terranova/tnstate/state.go b/bundle/terranova/tnstate/state.go index a249fe9866..471ca5f41e 100644 --- a/bundle/terranova/tnstate/state.go +++ b/bundle/terranova/tnstate/state.go @@ -74,7 +74,6 @@ func jsonRoundTrip(src, dest any) error { dec := json.NewDecoder(bytes.NewReader(raw)) dec.UseNumber() - dec.DisallowUnknownFields() return dec.Decode(dest) } From 7afaf23e7e514773fbd8b57859a476b7d4c9312b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 6 Jun 2025 11:14:42 +0200 Subject: [PATCH 40/41] state: use GetResourceEntry to replace both GetResourceID and GetSavedState --- bundle/terranova/deploy_mutator.go | 39 +++++++++++++++++++------- bundle/terranova/tnstate/state.go | 45 +++--------------------------- 2 files changed, 33 insertions(+), 51 deletions(-) diff --git a/bundle/terranova/deploy_mutator.go b/bundle/terranova/deploy_mutator.go index 8bd4a4419f..72f81bad39 100644 --- a/bundle/terranova/deploy_mutator.go +++ b/bundle/terranova/deploy_mutator.go @@ -1,8 +1,12 @@ package terranova import ( + "bytes" "context" + "encoding/json" + "errors" "fmt" + "reflect" "sync/atomic" "github.com/databricks/cli/bundle" @@ -113,11 +117,7 @@ func (d *Deployer) Deploy(ctx context.Context, inputConfig any) error { } func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { - // Note, oldID might be empty if resource is new - oldID, err := d.db.GetResourceID(d.section, d.resourceName) - if err != nil { - return err - } + entry, hasEntry := d.db.GetResourceEntry(d.section, d.resourceName) resource, err := tnresources.New(d.client, d.section, d.resourceName, inputConfig) if err != nil { @@ -126,20 +126,23 @@ func (d *Deployer) deploy(ctx context.Context, inputConfig any) error { config := resource.Config() - // Presence of id in the state file implies that the resource was created by us + if !hasEntry { + return d.Create(ctx, resource, config) + } + oldID := entry.ID if oldID == "" { - return d.Create(ctx, resource, config) + return errors.New("invalid state: empty id") } - savedState, err := d.db.GetSavedState(d.section, d.resourceName, resource.GetType()) + savedState, err := typeConvert(resource.GetType(), entry.State) if err != nil { - return fmt.Errorf("reading state: %w", err) + return fmt.Errorf("interpreting state: %w", err) } localDiff, err := structdiff.GetStructDiff(savedState, config) if err != nil { - return fmt.Errorf("state error: %w", err) + return fmt.Errorf("comparing state and config: %w", err) } localDiffType := tnresources.ChangeTypeNone @@ -240,3 +243,19 @@ func (d *Deployer) Update(ctx context.Context, resource tnresources.IResource, o } return nil } + +func typeConvert(destType reflect.Type, src any) (any, error) { + raw, err := json.Marshal(src) + if err != nil { + return nil, fmt.Errorf("marshalling: %w", err) + } + + destPtr := reflect.New(destType).Interface() + dec := json.NewDecoder(bytes.NewReader(raw)) + err = dec.Decode(destPtr) + if err != nil { + return nil, fmt.Errorf("unmarshalling into %s: %w", destType, err) + } + + return reflect.ValueOf(destPtr).Elem().Interface(), nil +} diff --git a/bundle/terranova/tnstate/state.go b/bundle/terranova/tnstate/state.go index 471ca5f41e..80c7dad7ae 100644 --- a/bundle/terranova/tnstate/state.go +++ b/bundle/terranova/tnstate/state.go @@ -1,10 +1,8 @@ package tnstate import ( - "bytes" "encoding/json" "os" - "reflect" "sync" "github.com/databricks/cli/libs/utils" @@ -66,49 +64,14 @@ func (db *TerranovaState) DeleteState(section, resourceName string) error { return nil } -func jsonRoundTrip(src, dest any) error { - raw, err := json.Marshal(src) - if err != nil { - return err - } - - dec := json.NewDecoder(bytes.NewReader(raw)) - dec.UseNumber() - - return dec.Decode(dest) -} - -func (db *TerranovaState) GetSavedState(section, resourceName string, stateType reflect.Type) (any, error) { - sectionData, ok := db.data.Resources[section] - if !ok { - return nil, nil - } - - entry, ok := sectionData[resourceName] - if !ok { - return nil, nil - } - - destPtr := reflect.New(stateType).Interface() - err := jsonRoundTrip(entry.State, destPtr) - if err != nil { - return nil, err - } - - return reflect.ValueOf(destPtr).Elem().Interface(), nil -} - -// Return resource id if the resource is present in the state or "" if it's not -func (db *TerranovaState) GetResourceID(section, resourceName string) (string, error) { - db.mu.Lock() - defer db.mu.Unlock() - +func (db *TerranovaState) GetResourceEntry(section, resourceName string) (ResourceEntry, bool) { sectionData, ok := db.data.Resources[section] if !ok { - return "", nil + return ResourceEntry{}, false } - return sectionData[resourceName].ID, nil + result, ok := sectionData[resourceName] + return result, ok } func (db *TerranovaState) GetAllResources() []ResourceNode { From bcffe8566f178e3fc66bdf0a38e58e97099d3776 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 12 Jun 2025 12:10:45 +0200 Subject: [PATCH 41/41] rebase fixups --- acceptance/bundle/generate/dashboard-inplace/test.toml | 1 + bundle/config/resources_types.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 acceptance/bundle/generate/dashboard-inplace/test.toml diff --git a/acceptance/bundle/generate/dashboard-inplace/test.toml b/acceptance/bundle/generate/dashboard-inplace/test.toml new file mode 100644 index 0000000000..29f550d538 --- /dev/null +++ b/acceptance/bundle/generate/dashboard-inplace/test.toml @@ -0,0 +1 @@ +EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # dashboards not supported yet diff --git a/bundle/config/resources_types.go b/bundle/config/resources_types.go index 230603fb88..4e0fbb39e5 100644 --- a/bundle/config/resources_types.go +++ b/bundle/config/resources_types.go @@ -3,7 +3,7 @@ package config import ( "reflect" - "github.com/databricks/cli/libs/structdiff/jsontag" + "github.com/databricks/cli/libs/structdiff/structtag" ) // ResourcesTypes maps the configuration key of each Databricks resource section (for example @@ -15,7 +15,7 @@ var ResourcesTypes = func() map[string]reflect.Type { res := make(map[string]reflect.Type, rt.NumField()) for _, field := range reflect.VisibleFields(rt) { - tag := jsontag.JSONTag(field.Tag.Get("json")) + tag := structtag.JSONTag(field.Tag.Get("json")) name := tag.Name() if name == "" || name == "-" { continue