Uh oh!
There was an error while loading. Please reload this page.
Skip app drift on deploy-only fields when there is no active deployment - #5943
Merged
Conversation
Collaborator
Integration test reportCommit: 167a01c
8 interesting tests: 4 RECOVERED, 4 SKIP
Top 5 slowest tests (at least 2 minutes):
|
radakamforce-pushed
the
fix-app-config-no-deployment-drift
branch
from
July 16, 2026 12:21
79153bb to
1957de9CompareThe app deploy-only fields (config, git_source) are applied through an app deployment, which only happens once the app compute is started (manageLifecycle). Until then DoRead leaves them nil, so an app that set config or git_source without lifecycle.started=true planned a perpetual update for a field that was never deployed. Extend OverrideChangeDesc to skip config and git_source while there is no active deployment, keying off remote.ActiveDeployment. source_code_path was already skipped; matching on the top-level field (Prefix(1)) now also catches nested config diffs. Real out-of-band drift is still reported once a deployment exists. Adds acceptance tests for the config and git_source cases.
radakamforce-pushed
the
fix-app-config-no-deployment-drift
branch
from
July 16, 2026 12:26
1957de9 to
a9a9561CompareUnify the deploy-only drift skip (source_code_path, config, git_source) on ActiveDeployment == nil, which the backend also returns for a stopped app, not just before the first deploy. Update the reason to "no active deployment" and fix the test server's stop handler to clear the active deployment so acceptance tests match cloud behavior. Add config-drift-stopped covering the stopped case.
The active deployment is cleared on stop only for non-scalable apps; scalable apps retain it (pending is always cleared). Soften the OverrideChangeDesc and test-server comments accordingly. Comment-only, no behavior change.
radakam
marked this pull request as ready for review
July 17, 2026 10:51
pietern
reviewed
Jul 20, 2026
Uh oh!
There was an error while loading. Please reload this page.
| if remote.ActiveDeployment == nil { | ||
| change.Action = deployplan.Skip | ||
| change.Reason = "no active deployment" | ||
| } |
Contributor
There was a problem hiding this comment.
@andrewnester Is there impact for the lifecycle block?
Contributor
There was a problem hiding this comment.
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
The OverrideChangeDesc and test-server stop comments used an informal "scalable/non-scalable" distinction that isn't API/SDK terminology and that the skip logic never branches on (it only checks ActiveDeployment == nil). Reword both to describe the observable behavior instead. Comment-only, no behavior change.
| } | ||
| } | ||
| === Stop the app: the backend clears the active deployment |
Contributor
There was a problem hiding this comment.
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
ContributorAuthor
There was a problem hiding this comment.
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.
andrewnester
approved these changes
Jul 22, 2026
eng-dev-ecosystem-bot
commented
Jul 22, 2026
Collaborator
Integration test reportCommit: 09d2395
53 interesting tests: 39 FAIL, 6 flaky, 4 KNOWN, 2 RECOVERED, 2 SKIP
Top 50 slowest tests (at least 2 minutes):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Skip drift reporting on the deploy-only app fields
source_code_path,config, andgit_sourcewhenever the app has no active deployment, unifying all three under a singleActiveDeployment == nilcheck (reason:no active deployment). Also fix the test server'sstophandler to clear the active deployment, matching the real backend, and add a test covering the stopped-app case.Why
DoReadcan only read these fields back from the app's active deployment. An app created withoutlifecycle.started(the defaultno_computecase) has none, so the fields read back empty while the bundle sets them — producing a phantomupdateinbundle planthat can never converge (these fields are excluded from the App Update call and only deploy on start). A stop can also clear theactive_deploymentfor some apps (pending_deploymentis always cleared), so the same spurious diff applies to a stopped app that has none; the previoussource_code_path-only check and the "no deployment" wording didn't capture that. Verified against a real workspace: a never-started app and a stopped app that cleared its deployment both returnactive_deployment: null, while a running app returns it, so real out-of-band drift is still reported once a deployment exists. This is a plan-idempotency fix. Aconfig/git_sourcechange made while the app has no active deployment is applied on the next start (seemanageLifecycle), not silently lost — it is deferred, not applied while stopped, because the API neither exposes nor accepts deployed config for a non-running app.Tests
config-no-deployment/git-source-no-deployment: no-op plan on a never-started app.config-drift-stopped: drift is detected while running, then skipped after stop.This gap was found by fuzz testing.