-
Notifications
You must be signed in to change notification settings - Fork 200
Skip app drift on deploy-only fields when there is no active deployment #5943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a9a9561
46fa9b5
c67e7f1
167a01c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import http.server, os | ||
|
|
||
| http.server.HTTPServer( | ||
| ("", int(os.environ["DATABRICKS_APP_PORT"])), http.server.SimpleHTTPRequestHandler | ||
| ).serve_forever() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| bundle: | ||
| name: config-drift-stopped-$UNIQUE_NAME | ||
|
|
||
| resources: | ||
| apps: | ||
| myapp: | ||
| name: $UNIQUE_NAME | ||
| description: my_app | ||
| source_code_path: ./app | ||
| config: | ||
| command: | ||
| - python | ||
| - app.py | ||
| env: | ||
| - name: MY_VAR | ||
| value: original_value | ||
| lifecycle: | ||
| started: true |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
|
|
||
| === Deploy with started=true so the app has an active deployment | ||
| >>> [CLI] bundle deploy | ||
| Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/config-drift-stopped-[UNIQUE_NAME]/default/files... | ||
| Deploying resources... | ||
| ✓ Deployment succeeded | ||
| Updating deployment state... | ||
| Deployment complete! | ||
|
|
||
| === Change config while running: drift is detected (active deployment present) | ||
| >>> update_file.py databricks.yml original_value changed_value | ||
|
|
||
| >>> [CLI] bundle plan -o json | ||
| { | ||
| "config.env[0].value": { | ||
| "action": "update", | ||
| "old": "original_value", | ||
| "new": "changed_value", | ||
| "remote": "original_value" | ||
| } | ||
| } | ||
|
|
||
| === Stop the app: the backend clears the active deployment | ||
| >>> [CLI] apps stop [UNIQUE_NAME] | ||
| "STOPPED" | ||
|
|
||
| === Same config change is now skipped: no active deployment to compare against | ||
| >>> [CLI] bundle plan -o json | ||
| { | ||
| "config": { | ||
| "action": "skip", | ||
| "reason": "no active deployment", | ||
| "old": { | ||
| "command": [ | ||
| "python", | ||
| "app.py" | ||
| ], | ||
| "env": [ | ||
| { | ||
| "name": "MY_VAR", | ||
| "value": "original_value" | ||
| } | ||
| ] | ||
| }, | ||
| "new": { | ||
| "command": [ | ||
| "python", | ||
| "app.py" | ||
| ], | ||
| "env": [ | ||
| { | ||
| "name": "MY_VAR", | ||
| "value": "changed_value" | ||
| } | ||
| ] | ||
| } | ||
| }, | ||
| "config.env[0].value": { | ||
| "action": "skip", | ||
| "reason": "no active deployment", | ||
| "old": "original_value", | ||
| "new": "changed_value" | ||
| } | ||
| } | ||
|
|
||
| >>> [CLI] bundle destroy --auto-approve | ||
| The following resources will be deleted: | ||
| delete resources.apps.myapp | ||
|
|
||
| All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/config-drift-stopped-[UNIQUE_NAME]/default | ||
|
|
||
| Deleting files... | ||
| Destroy complete! | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| envsubst < databricks.yml.tmpl > databricks.yml | ||
|
|
||
| cleanup() { | ||
| trace $CLI bundle destroy --auto-approve | ||
| } | ||
| trap cleanup EXIT | ||
|
|
||
| title "Deploy with started=true so the app has an active deployment" | ||
| trace $CLI bundle deploy | ||
|
|
||
| title "Change config while running: drift is detected (active deployment present)" | ||
| trace update_file.py databricks.yml original_value changed_value | ||
| trace $CLI bundle plan -o json | jq '.plan[].changes | with_entries(select(.key | startswith("config")))' | ||
|
|
||
| title "Stop the app: the backend clears the active deployment" | ||
| trace $CLI apps stop $UNIQUE_NAME | jq '.compute_status.state' | ||
|
|
||
| title "Same config change is now skipped: no active deployment to compare against" | ||
| trace $CLI bundle plan -o json | jq '.plan[].changes | with_entries(select(.key | startswith("config")))' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| Local = true | ||
| Cloud = false | ||
|
|
||
| # Asserts plan classification, not requests; drop the inherited RecordRequests. | ||
| RecordRequests = false | ||
|
|
||
| Ignore = [".databricks", "databricks.yml"] | ||
|
|
||
| # Direct engine only: the skip logic lives in OverrideChangeDesc. Mirrors | ||
| # config-no-deployment. | ||
| EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import os | ||
|
|
||
| from flask import Flask | ||
|
|
||
| app = Flask(__name__) | ||
|
|
||
|
|
||
| @app.route("/") | ||
| def home(): | ||
| return "Hello from config-no-deployment app!" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| command: | ||
| - python | ||
| - app.py |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| bundle: | ||
| name: app-config-no-deployment | ||
|
|
||
| resources: | ||
| apps: | ||
| myapp: | ||
| name: test-app-config-no-deployment | ||
| description: my_app_description | ||
| source_code_path: ./app | ||
| config: | ||
| command: ["python", "app.py"] | ||
| env: | ||
| - name: MY_ENV_VAR | ||
| value: test_value |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
|
|
||
| === Deploy: app created with no_compute; config is not deployed until the app starts | ||
| >>> [CLI] bundle deploy | ||
| Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/app-config-no-deployment/default/files... | ||
| Deploying resources... | ||
| Updating deployment state... | ||
| Deployment complete! | ||
|
|
||
| === Plan is a no-op: config/source_code_path drift is skipped while the app has no active deployment | ||
| >>> [CLI] bundle plan | ||
| Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged | ||
|
|
||
| === Both deploy-only fields are skipped with reason "no active deployment" | ||
| >>> [CLI] bundle plan -o json | ||
| { | ||
| "config": { | ||
| "action": "skip", | ||
| "reason": "no active deployment", | ||
| "old": { | ||
| "command": [ | ||
| "python", | ||
| "app.py" | ||
| ], | ||
| "env": [ | ||
| { | ||
| "name": "MY_ENV_VAR", | ||
| "value": "test_value" | ||
| } | ||
| ] | ||
| }, | ||
| "new": { | ||
| "command": [ | ||
| "python", | ||
| "app.py" | ||
| ], | ||
| "env": [ | ||
| { | ||
| "name": "MY_ENV_VAR", | ||
| "value": "test_value" | ||
| } | ||
| ] | ||
| } | ||
| }, | ||
| "source_code_path": { | ||
| "action": "skip", | ||
| "reason": "no active deployment", | ||
| "old": "/Workspace/Users/[USERNAME]/.bundle/app-config-no-deployment/default/files/app", | ||
| "new": "/Workspace/Users/[USERNAME]/.bundle/app-config-no-deployment/default/files/app" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| echo "*" > .gitignore | ||
|
|
||
| title "Deploy: app created with no_compute; config is not deployed until the app starts" | ||
| trace $CLI bundle deploy | ||
|
|
||
| title "Plan is a no-op: config/source_code_path drift is skipped while the app has no active deployment" | ||
| trace $CLI bundle plan | contains.py "Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged" | ||
|
|
||
| title "Both deploy-only fields are skipped with reason \"no active deployment\"" | ||
| trace $CLI bundle plan -o json | jq '.plan[].changes | {config, source_code_path}' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| Local = true | ||
| Cloud = false | ||
|
|
||
| # Asserts plan classification, not requests; drop the inherited RecordRequests. | ||
| RecordRequests = false | ||
|
|
||
| Ignore = [".databricks"] | ||
|
|
||
| # Direct engine only: the skip logic lives in OverrideChangeDesc. Mirrors | ||
| # lifecycle-started-omitted. | ||
| EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| bundle: | ||
| name: app-git-source-no-deployment | ||
|
|
||
| resources: | ||
| apps: | ||
| myapp: | ||
| name: test-app-git-source-no-deployment | ||
| description: my_app_description | ||
| git_source: | ||
| branch: main |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
|
|
||
| === Deploy: app created with no_compute; git_source is not deployed until the app starts | ||
| >>> [CLI] bundle deploy | ||
| Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/app-git-source-no-deployment/default/files... | ||
| Deploying resources... | ||
| Updating deployment state... | ||
| Deployment complete! | ||
|
|
||
| === Plan is a no-op: git_source drift is skipped while the app has no active deployment | ||
| >>> [CLI] bundle plan | ||
| Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged | ||
|
|
||
| === git_source is skipped with reason "no active deployment" | ||
| >>> [CLI] bundle plan -o json | ||
| { | ||
| "git_source": { | ||
| "action": "skip", | ||
| "reason": "no active deployment", | ||
| "old": { | ||
| "branch": "main" | ||
| }, | ||
| "new": { | ||
| "branch": "main" | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| echo "*" > .gitignore | ||
|
|
||
| title "Deploy: app created with no_compute; git_source is not deployed until the app starts" | ||
| trace $CLI bundle deploy | ||
|
|
||
| title "Plan is a no-op: git_source drift is skipped while the app has no active deployment" | ||
| trace $CLI bundle plan | contains.py "Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged" | ||
|
|
||
| title "git_source is skipped with reason \"no active deployment\"" | ||
| trace $CLI bundle plan -o json | jq '.plan[].changes | {git_source}' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| Local = true | ||
| Cloud = false | ||
|
|
||
| # Asserts plan classification, not requests; drop the inherited RecordRequests. | ||
| RecordRequests = false | ||
|
|
||
| Ignore = [".databricks"] | ||
|
|
||
| # Direct engine only: the skip logic lives in OverrideChangeDesc. Mirrors | ||
| # config-no-deployment. | ||
| EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -246,12 +246,18 @@ func hasAppChanges(entry *PlanEntry) bool { | |
| return entry.Changes.HasChangeExcept("source_code_path", "config", "git_source", "lifecycle", "lifecycle.started") | ||
| } | ||
|
|
||
| // OverrideChangeDesc skips source_code_path drift when the remote value is empty. | ||
| // This happens when an app has no deployment yet (DefaultSourceCodePath is unset). | ||
| // OverrideChangeDesc skips drift on the deploy-only fields (source_code_path, config, | ||
| // git_source) while the app has no active deployment. DoRead reads them only from the | ||
| // active deployment, so before the first deploy (or once a stop clears it) the remote | ||
| // side is empty and the diff is spurious; it applies on the next start (manageLifecycle). | ||
| func (*ResourceApp) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, remote *AppRemote) error { | ||
| if path.String() == "source_code_path" && (remote.SourceCodePath == "" || remote.SourceCodePath == "null") { | ||
| change.Action = deployplan.Skip | ||
| change.Reason = "no deployment" | ||
| // Prefix(1) so a nested diff (e.g. config.command) matches its top-level field. | ||
| switch path.Prefix(1).String() { | ||
| case "source_code_path", "config", "git_source": | ||
| if remote.ActiveDeployment == nil { | ||
| change.Action = deployplan.Skip | ||
| change.Reason = "no active deployment" | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @andrewnester Is there impact for the lifecycle block?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From looks of it the change seems to be correct, the tets coverage for lifecycle seems to be pretty okay so I'd also rely on it to validate |
||
| } | ||
| return nil | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure stopping actually clears active deployment, could you confirm this with real backend behaviour? I think active is only empty when the app was created but never deployed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed against dogfood: a normally stopped app that was previously deployed returns active_deployment: null, so it seems that it's not limited to never-deployed apps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for confirming!