diff --git a/bundle/apps/upload_config.go b/bundle/apps/upload_config.go new file mode 100644 index 000000000..4f70976a3 --- /dev/null +++ b/bundle/apps/upload_config.go @@ -0,0 +1,105 @@ +package apps + +import ( + "bytes" + "context" + "fmt" + "path" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/deploy" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/filer" + "golang.org/x/sync/errgroup" + + "gopkg.in/yaml.v3" +) + +type uploadConfig struct { + filerFactory deploy.FilerFactory +} + +func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + errGroup, ctx := errgroup.WithContext(ctx) + + for key, app := range b.Config.Resources.Apps { + // If the app has a config, we need to deploy it first. + // It means we need to write app.yml file with the content of the config field + // to the remote source code path of the app. + if app.Config != nil { + if !strings.HasPrefix(app.SourceCodePath, b.Config.Workspace.FilePath) { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "App source code invalid", + Detail: fmt.Sprintf("App source code path %s is not within file path %s", app.SourceCodePath, b.Config.Workspace.FilePath), + Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s.source_code_path", key)), + }) + + continue + } + + appPath := strings.TrimPrefix(app.SourceCodePath, b.Config.Workspace.FilePath) + + buf, err := configToYaml(app) + if err != nil { + return diag.FromErr(err) + } + + // When the app is started, create a new app deployment and wait for it to complete. + f, err := u.filerFactory(b) + if err != nil { + return diag.FromErr(err) + } + + errGroup.Go(func() error { + err = f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists) + if err != nil { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "Failed to save config", + Detail: fmt.Sprintf("Failed to write %s file: %s", path.Join(app.SourceCodePath, "app.yml"), err), + Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s", key)), + }) + } + return nil + }) + } + } + + if err := errGroup.Wait(); err != nil { + return diag.FromErr(err) + } + + return diags +} + +// Name implements bundle.Mutator. +func (u *uploadConfig) Name() string { + return "apps:UploadConfig" +} + +func UploadConfig() bundle.Mutator { + return &uploadConfig{ + filerFactory: func(b *bundle.Bundle) (filer.Filer, error) { + return filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.FilePath) + }, + } +} + +func configToYaml(app *resources.App) (*bytes.Buffer, error) { + buf := bytes.NewBuffer(nil) + enc := yaml.NewEncoder(buf) + enc.SetIndent(2) + + err := enc.Encode(app.Config) + defer enc.Close() + + if err != nil { + return nil, fmt.Errorf("failed to encode app config to yaml: %w", err) + } + + return buf, nil +} diff --git a/bundle/apps/upload_config_test.go b/bundle/apps/upload_config_test.go new file mode 100644 index 000000000..56e3101e9 --- /dev/null +++ b/bundle/apps/upload_config_test.go @@ -0,0 +1,69 @@ +package apps + +import ( + "bytes" + "context" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + mockfiler "github.com/databricks/cli/internal/mocks/libs/filer" + "github.com/databricks/cli/libs/filer" + "github.com/databricks/cli/libs/vfs" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestAppUploadConfig(t *testing.T) { + root := t.TempDir() + err := os.MkdirAll(filepath.Join(root, "my_app"), 0700) + require.NoError(t, err) + + b := &bundle.Bundle{ + BundleRootPath: root, + SyncRoot: vfs.MustNew(root), + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Users/foo@bar.com/", + }, + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "my_app": { + App: &apps.App{ + Name: "my_app", + }, + SourceCodePath: "./my_app", + Config: map[string]any{ + "command": []string{"echo", "hello"}, + "env": []map[string]string{ + {"name": "MY_APP", "value": "my value"}, + }, + }, + }, + }, + }, + }, + } + + mockFiler := mockfiler.NewMockFiler(t) + mockFiler.EXPECT().Write(mock.Anything, "my_app/app.yml", bytes.NewBufferString(`command: + - echo + - hello +env: + - name: MY_APP + value: my value +`), filer.OverwriteIfExists).Return(nil) + + u := uploadConfig{ + filerFactory: func(b *bundle.Bundle) (filer.Filer, error) { + return mockFiler, nil + }, + } + + diags := bundle.Apply(context.Background(), b, &u) + require.NoError(t, diags.Error()) +} diff --git a/bundle/apps/validate.go b/bundle/apps/validate.go index 5e3082f2d..d12d01313 100644 --- a/bundle/apps/validate.go +++ b/bundle/apps/validate.go @@ -3,8 +3,7 @@ package apps import ( "context" "fmt" - "os" - "path/filepath" + "path" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" @@ -15,14 +14,15 @@ type validate struct { func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { var diags diag.Diagnostics + possibleConfigFiles := []string{"app.yml", "app.yaml"} + for _, app := range b.Config.Resources.Apps { - possibleConfigFiles := []string{"app.yml", "app.yaml"} for _, configFile := range possibleConfigFiles { - cf := filepath.Join(b.SyncRootPath, app.SourceCodePath, configFile) - if _, err := os.Stat(cf); err == nil { + cf := path.Join(app.SourceCodePath, configFile) + if _, err := b.SyncRoot.Stat(cf); err == nil { diags = append(diags, diag.Diagnostic{ Severity: diag.Error, - Summary: "app.yml detected", + Summary: fmt.Sprintf("%s detected", configFile), Detail: fmt.Sprintf("remove %s and use 'config' property for app resource '%s' instead", cf, app.Name), }) } diff --git a/bundle/config/mutator/merge_apps.go b/bundle/config/mutator/merge_apps.go index 3be9ae6d2..d91e8dd7f 100644 --- a/bundle/config/mutator/merge_apps.go +++ b/bundle/config/mutator/merge_apps.go @@ -26,7 +26,7 @@ func (m *mergeApps) resourceName(v dyn.Value) string { case dyn.KindString: return v.MustString() default: - panic("job cluster key must be a string") + panic("app name must be a string") } } diff --git a/bundle/config/resources/apps.go b/bundle/config/resources/apps.go index 748411096..00b7de4dd 100644 --- a/bundle/config/resources/apps.go +++ b/bundle/config/resources/apps.go @@ -17,15 +17,15 @@ type App struct { // This equals to app name and added for symmetry with other resources. ID string `json:"id,omitempty" bundle:"readonly"` - // SourceCodePath is a required field used by DABs to point databricks app source code - // on local disk and use it to point to this source code in the app deployment + // SourceCodePath is a required field used by DABs to point to Databricks app source code + // on local disk and to the corresponding workspace path during app deployment. SourceCodePath string `json:"source_code_path"` // Config is an optional field which allows configuring the app following Databricks app configuration format like in app.yml. // When this field is set, DABs read the configuration set in this field and write // it to app.yml in the root of the source code folder in Databricks workspace. // If there’s app.yml defined locally, DABs will raise an error. - Config map[string]interface{} `json:"config,omitempty"` + Config map[string]any `json:"config,omitempty"` Permissions []Permission `json:"permissions,omitempty"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` diff --git a/bundle/deploy/terraform/tfdyn/convert_app_test.go b/bundle/deploy/terraform/tfdyn/convert_app_test.go index 95b9bdae4..558961059 100644 --- a/bundle/deploy/terraform/tfdyn/convert_app_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_app_test.go @@ -16,7 +16,7 @@ import ( func TestConvertApp(t *testing.T) { var src = resources.App{ SourceCodePath: "./app", - Config: map[string]interface{}{ + Config: map[string]any{ "command": []string{"python", "app.py"}, }, App: &apps.App{ @@ -60,20 +60,20 @@ func TestConvertApp(t *testing.T) { require.NoError(t, err) app := out.App["my_app"] - assert.Equal(t, map[string]interface{}{ + assert.Equal(t, map[string]any{ "description": "app description", "name": "app_id", - "resource": []interface{}{ - map[string]interface{}{ + "resource": []any{ + map[string]any{ "name": "job1", - "job": map[string]interface{}{ + "job": map[string]any{ "id": "1234", "permission": "CAN_MANAGE_RUN", }, }, - map[string]interface{}{ + map[string]any{ "name": "sql1", - "sql_warehouse": map[string]interface{}{ + "sql_warehouse": map[string]any{ "id": "5678", "permission": "CAN_USE", }, diff --git a/bundle/run/app.go b/bundle/run/app.go index 8a4dc3402..2bbce10cc 100644 --- a/bundle/run/app.go +++ b/bundle/run/app.go @@ -1,23 +1,16 @@ package run import ( - "bytes" "context" "fmt" - "path" - "path/filepath" "time" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/bundle/deploy" "github.com/databricks/cli/bundle/run/output" "github.com/databricks/cli/libs/cmdio" - "github.com/databricks/cli/libs/filer" "github.com/databricks/databricks-sdk-go/service/apps" "github.com/spf13/cobra" - - "gopkg.in/yaml.v3" ) func logProgress(ctx context.Context, msg string) { @@ -32,8 +25,6 @@ type appRunner struct { bundle *bundle.Bundle app *resources.App - - filerFactory deploy.FilerFactory } func (a *appRunner) Name() string { @@ -116,6 +107,11 @@ func (a *appRunner) start(ctx context.Context) error { return err } + // After the app is started (meaning the compute is running), the API will return the app object with the + // active and pending deployments fields (if any). If there are active or pending deployments, + // we need to wait for them to complete before we can do the new deployment. + // Otherwise, the new deployment will fail. + // Thus, we first wait for the active deployment to complete. if startedApp.ActiveDeployment != nil && startedApp.ActiveDeployment.Status.State == apps.AppDeploymentStateInProgress { logProgress(ctx, "Waiting for the active deployment to complete...") @@ -126,6 +122,7 @@ func (a *appRunner) start(ctx context.Context) error { logProgress(ctx, "Active deployment is completed!") } + // Then, we wait for the pending deployment to complete. if startedApp.PendingDeployment != nil && startedApp.PendingDeployment.Status.State == apps.AppDeploymentStateInProgress { logProgress(ctx, "Waiting for the pending deployment to complete...") @@ -145,32 +142,6 @@ func (a *appRunner) deploy(ctx context.Context) error { b := a.bundle w := b.WorkspaceClient() - // If the app has a config, we need to deploy it first. - // It means we need to write app.yml file with the content of the config field - // to the remote source code path of the app. - if app.Config != nil { - appPath, err := filepath.Rel(b.Config.Workspace.FilePath, app.SourceCodePath) - if err != nil { - return fmt.Errorf("failed to get relative path of app source code path: %w", err) - } - - buf, err := configToYaml(app) - if err != nil { - return err - } - - // When the app is started, create a new app deployment and wait for it to complete. - f, err := a.filerFactory(b) - if err != nil { - return err - } - - err = f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists) - if err != nil { - return fmt.Errorf("failed to write %s file: %w", path.Join(app.SourceCodePath, "app.yml"), err) - } - } - wait, err := w.Apps.Deploy(ctx, apps.CreateAppDeploymentRequest{ AppName: app.Name, AppDeployment: &apps.AppDeployment{ @@ -241,18 +212,3 @@ func (a *appRunner) ParseArgs(args []string, opts *Options) error { func (a *appRunner) CompleteArgs(args []string, toComplete string) ([]string, cobra.ShellCompDirective) { return nil, cobra.ShellCompDirectiveNoFileComp } - -func configToYaml(app *resources.App) (*bytes.Buffer, error) { - buf := bytes.NewBuffer(nil) - enc := yaml.NewEncoder(buf) - enc.SetIndent(2) - - err := enc.Encode(app.Config) - defer enc.Close() - - if err != nil { - return nil, fmt.Errorf("failed to encode app config to yaml: %w", err) - } - - return buf, nil -} diff --git a/bundle/run/app_test.go b/bundle/run/app_test.go index 98f05127b..ddbdc2a30 100644 --- a/bundle/run/app_test.go +++ b/bundle/run/app_test.go @@ -13,10 +13,8 @@ import ( "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/internal/bundletest" - mockfiler "github.com/databricks/cli/internal/mocks/libs/filer" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/flags" "github.com/databricks/cli/libs/vfs" "github.com/databricks/databricks-sdk-go/experimental/mocks" @@ -26,10 +24,9 @@ import ( ) type testAppRunner struct { - m *mocks.MockWorkspaceClient - b *bundle.Bundle - mockFiler *mockfiler.MockFiler - ctx context.Context + m *mocks.MockWorkspaceClient + b *bundle.Bundle + ctx context.Context } func (ta *testAppRunner) run(t *testing.T) { @@ -37,9 +34,6 @@ func (ta *testAppRunner) run(t *testing.T) { key: "my_app", bundle: ta.b, app: ta.b.Config.Resources.Apps["my_app"], - filerFactory: func(b *bundle.Bundle) (filer.Filer, error) { - return ta.mockFiler, nil - }, } _, err := r.Run(ta.ctx, &Options{}) @@ -65,7 +59,7 @@ func setupBundle(t *testing.T) (context.Context, *bundle.Bundle, *mocks.MockWork Name: "my_app", }, SourceCodePath: "./my_app", - Config: map[string]interface{}{ + Config: map[string]any{ "command": []string{"echo", "hello"}, "env": []map[string]string{ {"name": "MY_APP", "value": "my value"}, @@ -123,20 +117,10 @@ func setupTestApp(t *testing.T, initialAppState apps.ApplicationState, initialCo }, }).Return(wait, nil) - mockFiler := mockfiler.NewMockFiler(t) - mockFiler.EXPECT().Write(mock.Anything, "my_app/app.yml", bytes.NewBufferString(`command: - - echo - - hello -env: - - name: MY_APP - value: my value -`), filer.OverwriteIfExists).Return(nil) - return &testAppRunner{ - m: mwc, - b: b, - mockFiler: mockFiler, - ctx: ctx, + m: mwc, + b: b, + ctx: ctx, } } @@ -225,9 +209,6 @@ func TestStopApp(t *testing.T) { key: "my_app", bundle: b, app: b.Config.Resources.Apps["my_app"], - filerFactory: func(b *bundle.Bundle) (filer.Filer, error) { - return nil, nil - }, } err := r.Cancel(ctx) diff --git a/bundle/run/runner.go b/bundle/run/runner.go index e4706ca53..23c2c0a41 100644 --- a/bundle/run/runner.go +++ b/bundle/run/runner.go @@ -8,7 +8,6 @@ import ( "github.com/databricks/cli/bundle/config/resources" refs "github.com/databricks/cli/bundle/resources" "github.com/databricks/cli/bundle/run/output" - "github.com/databricks/cli/libs/filer" ) type key string @@ -62,9 +61,6 @@ func ToRunner(b *bundle.Bundle, ref refs.Reference) (Runner, error) { key: key(ref.KeyWithType), bundle: b, app: resource, - filerFactory: func(b *bundle.Bundle) (filer.Filer, error) { - return filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.FilePath) - }, }, nil default: return nil, fmt.Errorf("unsupported resource type: %T", resource) diff --git a/bundle/tests/apps_test.go b/bundle/tests/apps_test.go index 86c8eb08c..2b777a327 100644 --- a/bundle/tests/apps_test.go +++ b/bundle/tests/apps_test.go @@ -23,8 +23,8 @@ func TestApps(t *testing.T) { app := b.Config.Resources.Apps["my_app"] assert.Equal(t, "my-app", app.Name) assert.Equal(t, "My App", app.Description) - assert.Equal(t, []interface{}{"python", "app.py"}, app.Config["command"]) - assert.Equal(t, []interface{}{map[string]interface{}{"name": "SOME_ENV_VARIABLE", "value": "Some value"}}, app.Config["env"]) + assert.Equal(t, []any{"python", "app.py"}, app.Config["command"]) + assert.Equal(t, []any{map[string]any{"name": "SOME_ENV_VARIABLE", "value": "Some value"}}, app.Config["env"]) assert.Len(t, app.Resources, 2) assert.Equal(t, "1234", app.Resources[0].SqlWarehouse.Id) @@ -46,8 +46,8 @@ func TestAppsOverride(t *testing.T) { app := b.Config.Resources.Apps["my_app"] assert.Equal(t, "my-app", app.Name) assert.Equal(t, "My App", app.Description) - assert.Equal(t, []interface{}{"python", "dev.py"}, app.Config["command"]) - assert.Equal(t, []interface{}{map[string]interface{}{"name": "SOME_ENV_VARIABLE_2", "value": "Some value 2"}}, app.Config["env"]) + assert.Equal(t, []any{"python", "dev.py"}, app.Config["command"]) + assert.Equal(t, []any{map[string]any{"name": "SOME_ENV_VARIABLE_2", "value": "Some value 2"}}, app.Config["env"]) assert.Len(t, app.Resources, 3) assert.Equal(t, "1234", app.Resources[0].SqlWarehouse.Id) diff --git a/libs/dyn/merge/elements_by_key_test.go b/libs/dyn/merge/elements_by_key_test.go index ef316cc66..09efece07 100644 --- a/libs/dyn/merge/elements_by_key_test.go +++ b/libs/dyn/merge/elements_by_key_test.go @@ -50,3 +50,42 @@ func TestElementByKey(t *testing.T) { }, ) } + +func TestElementByKeyWithOverride(t *testing.T) { + vin := dyn.V([]dyn.Value{ + dyn.V(map[string]dyn.Value{ + "key": dyn.V("foo"), + "value": dyn.V(42), + }), + dyn.V(map[string]dyn.Value{ + "key": dyn.V("bar"), + "value": dyn.V(43), + }), + dyn.V(map[string]dyn.Value{ + "key": dyn.V("foo"), + "othervalue": dyn.V(44), + }), + }) + + keyFunc := func(v dyn.Value) string { + return strings.ToLower(v.MustString()) + } + + vout, err := dyn.MapByPath(vin, dyn.EmptyPath, ElementsByKeyWithOverride("key", keyFunc)) + require.NoError(t, err) + assert.Len(t, vout.MustSequence(), 2) + assert.Equal(t, + vout.Index(0).AsAny(), + map[string]any{ + "key": "foo", + "othervalue": 44, + }, + ) + assert.Equal(t, + vout.Index(1).AsAny(), + map[string]any{ + "key": "bar", + "value": 43, + }, + ) +}