From 8a5360464bcd676638482799260d69de5db38847 Mon Sep 17 00:00:00 2001 From: Ilia Babanov Date: Mon, 15 Jan 2024 10:32:53 +0100 Subject: [PATCH 1/9] Add "bundle remote-state" command The plan is to use the new command in the Databricks VSCode extension to render resource tree UI elements --- bundle/deploy/terraform/convert.go | 18 ++++++++ cmd/bundle/bundle.go | 1 + cmd/bundle/remote_state.go | 73 ++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+) create mode 100644 cmd/bundle/remote_state.go diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index 8d51a375ae..eed3b0a744 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -234,36 +234,54 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { case "databricks_job": var tmp schema.ResourceJob conv(resource.AttributeValues, &tmp) + if config.Resources.Jobs == nil { + config.Resources.Jobs = make(map[string]*resources.Job) + } cur := config.Resources.Jobs[resource.Name] conv(tmp, &cur) config.Resources.Jobs[resource.Name] = cur case "databricks_pipeline": var tmp schema.ResourcePipeline conv(resource.AttributeValues, &tmp) + if config.Resources.Pipelines == nil { + config.Resources.Pipelines = make(map[string]*resources.Pipeline) + } cur := config.Resources.Pipelines[resource.Name] conv(tmp, &cur) config.Resources.Pipelines[resource.Name] = cur case "databricks_mlflow_model": var tmp schema.ResourceMlflowModel conv(resource.AttributeValues, &tmp) + if config.Resources.Models == nil { + config.Resources.Models = make(map[string]*resources.MlflowModel) + } cur := config.Resources.Models[resource.Name] conv(tmp, &cur) config.Resources.Models[resource.Name] = cur case "databricks_mlflow_experiment": var tmp schema.ResourceMlflowExperiment conv(resource.AttributeValues, &tmp) + if config.Resources.Experiments == nil { + config.Resources.Experiments = make(map[string]*resources.MlflowExperiment) + } cur := config.Resources.Experiments[resource.Name] conv(tmp, &cur) config.Resources.Experiments[resource.Name] = cur case "databricks_model_serving": var tmp schema.ResourceModelServing conv(resource.AttributeValues, &tmp) + if config.Resources.ModelServingEndpoints == nil { + config.Resources.ModelServingEndpoints = make(map[string]*resources.ModelServingEndpoint) + } cur := config.Resources.ModelServingEndpoints[resource.Name] conv(tmp, &cur) config.Resources.ModelServingEndpoints[resource.Name] = cur case "databricks_registered_model": var tmp schema.ResourceRegisteredModel conv(resource.AttributeValues, &tmp) + if config.Resources.RegisteredModels == nil { + config.Resources.RegisteredModels = make(map[string]*resources.RegisteredModel) + } cur := config.Resources.RegisteredModels[resource.Name] conv(tmp, &cur) config.Resources.RegisteredModels[resource.Name] = cur diff --git a/cmd/bundle/bundle.go b/cmd/bundle/bundle.go index 3206b94eef..c952b329c4 100644 --- a/cmd/bundle/bundle.go +++ b/cmd/bundle/bundle.go @@ -22,5 +22,6 @@ func New() *cobra.Command { cmd.AddCommand(newTestCommand()) cmd.AddCommand(newValidateCommand()) cmd.AddCommand(newInitCommand()) + cmd.AddCommand(newRemoteStateCommand()) return cmd } diff --git a/cmd/bundle/remote_state.go b/cmd/bundle/remote_state.go new file mode 100644 index 0000000000..377ada4208 --- /dev/null +++ b/cmd/bundle/remote_state.go @@ -0,0 +1,73 @@ +package bundle + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/deploy/terraform" + "github.com/databricks/cli/cmd/root" + + "github.com/databricks/cli/bundle/phases" + "github.com/spf13/cobra" +) + +func newRemoteStateCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "remote-state", + Short: "Pull and print deployed state of the bundle", + + PreRunE: root.MustWorkspaceClient, + + // This command is currently intended only for the Databricks VSCode extension + Hidden: true, + } + + var forcePull bool + cmd.Flags().BoolVar(&forcePull, "force-pull", false, "Skip local cache and load the state from the remote workspace") + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + b := bundle.Get(cmd.Context()) + + err := bundle.Apply(cmd.Context(), b, phases.Initialize()) + if err != nil { + return err + } + + cacheDir, err := terraform.Dir(cmd.Context(), b) + if err != nil { + return err + } + _, err = os.Stat(filepath.Join(cacheDir, terraform.TerraformStateFileName)) + noCache := errors.Is(err, os.ErrNotExist) + + if forcePull || noCache { + err = bundle.Apply(cmd.Context(), b, terraform.StatePull()) + if err != nil { + return err + } + } + + // After the initialization phase the local bundle configuration is no longer necessary. + // We want to populate the config with the remote state only. + b.Config.Resources = config.Resources{} + + err = bundle.Apply(cmd.Context(), b, terraform.Load()) + if err != nil { + return err + } + + buf, err := json.MarshalIndent(b.Config, "", " ") + if err != nil { + return err + } + + cmd.OutOrStdout().Write(buf) + return nil + } + + return cmd +} From f0840f080a03f6d8af98d8b69b314cf133458265 Mon Sep 17 00:00:00 2001 From: Ilia Babanov Date: Tue, 16 Jan 2024 10:37:15 +0100 Subject: [PATCH 2/9] Replace config resources in the terraform.Load mutator --- bundle/deploy/terraform/load.go | 6 ++++++ cmd/bundle/remote_state.go | 10 +++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/bundle/deploy/terraform/load.go b/bundle/deploy/terraform/load.go index 624bf7a50a..ca450c827b 100644 --- a/bundle/deploy/terraform/load.go +++ b/bundle/deploy/terraform/load.go @@ -6,6 +6,7 @@ import ( "slices" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/hashicorp/terraform-exec/tfexec" tfjson "github.com/hashicorp/terraform-json" ) @@ -13,6 +14,7 @@ import ( type loadMode int const ErrorOnEmptyState loadMode = 0 +const ReplaceResources loadMode = 1 type load struct { modes []loadMode @@ -43,6 +45,10 @@ func (l *load) Apply(ctx context.Context, b *bundle.Bundle) error { return err } + if slices.Contains(l.modes, ReplaceResources) { + b.Config.Resources = config.Resources{} + } + // Merge state into configuration. err = TerraformToBundle(state, &b.Config) if err != nil { diff --git a/cmd/bundle/remote_state.go b/cmd/bundle/remote_state.go index 377ada4208..e703801aa9 100644 --- a/cmd/bundle/remote_state.go +++ b/cmd/bundle/remote_state.go @@ -7,9 +7,9 @@ import ( "path/filepath" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/bundle/phases" "github.com/spf13/cobra" @@ -51,11 +51,7 @@ func newRemoteStateCommand() *cobra.Command { } } - // After the initialization phase the local bundle configuration is no longer necessary. - // We want to populate the config with the remote state only. - b.Config.Resources = config.Resources{} - - err = bundle.Apply(cmd.Context(), b, terraform.Load()) + err = bundle.Apply(cmd.Context(), b, terraform.Load(terraform.ReplaceResources)) if err != nil { return err } @@ -65,7 +61,7 @@ func newRemoteStateCommand() *cobra.Command { return err } - cmd.OutOrStdout().Write(buf) + cmdio.LogString(cmd.Context(), string(buf)) return nil } From 03d5d9b80bba824bde25f1be3cb47d05849a0245 Mon Sep 17 00:00:00 2001 From: Ilia Babanov Date: Wed, 17 Jan 2024 09:50:00 +0100 Subject: [PATCH 3/9] Summarise command --- bundle/config/resources/job.go | 5 +- bundle/config/resources/mlflow_experiment.go | 4 +- bundle/config/resources/mlflow_model.go | 4 +- .../resources/model_serving_endpoint.go | 4 +- bundle/config/resources/modified_status.go | 9 ++ bundle/config/resources/pipeline.go | 5 +- bundle/config/resources/registered_model.go | 4 +- bundle/deploy/terraform/convert.go | 61 ++++++++++- bundle/deploy/terraform/convert_test.go | 103 +++++++++++++++--- bundle/deploy/terraform/load.go | 6 - cmd/bundle/bundle.go | 2 +- cmd/bundle/{remote_state.go => summarise.go} | 13 +-- 12 files changed, 179 insertions(+), 41 deletions(-) create mode 100644 bundle/config/resources/modified_status.go rename cmd/bundle/{remote_state.go => summarise.go} (76%) diff --git a/bundle/config/resources/job.go b/bundle/config/resources/job.go index bf29106a03..5e0e85aefc 100644 --- a/bundle/config/resources/job.go +++ b/bundle/config/resources/job.go @@ -8,8 +8,9 @@ import ( ) type Job struct { - ID string `json:"id,omitempty" bundle:"readonly"` - Permissions []Permission `json:"permissions,omitempty"` + ID string `json:"id,omitempty" bundle:"readonly"` + Permissions []Permission `json:"permissions,omitempty"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` paths.Paths diff --git a/bundle/config/resources/mlflow_experiment.go b/bundle/config/resources/mlflow_experiment.go index e4a9a8a887..96d28c8afe 100644 --- a/bundle/config/resources/mlflow_experiment.go +++ b/bundle/config/resources/mlflow_experiment.go @@ -7,7 +7,9 @@ import ( ) type MlflowExperiment struct { - Permissions []Permission `json:"permissions,omitempty"` + ID string `json:"id,omitempty" bundle:"readonly"` + Permissions []Permission `json:"permissions,omitempty"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` paths.Paths diff --git a/bundle/config/resources/mlflow_model.go b/bundle/config/resources/mlflow_model.go index 51fb0e0872..f3a2a85a84 100644 --- a/bundle/config/resources/mlflow_model.go +++ b/bundle/config/resources/mlflow_model.go @@ -7,7 +7,9 @@ import ( ) type MlflowModel struct { - Permissions []Permission `json:"permissions,omitempty"` + ID string `json:"id,omitempty" bundle:"readonly"` + Permissions []Permission `json:"permissions,omitempty"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` paths.Paths diff --git a/bundle/config/resources/model_serving_endpoint.go b/bundle/config/resources/model_serving_endpoint.go index 88a55ac80c..cb22e4c2c0 100644 --- a/bundle/config/resources/model_serving_endpoint.go +++ b/bundle/config/resources/model_serving_endpoint.go @@ -13,7 +13,7 @@ type ModelServingEndpoint struct { // This represents the id (ie serving_endpoint_id) that can be used // as a reference in other resources. This value is returned by terraform. - ID string + ID string `json:"id,omitempty" bundle:"readonly"` // Path to config file where the resource is defined. All bundle resources // include this for interpolation purposes. @@ -22,6 +22,8 @@ type ModelServingEndpoint struct { // This is a resource agnostic implementation of permissions for ACLs. // Implementation could be different based on the resource type. Permissions []Permission `json:"permissions,omitempty"` + + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` } func (s *ModelServingEndpoint) UnmarshalJSON(b []byte) error { diff --git a/bundle/config/resources/modified_status.go b/bundle/config/resources/modified_status.go new file mode 100644 index 0000000000..7634bae74a --- /dev/null +++ b/bundle/config/resources/modified_status.go @@ -0,0 +1,9 @@ +package resources + +type ModifiedStatus = string + +const ( + ModifiedStatusCreated ModifiedStatus = "CREATED" + ModifiedStatusUpdated ModifiedStatus = "UPDATED" + ModifiedStatusDeleted ModifiedStatus = "DELETED" +) diff --git a/bundle/config/resources/pipeline.go b/bundle/config/resources/pipeline.go index 5c741f8af6..fa10793db5 100644 --- a/bundle/config/resources/pipeline.go +++ b/bundle/config/resources/pipeline.go @@ -10,8 +10,9 @@ import ( ) type Pipeline struct { - ID string `json:"id,omitempty" bundle:"readonly"` - Permissions []Permission `json:"permissions,omitempty"` + ID string `json:"id,omitempty" bundle:"readonly"` + Permissions []Permission `json:"permissions,omitempty"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` paths.Paths diff --git a/bundle/config/resources/registered_model.go b/bundle/config/resources/registered_model.go index 32a451a2ab..fa57ad886a 100644 --- a/bundle/config/resources/registered_model.go +++ b/bundle/config/resources/registered_model.go @@ -14,7 +14,7 @@ type RegisteredModel struct { // This represents the id which is the full name of the model // (catalog_name.schema_name.model_name) that can be used // as a reference in other resources. This value is returned by terraform. - ID string + ID string `json:"id,omitempty" bundle:"readonly"` // Path to config file where the resource is defined. All bundle resources // include this for interpolation purposes. @@ -23,6 +23,8 @@ type RegisteredModel struct { // This represents the input args for terraform, and will get converted // to a HCL representation for CRUD *catalog.CreateRegisteredModelRequest + + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` } func (s *RegisteredModel) UnmarshalJSON(b []byte) error { diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index eed3b0a744..5251e132d6 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -3,6 +3,7 @@ package terraform import ( "encoding/json" "fmt" + "reflect" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" @@ -15,6 +16,16 @@ func conv(from any, to any) { json.Unmarshal(buf, &to) } +func convRemoteToLocal(remote any, local any) (resources.ModifiedStatus, error) { + var modifiedStatus resources.ModifiedStatus + if reflect.ValueOf(local).Elem().IsNil() { + modifiedStatus = resources.ModifiedStatusDeleted + } + buf, _ := json.Marshal(remote) + err := json.Unmarshal(buf, &local) + return modifiedStatus, err +} + func convPermissions(acl []resources.Permission) *schema.ResourcePermissions { if len(acl) == 0 { return nil @@ -238,7 +249,9 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { config.Resources.Jobs = make(map[string]*resources.Job) } cur := config.Resources.Jobs[resource.Name] - conv(tmp, &cur) + // TODO: make sure we can unmarshall tf state properly and don't swallow errors + modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus config.Resources.Jobs[resource.Name] = cur case "databricks_pipeline": var tmp schema.ResourcePipeline @@ -247,7 +260,8 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { config.Resources.Pipelines = make(map[string]*resources.Pipeline) } cur := config.Resources.Pipelines[resource.Name] - conv(tmp, &cur) + modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus config.Resources.Pipelines[resource.Name] = cur case "databricks_mlflow_model": var tmp schema.ResourceMlflowModel @@ -256,7 +270,8 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { config.Resources.Models = make(map[string]*resources.MlflowModel) } cur := config.Resources.Models[resource.Name] - conv(tmp, &cur) + modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus config.Resources.Models[resource.Name] = cur case "databricks_mlflow_experiment": var tmp schema.ResourceMlflowExperiment @@ -265,7 +280,8 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { config.Resources.Experiments = make(map[string]*resources.MlflowExperiment) } cur := config.Resources.Experiments[resource.Name] - conv(tmp, &cur) + modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus config.Resources.Experiments[resource.Name] = cur case "databricks_model_serving": var tmp schema.ResourceModelServing @@ -274,7 +290,8 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { config.Resources.ModelServingEndpoints = make(map[string]*resources.ModelServingEndpoint) } cur := config.Resources.ModelServingEndpoints[resource.Name] - conv(tmp, &cur) + modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus config.Resources.ModelServingEndpoints[resource.Name] = cur case "databricks_registered_model": var tmp schema.ResourceRegisteredModel @@ -283,7 +300,8 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { config.Resources.RegisteredModels = make(map[string]*resources.RegisteredModel) } cur := config.Resources.RegisteredModels[resource.Name] - conv(tmp, &cur) + modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus config.Resources.RegisteredModels[resource.Name] = cur case "databricks_permissions": case "databricks_grants": @@ -293,5 +311,36 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { } } + for _, src := range config.Resources.Jobs { + if src.ModifiedStatus == "" && src.ID == "" { + src.ModifiedStatus = resources.ModifiedStatusCreated + } + } + for _, src := range config.Resources.Pipelines { + if src.ModifiedStatus == "" && src.ID == "" { + src.ModifiedStatus = resources.ModifiedStatusCreated + } + } + for _, src := range config.Resources.Models { + if src.ModifiedStatus == "" && src.ID == "" { + src.ModifiedStatus = resources.ModifiedStatusCreated + } + } + for _, src := range config.Resources.Experiments { + if src.ModifiedStatus == "" && src.ID == "" { + src.ModifiedStatus = resources.ModifiedStatusCreated + } + } + for _, src := range config.Resources.ModelServingEndpoints { + if src.ModifiedStatus == "" && src.ID == "" { + src.ModifiedStatus = resources.ModifiedStatusCreated + } + } + for _, src := range config.Resources.RegisteredModels { + if src.ModifiedStatus == "" && src.ID == "" { + src.ModifiedStatus = resources.ModifiedStatusCreated + } + } + return nil } diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index 00086c7683..aae97dcd41 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -11,11 +11,12 @@ import ( "github.com/databricks/databricks-sdk-go/service/ml" "github.com/databricks/databricks-sdk-go/service/pipelines" "github.com/databricks/databricks-sdk-go/service/serving" + tfjson "github.com/hashicorp/terraform-json" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestConvertJob(t *testing.T) { +func TestBundleToTerraformJob(t *testing.T) { var src = resources.Job{ JobSettings: &jobs.JobSettings{ Name: "my job", @@ -62,7 +63,7 @@ func TestConvertJob(t *testing.T) { assert.Nil(t, out.Data) } -func TestConvertJobPermissions(t *testing.T) { +func TestBundleToTerraformJobPermissions(t *testing.T) { var src = resources.Job{ Permissions: []resources.Permission{ { @@ -89,7 +90,7 @@ func TestConvertJobPermissions(t *testing.T) { assert.Equal(t, "CAN_VIEW", p.PermissionLevel) } -func TestConvertJobTaskLibraries(t *testing.T) { +func TestBundleToTerraformJobTaskLibraries(t *testing.T) { var src = resources.Job{ JobSettings: &jobs.JobSettings{ Name: "my job", @@ -123,7 +124,7 @@ func TestConvertJobTaskLibraries(t *testing.T) { assert.Equal(t, "mlflow", out.Resource.Job["my_job"].Task[0].Library[0].Pypi.Package) } -func TestConvertPipeline(t *testing.T) { +func TestBundleToTerraformPipeline(t *testing.T) { var src = resources.Pipeline{ PipelineSpec: &pipelines.PipelineSpec{ Name: "my pipeline", @@ -182,7 +183,7 @@ func TestConvertPipeline(t *testing.T) { assert.Nil(t, out.Data) } -func TestConvertPipelinePermissions(t *testing.T) { +func TestBundleToTerraformPipelinePermissions(t *testing.T) { var src = resources.Pipeline{ Permissions: []resources.Permission{ { @@ -209,7 +210,7 @@ func TestConvertPipelinePermissions(t *testing.T) { assert.Equal(t, "CAN_VIEW", p.PermissionLevel) } -func TestConvertModel(t *testing.T) { +func TestBundleToTerraformModel(t *testing.T) { var src = resources.MlflowModel{ Model: &ml.Model{ Name: "name", @@ -246,7 +247,7 @@ func TestConvertModel(t *testing.T) { assert.Nil(t, out.Data) } -func TestConvertModelPermissions(t *testing.T) { +func TestBundleToTerraformModelPermissions(t *testing.T) { var src = resources.MlflowModel{ Permissions: []resources.Permission{ { @@ -273,7 +274,7 @@ func TestConvertModelPermissions(t *testing.T) { assert.Equal(t, "CAN_READ", p.PermissionLevel) } -func TestConvertExperiment(t *testing.T) { +func TestBundleToTerraformExperiment(t *testing.T) { var src = resources.MlflowExperiment{ Experiment: &ml.Experiment{ Name: "name", @@ -293,7 +294,7 @@ func TestConvertExperiment(t *testing.T) { assert.Nil(t, out.Data) } -func TestConvertExperimentPermissions(t *testing.T) { +func TestBundleToTerraformExperimentPermissions(t *testing.T) { var src = resources.MlflowExperiment{ Permissions: []resources.Permission{ { @@ -321,7 +322,7 @@ func TestConvertExperimentPermissions(t *testing.T) { } -func TestConvertModelServing(t *testing.T) { +func TestBundleToTerraformModelServing(t *testing.T) { var src = resources.ModelServingEndpoint{ CreateServingEndpoint: &serving.CreateServingEndpoint{ Name: "name", @@ -366,7 +367,7 @@ func TestConvertModelServing(t *testing.T) { assert.Nil(t, out.Data) } -func TestConvertModelServingPermissions(t *testing.T) { +func TestBundleToTerraformModelServingPermissions(t *testing.T) { var src = resources.ModelServingEndpoint{ Permissions: []resources.Permission{ { @@ -394,7 +395,7 @@ func TestConvertModelServingPermissions(t *testing.T) { } -func TestConvertRegisteredModel(t *testing.T) { +func TestBundleToTerraformRegisteredModel(t *testing.T) { var src = resources.RegisteredModel{ CreateRegisteredModelRequest: &catalog.CreateRegisteredModelRequest{ Name: "name", @@ -421,7 +422,7 @@ func TestConvertRegisteredModel(t *testing.T) { assert.Nil(t, out.Data) } -func TestConvertRegisteredModelGrants(t *testing.T) { +func TestBundleToTerraformRegisteredModelGrants(t *testing.T) { var src = resources.RegisteredModel{ Grants: []resources.Grant{ { @@ -446,5 +447,81 @@ func TestConvertRegisteredModelGrants(t *testing.T) { p := out.Resource.Grants["registered_model_my_registered_model"].Grant[0] assert.Equal(t, "jane@doe.com", p.Principal) assert.Equal(t, "EXECUTE", p.Privileges[0]) +} + +func TestTerraformToBundleEmptyJobs(t *testing.T) { + var config = config.Root{ + Resources: config.Resources{}, + } + var tfState = tfjson.State{ + Values: &tfjson.StateValues{ + RootModule: &tfjson.StateModule{ + Resources: []*tfjson.StateResource{ + { + Type: "databricks_job", + Mode: "managed", + Name: "test_job", + AttributeValues: map[string]interface{}{ + "id": "1", + }, + }, + }, + }, + }, + } + err := TerraformToBundle(&tfState, &config) + assert.NoError(t, err) + assert.Equal(t, "1", config.Resources.Jobs["test_job"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Jobs["test_job"].ModifiedStatus) +} +func TestTerrformToBundleModifiedJobs(t *testing.T) { + var config = config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "test_job": { + JobSettings: &jobs.JobSettings{ + Name: "test_job", + }, + }, + "test_job3": { + JobSettings: &jobs.JobSettings{ + Name: "test_job3", + }, + }, + }, + }, + } + var tfState = tfjson.State{ + Values: &tfjson.StateValues{ + RootModule: &tfjson.StateModule{ + Resources: []*tfjson.StateResource{ + { + Type: "databricks_job", + Mode: "managed", + Name: "test_job", + AttributeValues: map[string]interface{}{ + "id": "1", + }, + }, + { + Type: "databricks_job", + Mode: "managed", + Name: "test_job2", + AttributeValues: map[string]interface{}{ + "id": "2", + }, + }, + }, + }, + }, + } + err := TerraformToBundle(&tfState, &config) + assert.NoError(t, err) + assert.Equal(t, "1", config.Resources.Jobs["test_job"].ID) + assert.Equal(t, "", config.Resources.Jobs["test_job"].ModifiedStatus) + assert.Equal(t, "2", config.Resources.Jobs["test_job2"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Jobs["test_job2"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Jobs["test_job3"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Jobs["test_job3"].ModifiedStatus) } diff --git a/bundle/deploy/terraform/load.go b/bundle/deploy/terraform/load.go index ca450c827b..624bf7a50a 100644 --- a/bundle/deploy/terraform/load.go +++ b/bundle/deploy/terraform/load.go @@ -6,7 +6,6 @@ import ( "slices" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" "github.com/hashicorp/terraform-exec/tfexec" tfjson "github.com/hashicorp/terraform-json" ) @@ -14,7 +13,6 @@ import ( type loadMode int const ErrorOnEmptyState loadMode = 0 -const ReplaceResources loadMode = 1 type load struct { modes []loadMode @@ -45,10 +43,6 @@ func (l *load) Apply(ctx context.Context, b *bundle.Bundle) error { return err } - if slices.Contains(l.modes, ReplaceResources) { - b.Config.Resources = config.Resources{} - } - // Merge state into configuration. err = TerraformToBundle(state, &b.Config) if err != nil { diff --git a/cmd/bundle/bundle.go b/cmd/bundle/bundle.go index c952b329c4..a008ccd28d 100644 --- a/cmd/bundle/bundle.go +++ b/cmd/bundle/bundle.go @@ -22,6 +22,6 @@ func New() *cobra.Command { cmd.AddCommand(newTestCommand()) cmd.AddCommand(newValidateCommand()) cmd.AddCommand(newInitCommand()) - cmd.AddCommand(newRemoteStateCommand()) + cmd.AddCommand(newSummariseCommand()) return cmd } diff --git a/cmd/bundle/remote_state.go b/cmd/bundle/summarise.go similarity index 76% rename from cmd/bundle/remote_state.go rename to cmd/bundle/summarise.go index e703801aa9..603e771bf0 100644 --- a/cmd/bundle/remote_state.go +++ b/cmd/bundle/summarise.go @@ -8,21 +8,20 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/terraform" - "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/bundle/phases" "github.com/spf13/cobra" ) -func newRemoteStateCommand() *cobra.Command { +func newSummariseCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "remote-state", - Short: "Pull and print deployed state of the bundle", + Use: "summarise", + Short: "Describe the bundle resources and their deployment states", - PreRunE: root.MustWorkspaceClient, + PreRunE: ConfigureBundleWithVariables, - // This command is currently intended only for the Databricks VSCode extension + // This command is currently intended for the Databricks VSCode extension only Hidden: true, } @@ -51,7 +50,7 @@ func newRemoteStateCommand() *cobra.Command { } } - err = bundle.Apply(cmd.Context(), b, terraform.Load(terraform.ReplaceResources)) + err = bundle.Apply(cmd.Context(), b, terraform.Load()) if err != nil { return err } From e0e930618c0249b7e98881a4bd5d90978a7a29c5 Mon Sep 17 00:00:00 2001 From: Ilia Babanov Date: Wed, 17 Jan 2024 16:57:19 +0100 Subject: [PATCH 4/9] Print json to stdout --- cmd/bundle/summarise.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/bundle/summarise.go b/cmd/bundle/summarise.go index 603e771bf0..38fffe1ba6 100644 --- a/cmd/bundle/summarise.go +++ b/cmd/bundle/summarise.go @@ -8,7 +8,6 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/terraform" - "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/bundle/phases" "github.com/spf13/cobra" @@ -60,7 +59,7 @@ func newSummariseCommand() *cobra.Command { return err } - cmdio.LogString(cmd.Context(), string(buf)) + cmd.OutOrStdout().Write(buf) return nil } From 58fa44416525a620155ad5cdbb1becaef03a3a38 Mon Sep 17 00:00:00 2001 From: Ilia Babanov Date: Thu, 18 Jan 2024 11:33:25 +0100 Subject: [PATCH 5/9] Rename to summary, mark new fields as internal --- bundle/config/resources/job.go | 2 +- bundle/config/resources/mlflow_experiment.go | 2 +- bundle/config/resources/mlflow_model.go | 2 +- .../resources/model_serving_endpoint.go | 2 +- bundle/config/resources/pipeline.go | 2 +- bundle/config/resources/registered_model.go | 2 +- bundle/deploy/terraform/convert.go | 19 +++++++++---------- cmd/bundle/bundle.go | 2 +- cmd/bundle/{summarise.go => summary.go} | 4 ++-- 9 files changed, 18 insertions(+), 19 deletions(-) rename cmd/bundle/{summarise.go => summary.go} (95%) diff --git a/bundle/config/resources/job.go b/bundle/config/resources/job.go index 5e0e85aefc..bd43ed0af6 100644 --- a/bundle/config/resources/job.go +++ b/bundle/config/resources/job.go @@ -10,7 +10,7 @@ import ( type Job struct { ID string `json:"id,omitempty" bundle:"readonly"` Permissions []Permission `json:"permissions,omitempty"` - ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` paths.Paths diff --git a/bundle/config/resources/mlflow_experiment.go b/bundle/config/resources/mlflow_experiment.go index 96d28c8afe..0f53096a0e 100644 --- a/bundle/config/resources/mlflow_experiment.go +++ b/bundle/config/resources/mlflow_experiment.go @@ -9,7 +9,7 @@ import ( type MlflowExperiment struct { ID string `json:"id,omitempty" bundle:"readonly"` Permissions []Permission `json:"permissions,omitempty"` - ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` paths.Paths diff --git a/bundle/config/resources/mlflow_model.go b/bundle/config/resources/mlflow_model.go index f3a2a85a84..59893aa475 100644 --- a/bundle/config/resources/mlflow_model.go +++ b/bundle/config/resources/mlflow_model.go @@ -9,7 +9,7 @@ import ( type MlflowModel struct { ID string `json:"id,omitempty" bundle:"readonly"` Permissions []Permission `json:"permissions,omitempty"` - ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` paths.Paths diff --git a/bundle/config/resources/model_serving_endpoint.go b/bundle/config/resources/model_serving_endpoint.go index cb22e4c2c0..d1d57bafc2 100644 --- a/bundle/config/resources/model_serving_endpoint.go +++ b/bundle/config/resources/model_serving_endpoint.go @@ -23,7 +23,7 @@ type ModelServingEndpoint struct { // Implementation could be different based on the resource type. Permissions []Permission `json:"permissions,omitempty"` - ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` } func (s *ModelServingEndpoint) UnmarshalJSON(b []byte) error { diff --git a/bundle/config/resources/pipeline.go b/bundle/config/resources/pipeline.go index fa10793db5..43450dc49a 100644 --- a/bundle/config/resources/pipeline.go +++ b/bundle/config/resources/pipeline.go @@ -12,7 +12,7 @@ import ( type Pipeline struct { ID string `json:"id,omitempty" bundle:"readonly"` Permissions []Permission `json:"permissions,omitempty"` - ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` paths.Paths diff --git a/bundle/config/resources/registered_model.go b/bundle/config/resources/registered_model.go index fa57ad886a..7b4b70d1ab 100644 --- a/bundle/config/resources/registered_model.go +++ b/bundle/config/resources/registered_model.go @@ -24,7 +24,7 @@ type RegisteredModel struct { // to a HCL representation for CRUD *catalog.CreateRegisteredModelRequest - ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"readonly"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` } func (s *RegisteredModel) UnmarshalJSON(b []byte) error { diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index 5251e132d6..b6fc2d18a3 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -16,14 +16,13 @@ func conv(from any, to any) { json.Unmarshal(buf, &to) } -func convRemoteToLocal(remote any, local any) (resources.ModifiedStatus, error) { +func convRemoteToLocal(remote any, local any) resources.ModifiedStatus { var modifiedStatus resources.ModifiedStatus if reflect.ValueOf(local).Elem().IsNil() { modifiedStatus = resources.ModifiedStatusDeleted } - buf, _ := json.Marshal(remote) - err := json.Unmarshal(buf, &local) - return modifiedStatus, err + conv(remote, local) + return modifiedStatus } func convPermissions(acl []resources.Permission) *schema.ResourcePermissions { @@ -250,7 +249,7 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { } cur := config.Resources.Jobs[resource.Name] // TODO: make sure we can unmarshall tf state properly and don't swallow errors - modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + modifiedStatus := convRemoteToLocal(tmp, &cur) cur.ModifiedStatus = modifiedStatus config.Resources.Jobs[resource.Name] = cur case "databricks_pipeline": @@ -260,7 +259,7 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { config.Resources.Pipelines = make(map[string]*resources.Pipeline) } cur := config.Resources.Pipelines[resource.Name] - modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + modifiedStatus := convRemoteToLocal(tmp, &cur) cur.ModifiedStatus = modifiedStatus config.Resources.Pipelines[resource.Name] = cur case "databricks_mlflow_model": @@ -270,7 +269,7 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { config.Resources.Models = make(map[string]*resources.MlflowModel) } cur := config.Resources.Models[resource.Name] - modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + modifiedStatus := convRemoteToLocal(tmp, &cur) cur.ModifiedStatus = modifiedStatus config.Resources.Models[resource.Name] = cur case "databricks_mlflow_experiment": @@ -280,7 +279,7 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { config.Resources.Experiments = make(map[string]*resources.MlflowExperiment) } cur := config.Resources.Experiments[resource.Name] - modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + modifiedStatus := convRemoteToLocal(tmp, &cur) cur.ModifiedStatus = modifiedStatus config.Resources.Experiments[resource.Name] = cur case "databricks_model_serving": @@ -290,7 +289,7 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { config.Resources.ModelServingEndpoints = make(map[string]*resources.ModelServingEndpoint) } cur := config.Resources.ModelServingEndpoints[resource.Name] - modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + modifiedStatus := convRemoteToLocal(tmp, &cur) cur.ModifiedStatus = modifiedStatus config.Resources.ModelServingEndpoints[resource.Name] = cur case "databricks_registered_model": @@ -300,7 +299,7 @@ func TerraformToBundle(state *tfjson.State, config *config.Root) error { config.Resources.RegisteredModels = make(map[string]*resources.RegisteredModel) } cur := config.Resources.RegisteredModels[resource.Name] - modifiedStatus, _ := convRemoteToLocal(tmp, &cur) + modifiedStatus := convRemoteToLocal(tmp, &cur) cur.ModifiedStatus = modifiedStatus config.Resources.RegisteredModels[resource.Name] = cur case "databricks_permissions": diff --git a/cmd/bundle/bundle.go b/cmd/bundle/bundle.go index 5a82a57a7a..a82311d83a 100644 --- a/cmd/bundle/bundle.go +++ b/cmd/bundle/bundle.go @@ -22,7 +22,7 @@ func New() *cobra.Command { cmd.AddCommand(newTestCommand()) cmd.AddCommand(newValidateCommand()) cmd.AddCommand(newInitCommand()) - cmd.AddCommand(newSummariseCommand()) + cmd.AddCommand(newSummaryCommand()) cmd.AddCommand(newGenerateCommand()) return cmd } diff --git a/cmd/bundle/summarise.go b/cmd/bundle/summary.go similarity index 95% rename from cmd/bundle/summarise.go rename to cmd/bundle/summary.go index 38fffe1ba6..a38732b49d 100644 --- a/cmd/bundle/summarise.go +++ b/cmd/bundle/summary.go @@ -13,9 +13,9 @@ import ( "github.com/spf13/cobra" ) -func newSummariseCommand() *cobra.Command { +func newSummaryCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "summarise", + Use: "summary", Short: "Describe the bundle resources and their deployment states", PreRunE: ConfigureBundleWithVariables, From 80d3a7d7d1ddfb2373b10f23f97153a02474801a Mon Sep 17 00:00:00 2001 From: Ilia Babanov Date: Thu, 18 Jan 2024 13:15:36 +0100 Subject: [PATCH 6/9] Improve tests --- bundle/deploy/terraform/convert_test.go | 268 +++++++++++++++++++++--- 1 file changed, 242 insertions(+), 26 deletions(-) diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index aae97dcd41..98bd919988 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -1,6 +1,7 @@ package terraform import ( + "reflect" "testing" "github.com/databricks/cli/bundle/config" @@ -449,7 +450,7 @@ func TestBundleToTerraformRegisteredModelGrants(t *testing.T) { assert.Equal(t, "EXECUTE", p.Privileges[0]) } -func TestTerraformToBundleEmptyJobs(t *testing.T) { +func TestTerraformToBundleEmptyLocalResources(t *testing.T) { var config = config.Root{ Resources: config.Resources{}, } @@ -458,12 +459,40 @@ func TestTerraformToBundleEmptyJobs(t *testing.T) { RootModule: &tfjson.StateModule{ Resources: []*tfjson.StateResource{ { - Type: "databricks_job", - Mode: "managed", - Name: "test_job", - AttributeValues: map[string]interface{}{ - "id": "1", - }, + Type: "databricks_job", + Mode: "managed", + Name: "test_job", + AttributeValues: map[string]interface{}{"id": "1"}, + }, + { + Type: "databricks_pipeline", + Mode: "managed", + Name: "test_pipeline", + AttributeValues: map[string]interface{}{"id": "1"}, + }, + { + Type: "databricks_mlflow_model", + Mode: "managed", + Name: "test_mlflow_model", + AttributeValues: map[string]interface{}{"id": "1"}, + }, + { + Type: "databricks_mlflow_experiment", + Mode: "managed", + Name: "test_mlflow_experiment", + AttributeValues: map[string]interface{}{"id": "1"}, + }, + { + Type: "databricks_model_serving", + Mode: "managed", + Name: "test_model_serving", + AttributeValues: map[string]interface{}{"id": "1"}, + }, + { + Type: "databricks_registered_model", + Mode: "managed", + Name: "test_registered_model", + AttributeValues: map[string]interface{}{"id": "1"}, }, }, }, @@ -471,11 +500,29 @@ func TestTerraformToBundleEmptyJobs(t *testing.T) { } err := TerraformToBundle(&tfState, &config) assert.NoError(t, err) + assert.Equal(t, "1", config.Resources.Jobs["test_job"].ID) assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Jobs["test_job"].ModifiedStatus) + + assert.Equal(t, "1", config.Resources.Pipelines["test_pipeline"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Pipelines["test_pipeline"].ModifiedStatus) + + assert.Equal(t, "1", config.Resources.Models["test_mlflow_model"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Models["test_mlflow_model"].ModifiedStatus) + + assert.Equal(t, "1", config.Resources.Experiments["test_mlflow_experiment"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Experiments["test_mlflow_experiment"].ModifiedStatus) + + assert.Equal(t, "1", config.Resources.ModelServingEndpoints["test_model_serving"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.ModelServingEndpoints["test_model_serving"].ModifiedStatus) + + assert.Equal(t, "1", config.Resources.RegisteredModels["test_registered_model"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.RegisteredModels["test_registered_model"].ModifiedStatus) + + AssertFullResourceCoverage(t, &config) } -func TestTerrformToBundleModifiedJobs(t *testing.T) { +func TestTerraformToBundleModifiedResources(t *testing.T) { var config = config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -484,9 +531,69 @@ func TestTerrformToBundleModifiedJobs(t *testing.T) { Name: "test_job", }, }, - "test_job3": { + "test_job_new": { JobSettings: &jobs.JobSettings{ - Name: "test_job3", + Name: "test_job_new", + }, + }, + }, + Pipelines: map[string]*resources.Pipeline{ + "test_pipeline": { + PipelineSpec: &pipelines.PipelineSpec{ + Name: "test_pipeline", + }, + }, + "test_pipeline_new": { + PipelineSpec: &pipelines.PipelineSpec{ + Name: "test_pipeline_new", + }, + }, + }, + Models: map[string]*resources.MlflowModel{ + "test_mlflow_model": { + Model: &ml.Model{ + Name: "test_mlflow_model", + }, + }, + "test_mlflow_model_new": { + Model: &ml.Model{ + Name: "test_mlflow_model_new", + }, + }, + }, + Experiments: map[string]*resources.MlflowExperiment{ + "test_mlflow_experiment": { + Experiment: &ml.Experiment{ + Name: "test_mlflow_experiment", + }, + }, + "test_mlflow_experiment_new": { + Experiment: &ml.Experiment{ + Name: "test_mlflow_experiment_new", + }, + }, + }, + ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{ + "test_model_serving": { + CreateServingEndpoint: &serving.CreateServingEndpoint{ + Name: "test_model_serving", + }, + }, + "test_model_serving_new": { + CreateServingEndpoint: &serving.CreateServingEndpoint{ + Name: "test_model_serving_new", + }, + }, + }, + RegisteredModels: map[string]*resources.RegisteredModel{ + "test_registered_model": { + CreateRegisteredModelRequest: &catalog.CreateRegisteredModelRequest{ + Name: "test_registered_model", + }, + }, + "test_registered_model_new": { + CreateRegisteredModelRequest: &catalog.CreateRegisteredModelRequest{ + Name: "test_registered_model_new", }, }, }, @@ -497,20 +604,76 @@ func TestTerrformToBundleModifiedJobs(t *testing.T) { RootModule: &tfjson.StateModule{ Resources: []*tfjson.StateResource{ { - Type: "databricks_job", - Mode: "managed", - Name: "test_job", - AttributeValues: map[string]interface{}{ - "id": "1", - }, + Type: "databricks_job", + Mode: "managed", + Name: "test_job", + AttributeValues: map[string]interface{}{"id": "1"}, }, { - Type: "databricks_job", - Mode: "managed", - Name: "test_job2", - AttributeValues: map[string]interface{}{ - "id": "2", - }, + Type: "databricks_job", + Mode: "managed", + Name: "test_job_old", + AttributeValues: map[string]interface{}{"id": "2"}, + }, + { + Type: "databricks_pipeline", + Mode: "managed", + Name: "test_pipeline", + AttributeValues: map[string]interface{}{"id": "1"}, + }, + { + Type: "databricks_pipeline", + Mode: "managed", + Name: "test_pipeline_old", + AttributeValues: map[string]interface{}{"id": "2"}, + }, + { + Type: "databricks_mlflow_model", + Mode: "managed", + Name: "test_mlflow_model", + AttributeValues: map[string]interface{}{"id": "1"}, + }, + { + Type: "databricks_mlflow_model", + Mode: "managed", + Name: "test_mlflow_model_old", + AttributeValues: map[string]interface{}{"id": "2"}, + }, + { + Type: "databricks_mlflow_experiment", + Mode: "managed", + Name: "test_mlflow_experiment", + AttributeValues: map[string]interface{}{"id": "1"}, + }, + { + Type: "databricks_mlflow_experiment", + Mode: "managed", + Name: "test_mlflow_experiment_old", + AttributeValues: map[string]interface{}{"id": "2"}, + }, + { + Type: "databricks_model_serving", + Mode: "managed", + Name: "test_model_serving", + AttributeValues: map[string]interface{}{"id": "1"}, + }, + { + Type: "databricks_model_serving", + Mode: "managed", + Name: "test_model_serving_old", + AttributeValues: map[string]interface{}{"id": "2"}, + }, + { + Type: "databricks_registered_model", + Mode: "managed", + Name: "test_registered_model", + AttributeValues: map[string]interface{}{"id": "1"}, + }, + { + Type: "databricks_registered_model", + Mode: "managed", + Name: "test_registered_model_old", + AttributeValues: map[string]interface{}{"id": "2"}, }, }, }, @@ -518,10 +681,63 @@ func TestTerrformToBundleModifiedJobs(t *testing.T) { } err := TerraformToBundle(&tfState, &config) assert.NoError(t, err) + assert.Equal(t, "1", config.Resources.Jobs["test_job"].ID) assert.Equal(t, "", config.Resources.Jobs["test_job"].ModifiedStatus) - assert.Equal(t, "2", config.Resources.Jobs["test_job2"].ID) - assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Jobs["test_job2"].ModifiedStatus) - assert.Equal(t, "", config.Resources.Jobs["test_job3"].ID) - assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Jobs["test_job3"].ModifiedStatus) + assert.Equal(t, "2", config.Resources.Jobs["test_job_old"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Jobs["test_job_old"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Jobs["test_job_new"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Jobs["test_job_new"].ModifiedStatus) + + assert.Equal(t, "1", config.Resources.Pipelines["test_pipeline"].ID) + assert.Equal(t, "", config.Resources.Pipelines["test_pipeline"].ModifiedStatus) + assert.Equal(t, "2", config.Resources.Pipelines["test_pipeline_old"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Pipelines["test_pipeline_old"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Pipelines["test_pipeline_new"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Pipelines["test_pipeline_new"].ModifiedStatus) + + assert.Equal(t, "1", config.Resources.Models["test_mlflow_model"].ID) + assert.Equal(t, "", config.Resources.Models["test_mlflow_model"].ModifiedStatus) + assert.Equal(t, "2", config.Resources.Models["test_mlflow_model_old"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Models["test_mlflow_model_old"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Models["test_mlflow_model_new"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Models["test_mlflow_model_new"].ModifiedStatus) + + assert.Equal(t, "1", config.Resources.RegisteredModels["test_registered_model"].ID) + assert.Equal(t, "", config.Resources.RegisteredModels["test_registered_model"].ModifiedStatus) + assert.Equal(t, "2", config.Resources.RegisteredModels["test_registered_model_old"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.RegisteredModels["test_registered_model_old"].ModifiedStatus) + assert.Equal(t, "", config.Resources.RegisteredModels["test_registered_model_new"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.RegisteredModels["test_registered_model_new"].ModifiedStatus) + + assert.Equal(t, "1", config.Resources.Experiments["test_mlflow_experiment"].ID) + assert.Equal(t, "", config.Resources.Experiments["test_mlflow_experiment"].ModifiedStatus) + assert.Equal(t, "2", config.Resources.Experiments["test_mlflow_experiment_old"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Experiments["test_mlflow_experiment_old"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Experiments["test_mlflow_experiment_new"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Experiments["test_mlflow_experiment_new"].ModifiedStatus) + + assert.Equal(t, "1", config.Resources.ModelServingEndpoints["test_model_serving"].ID) + assert.Equal(t, "", config.Resources.ModelServingEndpoints["test_model_serving"].ModifiedStatus) + assert.Equal(t, "2", config.Resources.ModelServingEndpoints["test_model_serving_old"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.ModelServingEndpoints["test_model_serving_old"].ModifiedStatus) + assert.Equal(t, "", config.Resources.ModelServingEndpoints["test_model_serving_new"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.ModelServingEndpoints["test_model_serving_new"].ModifiedStatus) + + AssertFullResourceCoverage(t, &config) +} + +func AssertFullResourceCoverage(t *testing.T, config *config.Root) { + resources := reflect.ValueOf(config.Resources) + for i := 0; i < resources.NumField(); i++ { + field := resources.Field(i) + if field.Kind() == reflect.Map { + assert.True( + t, + !field.IsNil() && field.Len() > 0, + "TerraformToBundle should support '%s' (please add it to convert.go and extend the test suite)", + resources.Type().Field(i).Name, + ) + } + } } From ccf33293002131d13e23b684c365760d2a0fd5d5 Mon Sep 17 00:00:00 2001 From: Ilia Babanov Date: Thu, 18 Jan 2024 13:32:05 +0100 Subject: [PATCH 7/9] Explain ModifiedStatus enum --- bundle/config/resources/modified_status.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bundle/config/resources/modified_status.go b/bundle/config/resources/modified_status.go index 7634bae74a..c10bc8f07e 100644 --- a/bundle/config/resources/modified_status.go +++ b/bundle/config/resources/modified_status.go @@ -1,5 +1,11 @@ package resources +// ModifiedStatus is an enum of the possible statuses of a resource from the local perspective. +// CREATED - new resources that have been added to the local bundle configuration and don't yet exist remotely. +// DELETED - existing resources that have been removed from the local bundle but still exist remotely. +// UPDATED - existing resources that have been modified. +// An empty status means that the resource is unchanged. +// We use these statuses to build git-status-like UI of the resources in the Databricks VSCode extension. type ModifiedStatus = string const ( From d133130f0cd481b4e7ed847df3bcf6a2d3eee87a Mon Sep 17 00:00:00 2001 From: Ilia Babanov Date: Mon, 22 Jan 2024 10:34:23 +0100 Subject: [PATCH 8/9] Explicitly require json output --- bundle/config/resources/modified_status.go | 6 +++--- cmd/bundle/summary.go | 19 ++++++++++++++----- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/bundle/config/resources/modified_status.go b/bundle/config/resources/modified_status.go index c10bc8f07e..d135c196dd 100644 --- a/bundle/config/resources/modified_status.go +++ b/bundle/config/resources/modified_status.go @@ -9,7 +9,7 @@ package resources type ModifiedStatus = string const ( - ModifiedStatusCreated ModifiedStatus = "CREATED" - ModifiedStatusUpdated ModifiedStatus = "UPDATED" - ModifiedStatusDeleted ModifiedStatus = "DELETED" + ModifiedStatusCreated ModifiedStatus = "created" + ModifiedStatusUpdated ModifiedStatus = "updated" + ModifiedStatusDeleted ModifiedStatus = "deleted" ) diff --git a/cmd/bundle/summary.go b/cmd/bundle/summary.go index a38732b49d..efa3c679d8 100644 --- a/cmd/bundle/summary.go +++ b/cmd/bundle/summary.go @@ -3,13 +3,15 @@ package bundle import ( "encoding/json" "errors" + "fmt" "os" "path/filepath" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/terraform" - "github.com/databricks/cli/bundle/phases" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/flags" "github.com/spf13/cobra" ) @@ -54,12 +56,19 @@ func newSummaryCommand() *cobra.Command { return err } - buf, err := json.MarshalIndent(b.Config, "", " ") - if err != nil { - return err + switch root.OutputType(cmd) { + case flags.OutputText: + return fmt.Errorf("%w, only json output is supported", errors.ErrUnsupported) + case flags.OutputJSON: + buf, err := json.MarshalIndent(b.Config, "", " ") + if err != nil { + return err + } + cmd.OutOrStdout().Write(buf) + default: + return fmt.Errorf("unknown output type %s", root.OutputType(cmd)) } - cmd.OutOrStdout().Write(buf) return nil } From b4054de8abac91291babe041277630e4f9fab7a8 Mon Sep 17 00:00:00 2001 From: Ilia Babanov Date: Tue, 23 Jan 2024 16:29:46 +0100 Subject: [PATCH 9/9] Remove early return to properly set created statuses --- bundle/deploy/terraform/convert.go | 149 ++++++++++++------------ bundle/deploy/terraform/convert_test.go | 74 ++++++++++++ 2 files changed, 147 insertions(+), 76 deletions(-) diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index b6fc2d18a3..6723caee31 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -229,84 +229,81 @@ func BundleToTerraform(config *config.Root) *schema.Root { } func TerraformToBundle(state *tfjson.State, config *config.Root) error { - // This is a no-op if the state is empty. - if state.Values == nil || state.Values.RootModule == nil { - return nil - } - - for _, resource := range state.Values.RootModule.Resources { - // Limit to resources. - if resource.Mode != tfjson.ManagedResourceMode { - continue - } - - switch resource.Type { - case "databricks_job": - var tmp schema.ResourceJob - conv(resource.AttributeValues, &tmp) - if config.Resources.Jobs == nil { - config.Resources.Jobs = make(map[string]*resources.Job) - } - cur := config.Resources.Jobs[resource.Name] - // TODO: make sure we can unmarshall tf state properly and don't swallow errors - modifiedStatus := convRemoteToLocal(tmp, &cur) - cur.ModifiedStatus = modifiedStatus - config.Resources.Jobs[resource.Name] = cur - case "databricks_pipeline": - var tmp schema.ResourcePipeline - conv(resource.AttributeValues, &tmp) - if config.Resources.Pipelines == nil { - config.Resources.Pipelines = make(map[string]*resources.Pipeline) - } - cur := config.Resources.Pipelines[resource.Name] - modifiedStatus := convRemoteToLocal(tmp, &cur) - cur.ModifiedStatus = modifiedStatus - config.Resources.Pipelines[resource.Name] = cur - case "databricks_mlflow_model": - var tmp schema.ResourceMlflowModel - conv(resource.AttributeValues, &tmp) - if config.Resources.Models == nil { - config.Resources.Models = make(map[string]*resources.MlflowModel) - } - cur := config.Resources.Models[resource.Name] - modifiedStatus := convRemoteToLocal(tmp, &cur) - cur.ModifiedStatus = modifiedStatus - config.Resources.Models[resource.Name] = cur - case "databricks_mlflow_experiment": - var tmp schema.ResourceMlflowExperiment - conv(resource.AttributeValues, &tmp) - if config.Resources.Experiments == nil { - config.Resources.Experiments = make(map[string]*resources.MlflowExperiment) + if state.Values != nil && state.Values.RootModule != nil { + for _, resource := range state.Values.RootModule.Resources { + // Limit to resources. + if resource.Mode != tfjson.ManagedResourceMode { + continue } - cur := config.Resources.Experiments[resource.Name] - modifiedStatus := convRemoteToLocal(tmp, &cur) - cur.ModifiedStatus = modifiedStatus - config.Resources.Experiments[resource.Name] = cur - case "databricks_model_serving": - var tmp schema.ResourceModelServing - conv(resource.AttributeValues, &tmp) - if config.Resources.ModelServingEndpoints == nil { - config.Resources.ModelServingEndpoints = make(map[string]*resources.ModelServingEndpoint) - } - cur := config.Resources.ModelServingEndpoints[resource.Name] - modifiedStatus := convRemoteToLocal(tmp, &cur) - cur.ModifiedStatus = modifiedStatus - config.Resources.ModelServingEndpoints[resource.Name] = cur - case "databricks_registered_model": - var tmp schema.ResourceRegisteredModel - conv(resource.AttributeValues, &tmp) - if config.Resources.RegisteredModels == nil { - config.Resources.RegisteredModels = make(map[string]*resources.RegisteredModel) + + switch resource.Type { + case "databricks_job": + var tmp schema.ResourceJob + conv(resource.AttributeValues, &tmp) + if config.Resources.Jobs == nil { + config.Resources.Jobs = make(map[string]*resources.Job) + } + cur := config.Resources.Jobs[resource.Name] + // TODO: make sure we can unmarshall tf state properly and don't swallow errors + modifiedStatus := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus + config.Resources.Jobs[resource.Name] = cur + case "databricks_pipeline": + var tmp schema.ResourcePipeline + conv(resource.AttributeValues, &tmp) + if config.Resources.Pipelines == nil { + config.Resources.Pipelines = make(map[string]*resources.Pipeline) + } + cur := config.Resources.Pipelines[resource.Name] + modifiedStatus := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus + config.Resources.Pipelines[resource.Name] = cur + case "databricks_mlflow_model": + var tmp schema.ResourceMlflowModel + conv(resource.AttributeValues, &tmp) + if config.Resources.Models == nil { + config.Resources.Models = make(map[string]*resources.MlflowModel) + } + cur := config.Resources.Models[resource.Name] + modifiedStatus := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus + config.Resources.Models[resource.Name] = cur + case "databricks_mlflow_experiment": + var tmp schema.ResourceMlflowExperiment + conv(resource.AttributeValues, &tmp) + if config.Resources.Experiments == nil { + config.Resources.Experiments = make(map[string]*resources.MlflowExperiment) + } + cur := config.Resources.Experiments[resource.Name] + modifiedStatus := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus + config.Resources.Experiments[resource.Name] = cur + case "databricks_model_serving": + var tmp schema.ResourceModelServing + conv(resource.AttributeValues, &tmp) + if config.Resources.ModelServingEndpoints == nil { + config.Resources.ModelServingEndpoints = make(map[string]*resources.ModelServingEndpoint) + } + cur := config.Resources.ModelServingEndpoints[resource.Name] + modifiedStatus := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus + config.Resources.ModelServingEndpoints[resource.Name] = cur + case "databricks_registered_model": + var tmp schema.ResourceRegisteredModel + conv(resource.AttributeValues, &tmp) + if config.Resources.RegisteredModels == nil { + config.Resources.RegisteredModels = make(map[string]*resources.RegisteredModel) + } + cur := config.Resources.RegisteredModels[resource.Name] + modifiedStatus := convRemoteToLocal(tmp, &cur) + cur.ModifiedStatus = modifiedStatus + config.Resources.RegisteredModels[resource.Name] = cur + case "databricks_permissions": + case "databricks_grants": + // Ignore; no need to pull these back into the configuration. + default: + return fmt.Errorf("missing mapping for %s", resource.Type) } - cur := config.Resources.RegisteredModels[resource.Name] - modifiedStatus := convRemoteToLocal(tmp, &cur) - cur.ModifiedStatus = modifiedStatus - config.Resources.RegisteredModels[resource.Name] = cur - case "databricks_permissions": - case "databricks_grants": - // Ignore; no need to pull these back into the configuration. - default: - return fmt.Errorf("missing mapping for %s", resource.Type) } } diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index 98bd919988..bb77f287bb 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -522,6 +522,80 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) { AssertFullResourceCoverage(t, &config) } +func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { + var config = config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "test_job": { + JobSettings: &jobs.JobSettings{ + Name: "test_job", + }, + }, + }, + Pipelines: map[string]*resources.Pipeline{ + "test_pipeline": { + PipelineSpec: &pipelines.PipelineSpec{ + Name: "test_pipeline", + }, + }, + }, + Models: map[string]*resources.MlflowModel{ + "test_mlflow_model": { + Model: &ml.Model{ + Name: "test_mlflow_model", + }, + }, + }, + Experiments: map[string]*resources.MlflowExperiment{ + "test_mlflow_experiment": { + Experiment: &ml.Experiment{ + Name: "test_mlflow_experiment", + }, + }, + }, + ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{ + "test_model_serving": { + CreateServingEndpoint: &serving.CreateServingEndpoint{ + Name: "test_model_serving", + }, + }, + }, + RegisteredModels: map[string]*resources.RegisteredModel{ + "test_registered_model": { + CreateRegisteredModelRequest: &catalog.CreateRegisteredModelRequest{ + Name: "test_registered_model", + }, + }, + }, + }, + } + var tfState = tfjson.State{ + Values: nil, + } + err := TerraformToBundle(&tfState, &config) + assert.NoError(t, err) + + assert.Equal(t, "", config.Resources.Jobs["test_job"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Jobs["test_job"].ModifiedStatus) + + assert.Equal(t, "", config.Resources.Pipelines["test_pipeline"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Pipelines["test_pipeline"].ModifiedStatus) + + assert.Equal(t, "", config.Resources.Models["test_mlflow_model"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Models["test_mlflow_model"].ModifiedStatus) + + assert.Equal(t, "", config.Resources.Experiments["test_mlflow_experiment"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Experiments["test_mlflow_experiment"].ModifiedStatus) + + assert.Equal(t, "", config.Resources.ModelServingEndpoints["test_model_serving"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.ModelServingEndpoints["test_model_serving"].ModifiedStatus) + + assert.Equal(t, "", config.Resources.RegisteredModels["test_registered_model"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.RegisteredModels["test_registered_model"].ModifiedStatus) + + AssertFullResourceCoverage(t, &config) +} + func TestTerraformToBundleModifiedResources(t *testing.T) { var config = config.Root{ Resources: config.Resources{