Skip to content

Commit c8cc74e

Browse files
denikdenik
authored
internal: decouple terranova from bundle (#3513)
## Changes - Refactor terranova not to depend on bundle. - Add new terranova.BundleDeployer that is responsible for all resources and embed it in Bundle struct. - No functional changes, but a bunch of code moved: from apply.go to bundle_apply.go and from plan.go to bundle_plan.go. ## Why This struct can carry state from planning to deploy phases, this is important to keep fresh remote state. Better logical grouping - graph, actions, state are all related and should be kept together. ## Tests Existing tests.
1 parent b2c1182 commit c8cc74e

File tree

10 files changed

+356
-353
lines changed

10 files changed

+356
-353
lines changed

‎bundle/bundle.go‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,10 @@ import (
1515
"sync"
1616

1717
"github.com/databricks/cli/bundle/config"
18-
"github.com/databricks/cli/bundle/deployplan"
1918
"github.com/databricks/cli/bundle/env"
2019
"github.com/databricks/cli/bundle/metadata"
21-
"github.com/databricks/cli/bundle/terranova/tnstate"
20+
"github.com/databricks/cli/bundle/terranova"
2221
"github.com/databricks/cli/libs/auth"
23-
"github.com/databricks/cli/libs/dagrun"
2422
"github.com/databricks/cli/libs/fileset"
2523
"github.com/databricks/cli/libs/locker"
2624
"github.com/databricks/cli/libs/log"
@@ -132,10 +130,7 @@ type Bundle struct {
132130
TerraformPlanIsEmpty bool
133131

134132
// (direct only) graph of dependencies between resources
135-
Graph *dagrun.Graph[deployplan.ResourceNode]
136-
137-
// (direct only) planned action for each resource
138-
PlannedActions map[deployplan.ResourceNode]deployplan.ActionType
133+
BundleDeployer terranova.BundleDeployer
139134

140135
// if true, we skip approval checks for deploy, destroy resources and delete
141136
// files
@@ -149,9 +144,6 @@ type Bundle struct {
149144

150145
// If true, don't use terraform. Set by DATABRICKS_CLI_DEPLOYMENT=direct
151146
DirectDeployment bool
152-
153-
// State file access for direct deployment (only initialized if DirectDeployment = true)
154-
ResourceDatabase tnstate.TerranovaState
155147
}
156148

157149
func Load(ctx context.Context, path string) (*Bundle, error) {
@@ -349,7 +341,7 @@ func (b *Bundle) StateLocalPath(ctx context.Context) (string, error) {
349341
}
350342
}
351343

352-
func (b *Bundle) OpenResourceDatabase(ctx context.Context) error {
344+
func (b *Bundle) OpenStateFile(ctx context.Context) error {
353345
if !b.DirectDeployment {
354346
panic("internal error: OpenResourceDatabase must be called with DirectDeployment")
355347
}
@@ -359,9 +351,9 @@ func (b *Bundle) OpenResourceDatabase(ctx context.Context) error {
359351
return err
360352
}
361353

362-
err = b.ResourceDatabase.Open(statePath)
354+
err = b.BundleDeployer.OpenStateFile(statePath)
363355
if err != nil {
364-
return fmt.Errorf("failed to open/create resoruce database in %s: %s", statePath, err)
356+
return fmt.Errorf("failed to open/create state file at %s: %s", statePath, err)
365357
}
366358

367359
return nil

‎bundle/phases/deploy.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"github.com/databricks/cli/bundle/permissions"
2121
"github.com/databricks/cli/bundle/scripts"
2222
"github.com/databricks/cli/bundle/statemgmt"
23-
"github.com/databricks/cli/bundle/terranova"
2423
"github.com/databricks/cli/bundle/trampoline"
2524
"github.com/databricks/cli/libs/cmdio"
2625
"github.com/databricks/cli/libs/log"
@@ -30,11 +29,15 @@ import (
3029

3130
func getActions(ctx context.Context, b *bundle.Bundle) ([]deployplan.Action, error) {
3231
if b.DirectDeployment {
33-
err := terranova.CalculatePlanForDeploy(ctx, b)
32+
err := b.OpenStateFile(ctx)
3433
if err != nil {
3534
return nil, err
3635
}
37-
return terranova.GetDeployActions(ctx, b), nil
36+
err = b.BundleDeployer.CalculatePlanForDeploy(ctx, b.WorkspaceClient(), &b.Config)
37+
if err != nil {
38+
return nil, err
39+
}
40+
return b.BundleDeployer.GetActions(ctx), nil
3841
} else {
3942
tf := b.Terraform
4043
if tf == nil {
@@ -117,7 +120,7 @@ func deployCore(ctx context.Context, b *bundle.Bundle) {
117120
cmdio.LogString(ctx, "Deploying resources...")
118121

119122
if b.DirectDeployment {
120-
bundle.ApplyContext(ctx, b, terranova.TerranovaApply())
123+
b.BundleDeployer.Apply(ctx, b.WorkspaceClient(), &b.Config)
121124
} else {
122125
bundle.ApplyContext(ctx, b, terraform.Apply())
123126
}
@@ -132,8 +135,13 @@ func deployCore(ctx context.Context, b *bundle.Bundle) {
132135

133136
bundle.ApplySeqContext(ctx, b,
134137
statemgmt.Load(),
138+
139+
// TODO: this does terraform specific transformation.
135140
apps.InterpolateVariables(),
141+
142+
// TODO: this should either be part of app resource or separate AppConfig resource that depends on main resource.
136143
apps.UploadConfig(),
144+
137145
metadata.Compute(),
138146
metadata.Upload(),
139147
)

‎bundle/phases/destroy.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"github.com/databricks/cli/bundle/deploy/terraform"
1212
"github.com/databricks/cli/bundle/deployplan"
1313
"github.com/databricks/cli/bundle/statemgmt"
14-
"github.com/databricks/cli/bundle/terranova"
1514
"github.com/databricks/cli/libs/cmdio"
1615
"github.com/databricks/cli/libs/log"
1716
"github.com/databricks/cli/libs/logdiag"
@@ -33,11 +32,15 @@ func assertRootPathExists(ctx context.Context, b *bundle.Bundle) (bool, error) {
3332

3433
func getDeleteActions(ctx context.Context, b *bundle.Bundle) ([]deployplan.Action, error) {
3534
if b.DirectDeployment {
36-
err := terranova.CalculatePlanForDestroy(ctx, b)
35+
err := b.OpenStateFile(ctx)
3736
if err != nil {
3837
return nil, err
3938
}
40-
return terranova.GetDeployActions(ctx, b), nil
39+
err = b.BundleDeployer.CalculatePlanForDestroy(ctx)
40+
if err != nil {
41+
return nil, err
42+
}
43+
return b.BundleDeployer.GetActions(ctx), nil
4144
}
4245

4346
tf := b.Terraform
@@ -116,7 +119,7 @@ func approvalForDestroy(ctx context.Context, b *bundle.Bundle) (bool, error) {
116119

117120
func destroyCore(ctx context.Context, b *bundle.Bundle) {
118121
if b.DirectDeployment {
119-
bundle.ApplyContext(ctx, b, terranova.TerranovaApply())
122+
b.BundleDeployer.Apply(ctx, b.WorkspaceClient(), &b.Config)
120123
} else {
121124
// Core destructive mutators for destroy. These require informed user consent.
122125
bundle.ApplyContext(ctx, b, terraform.Apply())

‎bundle/statemgmt/state_load.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,32 +34,31 @@ func (l *load) Name() string {
3434

3535
func (l *load) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
3636
var state ExportedResourcesMap
37-
var err error
3837

3938
if b.DirectDeployment {
40-
err = b.OpenResourceDatabase(ctx)
39+
err := b.OpenStateFile(ctx)
4140
if err != nil {
4241
return diag.FromErr(err)
4342
}
44-
state = b.ResourceDatabase.ExportState(ctx)
43+
state = b.BundleDeployer.StateDB.ExportState(ctx)
4544
} else {
4645
tf := b.Terraform
4746
if tf == nil {
4847
return diag.Errorf("terraform not initialized")
4948
}
5049

51-
err = tf.Init(ctx, tfexec.Upgrade(true))
50+
err := tf.Init(ctx, tfexec.Upgrade(true))
5251
if err != nil {
5352
return diag.Errorf("terraform init: %v", err)
5453
}
5554

5655
state, err = terraform.ParseResourcesState(ctx, b)
57-
}
58-
if err != nil {
59-
return diag.FromErr(err)
56+
if err != nil {
57+
return diag.FromErr(err)
58+
}
6059
}
6160

62-
err = l.validateState(state)
61+
err := l.validateState(state)
6362
if err != nil {
6463
return diag.FromErr(err)
6564
}

‎bundle/terranova/apply.go‎

Lines changed: 0 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -8,134 +8,12 @@ import (
88
"fmt"
99
"reflect"
1010

11-
"github.com/databricks/cli/bundle"
1211
"github.com/databricks/cli/bundle/deployplan"
1312
"github.com/databricks/cli/bundle/terranova/tnstate"
14-
"github.com/databricks/cli/libs/diag"
1513
"github.com/databricks/cli/libs/log"
16-
"github.com/databricks/cli/libs/logdiag"
1714
"github.com/databricks/databricks-sdk-go"
1815
)
1916

20-
// How many parallel operations (API calls) are allowed
21-
const defaultParallelism = 10
22-
23-
type terranovaApplyMutator struct{}
24-
25-
func TerranovaApply() bundle.Mutator {
26-
return &terranovaApplyMutator{}
27-
}
28-
29-
func (m *terranovaApplyMutator) Name() string {
30-
return "TerranovaDeploy"
31-
}
32-
33-
func (m *terranovaApplyMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
34-
if b.Graph == nil {
35-
panic("Planning is not done")
36-
}
37-
38-
if len(b.PlannedActions) == 0 {
39-
// Avoid creating state file if nothing to deploy
40-
return nil
41-
}
42-
43-
if logdiag.HasError(ctx) {
44-
return nil
45-
}
46-
47-
client := b.WorkspaceClient()
48-
49-
b.Graph.Run(defaultParallelism, func(node deployplan.ResourceNode, failedDependency *deployplan.ResourceNode) bool {
50-
actionType := b.PlannedActions[node]
51-
52-
errorPrefix := fmt.Sprintf("cannot %s %s.%s", actionType.String(), node.Group, node.Key)
53-
54-
// If a dependency failed, report and skip execution for this node by returning false
55-
if failedDependency != nil {
56-
logdiag.LogError(ctx, fmt.Errorf("%s: dependency failed: %s", errorPrefix, failedDependency.String()))
57-
return false
58-
}
59-
60-
settings, ok := SupportedResources[node.Group]
61-
if !ok {
62-
// Unexpected, this should be filtered at plan.
63-
return false
64-
}
65-
66-
// The way plan currently works, is that it does not add resources with Noop action, turning them into Unset.
67-
// So we skip both, although at this point we will not see Noop here.
68-
if actionType == deployplan.ActionTypeUnset || actionType == deployplan.ActionTypeNoop {
69-
return true
70-
}
71-
72-
d := Deployer{
73-
client: client,
74-
db: &b.ResourceDatabase,
75-
group: node.Group,
76-
resourceName: node.Key,
77-
settings: settings,
78-
}
79-
80-
if actionType == deployplan.ActionTypeDelete {
81-
err := d.Destroy(ctx)
82-
if err != nil {
83-
logdiag.LogError(ctx, fmt.Errorf("%s: %w", errorPrefix, err))
84-
return false
85-
}
86-
return true
87-
}
88-
89-
// Fetch the references to ensure all are resolved
90-
myReferences, err := extractReferences(b.Config.Value(), node)
91-
if err != nil {
92-
logdiag.LogError(ctx, fmt.Errorf("%s: reading references from config: %w", errorPrefix, err))
93-
return false
94-
}
95-
96-
// At this point it's an error to have unresolved deps
97-
if len(myReferences) > 0 {
98-
// TODO: include the deps themselves in the message
99-
logdiag.LogError(ctx, fmt.Errorf("%s: unresolved deps", errorPrefix))
100-
return false
101-
}
102-
103-
config, ok := b.Config.GetResourceConfig(node.Group, node.Key)
104-
if !ok {
105-
logdiag.LogError(ctx, fmt.Errorf("%s: internal error when reading config", errorPrefix))
106-
return false
107-
}
108-
109-
// TODO: redo calcDiff to downgrade planned action if possible (?)
110-
111-
err = d.Deploy(ctx, config, actionType)
112-
if err != nil {
113-
logdiag.LogError(ctx, fmt.Errorf("%s: %w", errorPrefix, err))
114-
return false
115-
}
116-
117-
// Update resources.id after successful deploy so that future ${resources...id} refs are replaced
118-
if b.Graph.HasOutgoingEdges(node) {
119-
err = resolveIDReference(ctx, b, node.Group, node.Key)
120-
if err != nil {
121-
// not using errorPrefix because resource was deployed
122-
logdiag.LogError(ctx, fmt.Errorf("failed to replace ref to resources.%s.%s.id: %w", node.Group, node.Key, err))
123-
return false
124-
}
125-
}
126-
127-
return true
128-
})
129-
130-
// This must run even if deploy failed:
131-
err := b.ResourceDatabase.Finalize()
132-
if err != nil {
133-
logdiag.LogError(ctx, err)
134-
}
135-
136-
return nil
137-
}
138-
13917
type Deployer struct {
14018
client *databricks.WorkspaceClient
14119
db *tnstate.TerranovaState

0 commit comments

Comments
 (0)