wip

Author: denik

acceptance/bundle/resource_deps/remote_delete_volume/databricks.yml

@@ -0,0 +1,6 @@
+resources:
+  volumes:
+    foo:
+      catalog_name: mycatalog
+      schema_name: myschema
+      name: myname

acceptance/bundle/resource_deps/remote_delete_volume/out.test.toml

@@ -0,0 +1,13 @@
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
+Deploying resources...
+Updating deployment state...
+Deployment complete!
+
+>>> [CLI] volumes delete mycatalog.myschema.myname
+
+>>> [CLI] bundle plan
+create volumes.foo
+
+Plan: 1 to add, 0 to change, 0 to delete, 0 unchanged

acceptance/bundle/resource_deps/remote_delete_volume/output.txt

@@ -0,0 +1,3 @@
+trace $CLI bundle deploy
+trace $CLI volumes delete mycatalog.myschema.myname
+trace $CLI bundle plan

acceptance/bundle/resource_deps/remote_delete_volume/script

@@ -0,0 +1 @@
+RecordRequests = false

acceptance/bundle/resource_deps/remote_delete_volume/test.toml

@@ -16,6 +16,12 @@
           "name": {
             "action": "update_id"
           }
+        },
+        "remote": {
+          "storage_location": {
+            "action": "skip",
+            "reason": "remote_only"
+          }
         }
       }
     }

acceptance/bundle/resources/volumes/change-name/out.plan.direct-exp.json

@@ -21,6 +21,7 @@ import (
 	"github.com/databricks/cli/libs/structs/structvar"
 	"github.com/databricks/cli/libs/utils"
 	"github.com/databricks/databricks-sdk-go"
+	"github.com/databricks/databricks-sdk-go/apierr"
 )
 
 var errDelayed = errors.New("must be resolved after apply")
@@ -169,22 +170,34 @@ func (b *DeploymentBundle) CalculatePlanForDeploy(ctx context.Context, client *d
 
 		remoteState, err := adapter.DoRefresh(ctx, dbentry.ID)
 		if err != nil {
-			logdiag.LogError(ctx, fmt.Errorf("%s: failed to read id=%q: %w", errorPrefix, dbentry.ID, err))
-			return false
+			if errors.Is(err, apierr.ErrResourceDoesNotExist) || errors.Is(err, apierr.ErrNotFound) {
+				remoteState = nil
+			} else {
+				logdiag.LogError(ctx, fmt.Errorf("%s: failed to read id=%q: %w (localAction=%q)", errorPrefix, dbentry.ID, err, localAction.String()))
+				return false
+			}
 		}
 
-		remoteStateComparable, err := adapter.RemapState(remoteState)
-		if err != nil {
-			logdiag.LogError(ctx, fmt.Errorf("%s: failed to interpret remote state id=%q: %w", errorPrefix, dbentry.ID, err))
-		}
+		var remoteAction deployplan.ActionType
+		var remoteChangeMap map[string]deployplan.Trigger
 
-		remoteDiff, err := structdiff.GetStructDiff(remoteStateComparable, entry.NewState.Config)
-		if err != nil {
-			logdiag.LogError(ctx, fmt.Errorf("%s: diffing remote state: %w", errorPrefix, err))
-			return false
+		if remoteState == nil {
+			remoteAction = deployplan.ActionTypeCreate
+		} else {
+			remoteStateComparable, err := adapter.RemapState(remoteState)
+			if err != nil {
+				logdiag.LogError(ctx, fmt.Errorf("%s: failed to interpret remote state id=%q: %w", errorPrefix, dbentry.ID, err))
+			}
+
+			remoteDiff, err := structdiff.GetStructDiff(savedState, remoteStateComparable)
+			if err != nil {
+				logdiag.LogError(ctx, fmt.Errorf("%s: diffing remote state: %w", errorPrefix, err))
+				return false
+			}
+
+			remoteAction, remoteChangeMap = interpretOldStateVsRemoteState(adapter, remoteDiff)
 		}
 
-		remoteAction, remoteChangeMap := convertChangesToTriggersMap(adapter, remoteDiff)
 		entry.Action = max(localAction, remoteAction).String()
 
 		if len(localChangeMap) > 0 || len(remoteChangeMap) > 0 {
@@ -250,6 +263,68 @@ func convertChangesToTriggersMap(adapter *dresources.Adapter, diff []structdiff.
 	return action, m
 }
 
+/*
+
+How we interpret remote and local differences, few examples:
+
+previous state: x
+new state : y
+remote state: z
+
+previous state: x
+new state: y
+remote state: x
+
+remote-value is the same as last deployed, this is the default and not interesting, so it won’t be included in Changes.Remote
+
+
+diff(savedState, remoteValue) —> no changes there, not interesting
+
+
+previous state: x
+new state: y
+remote state: y
+
+remote-value is different from last-deployed. One can say that because new-config and remote-value are converged on the same value, this should be noop/skip, but we’ve chosen to always act on local config changes, to avoid surprises.
+
+
+To summarize, Remote.Changes is diff between previous state and remote state. If there are both local and remote actions, we take maximum possible action.
+
+
+previous state: nil
+new state: <irrelevant>
+remote state: z
+
+This is a case of server-side default being set. We’ll include it into Changes.Remote with action="skip” and reason="server_side_only”
+*/
+
+func interpretOldStateVsRemoteState(adapter *dresources.Adapter, diff []structdiff.Change) (deployplan.ActionType, map[string]deployplan.Trigger) {
+	action := deployplan.ActionTypeSkip
+	m := make(map[string]deployplan.Trigger)
+
+	for _, ch := range diff {
+		if ch.Old == nil {
+			m[ch.Path.String()] = deployplan.Trigger{
+				Action: deployplan.ActionTypeSkipString,
+				// a.k.a server-side default
+				Reason: "remote_only",
+			}
+			continue
+		}
+		fieldAction := adapter.ClassifyByTriggers(ch)
+		if fieldAction > action {
+			action = fieldAction
+		}
+		m[ch.Path.String()] = deployplan.Trigger{Action: fieldAction.String()}
+	}
+
+	if len(m) == 0 {
+		m = nil
+	}
+
+	return action, m
+}
+
 func (b *DeploymentBundle) CalculatePlanForDestroy(ctx context.Context, client *databricks.WorkspaceClient) (*deployplan.Plan, error) {
 	b.StateDB.AssertOpened()
 

bundle/direct/bundle_plan.go

@@ -42,7 +42,8 @@ func (*ResourcePipeline) RemapState(p *pipelines.GetPipelineResponse) *pipelines
 		EventLog:            spec.EventLog,
 		Filters:             spec.Filters,
 		GatewayDefinition:   spec.GatewayDefinition,
-		Id:                  spec.Id,
+		// Clear "id" field, otherwise it shows up in changes.remote
+		Id:                  "",
 		IngestionDefinition: spec.IngestionDefinition,
 		Libraries:           spec.Libraries,
 		Name:                spec.Name,
@@ -57,7 +58,7 @@ func (*ResourcePipeline) RemapState(p *pipelines.GetPipelineResponse) *pipelines
 		Tags:                spec.Tags,
 		Target:              spec.Target,
 		Trigger:             spec.Trigger,
-		ForceSendFields:     filterFields[pipelines.CreatePipeline](spec.ForceSendFields, "AllowDuplicateNames", "DryRun", "RunAs"),
+		ForceSendFields:     filterFields[pipelines.CreatePipeline](spec.ForceSendFields, "AllowDuplicateNames", "DryRun", "RunAs", "Id"),
 	}
 }