Repository navigation
Enable the DCGM exporter on Civo, document it for existing clusters - #526
Conversation
|
Docs preview: https://modelplane-docs-pr-526.vercel.app (ready once the site's Content workflow finishes) |
dennis-upbound
left a comment
There was a problem hiding this comment.
Agree on not installing an exporter on Existing. It's the same call as the GPU driver: a cluster running the GPU Operator already has one, and a second double-counts every GPU, which is worse than empty series. Documenting the contract plus the curl check is right.
On the Civo half:
- Is the ServiceMonitor needed? The collector scrapes the exporter pods directly, and Civo's Prometheus only opens up PodMonitor selection, so as far as I can tell nothing reads it. With
dcgmExporter.serviceMonitor.enabled: false, the new kube-prometheus-stack edge, itswait=True, and the Usage all go away, and the GPU Operator and DRA driver don't wait on a healthy Prometheus. The cost is Civo's values no longer matching the generated clouds verbatim. Your call on which matters more. - The CSV is a copy. It matches GKE/EKS/AKS today, but nothing enforces it. A test asserting Civo's CSV equals the generated one would make "one review moves both" true.
- Nit: the
E501ignore covers all ofcivo.py; a# noqa: E501on the CSV string would keep it scoped. - Nit: the new
inference-cluster.mdbullet repeats most of the telemetry GPUs section. A couple of lines and a link would do.
…for existing Signed-off-by: Christopher Haar <christopher.haar@upbound.io>
…le serviceMonitor Signed-off-by: Christopher Haar <christopher.haar@upbound.io>
|
updated the PR:
|
dennis-upbound
left a comment
There was a problem hiding this comment.
Thanks, both points are handled, and the generated dcgm.py with its fail-closed checks is better than the test I suggested. Two nits before merge: the commit subjects carry typed prefixes (fix(servingstack):, feat(refactor):), which CONTRIBUTING.md asks us to drop, and the new paragraph in serving-stack-generation.md could lose its em dashes and split its long middle sentence.
Description of your changes
On a Civo cluster Modelplane provisions, every
modelplane_gpu_*series andmodelplane_energy_joules_totalwas empty. The hand-written Civo stack disabled the GPU Operator's DCGM exporter - not catched as other handwritten installing the GPU Operator as pre-req.The telemetry collector discovers a GPU exporter by pod label, so with no exporter DaemonSet there was nothing to scrape.
This enables the exporter on Civo with the same
dcgm/dcgmExportervalues the generated clouds pin, so one review moves both halves; the metrics CSV covers every fieldmetrics.pymaps. The gpu-operator chart gains a kube-prometheus-stack edge: unlike the generated clouds, Civo's kube-prometheus-stack keeps its bundled CRDs enabled, so that release is what provides the ServiceMonitor CRD, and it now setswait=Trueas a depended-on chart must.On
source: Existingclusters Modelplane doesn't install an exporter, on purpose: it can't know whether the cluster already runs one, and the collector scrapes every pod matching.*dcgm.*, so a second exporter would collect each GPU twice. The docs now say so. A new GPUs section in the telemetry page documents the discovery contract, the seven DCGM fields with their unit conversions, and how to check what an exporter publishes; the existing-cluster requirements gain a DCGM exporter bullet pointing at it.Fixes #519.
Towards #524.
I have:
nix flake check(or./nix.sh flake check) and made sure it passes.git commit -s.Validated
on a live Civo NYC1 cluster: all seven renamed series arrive at the collector's debug sink.