This commit is contained in:
Lennart Kats (databricks) 2024-11-20 22:12:16 +05:30 committed by GitHub
commit ae74a8498d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 36 additions and 15 deletions

View File

@ -38,18 +38,27 @@ func overrideJobCompute(j *resources.Job, compute string) {
}
func (m *overrideCompute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
if b.Config.Bundle.Mode != config.Development {
var diags diag.Diagnostics
if b.Config.Bundle.Mode == config.Production {
if b.Config.Bundle.ClusterId != "" {
return diag.Errorf("cannot override compute for an target that does not use 'mode: development'")
// Overriding compute via a command-line flag for production works, but is not recommended.
diags = diags.Extend(diag.Warningf("overriding compute for a target that uses 'mode: production' is not recommended"))
}
if env.Get(ctx, "DATABRICKS_CLUSTER_ID") != "" {
// The DATABRICKS_CLUSTER_ID may be set by accident when doing a production deploy.
// Overriding the cluster in production is almost always a mistake since customers
// want consistency in production and not compute that is different each deploy.
// For this reason we log a warning and ignore the environment variable.
return diag.Warningf("the DATABRICKS_CLUSTER_ID variable is set but is ignored since the current target uses 'mode: production'")
}
return nil
}
if v := env.Get(ctx, "DATABRICKS_CLUSTER_ID"); v != "" {
b.Config.Bundle.ClusterId = v
}
if b.Config.Bundle.ClusterId == "" {
return nil
return diags
}
r := b.Config.Resources
@ -57,5 +66,5 @@ func (m *overrideCompute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diag
overrideJobCompute(r.Jobs[i], b.Config.Bundle.ClusterId)
}
return nil
return diags
}

View File

@ -14,7 +14,7 @@ import (
"github.com/stretchr/testify/require"
)
func TestOverrideDevelopment(t *testing.T) {
func TestOverrideComputeModeDevelopment(t *testing.T) {
t.Setenv("DATABRICKS_CLUSTER_ID", "")
b := &bundle.Bundle{
Config: config.Root{
@ -62,10 +62,13 @@ func TestOverrideDevelopment(t *testing.T) {
assert.Empty(t, b.Config.Resources.Jobs["job1"].Tasks[3].JobClusterKey)
}
func TestOverrideDevelopmentEnv(t *testing.T) {
func TestOverrideComputeModeDefault(t *testing.T) {
t.Setenv("DATABRICKS_CLUSTER_ID", "newClusterId")
b := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Mode: "",
},
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job1": {JobSettings: &jobs.JobSettings{
@ -86,11 +89,12 @@ func TestOverrideDevelopmentEnv(t *testing.T) {
m := mutator.OverrideCompute()
diags := bundle.Apply(context.Background(), b, m)
require.NoError(t, diags.Error())
assert.Equal(t, "cluster2", b.Config.Resources.Jobs["job1"].Tasks[1].ExistingClusterId)
require.Empty(t, diags)
assert.Equal(t, "newClusterId", b.Config.Resources.Jobs["job1"].Tasks[0].ExistingClusterId)
assert.Equal(t, "newClusterId", b.Config.Resources.Jobs["job1"].Tasks[1].ExistingClusterId)
}
func TestOverridePipelineTask(t *testing.T) {
func TestOverrideComputePipelineTask(t *testing.T) {
t.Setenv("DATABRICKS_CLUSTER_ID", "newClusterId")
b := &bundle.Bundle{
Config: config.Root{
@ -115,7 +119,7 @@ func TestOverridePipelineTask(t *testing.T) {
assert.Empty(t, b.Config.Resources.Jobs["job1"].Tasks[0].ExistingClusterId)
}
func TestOverrideForEachTask(t *testing.T) {
func TestOverrideComputeForEachTask(t *testing.T) {
t.Setenv("DATABRICKS_CLUSTER_ID", "newClusterId")
b := &bundle.Bundle{
Config: config.Root{
@ -140,10 +144,11 @@ func TestOverrideForEachTask(t *testing.T) {
assert.Empty(t, b.Config.Resources.Jobs["job1"].Tasks[0].ForEachTask.Task)
}
func TestOverrideProduction(t *testing.T) {
func TestOverrideComputeModeProduction(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Mode: config.Production,
ClusterId: "newClusterID",
},
Resources: config.Resources{
@ -166,13 +171,18 @@ func TestOverrideProduction(t *testing.T) {
m := mutator.OverrideCompute()
diags := bundle.Apply(context.Background(), b, m)
require.True(t, diags.HasError())
require.Len(t, diags, 1)
assert.Equal(t, "overriding compute for a target that uses 'mode: production' is not recommended", diags[0].Summary)
assert.Equal(t, "newClusterID", b.Config.Resources.Jobs["job1"].Tasks[0].ExistingClusterId)
}
func TestOverrideProductionEnv(t *testing.T) {
func TestOverrideComputeModeProductionIgnoresVariable(t *testing.T) {
t.Setenv("DATABRICKS_CLUSTER_ID", "newClusterId")
b := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Mode: config.Production,
},
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job1": {JobSettings: &jobs.JobSettings{
@ -193,5 +203,7 @@ func TestOverrideProductionEnv(t *testing.T) {
m := mutator.OverrideCompute()
diags := bundle.Apply(context.Background(), b, m)
require.NoError(t, diags.Error())
require.Len(t, diags, 1)
assert.Equal(t, "the DATABRICKS_CLUSTER_ID variable is set but is ignored since the current target uses 'mode: production'", diags[0].Summary)
assert.Equal(t, "cluster2", b.Config.Resources.Jobs["job1"].Tasks[1].ExistingClusterId)
}