Use normalized short name for tag value in development mode (#821)

## Changes

The jobs backend propagates job tags to the underlying cloud provider's
resources. As such, they need to match the constraints a cloud provider
places on tag values. The display name can contain anything. With this
change, we modify the tag value to equal the short name as used in the
name prefix.

Additionally, we leverage tag normalization as introduced in #819 to
make sure characters that aren't accepted are removed before using the
value as a tag value.

This is a new stab at #810 and should completely eliminate this class of
problems.

## Tests

Tests pass.
This commit is contained in:
Pieter Noordhuis 2023-10-02 08:58:51 +02:00 committed by GitHub
parent 775251d0dc
commit f1b068cefe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 83 additions and 4 deletions

View File

@ -19,6 +19,7 @@ import (
"github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/locker"
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/tags"
"github.com/databricks/cli/libs/terraform"
"github.com/databricks/databricks-sdk-go"
sdkconfig "github.com/databricks/databricks-sdk-go/config"
@ -46,6 +47,10 @@ type Bundle struct {
// if true, we skip approval checks for deploy, destroy resources and delete
// files
AutoApprove bool
// Tagging is used to normalize tag keys and values.
// The implementation depends on the cloud being targeted.
Tagging tags.Cloud
}
func Load(ctx context.Context, path string) (*Bundle, error) {

View File

@ -7,6 +7,7 @@ import (
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/libs/tags"
)
type populateCurrentUser struct{}
@ -35,6 +36,10 @@ func (m *populateCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) error
ShortName: getShortUserName(me.UserName),
User: me,
}
// Configure tagging object now that we know we have a valid client.
b.Tagging = tags.ForCloud(w.Config)
return nil
}

View File

@ -32,14 +32,18 @@ func (m *processTargetMode) Name() string {
func transformDevelopmentMode(b *bundle.Bundle) error {
r := b.Config.Resources
prefix := "[dev " + b.Config.Workspace.CurrentUser.ShortName + "] "
shortName := b.Config.Workspace.CurrentUser.ShortName
prefix := "[dev " + shortName + "] "
// Generate a normalized version of the short name that can be used as a tag value.
tagValue := b.Tagging.NormalizeValue(shortName)
for i := range r.Jobs {
r.Jobs[i].Name = prefix + r.Jobs[i].Name
if r.Jobs[i].Tags == nil {
r.Jobs[i].Tags = make(map[string]string)
}
r.Jobs[i].Tags["dev"] = b.Config.Workspace.CurrentUser.DisplayName
r.Jobs[i].Tags["dev"] = tagValue
if r.Jobs[i].MaxConcurrentRuns == 0 {
r.Jobs[i].MaxConcurrentRuns = developmentConcurrentRuns
}
@ -74,7 +78,7 @@ func transformDevelopmentMode(b *bundle.Bundle) error {
} else {
r.Experiments[i].Name = dir + "/" + prefix + base
}
r.Experiments[i].Tags = append(r.Experiments[i].Tags, ml.ExperimentTag{Key: "dev", Value: b.Config.Workspace.CurrentUser.DisplayName})
r.Experiments[i].Tags = append(r.Experiments[i].Tags, ml.ExperimentTag{Key: "dev", Value: tagValue})
}
for i := range r.ModelServingEndpoints {

View File

@ -9,6 +9,8 @@ import (
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/libs/tags"
sdkconfig "github.com/databricks/databricks-sdk-go/config"
"github.com/databricks/databricks-sdk-go/service/iam"
"github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/databricks/databricks-sdk-go/service/ml"
@ -59,6 +61,10 @@ func mockBundle(mode config.Mode) *bundle.Bundle {
},
},
},
// Use AWS implementation for testing.
Tagging: tags.ForCloud(&sdkconfig.Config{
Host: "https://company.cloud.databricks.com",
}),
}
}
@ -68,14 +74,71 @@ func TestProcessTargetModeDevelopment(t *testing.T) {
m := ProcessTargetMode()
err := m.Apply(context.Background(), bundle)
require.NoError(t, err)
// Job 1
assert.Equal(t, "[dev lennart] job1", bundle.Config.Resources.Jobs["job1"].Name)
assert.Equal(t, bundle.Config.Resources.Jobs["job1"].Tags["dev"], "lennart")
// Pipeline 1
assert.Equal(t, "[dev lennart] pipeline1", bundle.Config.Resources.Pipelines["pipeline1"].Name)
assert.True(t, bundle.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development)
// Experiment 1
assert.Equal(t, "/Users/lennart.kats@databricks.com/[dev lennart] experiment1", bundle.Config.Resources.Experiments["experiment1"].Name)
assert.Contains(t, bundle.Config.Resources.Experiments["experiment1"].Experiment.Tags, ml.ExperimentTag{Key: "dev", Value: "lennart"})
// Experiment 2
assert.Equal(t, "[dev lennart] experiment2", bundle.Config.Resources.Experiments["experiment2"].Name)
assert.Contains(t, bundle.Config.Resources.Experiments["experiment2"].Experiment.Tags, ml.ExperimentTag{Key: "dev", Value: "lennart"})
// Model 1
assert.Equal(t, "[dev lennart] model1", bundle.Config.Resources.Models["model1"].Name)
// Model serving endpoint 1
assert.Equal(t, "dev_lennart_servingendpoint1", bundle.Config.Resources.ModelServingEndpoints["servingendpoint1"].Name)
assert.Equal(t, "dev", bundle.Config.Resources.Experiments["experiment1"].Experiment.Tags[0].Key)
assert.True(t, bundle.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development)
}
func TestProcessTargetModeDevelopmentTagNormalizationForAws(t *testing.T) {
bundle := mockBundle(config.Development)
bundle.Tagging = tags.ForCloud(&sdkconfig.Config{
Host: "https://dbc-XXXXXXXX-YYYY.cloud.databricks.com/",
})
bundle.Config.Workspace.CurrentUser.ShortName = "Héllö wörld?!"
err := ProcessTargetMode().Apply(context.Background(), bundle)
require.NoError(t, err)
// Assert that tag normalization took place.
assert.Equal(t, "Hello world__", bundle.Config.Resources.Jobs["job1"].Tags["dev"])
}
func TestProcessTargetModeDevelopmentTagNormalizationForAzure(t *testing.T) {
bundle := mockBundle(config.Development)
bundle.Tagging = tags.ForCloud(&sdkconfig.Config{
Host: "https://adb-xxx.y.azuredatabricks.net/",
})
bundle.Config.Workspace.CurrentUser.ShortName = "Héllö wörld?!"
err := ProcessTargetMode().Apply(context.Background(), bundle)
require.NoError(t, err)
// Assert that tag normalization took place (Azure allows more characters than AWS).
assert.Equal(t, "Héllö wörld?!", bundle.Config.Resources.Jobs["job1"].Tags["dev"])
}
func TestProcessTargetModeDevelopmentTagNormalizationForGcp(t *testing.T) {
bundle := mockBundle(config.Development)
bundle.Tagging = tags.ForCloud(&sdkconfig.Config{
Host: "https://123.4.gcp.databricks.com/",
})
bundle.Config.Workspace.CurrentUser.ShortName = "Héllö wörld?!"
err := ProcessTargetMode().Apply(context.Background(), bundle)
require.NoError(t, err)
// Assert that tag normalization took place.
assert.Equal(t, "Hello_world", bundle.Config.Resources.Jobs["job1"].Tags["dev"])
}
func TestProcessTargetModeDefault(t *testing.T) {

View File

@ -17,6 +17,7 @@ import (
"github.com/databricks/cli/bundle/config/mutator"
"github.com/databricks/cli/bundle/phases"
"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/tags"
"github.com/databricks/databricks-sdk-go"
workspaceConfig "github.com/databricks/databricks-sdk-go/config"
"github.com/databricks/databricks-sdk-go/service/iam"
@ -66,6 +67,7 @@ func assertBuiltinTemplateValid(t *testing.T, settings map[string]any, target st
// Apply initialize / validation mutators
b.Config.Workspace.CurrentUser = &bundleConfig.User{User: cachedUser}
b.Tagging = tags.ForCloud(w.Config)
b.WorkspaceClient()
b.Config.Bundle.Terraform = &bundleConfig.Terraform{
ExecPath: "sh",