Skip to content

Commit 8756548

Browse files
shreyas-goenkashreyas-goenka
committed
Use string enums, report failed operations, and refactor lock acquisition
- Change all enum types from int to string using proto enum name strings (e.g. "OPERATION_ACTION_TYPE_CREATE" instead of 4), matching proto-over-HTTP serialization format. - Report failed operations to the metadata service with error messages, not just successful ones. - Enforce direct deployment engine for managed state (early return). - Extract acquireMetadataLock helper to deduplicate deploy/destroy lock blocks. - Add deploy-error acceptance test verifying failed operation reporting. Co-authored-by: Isaac
1 parent b1a9a0a commit 8756548

File tree

12 files changed

+213
-109
lines changed

12 files changed

+213
-109
lines changed

‎acceptance/bundle/deploy/metadata-service/deploy-error/databricks.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
bundle:
2+
name: metadata-service-error-test
3+
4+
resources:
5+
jobs:
6+
test_job:
7+
name: test-job

‎acceptance/bundle/deploy/metadata-service/deploy-error/out.test.toml‎

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎acceptance/bundle/deploy/metadata-service/deploy-error/output.txt‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
2+
>>> musterr [CLI] bundle deploy
3+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/metadata-service-error-test/default/files...
4+
Deploying resources...
5+
Error: cannot create resources.jobs.test_job: Invalid job configuration. (400 INVALID_PARAMETER_VALUE)
6+
7+
Endpoint: POST [DATABRICKS_URL]/api/2.2/jobs/create
8+
HTTP Status: 400 Bad Request
9+
API error_code: INVALID_PARAMETER_VALUE
10+
API message: Invalid job configuration.
11+
12+
Updating deployment state...
13+
14+
>>> print_requests.py --get //bundle
15+
{
16+
"method": "POST",
17+
"path": "/api/2.0/bundle/deployments",
18+
"q": {
19+
"deployment_id": "[UUID]"
20+
},
21+
"body": {
22+
"target_name": "default"
23+
}
24+
}
25+
{
26+
"method": "GET",
27+
"path": "/api/2.0/bundle/deployments/[UUID]"
28+
}
29+
{
30+
"method": "POST",
31+
"path": "/api/2.0/bundle/deployments/[UUID]/versions",
32+
"q": {
33+
"version_id": "1"
34+
},
35+
"body": {
36+
"cli_version": "[DEV_VERSION]",
37+
"version_type": "VERSION_TYPE_DEPLOY",
38+
"target_name": "default"
39+
}
40+
}
41+
{
42+
"method": "POST",
43+
"path": "/api/2.0/bundle/deployments/[UUID]/versions/1/operations",
44+
"q": {
45+
"resource_key": "resources.jobs.test_job"
46+
},
47+
"body": {
48+
"resource_key": "resources.jobs.test_job",
49+
"action_type": "OPERATION_ACTION_TYPE_CREATE",
50+
"status": "OPERATION_STATUS_FAILED",
51+
"error_message": "Invalid job configuration."
52+
}
53+
}
54+
{
55+
"method": "POST",
56+
"path": "/api/2.0/bundle/deployments/[UUID]/versions/1/complete",
57+
"body": {
58+
"name": "deployments/[UUID]/versions/1",
59+
"completion_reason": "VERSION_COMPLETE_FAILURE"
60+
}
61+
}

‎acceptance/bundle/deploy/metadata-service/deploy-error/script‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Deploy with the metadata service enabled, expecting a resource creation failure.
2+
trace musterr $CLI bundle deploy
3+
4+
# Print the metadata service requests to verify the failed operation is reported.
5+
trace print_requests.py --get //bundle

‎acceptance/bundle/deploy/metadata-service/deploy-error/test.toml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]
2+
EnvMatrix.DATABRICKS_BUNDLE_MANAGED_STATE = ["true"]
3+
RecordRequests = true
4+
5+
[[Server]]
6+
Pattern = "POST /api/2.2/jobs/create"
7+
Response.StatusCode = 400
8+
Response.Body = '{"error_code": "INVALID_PARAMETER_VALUE", "message": "Invalid job configuration."}'

‎acceptance/bundle/deploy/metadata-service/output.txt‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ Deployment complete!
2828
},
2929
"body": {
3030
"cli_version": "[DEV_VERSION]",
31-
"version_type": 1,
31+
"version_type": "VERSION_TYPE_DEPLOY",
3232
"target_name": "default"
3333
}
3434
}
@@ -40,17 +40,17 @@ Deployment complete!
4040
},
4141
"body": {
4242
"resource_key": "resources.jobs.test_job",
43-
"action_type": 4,
43+
"action_type": "OPERATION_ACTION_TYPE_CREATE",
4444
"resource_id": "[NUMID]",
45-
"status": 1
45+
"status": "OPERATION_STATUS_SUCCEEDED"
4646
}
4747
}
4848
{
4949
"method": "POST",
5050
"path": "/api/2.0/bundle/deployments/[UUID]/versions/1/complete",
5151
"body": {
5252
"name": "deployments/[UUID]/versions/1",
53-
"completion_reason": 1
53+
"completion_reason": "VERSION_COMPLETE_SUCCESS"
5454
}
5555
}
5656

@@ -86,7 +86,7 @@ Destroy complete!
8686
},
8787
"body": {
8888
"cli_version": "[DEV_VERSION]",
89-
"version_type": 2,
89+
"version_type": "VERSION_TYPE_DESTROY",
9090
"target_name": "default"
9191
}
9292
}
@@ -98,16 +98,16 @@ Destroy complete!
9898
},
9999
"body": {
100100
"resource_key": "resources.jobs.test_job",
101-
"action_type": 6,
101+
"action_type": "OPERATION_ACTION_TYPE_DELETE",
102102
"resource_id": "[NUMID]",
103-
"status": 1
103+
"status": "OPERATION_STATUS_SUCCEEDED"
104104
}
105105
}
106106
{
107107
"method": "POST",
108108
"path": "/api/2.0/bundle/deployments/[UUID]/versions/1/complete",
109109
"body": {
110110
"name": "deployments/[UUID]/versions/1",
111-
"completion_reason": 1
111+
"completion_reason": "VERSION_COMPLETE_SUCCESS"
112112
}
113113
}

‎bundle/direct/bundle_apply.go‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,13 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
9494
}
9595

9696
err = d.Destroy(ctx, &b.StateDB)
97+
if b.OperationReporter != nil {
98+
b.OperationReporter(ctx, resourceKey, deleteResourceID, action, err)
99+
}
97100
if err != nil {
98101
logdiag.LogError(ctx, fmt.Errorf("%s: %w", errorPrefix, err))
99102
return false
100103
}
101-
102-
if b.OperationReporter != nil {
103-
b.OperationReporter(ctx, resourceKey, deleteResourceID, action)
104-
}
105104
return true
106105
}
107106

@@ -137,16 +136,19 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
137136
err = d.Deploy(ctx, &b.StateDB, sv.Value, action, entry)
138137
}
139138

139+
// Report the operation inline to the metadata service.
140+
if b.OperationReporter != nil && !migrateMode {
141+
var resourceID string
142+
if dbentry, ok := b.StateDB.GetResourceEntry(resourceKey); ok {
143+
resourceID = dbentry.ID
144+
}
145+
b.OperationReporter(ctx, resourceKey, resourceID, action, err)
146+
}
147+
140148
if err != nil {
141149
logdiag.LogError(ctx, fmt.Errorf("%s: %w", errorPrefix, err))
142150
return false
143151
}
144-
145-
// Report the operation inline to the metadata service.
146-
if b.OperationReporter != nil && !migrateMode {
147-
dbentry, _ := b.StateDB.GetResourceEntry(resourceKey)
148-
b.OperationReporter(ctx, resourceKey, dbentry.ID, action)
149-
}
150152
}
151153

152154
// TODO: Note, we only really need remote state if there are remote references.

‎bundle/direct/pkg.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,11 @@ type DeploymentUnit struct {
3737
DependsOn []deployplan.DependsOnEntry
3838
}
3939

40-
// OperationReporter is called after each successful resource operation to report
41-
// it to the deployment metadata service. It is best-effort: failures are logged
42-
// as warnings by the caller.
43-
type OperationReporter func(ctx context.Context, resourceKey string, resourceID string, action deployplan.ActionType)
40+
// OperationReporter is called after each resource operation (success or failure)
41+
// to report it to the deployment metadata service. If operationErr is non-nil the
42+
// operation is recorded as failed with the error message. It is best-effort:
43+
// reporting failures are logged as warnings by the caller.
44+
type OperationReporter func(ctx context.Context, resourceKey string, resourceID string, action deployplan.ActionType, operationErr error)
4445

4546
// DeploymentBundle holds everything needed to deploy a bundle
4647
type DeploymentBundle struct {

‎bundle/phases/deploy.go‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package phases
33
import (
44
"context"
55
"errors"
6-
"fmt"
76

87
"github.com/databricks/cli/bundle"
98
"github.com/databricks/cli/bundle/artifacts"
@@ -109,9 +108,7 @@ func deployCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, ta
109108
bundle.ApplyContext(ctx, b, terraform.Apply())
110109
}
111110

112-
// Even if deployment failed, there might be updates in states that we need to upload.
113111
statemgmt.PushResourcesState(ctx, b, targetEngine)
114-
115112
if logdiag.HasError(ctx) {
116113
return
117114
}
@@ -158,24 +155,19 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand
158155
var failed bool
159156

160157
if useMetadataService == "true" {
161-
svc, err := tmpdms.NewDeploymentMetadataAPI(b.WorkspaceClient())
162-
if err != nil {
163-
logdiag.LogError(ctx, fmt.Errorf("failed to create metadata service client: %w", err))
158+
if !targetEngine.IsDirect() {
159+
logdiag.LogError(ctx, errors.New("managed state is only supported with the direct deployment engine"))
164160
return
165161
}
166162

167-
deploymentID, versionID, cleanup, err := deployMetadataLock(ctx, b, svc, tmpdms.VersionTypeDeploy)
163+
cleanup, err := acquireMetadataLock(ctx, b, tmpdms.VersionTypeDeploy)
168164
if err != nil {
169165
logdiag.LogError(ctx, err)
170166
return
171167
}
172168
defer func() {
173169
cleanup(failed || logdiag.HasError(ctx))
174170
}()
175-
176-
if targetEngine.IsDirect() {
177-
b.DeploymentBundle.OperationReporter = makeOperationReporter(svc, deploymentID, versionID)
178-
}
179171
} else {
180172
bundle.ApplyContext(ctx, b, lock.Acquire())
181173
if logdiag.HasError(ctx) {

‎bundle/phases/deploy_metadata.go‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,24 @@ import (
2121
"github.com/google/uuid"
2222
)
2323

24+
// acquireMetadataLock creates the metadata service client, acquires the deployment
25+
// lock, and sets up the operation reporter on the bundle. It returns a cleanup
26+
// function that releases the lock, or an error if the lock could not be acquired.
27+
func acquireMetadataLock(ctx context.Context, b *bundle.Bundle, versionType tmpdms.VersionType) (cleanup func(failed bool), err error) {
28+
svc, err := tmpdms.NewDeploymentMetadataAPI(b.WorkspaceClient())
29+
if err != nil {
30+
return nil, fmt.Errorf("failed to create metadata service client: %w", err)
31+
}
32+
33+
deploymentID, versionID, cleanup, err := deployMetadataLock(ctx, b, svc, versionType)
34+
if err != nil {
35+
return nil, err
36+
}
37+
38+
b.DeploymentBundle.OperationReporter = makeOperationReporter(svc, deploymentID, versionID)
39+
return cleanup, nil
40+
}
41+
2442
// deployMetadataLock implements the lock acquire/release lifecycle using the
2543
// deployment metadata service (CreateVersion / CompleteVersion).
2644
//
@@ -120,7 +138,7 @@ func deployMetadataLock(ctx context.Context, b *bundle.Bundle, svc *tmpdms.Deplo
120138
if completeErr != nil {
121139
log.Warnf(ctx, "Failed to release deployment lock: %v", completeErr)
122140
} else {
123-
log.Infof(ctx, "Released deployment lock: deployment=%s version=%s reason=%d", deploymentID, versionID, reason)
141+
log.Infof(ctx, "Released deployment lock: deployment=%s version=%s reason=%s", deploymentID, versionID, reason)
124142
}
125143
}
126144

@@ -144,24 +162,33 @@ func planActionToOperationAction(action deployplan.ActionType) tmpdms.OperationA
144162
}
145163

146164
// makeOperationReporter returns an OperationReporter that reports each resource
147-
// operation to the metadata service. Failures are logged as warnings.
165+
// operation (success or failure) to the metadata service. Reporting failures are
166+
// logged as warnings and do not affect the deploy outcome.
148167
func makeOperationReporter(svc *tmpdms.DeploymentMetadataAPI, deploymentID, versionID string) direct.OperationReporter {
149-
return func(ctx context.Context, resourceKey string, resourceID string, action deployplan.ActionType) {
168+
return func(ctx context.Context, resourceKey string, resourceID string, action deployplan.ActionType, operationErr error) {
150169
actionType := planActionToOperationAction(action)
151170
if actionType == tmpdms.OperationActionTypeUnspecified {
152171
return
153172
}
154173

174+
status := tmpdms.OperationStatusSucceeded
175+
var errorMessage string
176+
if operationErr != nil {
177+
status = tmpdms.OperationStatusFailed
178+
errorMessage = operationErr.Error()
179+
}
180+
155181
_, err := svc.CreateOperation(ctx, tmpdms.CreateOperationRequest{
156182
DeploymentID: deploymentID,
157183
VersionID: versionID,
158184
Parent: fmt.Sprintf("deployments/%s/versions/%s", deploymentID, versionID),
159185
ResourceKey: resourceKey,
160186
Operation: &tmpdms.Operation{
161-
ResourceKey: resourceKey,
162-
ResourceID: resourceID,
163-
Status: tmpdms.OperationStatusSucceeded,
164-
ActionType: actionType,
187+
ResourceKey: resourceKey,
188+
ResourceID: resourceID,
189+
Status: status,
190+
ActionType: actionType,
191+
ErrorMessage: errorMessage,
165192
},
166193
})
167194
if err != nil {

0 commit comments

Comments
 (0)