Skip to content

Commit 89c5e1b

Browse files
authored
internal: remove DirectDeployment bundle attribute (#3848)
## Changes Instead of having global *bool for directDeployment, make it explicit parameter that is passed around. ## Why Enables migration: we need first run deploy on terraform then in direct, having parameter helps. Prevents panic, make it explicit which parts depend on engine. ## Tests Existing tests.
1 parent 887c12f commit 89c5e1b

File tree

19 files changed

+119
-135
lines changed

19 files changed

+119
-135
lines changed

bundle/bundle.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,6 @@ type Bundle struct {
141141
Tagging tags.Cloud
142142

143143
Metrics Metrics
144-
145-
// If true, don't use terraform. Set by DATABRICKS_BUNDLE_ENGINE=direct
146-
DirectDeployment *bool
147144
}
148145

149146
func Load(ctx context.Context, path string) (*Bundle, error) {

bundle/deploy/terraform/check_dashboards_modified_remotely.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ func collectDashboardsFromState(ctx context.Context, b *bundle.Bundle) ([]dashbo
3434
}
3535

3636
type checkDashboardsModifiedRemotely struct {
37-
isPlan bool
37+
isPlan bool
38+
directDeployment bool
3839
}
3940

4041
func (l *checkDashboardsModifiedRemotely) Name() string {
@@ -47,7 +48,7 @@ func (l *checkDashboardsModifiedRemotely) Apply(ctx context.Context, b *bundle.B
4748
return nil
4849
}
4950

50-
if *b.DirectDeployment {
51+
if l.directDeployment {
5152
// TODO: not implemented yet
5253
return nil
5354
}
@@ -115,6 +116,6 @@ func (l *checkDashboardsModifiedRemotely) Apply(ctx context.Context, b *bundle.B
115116
return diags
116117
}
117118

118-
func CheckDashboardsModifiedRemotely(isPlan bool) *checkDashboardsModifiedRemotely {
119-
return &checkDashboardsModifiedRemotely{isPlan: isPlan}
119+
func CheckDashboardsModifiedRemotely(isPlan, directDeployment bool) *checkDashboardsModifiedRemotely {
120+
return &checkDashboardsModifiedRemotely{isPlan: isPlan, directDeployment: directDeployment}
120121
}

bundle/deploy/terraform/check_dashboards_modified_remotely_test.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,9 @@ import (
1717
)
1818

1919
func mockDashboardBundle(t *testing.T) *bundle.Bundle {
20-
falseBool := false
2120
dir := t.TempDir()
2221
b := &bundle.Bundle{
23-
DirectDeployment: &falseBool,
24-
BundleRootPath: dir,
22+
BundleRootPath: dir,
2523
Config: config.Root{
2624
Bundle: config.Bundle{
2725
Target: "test",
@@ -54,13 +52,13 @@ func TestCheckDashboardsModifiedRemotely_NoDashboards(t *testing.T) {
5452
},
5553
}
5654

57-
diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely(false))
55+
diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely(false, false))
5856
assert.Empty(t, diags)
5957
}
6058

6159
func TestCheckDashboardsModifiedRemotely_FirstDeployment(t *testing.T) {
6260
b := mockDashboardBundle(t)
63-
diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely(false))
61+
diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely(false, false))
6462
assert.Empty(t, diags)
6563
}
6664

@@ -83,7 +81,7 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateNoChange(t *testing.T) {
8381
b.SetWorkpaceClient(m.WorkspaceClient)
8482

8583
// No changes, so no diags.
86-
diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(false))
84+
diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(false, false))
8785
assert.Empty(t, diags)
8886
}
8987

@@ -106,7 +104,7 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateChange(t *testing.T) {
106104
b.SetWorkpaceClient(m.WorkspaceClient)
107105

108106
// The dashboard has changed, so expect an error.
109-
diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(false))
107+
diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(false, false))
110108
if assert.Len(t, diags, 1) {
111109
assert.Equal(t, diag.Error, diags[0].Severity)
112110
assert.Equal(t, `dashboard "dash1" has been modified remotely`, diags[0].Summary)
@@ -129,7 +127,7 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateFailureToGet(t *testing.T)
129127
b.SetWorkpaceClient(m.WorkspaceClient)
130128

131129
// Unable to get the dashboard, so expect an error.
132-
diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(false))
130+
diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(false, false))
133131
if assert.Len(t, diags, 1) {
134132
assert.Equal(t, diag.Error, diags[0].Severity)
135133
assert.Equal(t, `failed to get dashboard "dash1"`, diags[0].Summary)
@@ -155,7 +153,7 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateChangePlanMode(t *testing.
155153
b.SetWorkpaceClient(m.WorkspaceClient)
156154

157155
// The dashboard has changed, but in plan mode expect a warning instead of an error.
158-
diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(true))
156+
diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(true, false))
159157
if assert.Len(t, diags, 1) {
160158
assert.Equal(t, diag.Warning, diags[0].Severity)
161159
assert.Equal(t, `dashboard "dash1" has been modified remotely`, diags[0].Summary)

bundle/deploy/terraform/util_test.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,8 @@ import (
1212
)
1313

1414
func TestParseResourcesStateWithNoFile(t *testing.T) {
15-
falseBool := false
1615
b := &bundle.Bundle{
17-
DirectDeployment: &falseBool,
18-
BundleRootPath: t.TempDir(),
16+
BundleRootPath: t.TempDir(),
1917
Config: config.Root{
2018
Bundle: config.Bundle{
2119
Target: "whatever",
@@ -31,11 +29,9 @@ func TestParseResourcesStateWithNoFile(t *testing.T) {
3129
}
3230

3331
func TestParseResourcesStateWithExistingStateFile(t *testing.T) {
34-
falseBool := false
3532
ctx := context.Background()
3633
b := &bundle.Bundle{
37-
DirectDeployment: &falseBool,
38-
BundleRootPath: t.TempDir(),
34+
BundleRootPath: t.TempDir(),
3935
Config: config.Root{
4036
Bundle: config.Bundle{
4137
Target: "whatever",

bundle/phases/bind.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func Bind(ctx context.Context, b *bundle.Bundle, opts *terraform.BindOptions) {
2727
terraform.Interpolate(),
2828
terraform.Write(),
2929
terraform.Import(opts),
30-
statemgmt.StatePush(),
30+
statemgmt.StatePush(false),
3131
)
3232
}
3333

@@ -47,6 +47,6 @@ func Unbind(ctx context.Context, b *bundle.Bundle, bundleType, tfResourceType, r
4747
terraform.Interpolate(),
4848
terraform.Write(),
4949
terraform.Unbind(bundleType, tfResourceType, resourceKey),
50-
statemgmt.StatePush(),
50+
statemgmt.StatePush(false),
5151
)
5252
}

bundle/phases/deploy.go

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -92,27 +92,27 @@ func approvalForDeploy(ctx context.Context, b *bundle.Bundle, plan *deployplan.P
9292
return approved, nil
9393
}
9494

95-
func deployCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan) {
95+
func deployCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, directDeployment bool) {
9696
// Core mutators that CRUD resources and modify deployment state. These
9797
// mutators need informed consent if they are potentially destructive.
9898
cmdio.LogString(ctx, "Deploying resources...")
9999

100-
if *b.DirectDeployment {
100+
if directDeployment {
101101
b.DeploymentBundle.Apply(ctx, b.WorkspaceClient(), &b.Config, plan)
102102
} else {
103103
bundle.ApplyContext(ctx, b, terraform.Apply())
104104
}
105105

106106
// Even if deployment failed, there might be updates in states that we need to upload
107107
bundle.ApplyContext(ctx, b,
108-
statemgmt.StatePush(),
108+
statemgmt.StatePush(directDeployment),
109109
)
110110
if logdiag.HasError(ctx) {
111111
return
112112
}
113113

114114
bundle.ApplySeqContext(ctx, b,
115-
statemgmt.Load(),
115+
statemgmt.Load(directDeployment),
116116
metadata.Compute(),
117117
metadata.Upload(),
118118
)
@@ -133,7 +133,7 @@ func uploadLibraries(ctx context.Context, b *bundle.Bundle, libs map[string][]li
133133
}
134134

135135
// The deploy phase deploys artifacts and resources.
136-
func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHandler) {
136+
func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHandler, directDeployment bool) {
137137
log.Info(ctx, "Phase: deploy")
138138

139139
// Core mutators that CRUD resources and modify deployment state. These
@@ -153,7 +153,7 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand
153153
bundle.ApplyContext(ctx, b, lock.Release(lock.GoalDeploy))
154154
}()
155155

156-
libs := deployPrepare(ctx, b, false)
156+
libs := deployPrepare(ctx, b, false, directDeployment)
157157
if logdiag.HasError(ctx) {
158158
return
159159
}
@@ -176,7 +176,7 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand
176176
return
177177
}
178178

179-
plan := planWithoutPrepare(ctx, b)
179+
plan := planWithoutPrepare(ctx, b, directDeployment)
180180
if logdiag.HasError(ctx) {
181181
return
182182
}
@@ -187,7 +187,7 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand
187187
return
188188
}
189189
if haveApproval {
190-
deployCore(ctx, b, plan)
190+
deployCore(ctx, b, plan, directDeployment)
191191
} else {
192192
cmdio.LogString(ctx, "Deployment cancelled!")
193193
return
@@ -203,8 +203,8 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand
203203

204204
// planWithoutPrepare builds a deployment plan without running deployPrepare.
205205
// This is used when deployPrepare has already been called.
206-
func planWithoutPrepare(ctx context.Context, b *bundle.Bundle) *deployplan.Plan {
207-
if *b.DirectDeployment {
206+
func planWithoutPrepare(ctx context.Context, b *bundle.Bundle, directDeployment bool) *deployplan.Plan {
207+
if directDeployment {
208208
_, localPath := b.StateFilenameDirect(ctx)
209209
plan, err := b.DeploymentBundle.CalculatePlan(ctx, b.WorkspaceClient(), &b.Config, localPath)
210210
if err != nil {
@@ -236,16 +236,27 @@ func planWithoutPrepare(ctx context.Context, b *bundle.Bundle) *deployplan.Plan
236236
return nil
237237
}
238238

239+
for _, group := range b.Config.Resources.AllResources() {
240+
for rKey := range group.Resources {
241+
resourceKey := "resources." + group.Description.PluralName + "." + rKey
242+
if _, ok := plan.Plan[resourceKey]; !ok {
243+
plan.Plan[resourceKey] = &deployplan.PlanEntry{
244+
Action: deployplan.ActionTypeSkip.String(),
245+
}
246+
}
247+
}
248+
}
249+
239250
return plan
240251
}
241252

242-
func Plan(ctx context.Context, b *bundle.Bundle) *deployplan.Plan {
243-
deployPrepare(ctx, b, true)
253+
func Plan(ctx context.Context, b *bundle.Bundle, directDeployment bool) *deployplan.Plan {
254+
deployPrepare(ctx, b, true, directDeployment)
244255
if logdiag.HasError(ctx) {
245256
return nil
246257
}
247258

248-
return planWithoutPrepare(ctx, b)
259+
return planWithoutPrepare(ctx, b, directDeployment)
249260
}
250261

251262
// If there are more than 1 thousand of a resource type, do not

bundle/phases/destroy.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ func approvalForDestroy(ctx context.Context, b *bundle.Bundle, plan *deployplan.
8989
return approved, nil
9090
}
9191

92-
func destroyCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan) {
93-
if *b.DirectDeployment {
92+
func destroyCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, directDeployment bool) {
93+
if directDeployment {
9494
b.DeploymentBundle.Apply(ctx, b.WorkspaceClient(), &b.Config, plan)
9595
} else {
9696
// Core destructive mutators for destroy. These require informed user consent.
@@ -109,7 +109,7 @@ func destroyCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan) {
109109
}
110110

111111
// The destroy phase deletes artifacts and resources.
112-
func Destroy(ctx context.Context, b *bundle.Bundle) {
112+
func Destroy(ctx context.Context, b *bundle.Bundle, directDeployment bool) {
113113
log.Info(ctx, "Phase: destroy")
114114

115115
ok, err := assertRootPathExists(ctx, b)
@@ -132,7 +132,7 @@ func Destroy(ctx context.Context, b *bundle.Bundle) {
132132
bundle.ApplyContext(ctx, b, lock.Release(lock.GoalDestroy))
133133
}()
134134

135-
if !*b.DirectDeployment {
135+
if !directDeployment {
136136
bundle.ApplySeqContext(ctx, b,
137137
// We need to resolve artifact variable (how we do it in build phase)
138138
// because some of the to-be-destroyed resource might use this variable.
@@ -151,7 +151,7 @@ func Destroy(ctx context.Context, b *bundle.Bundle) {
151151
}
152152

153153
var plan *deployplan.Plan
154-
if *b.DirectDeployment {
154+
if directDeployment {
155155
_, localPath := b.StateFilenameDirect(ctx)
156156
plan, err = b.DeploymentBundle.CalculatePlan(ctx, b.WorkspaceClient(), nil, localPath)
157157
if err != nil {
@@ -179,7 +179,7 @@ func Destroy(ctx context.Context, b *bundle.Bundle) {
179179
}
180180

181181
if hasApproval {
182-
destroyCore(ctx, b, plan)
182+
destroyCore(ctx, b, plan, directDeployment)
183183
} else {
184184
cmdio.LogString(ctx, "Destroy cancelled!")
185185
}

bundle/phases/plan.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ import (
1818

1919
// deployPrepare is common set of mutators between "bundle plan" and "bundle deploy".
2020
// This function does not make any mutations in the workspace remotely, only in-memory bundle config mutations
21-
func deployPrepare(ctx context.Context, b *bundle.Bundle, isPlan bool) map[string][]libraries.LocationToUpdate {
21+
func deployPrepare(ctx context.Context, b *bundle.Bundle, isPlan, directDeployment bool) map[string][]libraries.LocationToUpdate {
2222
bundle.ApplySeqContext(ctx, b,
23-
terraform.CheckDashboardsModifiedRemotely(isPlan),
23+
terraform.CheckDashboardsModifiedRemotely(isPlan, directDeployment),
2424
deploy.StatePull(),
2525
mutator.ValidateGitDetails(),
2626
terraform.CheckRunningResource(),

bundle/statemgmt/state_load.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ type (
2424
const ErrorOnEmptyState LoadMode = 0
2525

2626
type load struct {
27-
modes []LoadMode
27+
modes []LoadMode
28+
directDeployment bool
2829
}
2930

3031
func (l *load) Name() string {
@@ -35,11 +36,7 @@ func (l *load) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
3536
var err error
3637
var state ExportedResourcesMap
3738

38-
if b.DirectDeployment == nil {
39-
return diag.Errorf("internal error: statemgmt.Load() called without statemgmt.PullResourcesState()")
40-
}
41-
42-
if *b.DirectDeployment {
39+
if l.directDeployment {
4340
_, fullPathDirect := b.StateFilenameDirect(ctx)
4441
state, err = b.DeploymentBundle.ExportState(ctx, fullPathDirect)
4542
if err != nil {
@@ -137,6 +134,6 @@ func (l *load) validateState(state ExportedResourcesMap) error {
137134
return nil
138135
}
139136

140-
func Load(modes ...LoadMode) bundle.Mutator {
141-
return &load{modes: modes}
137+
func Load(directDeployment bool, modes ...LoadMode) bundle.Mutator {
138+
return &load{modes: modes, directDeployment: directDeployment}
142139
}

0 commit comments

Comments
 (0)