Skip to content
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions acceptance/bundle/lifecycle/prevent-destroy/databricks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
bundle:
name: prevent-destroy

lifecycle: &lifecycle_base
lifecycle:
prevent_destroy: true

pipeline: &pipeline_base
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test coverage is unequal for direct and terraform - for Tf all resources are checked and for direct we only have this test. Can we have a test that loops through all resource types?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difficulty with writing an acceptance test for this is that we first need to deploy these resources, and doing this automatically in a loop is difficult, as they need to have a valid configuration, which is different per resource.
We can manually create the configuration for all resources but not sure if it's worth it. I've added a unit test for checkForPreventDestroy instead

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it work if we had large databricks.yml with all the resources? So we don't loop but test all at once. This requires all error messages to be printed rather than the first one, which may be a good idea anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could work, but if you add a new type of resource, you need to remember to add it to this databricks.yml file. The unit test I added will fail if the new resource does not handle prevent destroy correctly.
We can always follow up with additional tests if needed. This is a net new feature

Copy link
Copy Markdown
Contributor

@denik denik Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could work, but if you add a new type of resource, you need to remember to add it to this databricks.yml file.

It depends. If existing resource provide enough variety and implementation is general (it is) we can say we've covered our ground.

The unit test I added will fail if the new resource does not handle prevent destroy correctly.

It only covers TF, right?

Overall I agree we can proceed without full coverage, but it would be interesting to cover >1 resource to see what kind of error messages we get. Do we get them all? One at random?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only covers TF, right?

No, there are two separate ones

  1. terraform/TestConvertLifecycleForAllResources - for terraform
  2. phases/TestCheckPreventDestroyForAllResources - for direct

If existing resource provide enough variety and implementation is general (it is) we can say we've covered our ground.

Pipeline resource seem to provide enough variety, I'll add job just in case

resources:
pipelines:
my_pipelines:
name: "test-pipeline"
libraries:
- notebook:
path: "./test-notebook.py"
<<: *lifecycle_base
schema: test-schema
catalog: main

<<: *pipeline_base
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious: why use the YAML anchors if they aren't used anywhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use them in the test to remove the whole resource from the configuration

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative, could be easier to manage and follow than anchors:

  • have each resource in their own yaml
  • main databricks includes "resources/*.yml"

removing resources: rm resources/filename.yml
duplicating resource: cp resource/a.yml resource/b.yml, followed by update_file.py

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a preference here. If you think this makes it easier to understand, I'll change it to that

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done here 65bc5dd

48 changes: 48 additions & 0 deletions acceptance/bundle/lifecycle/prevent-destroy/out.direct-exp.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@

>>> errcode [CLI] bundle plan

>>> musterr [CLI] bundle destroy --auto-approve
Error: resource my_pipelines has lifecycle.prevent_destroy set, but the plan calls for this resource to be recreated or destroyed. To avoid this error, disable lifecycle.prevent_destroy for pipelines.my_pipelines


Exit code (musterr): 1

>>> errcode [CLI] bundle plan
recreate pipelines.my_pipelines

>>> musterr [CLI] bundle deploy
Comment thread
andrewnester marked this conversation as resolved.
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/prevent-destroy/default/files...
Error: resource my_pipelines has lifecycle.prevent_destroy set, but the plan calls for this resource to be recreated or destroyed. To avoid this error, disable lifecycle.prevent_destroy for pipelines.my_pipelines


Exit code (musterr): 1

>>> errcode [CLI] bundle plan
recreate pipelines.my_pipelines

>>> [CLI] bundle deploy --auto-approve
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/prevent-destroy/default/files...

This action will result in the deletion or recreation of the following Lakeflow Declarative Pipelines along with the
Streaming Tables (STs) and Materialized Views (MVs) managed by them. Recreating the pipelines will
restore the defined STs and MVs through full refresh. Note that recreation is necessary when pipeline
properties such as the 'catalog' or 'storage' are changed:
recreate pipeline my_pipelines
Deploying resources...
Updating deployment state...
Deployment complete!

>>> errcode [CLI] bundle plan
delete pipelines.my_pipelines

>>> [CLI] bundle deploy --auto-approve
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/prevent-destroy/default/files...

This action will result in the deletion or recreation of the following Lakeflow Declarative Pipelines along with the
Streaming Tables (STs) and Materialized Views (MVs) managed by them. Recreating the pipelines will
restore the defined STs and MVs through full refresh. Note that recreation is necessary when pipeline
properties such as the 'catalog' or 'storage' are changed:
delete pipeline my_pipelines
Deploying resources...
Updating deployment state...
Deployment complete!
84 changes: 84 additions & 0 deletions acceptance/bundle/lifecycle/prevent-destroy/out.terraform.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@

>>> errcode [CLI] bundle plan
Comment thread
andrewnester marked this conversation as resolved.

>>> musterr [CLI] bundle destroy --auto-approve
Error: exit status 1

Error: Instance cannot be destroyed

on bundle.tf.json line 15, in resource.databricks_pipeline:
15: "my_pipelines": {

Resource databricks_pipeline.my_pipelines has lifecycle.prevent_destroy set,
Comment thread
andrewnester marked this conversation as resolved.
but the plan calls for this resource to be destroyed. To avoid this error and
continue with the plan, either disable lifecycle.prevent_destroy or reduce
the scope of the plan using the -target flag.



Exit code (musterr): 1

>>> errcode [CLI] bundle plan
Error: exit status 1

Error: Instance cannot be destroyed

on bundle.tf.json line 15, in resource.databricks_pipeline:
15: "my_pipelines": {

Resource databricks_pipeline.my_pipelines has lifecycle.prevent_destroy set,
but the plan calls for this resource to be destroyed. To avoid this error and
continue with the plan, either disable lifecycle.prevent_destroy or reduce
the scope of the plan using the -target flag.



Exit code: 1

>>> musterr [CLI] bundle deploy
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/prevent-destroy/default/files...
Error: exit status 1

Error: Instance cannot be destroyed

on bundle.tf.json line 15, in resource.databricks_pipeline:
15: "my_pipelines": {

Resource databricks_pipeline.my_pipelines has lifecycle.prevent_destroy set,
but the plan calls for this resource to be destroyed. To avoid this error and
continue with the plan, either disable lifecycle.prevent_destroy or reduce
the scope of the plan using the -target flag.



Exit code (musterr): 1

>>> errcode [CLI] bundle plan
recreate pipelines.my_pipelines

>>> [CLI] bundle deploy --auto-approve
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/prevent-destroy/default/files...

This action will result in the deletion or recreation of the following Lakeflow Declarative Pipelines along with the
Streaming Tables (STs) and Materialized Views (MVs) managed by them. Recreating the pipelines will
restore the defined STs and MVs through full refresh. Note that recreation is necessary when pipeline
properties such as the 'catalog' or 'storage' are changed:
recreate pipeline my_pipelines
Deploying resources...
Updating deployment state...
Deployment complete!

>>> errcode [CLI] bundle plan
delete pipelines.my_pipelines

>>> [CLI] bundle deploy --auto-approve
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/prevent-destroy/default/files...

This action will result in the deletion or recreation of the following Lakeflow Declarative Pipelines along with the
Streaming Tables (STs) and Materialized Views (MVs) managed by them. Recreating the pipelines will
restore the defined STs and MVs through full refresh. Note that recreation is necessary when pipeline
properties such as the 'catalog' or 'storage' are changed:
delete pipeline my_pipelines
Deploying resources...
Updating deployment state...
Deployment complete!
5 changes: 5 additions & 0 deletions acceptance/bundle/lifecycle/prevent-destroy/out.test.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Local = true
Cloud = false

[EnvMatrix]
DATABRICKS_CLI_DEPLOYMENT = ["terraform", "direct-exp"]
15 changes: 15 additions & 0 deletions acceptance/bundle/lifecycle/prevent-destroy/output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

>>> [CLI] bundle validate
Name: prevent-destroy
Target: default
Workspace:
User: [USERNAME]
Path: /Workspace/Users/[USERNAME]/.bundle/prevent-destroy/default

Validation OK!

>>> [CLI] bundle deploy
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/prevent-destroy/default/files...
Deploying resources...
Updating deployment state...
Deployment complete!
22 changes: 22 additions & 0 deletions acceptance/bundle/lifecycle/prevent-destroy/script
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
trace $CLI bundle validate

trace $CLI bundle deploy

trace errcode $CLI bundle plan >out.$DATABRICKS_CLI_DEPLOYMENT.txt 2>&1
trace musterr $CLI bundle destroy --auto-approve >>out.$DATABRICKS_CLI_DEPLOYMENT.txt 2>&1

# Changing the catalog name, deploy must fail because pipeline will be recreated
update_file.py databricks.yml 'catalog: main' 'catalog: mainnew'
trace errcode $CLI bundle plan >>out.$DATABRICKS_CLI_DEPLOYMENT.txt 2>&1
trace musterr $CLI bundle deploy >>out.$DATABRICKS_CLI_DEPLOYMENT.txt 2>&1

# Removing the prevent_destroy, deploy must succeed
update_file.py databricks.yml 'prevent_destroy: true' 'prevent_destroy: false'
trace errcode $CLI bundle plan >>out.$DATABRICKS_CLI_DEPLOYMENT.txt 2>&1
trace $CLI bundle deploy --auto-approve >>out.$DATABRICKS_CLI_DEPLOYMENT.txt 2>&1

update_file.py databricks.yml 'prevent_destroy: false' 'prevent_destroy: true'
# Removing the pipeline, deploy must succeed
update_file.py databricks.yml '<<: *pipeline_base' ''
trace errcode $CLI bundle plan >>out.$DATABRICKS_CLI_DEPLOYMENT.txt 2>&1
trace $CLI bundle deploy --auto-approve >>out.$DATABRICKS_CLI_DEPLOYMENT.txt 2>&1
3 changes: 3 additions & 0 deletions acceptance/bundle/lifecycle/prevent-destroy/test-notebook.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Databricks notebook source

print("Hello, World!")
5 changes: 5 additions & 0 deletions acceptance/bundle/lifecycle/prevent-destroy/test.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
EnvVaryOutput = "DATABRICKS_CLI_DEPLOYMENT"

Ignore = [
".databricks"
]
2 changes: 2 additions & 0 deletions acceptance/bundle/lifecycle/test.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Local = true
Cloud = false
18 changes: 18 additions & 0 deletions acceptance/bundle/refschema/out.fields.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ resources.apps.*.effective_budget_policy_id string ALL
resources.apps.*.effective_user_api_scopes []string ALL
resources.apps.*.effective_user_api_scopes[*] string ALL
resources.apps.*.id string ALL
resources.apps.*.lifecycle resources.Lifecycle INPUT
resources.apps.*.lifecycle.prevent_destroy bool INPUT
resources.apps.*.modified_status string INPUT
resources.apps.*.name string ALL
resources.apps.*.oauth2_app_client_id string ALL
Expand Down Expand Up @@ -87,6 +89,8 @@ resources.database_catalogs.*.create_database_if_not_exists bool ALL
resources.database_catalogs.*.database_instance_name string ALL
resources.database_catalogs.*.database_name string ALL
resources.database_catalogs.*.id string INPUT
resources.database_catalogs.*.lifecycle resources.Lifecycle INPUT
resources.database_catalogs.*.lifecycle.prevent_destroy bool INPUT
resources.database_catalogs.*.modified_status string INPUT
resources.database_catalogs.*.name string ALL
resources.database_catalogs.*.uid string ALL
Expand All @@ -109,6 +113,8 @@ resources.database_instances.*.effective_stopped bool ALL
resources.database_instances.*.enable_pg_native_login bool ALL
resources.database_instances.*.enable_readable_secondaries bool ALL
resources.database_instances.*.id string INPUT
resources.database_instances.*.lifecycle resources.Lifecycle INPUT
resources.database_instances.*.lifecycle.prevent_destroy bool INPUT
resources.database_instances.*.modified_status string INPUT
resources.database_instances.*.name string ALL
resources.database_instances.*.node_count int ALL
Expand Down Expand Up @@ -294,6 +300,8 @@ resources.jobs.*.job_clusters[*].new_cluster.workload_type.clients compute.Clien
resources.jobs.*.job_clusters[*].new_cluster.workload_type.clients.jobs bool INPUT STATE
resources.jobs.*.job_clusters[*].new_cluster.workload_type.clients.notebooks bool INPUT STATE
resources.jobs.*.job_id int64 REMOTE
resources.jobs.*.lifecycle resources.Lifecycle INPUT
resources.jobs.*.lifecycle.prevent_destroy bool INPUT
resources.jobs.*.max_concurrent_runs int INPUT STATE
resources.jobs.*.modified_status string INPUT
resources.jobs.*.name string INPUT STATE
Expand Down Expand Up @@ -2082,6 +2090,8 @@ resources.pipelines.*.libraries[*].maven.repo string INPUT STATE
resources.pipelines.*.libraries[*].notebook *pipelines.NotebookLibrary INPUT STATE
resources.pipelines.*.libraries[*].notebook.path string INPUT STATE
resources.pipelines.*.libraries[*].whl string INPUT STATE
resources.pipelines.*.lifecycle resources.Lifecycle INPUT
resources.pipelines.*.lifecycle.prevent_destroy bool INPUT
resources.pipelines.*.modified_status string INPUT
resources.pipelines.*.name string ALL
resources.pipelines.*.notifications []pipelines.Notifications INPUT STATE
Expand Down Expand Up @@ -2389,6 +2399,8 @@ resources.schemas.*.grants[*].principal string INPUT
resources.schemas.*.grants[*].privileges []resources.SchemaGrantPrivilege INPUT
resources.schemas.*.grants[*].privileges[*] resources.SchemaGrantPrivilege INPUT
resources.schemas.*.id string INPUT
resources.schemas.*.lifecycle resources.Lifecycle INPUT
resources.schemas.*.lifecycle.prevent_destroy bool INPUT
resources.schemas.*.metastore_id string REMOTE
resources.schemas.*.modified_status string INPUT
resources.schemas.*.name string ALL
Expand Down Expand Up @@ -2422,6 +2434,8 @@ resources.sql_warehouses.*.health.summary string REMOTE
resources.sql_warehouses.*.id string INPUT REMOTE
resources.sql_warehouses.*.instance_profile_arn string ALL
resources.sql_warehouses.*.jdbc_url string REMOTE
resources.sql_warehouses.*.lifecycle resources.Lifecycle INPUT
resources.sql_warehouses.*.lifecycle.prevent_destroy bool INPUT
resources.sql_warehouses.*.max_num_clusters int ALL
resources.sql_warehouses.*.min_num_clusters int ALL
resources.sql_warehouses.*.modified_status string INPUT
Expand Down Expand Up @@ -2494,6 +2508,8 @@ resources.synced_database_tables.*.database_instance_name string ALL
resources.synced_database_tables.*.effective_database_instance_name string ALL
resources.synced_database_tables.*.effective_logical_database_name string ALL
resources.synced_database_tables.*.id string INPUT
resources.synced_database_tables.*.lifecycle resources.Lifecycle INPUT
resources.synced_database_tables.*.lifecycle.prevent_destroy bool INPUT
resources.synced_database_tables.*.logical_database_name string ALL
resources.synced_database_tables.*.modified_status string INPUT
resources.synced_database_tables.*.name string ALL
Expand Down Expand Up @@ -2527,6 +2543,8 @@ resources.volumes.*.grants[*].principal string INPUT
resources.volumes.*.grants[*].privileges []resources.VolumeGrantPrivilege INPUT
resources.volumes.*.grants[*].privileges[*] resources.VolumeGrantPrivilege INPUT
resources.volumes.*.id string INPUT
resources.volumes.*.lifecycle resources.Lifecycle INPUT
resources.volumes.*.lifecycle.prevent_destroy bool INPUT
resources.volumes.*.metastore_id string REMOTE
resources.volumes.*.modified_status string INPUT
resources.volumes.*.name string ALL
Expand Down
3 changes: 1 addition & 2 deletions bundle/config/resources/apps.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type AppPermission struct {

type App struct {
BaseResource
apps.App // nolint App struct also defines Id and URL field with the same json tag "id" and "url"

// SourceCodePath is a required field used by DABs to point to Databricks app source code
// on local disk and to the corresponding workspace path during app deployment.
Expand All @@ -36,8 +37,6 @@ type App struct {
Config map[string]any `json:"config,omitempty"`

Permissions []AppPermission `json:"permissions,omitempty"`

apps.App // nolint App struct also defines Id and URL field with the same json tag "id" and "url"
}

func (a *App) UnmarshalJSON(b []byte) error {
Expand Down
1 change: 1 addition & 0 deletions bundle/config/resources/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ type BaseResource struct {
ID string `json:"id,omitempty" bundle:"readonly"`
ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"`
URL string `json:"url,omitempty" bundle:"internal"`
Lifecycle Lifecycle `json:"lifecycle,omitempty"`
}
3 changes: 1 addition & 2 deletions bundle/config/resources/clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ type ClusterPermission struct {

type Cluster struct {
BaseResource
compute.ClusterSpec

Permissions []ClusterPermission `json:"permissions,omitempty"`

compute.ClusterSpec
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious about this move -- was it needed for this PR to work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, just grouping them together, it's less readable when these structs are separated by custom fields

}

func (s *Cluster) UnmarshalJSON(b []byte) error {
Expand Down
4 changes: 2 additions & 2 deletions bundle/config/resources/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ type DashboardConfig struct {

type Dashboard struct {
BaseResource
Permissions []DashboardPermission `json:"permissions,omitempty"`

DashboardConfig

Permissions []DashboardPermission `json:"permissions,omitempty"`

// FilePath points to the local `.lvdash.json` file containing the dashboard definition.
// This is inlined into serialized_dashboard during deployment. The file_path is kept around
// as metadata which is needed for `databricks bundle generate dashboard --resource <dashboard_key>` to work.
Expand Down
1 change: 0 additions & 1 deletion bundle/config/resources/database_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

type DatabaseCatalog struct {
BaseResource

database.DatabaseCatalog
}

Expand Down
3 changes: 1 addition & 2 deletions bundle/config/resources/database_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ type DatabaseInstancePermission struct {

type DatabaseInstance struct {
BaseResource
database.DatabaseInstance

Permissions []DatabaseInstancePermission `json:"permissions,omitempty"`

database.DatabaseInstance
}

func (d *DatabaseInstance) Exists(ctx context.Context, w *databricks.WorkspaceClient, name string) (bool, error) {
Expand Down
3 changes: 1 addition & 2 deletions bundle/config/resources/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@ type JobPermission struct {

type Job struct {
BaseResource
jobs.JobSettings

Permissions []JobPermission `json:"permissions,omitempty"`

jobs.JobSettings
}

func (j *Job) UnmarshalJSON(b []byte) error {
Expand Down
8 changes: 8 additions & 0 deletions bundle/config/resources/lifecycle.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package resources

// Lifecycle is a struct that contains the lifecycle settings for a resource.
// It controls the behavior of the resource when it is deployed or destroyed.
type Lifecycle struct {
// Lifecycle setting to prevent the resource from being destroyed.
PreventDestroy bool `json:"prevent_destroy,omitempty"`
}
3 changes: 1 addition & 2 deletions bundle/config/resources/mlflow_experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ type MlflowExperimentPermission struct {

type MlflowExperiment struct {
BaseResource
ml.Experiment

Permissions []MlflowExperimentPermission `json:"permissions,omitempty"`

ml.Experiment
}

func (s *MlflowExperiment) UnmarshalJSON(b []byte) error {
Expand Down
Loading
Loading