Merge c4301b7a44 into 984c38e03e

bundle/config/mutator/override_compute.go

@@ -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
 }

bundle/config/mutator/override_compute_test.go

@@ -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)
 }