Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DABs support for Unity Catalog volumes #1762

Merged
merged 78 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 75 commits
Commits
Show all changes
78 commits
Select commit Hold shift + click to select a range
f772ce4
first comment
shreyas-goenka Sep 9, 2024
8f4f3ae
Merge remote-tracking branch 'origin' into feature/uc-volumes
shreyas-goenka Sep 9, 2024
ce5792c
fix convertor and add unit test
shreyas-goenka Sep 9, 2024
7c7abef
run as support
shreyas-goenka Sep 9, 2024
d04b6b0
-
shreyas-goenka Sep 9, 2024
9b66cd5
add apply target mode prefix functionality
shreyas-goenka Sep 9, 2024
4b22e2d
add conversion tests
shreyas-goenka Sep 9, 2024
88d0402
add inteprolation for volumes fields
shreyas-goenka Sep 9, 2024
6f9817e
add prompt and crud test for volumes
shreyas-goenka Sep 10, 2024
d47b0d6
Merge remote-tracking branch 'origin' into feature/uc-volumes
shreyas-goenka Sep 10, 2024
d180bab
add filer
shreyas-goenka Sep 15, 2024
fa54577
Merge remote-tracking branch 'origin' into feature/uc-volumes
shreyas-goenka Sep 15, 2024
73826ac
fix test and improve error
shreyas-goenka Sep 15, 2024
de7eb94
-
shreyas-goenka Sep 15, 2024
f10038a
-
shreyas-goenka Sep 15, 2024
df3bbad
add integration tests for artifacts portion:
shreyas-goenka Sep 15, 2024
aeab4ef
unit test for comparision of locatoin
shreyas-goenka Sep 15, 2024
aa2e16d
cleanup and add test
shreyas-goenka Sep 16, 2024
a90eb57
fix unit tests
shreyas-goenka Sep 16, 2024
39cb5e8
fix test on windows
shreyas-goenka Sep 16, 2024
13748f1
cleanup todos
shreyas-goenka Sep 16, 2024
bdecd08
-
shreyas-goenka Sep 16, 2024
274fd63
iter
shreyas-goenka Sep 16, 2024
d3d5d4c
-
shreyas-goenka Sep 16, 2024
227dfe9
fixes
shreyas-goenka Sep 16, 2024
e43f566
Merge remote-tracking branch 'origin' into feature/uc-volumes
shreyas-goenka Oct 14, 2024
f919e94
typo fix
shreyas-goenka Oct 14, 2024
9921263
separate GetFilerForLibraries tests
shreyas-goenka Oct 14, 2024
266c26c
fix tesT
shreyas-goenka Oct 14, 2024
a9b8575
fmt and fix test
shreyas-goenka Oct 14, 2024
c5a02ef
split into filer files
shreyas-goenka Oct 15, 2024
eb94cd6
-
shreyas-goenka Oct 15, 2024
3e3ddfd
fix test
shreyas-goenka Oct 15, 2024
d241c2b
add integration test for grant on volume
shreyas-goenka Oct 15, 2024
6192835
add custom prefixing behaviour for volumes
shreyas-goenka Oct 15, 2024
701b178
fmt
shreyas-goenka Oct 15, 2024
810da66
-
shreyas-goenka Oct 16, 2024
1a961eb
-
shreyas-goenka Oct 16, 2024
8a2fe49
merge
shreyas-goenka Oct 29, 2024
49b2cf2
Merge remote-tracking branch 'origin' into feature/uc-volumes
shreyas-goenka Oct 31, 2024
e32ebd0
use IsVolumesPath
shreyas-goenka Oct 31, 2024
6b12234
better message
shreyas-goenka Oct 31, 2024
f9287e0
address comments
shreyas-goenka Oct 31, 2024
250d426
rename to volume
shreyas-goenka Oct 31, 2024
1218178
-
shreyas-goenka Oct 31, 2024
68dc6c1
Merge remote-tracking branch 'origin' into feature/uc-volumes
shreyas-goenka Nov 18, 2024
4cc2790
remove prefixing for uc volumes
shreyas-goenka Nov 18, 2024
0406265
-
shreyas-goenka Nov 18, 2024
b0e527e
-
shreyas-goenka Nov 18, 2024
e6723de
undo containsUsername
shreyas-goenka Nov 18, 2024
f5ea8da
remove other mutator
shreyas-goenka Nov 18, 2024
ea6906e
add support for initializeUrl
shreyas-goenka Nov 18, 2024
76092cc
remove todo
shreyas-goenka Nov 18, 2024
039057f
fix renaming test
shreyas-goenka Nov 18, 2024
4f5f9ec
Merge remote-tracking branch 'origin' into feature/uc-volumes
shreyas-goenka Nov 29, 2024
5ec784b
merge
shreyas-goenka Nov 29, 2024
fcb8fff
-
shreyas-goenka Nov 29, 2024
e9a5544
-
shreyas-goenka Nov 29, 2024
5531e2a
-
shreyas-goenka Nov 29, 2024
3090588
Update bundle/libraries/filer.go
shreyas-goenka Nov 29, 2024
015f8cd
Update bundle/libraries/filer_volume.go
shreyas-goenka Nov 29, 2024
d0385a0
add extractVolumeFromPath
shreyas-goenka Nov 29, 2024
5ac2d67
-
shreyas-goenka Nov 29, 2024
9493795
address comments
shreyas-goenka Nov 29, 2024
e51b3a1
add const for internal
shreyas-goenka Nov 29, 2024
01c3830
add TestFilerForWorkspace
shreyas-goenka Nov 29, 2024
e5f5618
-
shreyas-goenka Nov 29, 2024
23e87d5
-
shreyas-goenka Nov 29, 2024
d1ec088
remove defensive bit
shreyas-goenka Nov 29, 2024
42bf6ae
revert bundletest move
shreyas-goenka Dec 2, 2024
5f2db1e
move .internal var
shreyas-goenka Dec 2, 2024
07f888c
remove UC
shreyas-goenka Dec 2, 2024
7d544f4
lowercase volumes
shreyas-goenka Dec 2, 2024
406c073
use apierr.ErrNotFound
shreyas-goenka Dec 2, 2024
3461018
better error message
shreyas-goenka Dec 2, 2024
8e25bb4
Merge remote-tracking branch 'origin' into feature/uc-volumes
shreyas-goenka Dec 2, 2024
8d790ef
address comments
shreyas-goenka Dec 2, 2024
d460bd6
fix
shreyas-goenka Dec 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions bundle/artifacts/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,13 @@ func (m *cleanUp) Name() string {
}

func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
uploadPath, err := libraries.GetUploadBasePath(b)
if err != nil {
return diag.FromErr(err)
}

client, err := libraries.GetFilerForLibraries(b.WorkspaceClient(), uploadPath)
if err != nil {
return diag.FromErr(err)
client, uploadPath, diags := libraries.GetFilerForLibraries(ctx, b)
if diags.HasError() {
return diags
}

// We intentionally ignore the error because it is not critical to the deployment
err = client.Delete(ctx, ".", filer.DeleteRecursively)
err := client.Delete(ctx, ".", filer.DeleteRecursively)
if err != nil {
log.Errorf(ctx, "failed to delete %s: %v", uploadPath, err)
}
Expand Down
32 changes: 31 additions & 1 deletion bundle/config/mutator/apply_presets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestApplyPresetsPrefix(t *testing.T) {
}
}

func TestApplyPresetsPrefixForUcSchema(t *testing.T) {
func TestApplyPresetsPrefixForSchema(t *testing.T) {
tests := []struct {
name string
prefix string
Expand Down Expand Up @@ -129,6 +129,36 @@ func TestApplyPresetsPrefixForUcSchema(t *testing.T) {
}
}

func TestApplyPresetsVolumesShouldNotBePrefixed(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Resources: config.Resources{
Volumes: map[string]*resources.Volume{
"volume1": {
CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
Name: "volume1",
CatalogName: "catalog1",
SchemaName: "schema1",
},
},
},
},
Presets: config.Presets{
NamePrefix: "[prefix]",
},
},
}

ctx := context.Background()
diag := bundle.Apply(ctx, b, mutator.ApplyPresets())

if diag.HasError() {
t.Fatalf("unexpected error: %v", diag)
}

require.Equal(t, "volume1", b.Config.Resources.Volumes["volume1"].Name)
}

func TestApplyPresetsTags(t *testing.T) {
tests := []struct {
name string
Expand Down
36 changes: 25 additions & 11 deletions bundle/config/mutator/process_target_mode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"reflect"
"runtime"
"strings"
"slices"
"testing"

"github.com/databricks/cli/bundle"
Expand Down Expand Up @@ -131,6 +131,9 @@ func mockBundle(mode config.Mode) *bundle.Bundle {
Schemas: map[string]*resources.Schema{
"schema1": {CreateSchema: &catalog.CreateSchema{Name: "schema1"}},
},
Volumes: map[string]*resources.Volume{
"volume1": {CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{Name: "volume1"}},
},
Clusters: map[string]*resources.Cluster{
"cluster1": {ClusterSpec: &compute.ClusterSpec{ClusterName: "cluster1", SparkVersion: "13.2.x", NumWorkers: 1}},
},
Expand Down Expand Up @@ -311,6 +314,8 @@ func TestProcessTargetModeDefault(t *testing.T) {
assert.Equal(t, "servingendpoint1", b.Config.Resources.ModelServingEndpoints["servingendpoint1"].Name)
assert.Equal(t, "registeredmodel1", b.Config.Resources.RegisteredModels["registeredmodel1"].Name)
assert.Equal(t, "qualityMonitor1", b.Config.Resources.QualityMonitors["qualityMonitor1"].TableName)
assert.Equal(t, "schema1", b.Config.Resources.Schemas["schema1"].Name)
assert.Equal(t, "volume1", b.Config.Resources.Volumes["volume1"].Name)
assert.Equal(t, "cluster1", b.Config.Resources.Clusters["cluster1"].ClusterName)
}

Expand Down Expand Up @@ -355,6 +360,8 @@ func TestProcessTargetModeProduction(t *testing.T) {
assert.Equal(t, "servingendpoint1", b.Config.Resources.ModelServingEndpoints["servingendpoint1"].Name)
assert.Equal(t, "registeredmodel1", b.Config.Resources.RegisteredModels["registeredmodel1"].Name)
assert.Equal(t, "qualityMonitor1", b.Config.Resources.QualityMonitors["qualityMonitor1"].TableName)
assert.Equal(t, "schema1", b.Config.Resources.Schemas["schema1"].Name)
assert.Equal(t, "volume1", b.Config.Resources.Volumes["volume1"].Name)
assert.Equal(t, "cluster1", b.Config.Resources.Clusters["cluster1"].ClusterName)
}

Expand Down Expand Up @@ -388,10 +395,17 @@ func TestAllResourcesMocked(t *testing.T) {
}
}

// Make sure that we at least rename all resources
func TestAllResourcesRenamed(t *testing.T) {
// Make sure that we at rename all non UC resources
func TestAllNonUcResourcesAreRenamed(t *testing.T) {
b := mockBundle(config.Development)

// UC resources should not have a prefix added to their name. Right now
// this list only contains the Volume resource since we have yet to remove
// prefixing support for UC schemas and registered models.
ucFields := []reflect.Type{
reflect.TypeOf(&resources.Volume{}),
}

m := bundle.Seq(ProcessTargetMode(), ApplyPresets())
diags := bundle.Apply(context.Background(), b, m)
require.NoError(t, diags.Error())
Expand All @@ -404,14 +418,14 @@ func TestAllResourcesRenamed(t *testing.T) {
for _, key := range field.MapKeys() {
resource := field.MapIndex(key)
nameField := resource.Elem().FieldByName("Name")
if nameField.IsValid() && nameField.Kind() == reflect.String {
assert.True(
t,
strings.Contains(nameField.String(), "dev"),
"process_target_mode should rename '%s' in '%s'",
key,
resources.Type().Field(i).Name,
)
if !nameField.IsValid() || nameField.Kind() != reflect.String {
continue
}

if slices.Contains(ucFields, resource.Type()) {
assert.NotContains(t, nameField.String(), "dev", "process_target_mode should not rename '%s' in '%s'", key, resources.Type().Field(i).Name)
} else {
assert.Contains(t, nameField.String(), "dev", "process_target_mode should rename '%s' in '%s'", key, resources.Type().Field(i).Name)
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions bundle/config/mutator/run_as_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func allResourceTypes(t *testing.T) []string {
"quality_monitors",
"registered_models",
"schemas",
"volumes",
},
resourceTypes,
)
Expand Down Expand Up @@ -141,6 +142,7 @@ func TestRunAsErrorForUnsupportedResources(t *testing.T) {
"registered_models",
"experiments",
"schemas",
"volumes",
}

base := config.Root{
Expand Down
8 changes: 8 additions & 0 deletions bundle/config/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type Resources struct {
RegisteredModels map[string]*resources.RegisteredModel `json:"registered_models,omitempty"`
QualityMonitors map[string]*resources.QualityMonitor `json:"quality_monitors,omitempty"`
Schemas map[string]*resources.Schema `json:"schemas,omitempty"`
Volumes map[string]*resources.Volume `json:"volumes,omitempty"`
Clusters map[string]*resources.Cluster `json:"clusters,omitempty"`
Dashboards map[string]*resources.Dashboard `json:"dashboards,omitempty"`
}
Expand Down Expand Up @@ -85,6 +86,7 @@ func (r *Resources) AllResources() []ResourceGroup {
collectResourceMap(descriptions["schemas"], r.Schemas),
collectResourceMap(descriptions["clusters"], r.Clusters),
collectResourceMap(descriptions["dashboards"], r.Dashboards),
collectResourceMap(descriptions["volumes"], r.Volumes),
}
}

Expand Down Expand Up @@ -189,5 +191,11 @@ func SupportedResources() map[string]ResourceDescription {
SingularTitle: "Dashboard",
PluralTitle: "Dashboards",
},
"volumes": {
SingularName: "volume",
PluralName: "volumes",
SingularTitle: "Volume",
PluralTitle: "Volumes",
},
}
}
62 changes: 62 additions & 0 deletions bundle/config/resources/volume.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package resources

import (
"context"
"fmt"
"net/url"
"strings"

"github.com/databricks/databricks-sdk-go"
"github.com/databricks/databricks-sdk-go/marshal"
"github.com/databricks/databricks-sdk-go/service/catalog"
)

type Volume struct {
// List of grants to apply on this volume.
Grants []Grant `json:"grants,omitempty"`

// Full name of the volume (catalog_name.schema_name.volume_name). This value is read from
// the terraform state after deployment succeeds.
ID string `json:"id,omitempty" bundle:"readonly"`

*catalog.CreateVolumeRequestContent
shreyas-goenka marked this conversation as resolved.
Show resolved Hide resolved

ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"`
URL string `json:"url,omitempty" bundle:"internal"`
}

func (v *Volume) UnmarshalJSON(b []byte) error {
return marshal.Unmarshal(b, v)
}

func (v Volume) MarshalJSON() ([]byte, error) {
return marshal.Marshal(v)
}

func (v *Volume) Exists(ctx context.Context, w *databricks.WorkspaceClient, id string) (bool, error) {
return false, fmt.Errorf("volume.Exists() is not supported")
}

func (v *Volume) TerraformResourceName() string {
return "databricks_volume"
}

func (v *Volume) InitializeURL(baseURL url.URL) {
if v.ID == "" {
return
}
baseURL.Path = fmt.Sprintf("explore/data/volumes/%s", strings.ReplaceAll(v.ID, ".", "/"))
v.URL = baseURL.String()
}

func (v *Volume) GetURL() string {
return v.URL
}

func (v *Volume) GetName() string {
return v.Name
}

func (v *Volume) IsNil() bool {
return v.CreateVolumeRequestContent == nil
}
15 changes: 15 additions & 0 deletions bundle/deploy/terraform/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,16 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error {
}
cur.ID = instance.Attributes.ID
config.Resources.Schemas[resource.Name] = cur
case "databricks_volume":
if config.Resources.Volumes == nil {
config.Resources.Volumes = make(map[string]*resources.Volume)
}
cur := config.Resources.Volumes[resource.Name]
if cur == nil {
cur = &resources.Volume{ModifiedStatus: resources.ModifiedStatusDeleted}
}
cur.ID = instance.Attributes.ID
config.Resources.Volumes[resource.Name] = cur
case "databricks_cluster":
if config.Resources.Clusters == nil {
config.Resources.Clusters = make(map[string]*resources.Cluster)
Expand Down Expand Up @@ -235,6 +245,11 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error {
src.ModifiedStatus = resources.ModifiedStatusCreated
}
}
for _, src := range config.Resources.Volumes {
if src.ModifiedStatus == "" && src.ID == "" {
src.ModifiedStatus = resources.ModifiedStatusCreated
}
}
for _, src := range config.Resources.Clusters {
if src.ModifiedStatus == "" && src.ID == "" {
src.ModifiedStatus = resources.ModifiedStatusCreated
Expand Down
56 changes: 56 additions & 0 deletions bundle/deploy/terraform/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,14 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) {
{Attributes: stateInstanceAttributes{ID: "1"}},
},
},
{
Type: "databricks_volume",
Mode: "managed",
Name: "test_volume",
Instances: []stateResourceInstance{
{Attributes: stateInstanceAttributes{ID: "1"}},
},
},
{
Type: "databricks_cluster",
Mode: "managed",
Expand Down Expand Up @@ -715,6 +723,9 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) {
assert.Equal(t, "1", config.Resources.Schemas["test_schema"].ID)
assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Schemas["test_schema"].ModifiedStatus)

assert.Equal(t, "1", config.Resources.Volumes["test_volume"].ID)
assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Volumes["test_volume"].ModifiedStatus)

assert.Equal(t, "1", config.Resources.Clusters["test_cluster"].ID)
assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Clusters["test_cluster"].ModifiedStatus)

Expand Down Expand Up @@ -783,6 +794,13 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) {
},
},
},
Volumes: map[string]*resources.Volume{
"test_volume": {
CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
Name: "test_volume",
},
},
},
Clusters: map[string]*resources.Cluster{
"test_cluster": {
ClusterSpec: &compute.ClusterSpec{
Expand Down Expand Up @@ -829,6 +847,9 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) {
assert.Equal(t, "", config.Resources.Schemas["test_schema"].ID)
assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Schemas["test_schema"].ModifiedStatus)

assert.Equal(t, "", config.Resources.Volumes["test_volume"].ID)
assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Volumes["test_volume"].ModifiedStatus)

assert.Equal(t, "", config.Resources.Clusters["test_cluster"].ID)
assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Clusters["test_cluster"].ModifiedStatus)

Expand Down Expand Up @@ -937,6 +958,18 @@ func TestTerraformToBundleModifiedResources(t *testing.T) {
},
},
},
Volumes: map[string]*resources.Volume{
"test_volume": {
CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
Name: "test_volume",
},
},
"test_volume_new": {
CreateVolumeRequestContent: &catalog.CreateVolumeRequestContent{
Name: "test_volume_new",
},
},
},
Clusters: map[string]*resources.Cluster{
"test_cluster": {
ClusterSpec: &compute.ClusterSpec{
Expand Down Expand Up @@ -1093,6 +1126,22 @@ func TestTerraformToBundleModifiedResources(t *testing.T) {
{Attributes: stateInstanceAttributes{ID: "2"}},
},
},
{
Type: "databricks_volume",
Mode: "managed",
Name: "test_volume",
Instances: []stateResourceInstance{
{Attributes: stateInstanceAttributes{ID: "1"}},
},
},
{
Type: "databricks_volume",
Mode: "managed",
Name: "test_volume_old",
Instances: []stateResourceInstance{
{Attributes: stateInstanceAttributes{ID: "2"}},
},
},
{
Type: "databricks_cluster",
Mode: "managed",
Expand Down Expand Up @@ -1186,6 +1235,13 @@ func TestTerraformToBundleModifiedResources(t *testing.T) {
assert.Equal(t, "", config.Resources.Schemas["test_schema_new"].ID)
assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Schemas["test_schema_new"].ModifiedStatus)

assert.Equal(t, "1", config.Resources.Volumes["test_volume"].ID)
assert.Equal(t, "", config.Resources.Volumes["test_volume"].ModifiedStatus)
assert.Equal(t, "2", config.Resources.Volumes["test_volume_old"].ID)
assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Volumes["test_volume_old"].ModifiedStatus)
assert.Equal(t, "", config.Resources.Volumes["test_volume_new"].ID)
assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Volumes["test_volume_new"].ModifiedStatus)

assert.Equal(t, "1", config.Resources.Clusters["test_cluster"].ID)
assert.Equal(t, "", config.Resources.Clusters["test_cluster"].ModifiedStatus)
assert.Equal(t, "2", config.Resources.Clusters["test_cluster_old"].ID)
Expand Down
2 changes: 2 additions & 0 deletions bundle/deploy/terraform/interpolate.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ func (m *interpolateMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.D
path = dyn.NewPath(dyn.Key("databricks_quality_monitor")).Append(path[2:]...)
case dyn.Key("schemas"):
path = dyn.NewPath(dyn.Key("databricks_schema")).Append(path[2:]...)
case dyn.Key("volumes"):
path = dyn.NewPath(dyn.Key("databricks_volume")).Append(path[2:]...)
case dyn.Key("clusters"):
path = dyn.NewPath(dyn.Key("databricks_cluster")).Append(path[2:]...)
case dyn.Key("dashboards"):
Expand Down
Loading
Loading