Skip to content

Commit 32bdcb8

Browse files
committed
fix(drain): don't claim acceptance with nothing evicted; latch plan-unsupported
The drain toast appended "evictions were accepted" even when every eviction failed or only skips happened, contradicting its own title. It now says that only when at least one pod was actually evicted; the node-stays-cordoned note is unconditional. The plan-unsupported fallback came straight from the current mutation error, which the next request clears, so toggling an option refired a request known to 404 and bounced the dialog out of fallback mode. The dialog now latches unsupported for as long as it stays open and stops refetching while latched. Claude-Session: https://claude.ai/code/session_01NE55VsYGkb5tV3neKDmtZw
1 parent c1aa6a3 commit 32bdcb8

3 files changed

Lines changed: 39 additions & 7 deletions

File tree

‎packages/k8s-ui/src/components/shared/ResourceActionsBar.tsx‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,13 +198,26 @@ export function ResourceActionsBar({
198198
const [showDrainConfirm, setShowDrainConfirm] = useState(false)
199199
const [drainOptions, setDrainOptions] = useState<DrainDialogOptions>(DEFAULT_DRAIN_DIALOG_OPTIONS)
200200

201+
// A backend without the plan endpoint stays unsupported for as long as the dialog is
202+
// open: the host reports it through a mutation error, which the next request clears,
203+
// so without latching every option change would refire a request known to 404 and
204+
// bounce the dialog out of its fallback mode.
205+
const [planUnsupported, setPlanUnsupported] = useState(false)
206+
useEffect(() => {
207+
if (drainPlanUnsupported) setPlanUnsupported(true)
208+
}, [drainPlanUnsupported])
209+
useEffect(() => {
210+
if (!showDrainConfirm) setPlanUnsupported(false)
211+
}, [showDrainConfirm])
212+
const planSupported = Boolean(onPlanDrain) && !planUnsupported
213+
201214
// Fetch (and refetch on option changes) the read-only plan while the drain dialog is open.
202215
useEffect(() => {
203-
if (showDrainConfirm && onPlanDrain) {
216+
if (showDrainConfirm && onPlanDrain && !planUnsupported) {
204217
onPlanDrain({ name: resource.name, options: drainOptions })
205218
}
206219
// eslint-disable-next-line react-hooks/exhaustive-deps
207-
}, [showDrainConfirm, drainOptions.force, drainOptions.deleteEmptyDirData, resource.name])
220+
}, [showDrainConfirm, drainOptions.force, drainOptions.deleteEmptyDirData, resource.name, planUnsupported])
208221

209222
// Rollback dialog state
210223
const [showRevisions, setShowRevisions] = useState(false)
@@ -702,11 +715,11 @@ export function ResourceActionsBar({
702715
nodeName={resource.name}
703716
plan={drainPlan}
704717
loading={Boolean(isPlanningDrain)}
705-
error={drainPlanUnsupported ? null : drainPlanError}
718+
error={planSupported ? drainPlanError : null}
706719
options={drainOptions}
707720
onOptionsChange={setDrainOptions}
708-
planSupported={Boolean(onPlanDrain) && !drainPlanUnsupported}
709-
onRefreshPlan={onPlanDrain ? () => onPlanDrain({ name: resource.name, options: drainOptions }) : undefined}
721+
planSupported={planSupported}
722+
onRefreshPlan={planSupported ? () => onPlanDrain?.({ name: resource.name, options: drainOptions }) : undefined}
710723
isDraining={Boolean(isDrainingNode)}
711724
onClose={() => {
712725
setShowDrainConfirm(false)

‎web/src/api/client.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5213,7 +5213,9 @@ export function describeDrainResult(data: {
52135213
parts.push(`Failed ${errors.length}: ${listWithOverflow(errors)}`);
52145214
}
52155215
parts.push(
5216-
"Evictions were accepted; pods may still be terminating. The node remains cordoned.",
5216+
evicted > 0
5217+
? "Evictions were accepted; those pods may still be terminating. The node remains cordoned."
5218+
: "The node remains cordoned.",
52175219
);
52185220
const detail = parts.join("\n");
52195221
if (errors.length > 0) {

‎web/src/api/drain-plan.test.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,24 @@ describe("describeDrainResult", () => {
6363
const r = describeDrainResult({ evictedPods: ["a", "b"] });
6464
expect(r.failed).toBe(false);
6565
expect(r.title).toBe("Node drained: 2 evicted, 0 skipped");
66-
expect(r.detail).toContain("pods may still be terminating");
66+
expect(r.detail).toContain("those pods may still be terminating");
6767
expect(r.detail).toContain("node remains cordoned");
6868
});
69+
70+
it("never claims evictions were accepted when nothing was evicted", () => {
71+
const allFailed = describeDrainResult({
72+
evictedPods: [],
73+
errors: ["shop/web-1: timed out waiting for PDB to allow eviction"],
74+
});
75+
expect(allFailed.detail).not.toContain("accepted");
76+
expect(allFailed.detail).toContain("node remains cordoned");
77+
78+
const skipOnly = describeDrainResult({
79+
evictedPods: [],
80+
skippedPods: [
81+
{ namespace: "shop", name: "agent", outcome: "skip", reason: "managed by a DaemonSet", emptyDir: false, pdbChecked: false },
82+
],
83+
});
84+
expect(skipOnly.detail).not.toContain("accepted");
85+
});
6986
});

0 commit comments

Comments
 (0)