Make bundle deploy work if no resources are defined (#767)

## Changes

This PR sets "resource" to nil in the terraform representation if no
resources are defined in the bundle configuration. This solves two
problems:

1. Makes bundle deploy work without any resources specified. 
2. Previously if a `resources` block was removed after a deployment,
that would fail with an error. Now the resources would get destroyed as
expected.

Also removes `TerraformHasNoResources` which is no longer needed.

## Tests
New e2e tests.
This commit is contained in:
shreyas-goenka 2023-09-14 00:50:37 +02:00 committed by GitHub
parent be55310cc9
commit fe32c46dc8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 138 additions and 23 deletions

View File

@ -38,10 +38,6 @@ type Bundle struct {
// Stores an initialized copy of this bundle's Terraform wrapper. // Stores an initialized copy of this bundle's Terraform wrapper.
Terraform *tfexec.Terraform Terraform *tfexec.Terraform
// Indicates that the Terraform definition based on this bundle is empty,
// i.e. that it would deploy no resources.
TerraformHasNoResources bool
// Stores the locker responsible for acquiring/releasing a deployment lock. // Stores the locker responsible for acquiring/releasing a deployment lock.
Locker *locker.Locker Locker *locker.Locker

View File

@ -16,10 +16,6 @@ func (w *apply) Name() string {
} }
func (w *apply) Apply(ctx context.Context, b *bundle.Bundle) error { func (w *apply) Apply(ctx context.Context, b *bundle.Bundle) error {
if b.TerraformHasNoResources {
cmdio.LogString(ctx, "Note: there are no resources to deploy for this bundle")
return nil
}
tf := b.Terraform tf := b.Terraform
if tf == nil { if tf == nil {
return fmt.Errorf("terraform not initialized") return fmt.Errorf("terraform not initialized")

View File

@ -49,7 +49,7 @@ func convPermission(ac resources.Permission) schema.ResourcePermissionsAccessCon
// //
// NOTE: THIS IS CURRENTLY A HACK. WE NEED A BETTER WAY TO // NOTE: THIS IS CURRENTLY A HACK. WE NEED A BETTER WAY TO
// CONVERT TO/FROM TERRAFORM COMPATIBLE FORMAT. // CONVERT TO/FROM TERRAFORM COMPATIBLE FORMAT.
func BundleToTerraform(config *config.Root) (*schema.Root, bool) { func BundleToTerraform(config *config.Root) *schema.Root {
tfroot := schema.NewRoot() tfroot := schema.NewRoot()
tfroot.Provider = schema.NewProviders() tfroot.Provider = schema.NewProviders()
tfroot.Resource = schema.NewResources() tfroot.Resource = schema.NewResources()
@ -174,7 +174,13 @@ func BundleToTerraform(config *config.Root) (*schema.Root, bool) {
} }
} }
return tfroot, noResources // We explicitly set "resource" to nil to omit it from a JSON encoding.
// This is required because the terraform CLI requires >= 1 resources defined
// if the "resource" property is used in a .tf.json file.
if noResources {
tfroot.Resource = nil
}
return tfroot
} }
func TerraformToBundle(state *tfjson.State, config *config.Root) error { func TerraformToBundle(state *tfjson.State, config *config.Root) error {

View File

@ -51,7 +51,7 @@ func TestConvertJob(t *testing.T) {
}, },
} }
out, _ := BundleToTerraform(&config) out := BundleToTerraform(&config)
assert.Equal(t, "my job", out.Resource.Job["my_job"].Name) assert.Equal(t, "my job", out.Resource.Job["my_job"].Name)
assert.Len(t, out.Resource.Job["my_job"].JobCluster, 1) assert.Len(t, out.Resource.Job["my_job"].JobCluster, 1)
assert.Equal(t, "https://github.com/foo/bar", out.Resource.Job["my_job"].GitSource.Url) assert.Equal(t, "https://github.com/foo/bar", out.Resource.Job["my_job"].GitSource.Url)
@ -79,7 +79,7 @@ func TestConvertJobPermissions(t *testing.T) {
}, },
} }
out, _ := BundleToTerraform(&config) out := BundleToTerraform(&config)
assert.NotEmpty(t, out.Resource.Permissions["job_my_job"].JobId) assert.NotEmpty(t, out.Resource.Permissions["job_my_job"].JobId)
assert.Len(t, out.Resource.Permissions["job_my_job"].AccessControl, 1) assert.Len(t, out.Resource.Permissions["job_my_job"].AccessControl, 1)
@ -115,7 +115,7 @@ func TestConvertJobTaskLibraries(t *testing.T) {
}, },
} }
out, _ := BundleToTerraform(&config) out := BundleToTerraform(&config)
assert.Equal(t, "my job", out.Resource.Job["my_job"].Name) assert.Equal(t, "my job", out.Resource.Job["my_job"].Name)
require.Len(t, out.Resource.Job["my_job"].Task, 1) require.Len(t, out.Resource.Job["my_job"].Task, 1)
require.Len(t, out.Resource.Job["my_job"].Task[0].Library, 1) require.Len(t, out.Resource.Job["my_job"].Task[0].Library, 1)
@ -149,7 +149,7 @@ func TestConvertPipeline(t *testing.T) {
}, },
} }
out, _ := BundleToTerraform(&config) out := BundleToTerraform(&config)
assert.Equal(t, "my pipeline", out.Resource.Pipeline["my_pipeline"].Name) assert.Equal(t, "my pipeline", out.Resource.Pipeline["my_pipeline"].Name)
assert.Len(t, out.Resource.Pipeline["my_pipeline"].Library, 2) assert.Len(t, out.Resource.Pipeline["my_pipeline"].Library, 2)
assert.Nil(t, out.Data) assert.Nil(t, out.Data)
@ -173,7 +173,7 @@ func TestConvertPipelinePermissions(t *testing.T) {
}, },
} }
out, _ := BundleToTerraform(&config) out := BundleToTerraform(&config)
assert.NotEmpty(t, out.Resource.Permissions["pipeline_my_pipeline"].PipelineId) assert.NotEmpty(t, out.Resource.Permissions["pipeline_my_pipeline"].PipelineId)
assert.Len(t, out.Resource.Permissions["pipeline_my_pipeline"].AccessControl, 1) assert.Len(t, out.Resource.Permissions["pipeline_my_pipeline"].AccessControl, 1)
@ -208,7 +208,7 @@ func TestConvertModel(t *testing.T) {
}, },
} }
out, _ := BundleToTerraform(&config) out := BundleToTerraform(&config)
assert.Equal(t, "name", out.Resource.MlflowModel["my_model"].Name) assert.Equal(t, "name", out.Resource.MlflowModel["my_model"].Name)
assert.Equal(t, "description", out.Resource.MlflowModel["my_model"].Description) assert.Equal(t, "description", out.Resource.MlflowModel["my_model"].Description)
assert.Len(t, out.Resource.MlflowModel["my_model"].Tags, 2) assert.Len(t, out.Resource.MlflowModel["my_model"].Tags, 2)
@ -237,7 +237,7 @@ func TestConvertModelPermissions(t *testing.T) {
}, },
} }
out, _ := BundleToTerraform(&config) out := BundleToTerraform(&config)
assert.NotEmpty(t, out.Resource.Permissions["mlflow_model_my_model"].RegisteredModelId) assert.NotEmpty(t, out.Resource.Permissions["mlflow_model_my_model"].RegisteredModelId)
assert.Len(t, out.Resource.Permissions["mlflow_model_my_model"].AccessControl, 1) assert.Len(t, out.Resource.Permissions["mlflow_model_my_model"].AccessControl, 1)
@ -261,7 +261,7 @@ func TestConvertExperiment(t *testing.T) {
}, },
} }
out, _ := BundleToTerraform(&config) out := BundleToTerraform(&config)
assert.Equal(t, "name", out.Resource.MlflowExperiment["my_experiment"].Name) assert.Equal(t, "name", out.Resource.MlflowExperiment["my_experiment"].Name)
assert.Nil(t, out.Data) assert.Nil(t, out.Data)
} }
@ -284,7 +284,7 @@ func TestConvertExperimentPermissions(t *testing.T) {
}, },
} }
out, _ := BundleToTerraform(&config) out := BundleToTerraform(&config)
assert.NotEmpty(t, out.Resource.Permissions["mlflow_experiment_my_experiment"].ExperimentId) assert.NotEmpty(t, out.Resource.Permissions["mlflow_experiment_my_experiment"].ExperimentId)
assert.Len(t, out.Resource.Permissions["mlflow_experiment_my_experiment"].AccessControl, 1) assert.Len(t, out.Resource.Permissions["mlflow_experiment_my_experiment"].AccessControl, 1)
@ -327,7 +327,7 @@ func TestConvertModelServing(t *testing.T) {
}, },
} }
out, _ := BundleToTerraform(&config) out := BundleToTerraform(&config)
resource := out.Resource.ModelServing["my_model_serving_endpoint"] resource := out.Resource.ModelServing["my_model_serving_endpoint"]
assert.Equal(t, "name", resource.Name) assert.Equal(t, "name", resource.Name)
assert.Equal(t, "model_name", resource.Config.ServedModels[0].ModelName) assert.Equal(t, "model_name", resource.Config.ServedModels[0].ModelName)
@ -357,7 +357,7 @@ func TestConvertModelServingPermissions(t *testing.T) {
}, },
} }
out, _ := BundleToTerraform(&config) out := BundleToTerraform(&config)
assert.NotEmpty(t, out.Resource.Permissions["model_serving_my_model_serving_endpoint"].ServingEndpointId) assert.NotEmpty(t, out.Resource.Permissions["model_serving_my_model_serving_endpoint"].ServingEndpointId)
assert.Len(t, out.Resource.Permissions["model_serving_my_model_serving_endpoint"].AccessControl, 1) assert.Len(t, out.Resource.Permissions["model_serving_my_model_serving_endpoint"].AccessControl, 1)

View File

@ -21,8 +21,7 @@ func (w *write) Apply(ctx context.Context, b *bundle.Bundle) error {
return err return err
} }
root, noResources := BundleToTerraform(&b.Config) root := BundleToTerraform(&b.Config)
b.TerraformHasNoResources = noResources
f, err := os.Create(filepath.Join(dir, "bundle.tf.json")) f, err := os.Create(filepath.Join(dir, "bundle.tf.json"))
if err != nil { if err != nil {
return err return err

View File

@ -0,0 +1,8 @@
{
"properties": {
"unique_id": {
"type": "string",
"description": "Unique ID for job name"
}
}
}

View File

@ -0,0 +1,8 @@
bundle:
name: deploy-then-remove
workspace:
root_path: "~/.bundle/{{.unique_id}}"
include:
- "./*.yml"

View File

@ -0,0 +1 @@
print("hello")

View File

@ -0,0 +1,7 @@
resources:
pipelines:
bar:
name: test-bundle-pipeline-{{.unique_id}}
libraries:
- notebook:
path: "./foo.py"

View File

@ -0,0 +1,2 @@
bundle:
name: abc

View File

@ -0,0 +1,55 @@
package bundle
import (
"context"
"os"
"path/filepath"
"testing"
"github.com/databricks/cli/internal"
"github.com/databricks/databricks-sdk-go"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestAccBundleDeployThenRemoveResources(t *testing.T) {
env := internal.GetEnvOrSkipTest(t, "CLOUD_ENV")
t.Log(env)
uniqueId := uuid.New().String()
bundleRoot, err := initTestTemplate(t, "deploy_then_remove_resources", map[string]any{
"unique_id": uniqueId,
})
require.NoError(t, err)
// deploy pipeline
err = deployBundle(t, bundleRoot)
require.NoError(t, err)
w, err := databricks.NewWorkspaceClient()
require.NoError(t, err)
// assert pipeline is created
pipelineName := "test-bundle-pipeline-" + uniqueId
pipeline, err := w.Pipelines.GetByName(context.Background(), pipelineName)
require.NoError(t, err)
assert.Equal(t, pipeline.Name, pipelineName)
// delete resources.yml
err = os.Remove(filepath.Join(bundleRoot, "resources.yml"))
require.NoError(t, err)
// deploy again
err = deployBundle(t, bundleRoot)
require.NoError(t, err)
// assert pipeline is deleted
_, err = w.Pipelines.GetByName(context.Background(), pipelineName)
assert.ErrorContains(t, err, "does not exist")
t.Cleanup(func() {
err = destroyBundle(t, bundleRoot)
require.NoError(t, err)
})
}

View File

@ -0,0 +1,37 @@
package bundle
import (
"fmt"
"os"
"path/filepath"
"testing"
"github.com/databricks/cli/internal"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
)
func TestAccEmptyBundleDeploy(t *testing.T) {
env := internal.GetEnvOrSkipTest(t, "CLOUD_ENV")
t.Log(env)
// create empty bundle
tmpDir := t.TempDir()
f, err := os.Create(filepath.Join(tmpDir, "databricks.yml"))
require.NoError(t, err)
bundleRoot := fmt.Sprintf(`bundle:
name: %s`, uuid.New().String())
_, err = f.WriteString(bundleRoot)
require.NoError(t, err)
f.Close()
// deploy empty bundle
err = deployBundle(t, tmpDir)
require.NoError(t, err)
t.Cleanup(func() {
err = destroyBundle(t, tmpDir)
require.NoError(t, err)
})
}