integrate cluster health check into toolkit. - #5917
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a passive health checking solution for NVIDIA GPUs in GKE clusters. By deploying a DaemonSet, the toolkit can now monitor GPU health, report failures via Kubernetes Node conditions and labels, and allow for configurable severity escalation for specific XID errors, enhancing cluster reliability and observability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new cluster-health-check community module, which deploys a Go-based agent as a Kubernetes DaemonSet to passively monitor NVIDIA GPUs on GKE. Feedback on the implementation highlights several critical and high-severity issues: a bug in the DaemonSet's LD_LIBRARY_PATH overrides necessary container library paths; shell redirection is incorrectly passed as a literal argument to execCommandContext; and hardcoded thresholds in the PCIe link width and NIC heartbeat checks present high false-positive risks on idle or non-RDMA nodes. Additionally, the agent should utilize the synced informer cache instead of making direct Kubernetes API calls, propagate context to all command executions, and register the new module in the main index in modules/README.md as required by the style guide.
3d9393c to
d5e7fd4
Compare
34fc3a1 to
03e92b4
Compare
- main.go: signal.NotifyContext for cooperative SIGTERM handling; watchdog goroutine checks ctx before Fatalf on nv-hostengine exit; waitForHostEngine polling loop replaces blind 5s sleep; dcgmi setup gets a timeout; dmesg calls use CommandContext; JSON patches built via json.Marshal on typed structs so control bytes in dmesg/dcgmi output can't corrupt the patch body; extractOverflowMessages sorts map keys for deterministic output; clearNodeHealth only patches the parts that actually need clearing (fixes per-tick PATCH storm on healthy nodes); passive severity and message now merge into an active-test-failed condition instead of being dropped; lastTransitionTime preserved when Status stays True; nvidia-smi failures now surface as Warning; 9 copy-paste check blocks refactored into a table-driven loop. - Dockerfile: switch base to public golang:1.25.10. - build-and-push script: run 'go mod vendor' before docker buildx; use absolute SCRIPT_DIR; drop dead ROOT_DIR. - daemonset.yaml: drop unused configmaps verbs from ClusterRole; comment out -check-interval=15s so the default 5m kicks in.
|
/gcbrun |
Head branch was pushed to by a user without write access
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
7d83193
into
GoogleCloudPlatform:develop
Co-authored-by: Neelabh94 <neelgoyal@google.com>
Co-authored-by: Neelabh94 <neelgoyal@google.com>
passive cluster healthcheck daemon