Skip to content

Commit 8442924

Browse files
authored
Generalize terraform to bundle conversion, prepare for direct deployment (#3134)
## Changes - Introduce ExportedResourcesMap type that is deployment-method neutral. Modify terraform.ParseResourcesState to return this type instead of struct matching terraform state format. - Modify terraform.TerraformToBundle() to rely on this new format. - Make terraform.TerraformToBundle() handle all resources automatically and operate on dynamic bundle structure. - Remove ModifedStatusUpdated constant and usage in apps. - Fix Secret scopes resource to properly set modified_status. Previously it was always "created". ## Why ExportedResourcesMap allows implementing terraform.Load() and thus bundle summary/run/open with direct deployment method (#2926) as it is not tied to terraform resource names or internal state structure. When implementing new resource, we no longer need to add an entry in TerraformToBundle, this both saves time and avoids bugs. ModifiedStatusUpdated is removed because it's not implemented. Only app resource ever sets it and it sets it always, not based on the actual situation. ## Tests summary/modified_status is expanded to handle both cases where resources: block is present and not present (regression during development of this PR).
1 parent 7c9ba16 commit 8442924

17 files changed

Lines changed: 275 additions & 634 deletions

File tree

acceptance/bundle/deploy/secret-scope/output.txt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,11 @@ Deploying resources...
2323
Updating deployment state...
2424
Deployment complete!
2525

26-
=== Print summary after deploy; it should have id and no modified_status. Badness: incorrectly shows modified_status=created
26+
=== Print summary after deploy; it should have id and no modified_status
2727
>>> [CLI] bundle summary --output json
2828
{
2929
"backend_type": "DATABRICKS",
3030
"id": "my-secrets-[UUID]",
31-
"modified_status": "created",
3231
"name": "my-secrets-[UUID]",
3332
"permissions": [
3433
{

acceptance/bundle/deploy/secret-scope/script

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ trap cleanup EXIT
1414
title "Print summary before deploy; it should have modified_status=created and no id"
1515
trace $CLI bundle summary --output json | jq '.resources.secret_scopes.secret_scope1'
1616
trace $CLI bundle deploy
17-
title "Print summary after deploy; it should have id and no modified_status. Badness: incorrectly shows modified_status=created"
17+
title "Print summary after deploy; it should have id and no modified_status"
1818
trace $CLI bundle summary --output json | jq '.resources.secret_scopes.secret_scope1'
1919
trace $CLI secrets list-scopes -o json | jq --arg value ${SECRET_SCOPE_NAME} '.[] | select(.name == $value)'
2020

acceptance/bundle/summary/modified_status/empty.yml renamed to acceptance/bundle/summary/modified_status/empty_resources.yml

File renamed without changes.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
bundle:
2+
name: test-bundle

acceptance/bundle/summary/modified_status/script

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ trace $CLI bundle deploy
55

66
title "Post-deployment view of resources with id and without modified_status"
77
trace $CLI bundle summary -o json | jq .resources
8-
mv empty.yml databricks.yml
8+
mv $VARIANT databricks.yml
99

1010
title "Expecting all resources to have modified_status=deleted"
1111
trace $CLI bundle summary -o json | jq .resources
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # "bundle summary" not implemented for direct deployment
2+
EnvMatrix.VARIANT = ["empty_resources.yml", "no_resources.yml"]

bundle/config/resources/modified_status.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,5 @@ type ModifiedStatus = string
1010

1111
const (
1212
ModifiedStatusCreated ModifiedStatus = "created"
13-
ModifiedStatusUpdated ModifiedStatus = "updated"
1413
ModifiedStatusDeleted ModifiedStatus = "deleted"
1514
)

bundle/deploy/terraform/check_dashboards_modified_remotely.go

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"github.com/databricks/cli/bundle"
88
"github.com/databricks/cli/libs/diag"
99
"github.com/databricks/cli/libs/dyn"
10-
tfjson "github.com/hashicorp/terraform-json"
1110
)
1211

1312
type dashboardState struct {
@@ -23,20 +22,12 @@ func collectDashboardsFromState(ctx context.Context, b *bundle.Bundle) ([]dashbo
2322
}
2423

2524
var dashboards []dashboardState
26-
for _, resource := range state.Resources {
27-
if resource.Mode != tfjson.ManagedResourceMode {
28-
continue
29-
}
30-
for _, instance := range resource.Instances {
31-
switch resource.Type {
32-
case "databricks_dashboard":
33-
dashboards = append(dashboards, dashboardState{
34-
Name: resource.Name,
35-
ID: instance.Attributes.ID,
36-
ETag: instance.Attributes.ETag,
37-
})
38-
}
39-
}
25+
for resourceName, instance := range state["dashboards"] {
26+
dashboards = append(dashboards, dashboardState{
27+
Name: resourceName,
28+
ID: instance.ID,
29+
ETag: instance.ETag,
30+
})
4031
}
4132

4233
return dashboards, nil

bundle/deploy/terraform/check_running_resources.go

Lines changed: 32 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"github.com/databricks/databricks-sdk-go"
1111
"github.com/databricks/databricks-sdk-go/service/jobs"
1212
"github.com/databricks/databricks-sdk-go/service/pipelines"
13-
tfjson "github.com/hashicorp/terraform-json"
1413
"golang.org/x/sync/errgroup"
1514
)
1615

@@ -51,50 +50,45 @@ func CheckRunningResource() *checkRunningResources {
5150
return &checkRunningResources{}
5251
}
5352

54-
func checkAnyResourceRunning(ctx context.Context, w *databricks.WorkspaceClient, state *resourcesState) error {
55-
if state == nil {
56-
return nil
57-
}
58-
53+
func checkAnyResourceRunning(ctx context.Context, w *databricks.WorkspaceClient, state ExportedResourcesMap) error {
5954
errs, errCtx := errgroup.WithContext(ctx)
6055

61-
for _, resource := range state.Resources {
62-
if resource.Mode != tfjson.ManagedResourceMode {
56+
for _, jobAttrs := range state["jobs"] {
57+
id := jobAttrs.ID
58+
if id == "" {
6359
continue
6460
}
65-
for _, instance := range resource.Instances {
66-
id := instance.Attributes.ID
67-
if id == "" {
68-
continue
69-
}
7061

71-
switch resource.Type {
72-
case "databricks_job":
73-
errs.Go(func() error {
74-
isRunning, err := IsJobRunning(errCtx, w, id)
75-
// If there's an error retrieving the job, we assume it's not running
76-
if err != nil {
77-
return err
78-
}
79-
if isRunning {
80-
return &ErrResourceIsRunning{resourceType: "job", resourceId: id}
81-
}
82-
return nil
83-
})
84-
case "databricks_pipeline":
85-
errs.Go(func() error {
86-
isRunning, err := IsPipelineRunning(errCtx, w, id)
87-
// If there's an error retrieving the pipeline, we assume it's not running
88-
if err != nil {
89-
return nil
90-
}
91-
if isRunning {
92-
return &ErrResourceIsRunning{resourceType: "pipeline", resourceId: id}
93-
}
94-
return nil
95-
})
62+
errs.Go(func() error {
63+
isRunning, err := IsJobRunning(errCtx, w, id)
64+
// If there's an error retrieving the job, we assume it's not running
65+
if err != nil {
66+
return err
67+
}
68+
if isRunning {
69+
return &ErrResourceIsRunning{resourceType: "job", resourceId: id}
9670
}
71+
return nil
72+
})
73+
}
74+
75+
for _, pipelineAttrs := range state["pipelines"] {
76+
id := pipelineAttrs.ID
77+
if id == "" {
78+
continue
9779
}
80+
81+
errs.Go(func() error {
82+
isRunning, err := IsPipelineRunning(errCtx, w, id)
83+
// If there's an error retrieving the pipeline, we assume it's not running
84+
if err != nil {
85+
return nil
86+
}
87+
if isRunning {
88+
return &ErrResourceIsRunning{resourceType: "pipeline", resourceId: id}
89+
}
90+
return nil
91+
})
9892
}
9993

10094
return errs.Wait()

bundle/deploy/terraform/check_running_resources_test.go

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,15 @@ import (
1414

1515
func TestIsAnyResourceRunningWithEmptyState(t *testing.T) {
1616
mock := mocks.NewMockWorkspaceClient(t)
17-
err := checkAnyResourceRunning(context.Background(), mock.WorkspaceClient, &resourcesState{})
17+
err := checkAnyResourceRunning(context.Background(), mock.WorkspaceClient, nil)
1818
require.NoError(t, err)
1919
}
2020

2121
func TestIsAnyResourceRunningWithJob(t *testing.T) {
2222
m := mocks.NewMockWorkspaceClient(t)
23-
resources := &resourcesState{
24-
Resources: []stateResource{
25-
{
26-
Type: "databricks_job",
27-
Mode: "managed",
28-
Name: "job1",
29-
Instances: []stateResourceInstance{
30-
{Attributes: stateInstanceAttributes{ID: "123"}},
31-
},
32-
},
23+
resources := ExportedResourcesMap{
24+
"jobs": map[string]ResourceState{
25+
"job1": {ID: "123"},
3326
},
3427
}
3528

@@ -55,16 +48,9 @@ func TestIsAnyResourceRunningWithJob(t *testing.T) {
5548

5649
func TestIsAnyResourceRunningWithPipeline(t *testing.T) {
5750
m := mocks.NewMockWorkspaceClient(t)
58-
resources := &resourcesState{
59-
Resources: []stateResource{
60-
{
61-
Type: "databricks_pipeline",
62-
Mode: "managed",
63-
Name: "pipeline1",
64-
Instances: []stateResourceInstance{
65-
{Attributes: stateInstanceAttributes{ID: "123"}},
66-
},
67-
},
51+
resources := ExportedResourcesMap{
52+
"pipelines": map[string]ResourceState{
53+
"pipeline1": {ID: "123"},
6854
},
6955
}
7056

@@ -91,16 +77,10 @@ func TestIsAnyResourceRunningWithPipeline(t *testing.T) {
9177

9278
func TestIsAnyResourceRunningWithAPIFailure(t *testing.T) {
9379
m := mocks.NewMockWorkspaceClient(t)
94-
resources := &resourcesState{
95-
Resources: []stateResource{
96-
{
97-
Type: "databricks_pipeline",
98-
Mode: "managed",
99-
Name: "pipeline1",
100-
Instances: []stateResourceInstance{
101-
{Attributes: stateInstanceAttributes{ID: "123"}},
102-
},
103-
},
80+
81+
resources := ExportedResourcesMap{
82+
"pipelines": map[string]ResourceState{
83+
"pipeline1": {ID: "123"},
10484
},
10585
}
10686

0 commit comments

Comments
 (0)