Skip to content

Commit e34cd7d

Browse files
fajarhidefajarhide
authored
fix(cloud): fingerprint pod tables by READY+RESTARTS, resolve columns from header (#113)
CloudDistiller treated any table containing NAMESPACE + NAME + STATUS as a pod table. That is not a fingerprint of anything — it is the common prefix of most `kubectl get <resource> -A` output. `kubectl get pvc -A` matched it, routed into distill_kubectl, and every healthy `Bound` claim landed in the catch-all arm and was reported as an error. Reproduced against the unfixed distiller, byte-for-byte with the report: k8s: 35 pods | 0 running, 0 pending, 35 error Problems: consul (Bound), consul (Bound), ... +30 more Three changes: 1. is_kubectl_table now requires READY + RESTARTS, which no other `kubectl get` resource prints. 2. distill_kubectl resolves NAME/STATUS from the header instead of assuming columns 0 and 2. This was a second, unreported instance of the same bug: `kubectl get pods -A` also carries a NAMESPACE column, so parts[2] read READY ("1/1") as the status and reported every healthy pod as an error too. A status outside the known lists now counts as `unknown`, never as `error` — an unrecognised status is not evidence of a failure. A header missing either column, or zero parseable rows, returns the input untouched. 3. Columnar listings with no distiller pass through instead of reaching distill_fallback, which kept 20 lines and dropped the rest with no marker. Without this, pvc/pv/ns would still lose 15 of 35 rows silently. Existing kubectl snapshots are unchanged: the non-`-A` fixtures resolve to the same column indices the old code hardcoded. Gates ran on rustc 1.94.0, not the pinned 1.97.0 (no rustup locally), so the clippy result does not predict CI. Refs #105
1 parent dbd86c0 commit e34cd7d

1 file changed

Lines changed: 167 additions & 19 deletions

File tree

‎src/distillers/cloud.rs‎

Lines changed: 167 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ impl Distiller for CloudDistiller {
2727
distill_helm(input)
2828
} else if input.contains("aws ") {
2929
distill_aws(segments, input)
30+
} else if is_resource_table(input) {
31+
// A columnar listing we have no distiller for (`kubectl get pvc|pv|
32+
// ns|certificates …`). The fallback would keep 20 lines and drop the
33+
// rest with no marker, so hand back the payload untouched.
34+
input.to_string()
3035
} else {
3136
distill_fallback(segments)
3237
}
@@ -37,10 +42,33 @@ impl Distiller for CloudDistiller {
3742
// Detection helpers
3843
// ---------------------------------------------------------------------------
3944

45+
/// A pod table is fingerprinted by `READY` + `RESTARTS`, which no other
46+
/// `kubectl get` resource prints. `NAMESPACE NAME STATUS` is *not* a
47+
/// fingerprint — it is the common prefix of nearly every `-A` listing (pvc, pv,
48+
/// namespaces, certificates, …), and matching on it routed those into the pod
49+
/// distiller, which then reported healthy objects as errors.
4050
fn is_kubectl_table(input: &str) -> bool {
41-
input.lines().any(|l| {
42-
(l.contains("READY") && l.contains("STATUS") && l.contains("RESTARTS"))
43-
|| (l.contains("NAMESPACE") && l.contains("NAME") && l.contains("STATUS"))
51+
input.lines().any(is_pod_header)
52+
}
53+
54+
fn is_pod_header(line: &str) -> bool {
55+
line.contains("NAME")
56+
&& line.contains("READY")
57+
&& line.contains("STATUS")
58+
&& line.contains("RESTARTS")
59+
}
60+
61+
/// Any tabular listing with a `NAME` column and an otherwise all-caps header.
62+
/// Used only to refuse: we can recognise the shape without knowing the schema.
63+
fn is_resource_table(input: &str) -> bool {
64+
input.lines().take(5).any(|l| {
65+
let cols: Vec<&str> = l.split_whitespace().collect();
66+
cols.len() >= 3
67+
&& cols.contains(&"NAME")
68+
&& cols.iter().all(|c| {
69+
c.chars()
70+
.all(|ch| ch.is_ascii_uppercase() || ch == '-' || ch == '_')
71+
})
4472
})
4573
}
4674

@@ -119,41 +147,96 @@ fn is_noise(line: &str) -> bool {
119147
// kubectl table (NAME READY STATUS RESTARTS AGE)
120148
// ---------------------------------------------------------------------------
121149

150+
/// Pod phases/reasons that mean "not healthy yet, but not broken".
151+
const POD_PENDING: &[&str] = &[
152+
"Pending",
153+
"ContainerCreating",
154+
"PodInitializing",
155+
"Terminating",
156+
];
157+
158+
/// Pod phases/reasons that mean "broken". Anything outside these three lists is
159+
/// counted as `unknown`, never as an error — a status this distiller has never
160+
/// heard of is not evidence of a failure.
161+
const POD_FAILED: &[&str] = &[
162+
"CrashLoopBackOff",
163+
"Error",
164+
"Failed",
165+
"ImagePullBackOff",
166+
"ErrImagePull",
167+
"OOMKilled",
168+
"Evicted",
169+
"CreateContainerConfigError",
170+
"CreateContainerError",
171+
"InvalidImageName",
172+
"RunContainerError",
173+
"NodeAffinity",
174+
"Unknown",
175+
];
176+
122177
fn distill_kubectl(input: &str) -> String {
178+
// Resolve NAME/STATUS from the header instead of assuming column 0 and 2:
179+
// `kubectl get pods -A` prefixes a NAMESPACE column, which shifted every
180+
// lookup by one and made READY ("1/1") read as the status.
181+
let Some((header_idx, header)) = input.lines().enumerate().find(|(_, l)| is_pod_header(l))
182+
else {
183+
return input.to_string();
184+
};
185+
let cols: Vec<&str> = header.split_whitespace().collect();
186+
let (Some(name_idx), Some(status_idx)) = (
187+
cols.iter().position(|c| *c == "NAME"),
188+
cols.iter().position(|c| *c == "STATUS"),
189+
) else {
190+
return input.to_string();
191+
};
192+
123193
let mut running = 0u32;
124194
let mut pending = 0u32;
125195
let mut failed = 0u32;
196+
let mut unknown = 0u32;
126197
let mut total = 0u32;
127198
let mut problems: Vec<String> = Vec::new();
128199

129-
for line in input.lines().skip(1) {
200+
for line in input.lines().skip(header_idx + 1) {
130201
let trimmed = line.trim();
131202
if trimmed.is_empty() {
132203
continue;
133204
}
134-
total += 1;
135205
let parts: Vec<&str> = trimmed.split_whitespace().collect();
136-
if parts.len() >= 3 {
137-
let name = parts[0];
138-
let status = parts[2];
139-
match status {
140-
"Running" | "Completed" | "Succeeded" => running += 1,
141-
"Pending" | "ContainerCreating" | "Init:0/1" => {
142-
pending += 1;
143-
problems.push(format!("{} ({})", name, status));
144-
}
145-
_ => {
146-
failed += 1;
147-
problems.push(format!("{} ({})", name, status));
148-
}
149-
}
206+
// A row that does not reach the STATUS column is not a row we can read.
207+
if parts.len() <= status_idx {
208+
continue;
150209
}
210+
total += 1;
211+
let name = parts[name_idx];
212+
let status = parts[status_idx];
213+
if matches!(status, "Running" | "Completed" | "Succeeded") {
214+
running += 1;
215+
} else if POD_PENDING.contains(&status) || status.starts_with("Init:") {
216+
pending += 1;
217+
problems.push(format!("{} ({})", name, status));
218+
} else if POD_FAILED.contains(&status) {
219+
failed += 1;
220+
problems.push(format!("{} ({})", name, status));
221+
} else {
222+
unknown += 1;
223+
problems.push(format!("{} ({})", name, status));
224+
}
225+
}
226+
227+
// Nothing parsed means the fingerprint matched something that only looks
228+
// like a pod table. Hand back the payload rather than summarise a guess.
229+
if total == 0 {
230+
return input.to_string();
151231
}
152232

153233
let mut out = format!(
154234
"k8s: {} pods | {} running, {} pending, {} error",
155235
total, running, pending, failed
156236
);
237+
if unknown > 0 {
238+
out.push_str(&format!(", {} unknown", unknown));
239+
}
157240

158241
if !problems.is_empty() {
159242
out.push_str("\nProblems: ");
@@ -586,3 +669,68 @@ fn distill_fallback(segments: &[OutputSegment]) -> String {
586669

587670
out.trim().to_string()
588671
}
672+
673+
// ---------------------------------------------------------------------------
674+
// Tests
675+
// ---------------------------------------------------------------------------
676+
677+
#[cfg(test)]
678+
mod tests {
679+
use super::*;
680+
681+
/// From issue #105: `kubectl get pvc -A` matched the pod fingerprint and
682+
/// every healthy `Bound` claim was reported as an error.
683+
const PVC_TABLE: &str = "\
684+
NAMESPACE NAME STATUS VOLUME CAPACITY ACCESS MODES STORAGECLASS AGE
685+
datasource qdrant-storage-qdrant-0 Bound pvc-0a75d858 5Gi RWO default 2y
686+
consul data-consul-0 Bound pvc-1b86e969 10Gi RWO default 2y";
687+
688+
const PODS_ALL_NAMESPACES: &str = "\
689+
NAMESPACE NAME READY STATUS RESTARTS AGE
690+
default api-7fb96c846b-f4jnm 1/1 Running 0 5d
691+
kube-系统 auth-5d4f6c8b99-abc12 0/1 CrashLoopBackOff 15 2h";
692+
693+
fn distill(input: &str) -> String {
694+
CloudDistiller.distill(&[], input, None)
695+
}
696+
697+
#[test]
698+
fn passes_non_pod_resource_tables_through_untouched() {
699+
assert_eq!(distill(PVC_TABLE), PVC_TABLE);
700+
}
701+
702+
#[test]
703+
fn does_not_fingerprint_namespace_name_status_as_pods() {
704+
assert!(!is_kubectl_table(PVC_TABLE));
705+
}
706+
707+
#[test]
708+
fn resolves_columns_from_header_when_namespace_is_present() {
709+
let out = distill(PODS_ALL_NAMESPACES);
710+
assert!(
711+
out.starts_with("k8s: 2 pods | 1 running, 0 pending, 1 error"),
712+
"{out}"
713+
);
714+
// The name column, not the namespace column.
715+
assert!(
716+
out.contains("auth-5d4f6c8b99-abc12 (CrashLoopBackOff)"),
717+
"{out}"
718+
);
719+
}
720+
721+
#[test]
722+
fn counts_unrecognised_status_as_unknown_not_error() {
723+
let input = "\
724+
NAME READY STATUS RESTARTS AGE
725+
web-0 1/1 SomeNewPhase 0 5d";
726+
let out = distill(input);
727+
assert!(out.contains("0 error"), "{out}");
728+
assert!(out.contains("1 unknown"), "{out}");
729+
}
730+
731+
#[test]
732+
fn returns_input_when_pod_table_has_no_rows() {
733+
let input = "NAME READY STATUS RESTARTS AGE\n";
734+
assert_eq!(distill(input), input);
735+
}
736+
}

0 commit comments

Comments
 (0)