fix: poll deploy status live so a rollback's actual progress is visible - #452
Merged
Conversation
recordInstantDeployAttempt (internal/api/deploys.go) marks a plain image-tag redeploy, rollback, or promote as 'succeeded' the instant its desired-state write lands, before the application controller's reconcile loop has run even once. Two frontend consequences, both silent: - deployAttemptsQueryOptions only polls while attempts[0].status === 'running', which is already false by the time the trigger's response comes back for this path, so it never polls at all. - deployStatusQueryOptions (the actual reconcile conditions) had no refetchInterval whatsoever. Net effect: for the most common redeploy/rollback action, the roll-out stage and DeployInProgressBanner could sit on stale, pre-convergence state indefinitely. An operator had no way to tell whether a rollback actually landed without manually reloading the page, and the "Deploy in progress" banner never appeared at all for this path (it was gated on attempt.status === 'running', which this path skips entirely). Adds useDeployProgress, a hook combining both queries with the one signal that actually reflects "still converging" for an already- finished instant attempt: computeDeployStages' own rollout-stage fallback (status 'running' when no condition transition has landed at or after the attempt's finished_at). Polls both queries every 3s while that holds, stops once the rollout stage lands on done/failed/skipped/ unknown. Wired into the three routes that render deploy progress (overview, deploy history, deploy detail logs), replacing their direct useDeployAttempts+useDeployStatus calls. Also fixed overview.tsx's DeployInProgressBanner gate to check the computed stage instead of raw attempt status, so it now actually shows during that window. What this doesn't do: - Doesn't change recordInstantDeployAttempt's own "succeeded means the write landed" semantics on the backend: the deploy_attempts row still reports 'succeeded' immediately, which is arguably correct (the desired-state write did succeed) - this is a frontend freshness fix, not a backend status-model change. - Doesn't add a hard timeout to the polling: it keeps polling for as long as computeDeployStages reports 'running', matching deployAttemptsQueryOptions' own existing "poll while running, no cap" precedent for a build in progress.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR adds ChangesDeployment progress
Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant RouteComponent
participant useDeployProgress
participant QueryClient
participant DeployQueries
RouteComponent->>useDeployProgress: request deployment progress
useDeployProgress->>DeployQueries: read attempts and conditions
useDeployProgress->>QueryClient: refetch status and attempts every 3 seconds
QueryClient->>DeployQueries: fetch updated deployment data
DeployQueries-->>useDeployProgress: return attempts and conditions
useDeployProgress-->>RouteComponent: return deployment progress
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const latestAttempt = attemptsQuery.data[0] | ||
| const stillConverging = | ||
| latestAttempt !== undefined && | ||
| computeDeployStages(latestAttempt, conditionsQuery.data, true)[1].status === 'running' |
There was a problem hiding this comment.
When an instant redeploy or rollback repeatedly fails because the container exits or is OOM-killed during readiness, the controller reports ExitedDuringReadiness or OOMKilledDuringReadiness. Neither reason is classified as a rollout failure, so computeDeployStages keeps returning running. This hook consequently polls both resources every three seconds indefinitely while the UI incorrectly presents the failed rollout as still progressing.
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
recordInstantDeployAttempt(internal/api/deploys.go) marks a plain image-tag redeploy, rollback, or promote assucceededthe instant its desired-state write lands, before the application controller's reconcile loop has run even once. Two frontend consequences, both silent:deployAttemptsQueryOptionsonly polls whileattempts[0].status === 'running', which is alreadyfalseby the time the trigger's response comes back for this path, so it never polls at all.deployStatusQueryOptions(the actual reconcile conditions) had norefetchIntervalwhatsoever.Net effect: for the most common redeploy/rollback action, the roll-out stage and
DeployInProgressBannercould sit on stale, pre-convergence state indefinitely. An operator had no way to tell whether a rollback actually landed without manually reloading the page, and the "Deploy in progress" banner never appeared at all for this path (it was gated onattempt.status === 'running', which this path skips entirely).Adds
useDeployProgress, a hook combining both queries with the one signal that actually reflects "still converging" for an already-finished instant attempt:computeDeployStages' own rollout-stage fallback (status'running'when no condition transition has landed at or after the attempt'sfinished_at). Polls both queries every 3s while that holds, stops once the rollout stage lands on done/failed/skipped/unknown. Wired into the three routes that render deploy progress (overview, deploy history, deploy detail logs), replacing their directuseDeployAttempts+useDeployStatuscalls. Also fixedoverview.tsx'sDeployInProgressBannergate to check the computed stage instead of raw attempt status, so it now actually shows during that window.Verification
useDeployProgress.test.tsx: proves polling continues while the rollout stage hasn't converged, stops once it has, and never starts at all when the latest attempt already converged on first load.npx tsc -b,npx eslint src, fullnpx vitest run(158 tests, 31 files) all pass.npm run buildsucceeds.What this doesn't do
recordInstantDeployAttempt's own "succeeded means the write landed" semantics on the backend: thedeploy_attemptsrow still reportssucceededimmediately, which is arguably correct (the desired-state write did succeed) - this is a frontend freshness fix, not a backend status-model change.computeDeployStagesreports'running', matchingdeployAttemptsQueryOptions' own existing "poll while running, no cap" precedent for a build in progress.Summary by CodeRabbit