feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting" - #7944
Open
eugenegujing wants to merge 2 commits into
Open
feat(computing-unit): surface failed/unhealthy computing units instead of an endless "Connecting"#7944eugenegujing wants to merge 2 commits into
eugenegujing wants to merge 2 commits into
Conversation
…d of an endless "Connecting" Add Failed, Unknown, and Terminating to ComputingUnitState, extract a PodStatusSnapshot per pod from the same single namespace-level pod listing as before, and map real pod state (eviction, image-pull failures, crash loops, OOM kills, unschedulable pods, deletion timestamps) to a status plus an owner-only, actionable statusReason on the DTO; shared users receive null and vanish-reconciliation semantics are unchanged. On the frontend, widen the status union and add statusReason, render Failed/Unknown as a red badge, replace the endless "Connecting" spinner with a disabled "Unit Unavailable" run button for dead units, and make each dropdown row a single reason-aware tooltip surface (removing the nested badge and name-URI tooltips and fixing a doubled period in the ". Cannot select." concatenation). Closes apache#7669
Remove the last text suffix next to a unit's name in the computing-unit dropdown: a Pending unit already shows the gold badge, the row tooltip explains the state (starting up, or waiting for cluster resources), and the run button shows the "Connecting" spinner for the selected unit, so the suffix was redundant and truncated to "(Co..." at real dropdown widths; status is now uniformly conveyed by badge color plus the row tooltip for every state.
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7944 +/- ##
============================================
+ Coverage 91.51% 92.13% +0.61%
- Complexity 4476 4530 +54
============================================
Files 1171 1180 +9
Lines 47199 47531 +332
Branches 5303 5329 +26
============================================
+ Hits 43196 43794 +598
+ Misses 2344 2120 -224
+ Partials 1659 1617 -42
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
/request-review @kunwp1 |
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
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.
What changes were proposed in this PR?
When a computing unit becomes unhealthy(the pod is OOM-killed into a crash loop, evicted for disk pressure, stuck on an image pull, or its node becomes unreachable), the UI used to show "(Connecting)" with a "starting up" tooltip forever, because
ComputingUnitStateonly hadRunningandPendingand status resolution only looked atpod.status.phase.This PR follows the decisions settled in #7670: mirror Kubernetes for the status vocabulary; owner-only, actionable failure reasons; no auto-recovery — users delete and recreate (the existing terminate flow already handles dead pods).
Screenshots
Two representative states as examples, and the complete set of states and reasons is the mapping table in the next section, and every display combination (status × reason × owner) was verified manually.
An
Unknownunit (node unreachable): red badge, disabled "Unit Unavailable" run button, and the owner-facing reason on the row tooltip:A recovered OOM-killed unit: stays green and runnable, but the tooltip carries the out-of-memory warning with the restart count:
Status vocabulary and reasons
ComputingUnitStategainsFailed,Unknown,Terminating. The full mapping from observed pod state to status and owner-facingstatusReason:statusReasondeletionTimestampsetTerminatingFailed+ reasonEvicted, message mentions ephemeral/diskFailedFailed+ reasonEvicted, otherFailedImagePullBackOff/ErrImagePull/InvalidImageNameFailedCrashLoopBackOff, last terminationOOMKilledFailedCrashLoopBackOff, otherFailedFailed, not evictedFailedUnknownUnknownPending+PodScheduled=False/UnschedulableconditionPendingRunning+ a container's last terminationOOMKilledRunningRunning, healthyRunninglocalunitPending/Running(unchanged)Precedence is top-to-bottom; in particular a
CrashLoopBackOfffailure wins over the recovered-OOM warning, andTerminatingwins over everything.statusReasonis owner-only: the backend sendsnullto shared users at every DTO construction site, and the frontend then falls back to a generic text. Frontend fallbacks whenstatusReasonis null:Running→ "Ready to use",Pending→ "Computing unit is starting up",Failed/Unknown→ "This computing unit is unavailable.",Terminating→ "Computing unit is shutting down", other states → the status word itself.Backend
PodStatusSnapshotper pod (phase, deletionTimestamp, pod reason/message,Unschedulablecondition, per-container waiting reason / last termination reason / restart count) via a pure, unit-testable transform — no additional Kubernetes round trips.restartPolicy: Always, an OOM-killed container restarts in place and the pod phase never leavesRunning, so OOM kills are only visible throughcontainerStatuses[].lastState.terminated.reason. A unit that recovered staysRunningwith a warning; a unit that cannot come back up lands inCrashLoopBackOffand is reportedFailed.Failedpod is not treated as vanished (covered by tests).localunits are unchanged (alwaysRunning) — local liveness is out of scope here, per the discussion.Frontend
statusunion widened to the five states;statusReason?added.Failed/Unknown: red badge in the dropdown (the previously dead red branch, now reachable).Runningunit reachable by hovering the row rather than a few-pixel dot.Failed/Unknown(the spinner branch never consulted unit status).statusReasonis a full sentence — previously this produced a doubled dot.Any related issues, documentation, discussions?
Closes #7669. The design questions (status vocabulary, how much detail reaches which users, auto-recovery) were discussed and settled in #7670.
How was this PR tested?
PodBuilder-built pods through the pure snapshot + mapping functions, including both eviction wordings, all three image-pull reasons, crash-loop with and without OOM history, the crash-loop-beats-recovered-OOM precedence, multi-container pods, owner gating, and the absent-pod path used by creation polling. fabric8 null-guard paths (status,conditions,containerStatuses,state,lastState,terminated) have dedicated tests.Unschedulableconditions, image-pull-beats-crash-loop precedence, empty-string statusReason not shadowing the fallback, and a pinning test documenting that phaseSucceededmaps toPending.ComputingUnitManagingServiceutil specs 90/90, resource specs 60/60, scalafmt clean; frontendtsc --noEmitclean, prettier clean, all changed spec files pass underng test.Notes for more information:
Option[String] → nullserialization relies onDefaultScalaModule, registered on the service's DropwizardObjectMapper; there is no end-to-end JSON test for the new field. A bare pod in phaseSucceededstill maps toPending(pinned by a test) — withrestartPolicy: Alwaysthis phase is practically unreachable, so it is left as a known limitation. Follow-ups deliberately out of scope (per #7670): frontend timeout for the "pod Running but engine unreachable" zombie case, UX for the silent vanish-reconcile, andlocalCU liveness.Was this PR authored or co-authored using generative AI tooling?
Co-authored by: Claude Code (Claude Fable 5)