feat: collect kube pod/node describes, namespace events, and object dumps

[k-rister]

Summary

• Add kubectl describe collection for pods, nodes, jobs, services, and secrets, plus namespace get events and get all, archived to the run's engine-logs/sysinfo directories so this evidence survives namespace deletion at cleanup. Container logs alone miss a lot of failure modes here since the per-engine Job spec uses restartPolicy: Never + backoffLimit: 0 (no logs at all for ImagePullBackOff/FailedScheduling/OOMKilled/FailedMount).
• Secrets are captured with describe, never -o yaml, so registry pull credentials never land in the run archive.
• Consolidate the previously duplicated per-pod log collection loop in kube_cleanup() and rescue_engine_logs() into two shared helpers, collect_namespace_diagnostics() and collect_pod_diagnostics(), mirroring the pattern remotehosts.py already uses for collect_podman_log()/collect_chroot_log() (return success/failure; cleanup gates destructive teardown on it, rescue discards it). The two collectors were deliberately kept separate in commit 8081e5c, but with several new diagnostic types landing in both, duplicating each addition twice was no longer worth it.

Test plan

Ran against a live OpenShift cluster with fio.kube.json, both the normal and error paths:

• Normal cleanup path: verified <pod>.describe.txt.xz + <engine>.txt.xz in engine-logs/, and get-all.txt.xz/events.txt.xz/node-<name>.describe.txt.xz/jobs.describe.txt.xz/services.describe.txt.xz/secrets.describe.txt.xz in sysinfo/endpoint/<label>/, with real content in each, correct per-node dedup, and clean namespace deletion afterward.
• Error/rescue path: forced a fast, deterministic failure via a syntactically-invalid Kubernetes annotation key (passes rickshaw's schema, fails k8s admission validation on --dry-run=server), which fails create_cs_pods() before any pod is ever created. Confirmed via endpoint-stderrout.txt.xz: rescue_engine_logs() fired at the correct log level, collect_namespace_diagnostics() wrote get-all.txt.xz/events.txt.xz even with zero pods, and collect_pod_diagnostics()'s "no pods" guard logged and returned cleanly instead of crashing on a missing pods key.
• python3 -m py_compile on kube.py

🤖 Generated with Claude Code

[k-rister]

PR Review: rickshaw#872 — feat: collect kube pod/node describes, namespace events, and object dumps

Summary: This PR enhances Kubernetes endpoint diagnostic collection by archiving kubectl describe outputs for pods, nodes, jobs, services, and secrets, plus namespace events and overall statuses to the run's archive, while consolidating duplicated log collection logic.
Changed files: 1
Review dimensions: Correctness, API & Contracts, Build & Deploy, Documentation, Style, Completeness

Bugs

• [endpoints/kube/kube.py:681-693] Optional kubectl describe failures on empty/absent resources block the entire namespace deletion — In the clean_k8s_namespace() cleanup loop, kubectl describe is run for jobs, services, and secrets. In many benchmark runs, resources of these types are not created (e.g., no Kubernetes Services are used). If any resource type is absent, kubectl describe exits with non-zero code 1. The code handles this as a failure, setting component_errors = True which returns 1 from clean_k8s_namespace(), causing kube_cleanup() to completely skip namespace deletion and flag a cleanup error.
• [endpoints/kube/kube.py:2302-2313] Privileged kubectl describe node failures prevent cleanup for non-admin cluster users — collect_pod_diagnostics() executes kubectl describe node <node_name>. In multi-tenant or enterprise environments, users often have admin permissions inside their own namespace, but lack cluster-level permissions to read or describe nodes. In such cases, the node describe command will fail with a forbidden error and exit with non-zero. The code treats this as a hard failure, causing collect_pod_diagnostics() to return False. This propagates to kube_cleanup(), which flags a cleanup error and skips namespace deletion, causing resource leaks on the cluster.

Issues

• [endpoints/kube/kube.py:2282] Safety check in collect_pod_diagnostics lacks defensive checking and can crash with KeyError — The safety check if "pods" not in settings["engines"]["endpoint"] or not settings["engines"]["endpoint"]["pods"]: assumes that "engines" and "endpoint" keys exist in the global settings dict. If a catastrophic error occurs before these are initialized, this check will crash with a KeyError, masking the original error during rescue diagnostics collection.
• [endpoints/kube/kube.py:2241-2247] Missing failure logging on get all command failure — If the get all command fails in collect_namespace_diagnostics(), it silently sets success = False without logging the failure or calling endpoints.log_result(result), which is inconsistent with how other command failures in the same module are logged.

Documentation

• [endpoints/kube/kube.py:2229,2276] Typos in settings docstring explanation — The docstring for both collect_namespace_diagnostics() and collect_pod_diagnostics() has a typo: "the one data structure to rule then all" where it should be "rule them all".

File Coverage

• endpoints/kube/kube.py — 2 bugs, 2 issues, 1 doc issue

Limitations

• Runtime Environment: This review could not verify execution behavior on actual Kubernetes/OpenShift endpoint hosts or the behavior of fabric remote connections under varying ssh configurations.
• Output Validation: We cannot verify whether lzma-compressed files are created successfully with correct file system permissions on remote hosts during high-concurrency cleanups.

Verdict

Request changes — While consolidating log collection into helper functions is a great design improvement, treating failures of optional/diagnostic commands like kubectl describe node or kubectl describe services (which routinely fail on empty/absent resources or restricted permissions) as hard errors blocks the deletion of the namespace during cleanup, causing critical resource leaks and false-positive cleanup errors.

[k-rister]

Thanks for the review. Addressed in 84ba5ca:

Fixed:

• Node describe RBAC (bug 2): Agreed, this is a real gap. Nodes are cluster-scoped, and a namespace-restricted user can easily lack describe node permission even with full rights in their own namespace. Fixed by making all new diagnostic collections (pod describe, node describe, namespace events, and the jobs/services/secrets describes) best-effort — a failure is logged but no longer sets the error flag that gates namespace deletion. Only the pre-existing gating signals (container log failures, namespace get all failures, and delete-command failures) still block cleanup, matching the exact behavior that existed before this PR.
• Docstring typo: fixed in the two new functions.

Investigated but not changed:

• Empty jobs/services/secrets describe (bug 1): Verified empirically against the live test cluster — oc describe <type> --namespace <ns> with zero matching objects returns exit 0 with "No resources found", not a failure. This also matches what I observed in this PR's own testing (services.describe.txt.xz/secrets.describe.txt.xz were legitimately empty and cleanup still completed). That said, the best-effort change above makes this moot either way — even if some cluster/CLI version behaves differently, it can no longer block deletion.
• KeyError guard (issue 1): This is pre-existing code, unchanged by this PR — it's the exact same guard rescue_engine_logs() already had (I only relocated it into the shared helper). Note this PR actually improves safety here, since kube_cleanup() previously had no such guard at all and would have raised on a missing pods key; now both callers share the same defensive check.
• Missing logging on get all failure (issue 2): endpoints.log_result(result) is already called unconditionally right after that command and logs at error level on non-zero exit, so the failure is logged today — just without an additional custom message like the events block has. Left as-is since it's not actually missing, just less verbose.

Re-tested both the normal cleanup path and the error/rescue path against the live cluster after these changes; namespace deletion completes cleanly with 0 occurrences of "Skipping namespace cleanup due to prior errors" and all diagnostic artifacts are still collected.

[atheurer]

LGTM! The refactoring is clean, and decoupling the new diagnostic collections so they don't block cleanup under restricted RBAC handles the multi-tenant edge case nicely.