Skip to content

feat(local): --observability Tilt consumer with Prometheus and Grafana - #663

Open
giuliocalzo wants to merge 29 commits into
NVIDIA:mainfrom
giuliocalzo:feat/issue-597-observability-demo
Open

feat(local): --observability Tilt consumer with Prometheus and Grafana#663
giuliocalzo wants to merge 29 commits into
NVIDIA:mainfrom
giuliocalzo:feat/issue-597-observability-demo

Conversation

@giuliocalzo

@giuliocalzo giuliocalzo commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Refs #597.

Prometheus scrapes the real, unmodified NVIDIA dcgm-exporter reading Mokka's mock libnvidia-ml.so, and Grafana renders it — on a cluster with no GPUs. Two manual triggers then inject a GPU fault and fail if it never reaches Prometheus, so the scrape path is asserted rather than eyeballed. That gives a scale run a "which component broke, and when" view.

make cluster-create
tilt up -- --observability     # Grafana: http://localhost:3000/d/mokka-gpu (admin/mokka)

What this adds

  • local/observability/ — a Tilt consumer in the shape of local/compute-domain/: kube-prometheus-stack (trimmed: Alertmanager off, default rules off, 2h retention), the Mokka dashboard in-tree, and two auto_init=False triggers inject-thermal / inject-xid.
  • --observability on the root Tiltfile. It implies --gpu-operator, since dcgm-exporter is one of the Operator's operands.
  • gpu_operator.tiltfile grows extra_values / extra_resource_deps (following dra.tiltfile's precedent) so the consumer can layer an overlay re-enabling dcgmExporter. Default behavior unchanged.
  • Replaces the standalone docs/demo/observability/run.sh from the earlier revision of this PR. Tilt composes with the GPU Operator instead of replacing it, and the script duplicated per-worker toolkit provisioning and CDI wiring that the shared kind-node image already bakes in.

Design notes

Install order is load-bearing. kube-prometheus-stack goes in before the GPU Operator: it ships the ServiceMonitor CRD that the Operator's chart needs. Expressed via resource_deps, not call order, so reordering the root Tiltfile can't silently break it.

The release label trap. kube-prometheus-stack defaults to serviceMonitorSelectorNilUsesHelmValues: true, so Prometheus only discovers ServiceMonitors labelled release: kube-prometheus-stack. A mismatch is silent — every panel is empty with no error anywhere. Pinned in the overlay with a comment naming the coupling, and the verification below asserts the target is actually up.

Assertions can't pass on residue. The thermal run resets every GPU on the node and waits for a reading that differs before asserting exact equality, so a re-run can't pass on the previous pin. The Xid run rotates between codes 79 and 48 and injects whichever is provably absent, because DCGM latches the last Xid per device and never retracts it — a fixed code would be satisfied by the previous run's residue, and would pass even with dcgm-exporter dead.

The temperature clamp. The mock clamps an injected temperature to the profile's shutdown_threshold_c, and the assertion checks exact equality, so too high a value burns the poll budget and reports a timeout that never mentions clamping. Default is 90 °C, the one band that works for every --gpu-profile: above the simulator's 73 °C ceiling (so a sibling can't wander onto it and fake a scope leak) and at or below the lowest threshold of any profile (92 °C on a100/h100). Guarded up front with messages that name both failure modes, plus a clamp hint on timeout.

Grafana access is a local_resource with serve_cmd port-forwarding the Grafana Service by name, gated on a readiness probe against Grafana's /api/health. Not helm_resource(port_forwards=...): that extension deploys a whole release as one Tilt resource owning every pod, so Tilt would forward to whichever of Prometheus, node-exporter, kube-state-metrics or Grafana it picked.

Verification

tilt ci -- --observabilitySUCCESS. All workloads are healthy. in 51s (warm cluster).

dcgm-exporter discovered through the ServiceMonitor and scraped on both workers:

pool=serviceMonitor/gpu-operator/nvidia-dcgm-exporter/0 job=nvidia-dcgm-exporter health=up node=mokka-worker
pool=serviceMonitor/gpu-operator/nvidia-dcgm-exporter/0 job=nvidia-dcgm-exporter health=up node=mokka-worker2
count(DCGM_FI_DEV_GPU_TEMP) = 16

Grafana imported the dashboard, via Grafana's own API rather than the ConfigMap existing:

uid=mokka-gpu title=Mokka — Mock GPU Fleet type=dash-db

Both triggers run twice, each showing the non-vacuity property holding:

inject-thermal #1: gpu 0 on mokka-worker stepped 63C -> 90C   (siblings 59-65C)
inject-thermal #2: gpu 0 on mokka-worker stepped 70C -> 90C   (different baseline: the reset works)
inject-xid #1: series absent -> 79   (did not exist before, so the whole path was travelled)
inject-xid #2: 79 -> 48              (rotated: a fresh delivery, not a latched re-read)

No regression with the flag off: tilt alpha tiltfile-result yields the identical resource graph as before (nvml-mock, gpu-validator), and the GPU Operator gets exactly its original single --values file; the overlay is layered second only when --observability is on.

Static: shellcheck clean, bash -n clean, jq empty on the dashboard, SPDX headers on all new YAML/shell.

Not verified

  • A cold-cluster run. The ServiceMonitor CRD survives a Helm uninstall, so the CRD-ordering dependency wasn't exercised on this pass; it's correct by construction via resource_deps.
  • Dashboard hot-reload. The ConfigMap is built with read_file + encode_yaml so Tilt watches the JSON and should re-apply on edit, but I didn't confirm the sidecar re-imports it.

@copy-pr-bot

copy-pr-bot Bot commented Aug 18, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@giuliocalzo
giuliocalzo force-pushed the feat/issue-597-observability-demo branch from 70f60d8 to 565bc72 Compare August 18, 2026 08:32
@giuliocalzo giuliocalzo changed the title feat(demo): observability demo with Prometheus and Grafana feat(local): --observability Tilt consumer with Prometheus and Grafana Aug 18, 2026
@giuliocalzo

Copy link
Copy Markdown
Collaborator Author

view from grafana

image

@giuliocalzo
giuliocalzo marked this pull request as ready for review August 20, 2026 14:54
Kind topology with CDI enabled and the Grafana/Prometheus NodePorts mapped to
the host, plus the run.sh scaffolding and container-toolkit install shared with
the nv-sentinel demo.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Re-provisioning the nvidia-container-toolkit on every invocation restarted
containerd on both workers, cycling them through NotReady and tearing a gap in
the GPU metrics series this demo exists to render -- and requiring network
access even when nothing needed installing. Skip provisioning on nodes that
already have it.

Also harden the copied setup block: give the config.toml rewrite a `set -e` so
a failure cannot leave the runtime silently in non-CDI mode, match CLUSTER_NAME
literally with `grep -qxF`, re-export the kubeconfig on the reuse path so a
pruned context heals itself, and bind the Grafana and Prometheus host ports to
loopback instead of every interface.

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The per-worker idempotency guard probed for the existence of
/etc/nvidia-container-runtime/config.toml, but the nvidia-container-toolkit
package ships that file as a default, non-CDI config. A first run interrupted
between the apt install and the CDI rewrite therefore left the runtime in
default mode while every later run claimed the node was already provisioned,
turning a cheap failure into "no GPUs in dcgm-exporter" several phases later.

Key the guard on the CDI rewrite instead, which is the last provisioning step
and so the only trustworthy completion marker.

Also gate the reuse path's `kind export kubeconfig` on the context actually
being missing, so a healthy reuse no longer repoints the caller's
current-context, and switch CLUSTER_NAME/IMAGE_NAME to the `: "${VAR:=...}"`
form used by the rest of the configuration block.

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Dynamic metrics keep temperature, power and utilization time-varying so the
Grafana panels show movement instead of flat profile constants.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
…riables

The nvml-mock install hardcoded image.repository/image.tag and the nodeSelector
key=value, duplicating IMAGE_NAME and GPU_NODE_LABEL declared in the same file.
Overriding the image or editing the label therefore built and labelled one thing
while the chart requested another, and both mistakes surfaced only as `helm
--wait` timing out after 180s without naming the image or the label.

Make IMAGE_REPO and IMAGE_TAG the overridable knobs and derive IMAGE_NAME from
them, so the chart and the build/load path cannot disagree; parsing a combined
reference apart instead would mis-handle a missing tag and a registry with a
port. Pass the nodeSelector as nodeSelector.${GPU_NODE_LABEL}, which already
holds the key=value form the flag wants.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Alertmanager and the Kind-noisy default rules are off and retention is capped at
2h to respect the per-node footprint budget from NVIDIA#597. Installed before the GPU
Operator so the ServiceMonitor CRD exists when the operator creates one.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Enables the GPU Operator's own ServiceMonitor rather than hand-writing one: the
exporter Service is created by the clusterpolicy controller, so only the
operator can be relied on to name its port correctly.

Asserts the target is actually up, because a ServiceMonitor missing the
kube-prometheus-stack release label is ignored silently.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Six panels covering GPU temperature, Xid errors, power, utilization, GPU
inventory and per-DaemonSet component health. Provisioned from a labelled
ConfigMap so the dashboard stays a reviewable file instead of UI state.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The observability demo now installs the mock into the `mokka` namespace
instead of `nvml-mock-system`, and the variable is renamed from
NVML_MOCK_NAMESPACE to MOKKA_NAMESPACE to match. The dashboard's restart
panel matcher moves with it, since a namespace matcher that drifts from
the Helm --namespace silently drops the mock's series from that panel.

This deliberately diverges from the nv-sentinel demo and from the chart
and E2E tests, which still use nvml-mock-system.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Grafana and Prometheus are mapped with listenAddress 0.0.0.0 so the
dashboard can be demoed from another machine, reversing the earlier
loopback-only default at the user's request.

The trade-off is real exposure: both ports are reachable from the local
network, and Grafana accepts the demo's default password, so the cluster
should be treated as throwaway and kept off untrusted networks.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The dashboard's restart panel hardcodes the mokka and gpu-operator namespace
names, so advertising MOKKA_NAMESPACE and GPU_OPERATOR_NAMESPACE as env
overrides promised something the static JSON cannot honour: an override
installed and imported cleanly while silently dropping that namespace's series.
Make both plain assignments and record why they must move with the dashboard.

Also: title the Xid panel as the last code reported rather than an error count,
since DCGM_FI_DEV_XID_ERRORS latches at the most recent Xid; surface the real
error on the dashboard-import failure path, whose 36 silent polls hid both a
missing curl and a password mismatch behind a message blaming the sidecar; and
pass grafana.adminPassword with --set-string, as --set coerces digit-only and
comma-bearing passwords into a password Grafana never accepts.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Heats a GPU and trips an uncorrectable ECC fault through nvml-mock-ctl, then
polls the Prometheus API until each change is recorded, failing if it never
arrives. This makes the demo a check on the whole scrape path rather than a
screenshot.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The Xid assertion could pass on residue: DCGM latches the last Xid per
device, so from the second run on the series already carried XID_CODE
before anything was injected -- and would have passed with dcgm-exporter
dead, since Prometheus keeps serving the last sample. Phase 2 now injects
whichever of XID_CODE / XID_CODE_ALT is not currently reported, so the
value it waits for cannot be present until this run's injection lands.

Two series can then describe the same GPU while the superseded err_code is
still un-stale, so prom_gpu_value takes an optional label to disambiguate
and rejects an ambiguous read instead of picking whichever series
Prometheus listed first. An instant query stamps every result with the
evaluation time, so recency cannot break that tie.

Also: fail when a pod-churn selector matches nothing, rather than
comparing two empty fingerprints and passing unconditionally; reject an
unrecognised comparison operator instead of degrading it to "!="; print
err_code alongside the identity labels the scope claim rests on; and stop
the phase-1 comment claiming the reset makes phase 2 deterministic.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Covers the pipeline, the dashboard panels and how to read them against the
mock's metric fidelity, the footprint cost, and the ServiceMonitor release-label
coupling that is the most common way to break the demo.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The three commands under "Injecting faults by hand" could not be pasted as a
block and show anything: `reset` drops the whole device override before the
exporter has ticked, so neither the temperature step nor the Xid ever rendered.
Split them into separate snippets, put the sequencing requirement above them
instead of after, and give `reset` its own block stating that it undoes the
other two.

Corrected alongside it: the clamp guidance moves next to the HOT_TEMP_C row and
now says values up to and including 92 read back verbatim (the clamp is
strictly-greater) and what exceeding it costs; the clamp is not conditional on
dynamicMetrics, since `temp` always writes a dynamic_metrics block the device
rebuilds its simulator from; a pinned namespace drops the restart series rather
than emptying the whole panel; two panel titles now match the dashboard JSON;
GPU_COUNT documents its minimum of 2; bash 4 joins the requirements, since
`mapfile` aborts on stock macOS bash.

In the closing summary: the temperature panel renders every GPU in the fleet,
not just the faulted node's, so report the series count Prometheus served; two
Xid lines during the rotation's staleness window are not a second fault; the
reset comment no longer claims a return to healthy when DCGM keeps reporting
the latched code; and the summary now points at the README.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Adds it to the canonical demo index and the root README table, which listed only
two of the seven demos.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Every previous demo landed with an entry under Added; this branch added a whole
demo without one.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
`kubectl rollout status` does not wait for an object to appear, so a first run
could reach it before the GPU Operator had created the DaemonSet and abort with
a bare NotFound. The only thing standing between the two was the nvidia.com/gpu
advertise loop, which breaks as soon as the device plugin is ready -- a
ClusterPolicy state earlier -- and which also fell through silently on expiry,
so a slow bring-up surfaced as an unexplained error on the next line.

Poll for the DaemonSet separately, warn when the advertise loop expires, and
record why that loop must not be deleted for looking redundant.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The component-health panel promised "which Mokka component broke and when" but
summed restarts by namespace, so on a healthy cluster it rendered
gpu-operator=56 from ordinary GPU Operator operand churn -- a Mokka pod
restarting three times was arithmetically invisible inside it. Sum by pod and
scope the selector to the two components the panel's other series track, so the
legend identifies what broke.

The ready/desired queries gain the namespace matcher they lacked, which kept a
cluster running the repo's nvml-mock-system DaemonSet from rendering two
identically legended lines.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Both the closing summary and the README handed the reader `temp --gpu 0 90` --
the exact override the run leaves in place -- and then told them to watch the
temperature step up and go flat. It was already flat. Reorder the walkthrough to
reset first, so every step changes what the dashboard shows, the way the Xid
line already did by handing over the rotated code.

Phase 1 now resets every GPU on the faulted node rather than only the target.
A pin left on a sibling by a by-hand session previously survived every re-run
and made the scope check accuse the run of leaking its override across the node;
it also falsified the summary's claim that the siblings keep wandering.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Correctness and diagnosability only; the demo proves the same things.

- Assert the exact DCGM series count (GPU_COUNT x workers) rather than "> 0",
  which one dead exporter passed. Polled, because the target wait returns on the
  first exporter to report up.
- Anchor the dcgm-exporter job match and the CDI provisioning guard, so a
  substring job name or a commented-out `mode = "cdi"` cannot satisfy either.
- Replay the Prometheus readiness and target queries once before failing, so a
  permanent error stops looking like five minutes of "not ready yet".
- `helm repo add --force-update` instead of `|| true`, which kept a same-name
  repo pointing elsewhere and surfaced as a chart-not-found.
- `--set-string` for image.tag and the ServiceMonitor release label, matching
  their neighbours.
- Fingerprint every container's restart count, not containerStatuses[0].
- Route the last bare `kubectl` through `command kubectl`, and stop `tee` from
  printing the apt sources list into the demo output.
- Move the two pinned namespace constants out of the "override via env" block
  they contradicted.
- Phrase the superseded-err_code coexistence conditionally: Prometheus usually
  staleness-marks the old series in the same scrape, so asserting that two lines
  are present was stating a timing detail as fact. Same for the measured
  25-45s propagation the comment still called ~25s.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
- Drop the dead grafana.adminPassword, which run.sh always overrides, and record
  that the dashboard's hardcoded "uid": "prometheus" is a chart default this
  values file cannot pin.
- Cross-reference the NodePort literals between kind.yaml and the values file.
  Drift there breaks the browser silently, since run.sh queries Prometheus
  through the API-server service proxy and keeps passing.
- Document HOT_TEMP_C's lower bound: the h100 simulator's band is 52-73 C, so a
  value inside it makes phase 1's scope check accuse the run of leaking an
  override it scoped correctly.
- Explain devicePlugin.config.name, the one knob in the values file without a
  why-comment.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The sibling docs/superpowers/ was already ignored, but .superpowers/ was merely
untracked, so a `git add -A` before opening a PR would sweep an agent review
ledger into the branch.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Self-review pass after the fix wave: the README still described the DCGM check as
"the series are present" and phase 1 as clearing "any leftover override", both of
which understate what the script now does. The scope-check comment said an
`--gpu all` mistake at a point where the script legitimately runs
`reset --gpu all`, so name the command that would actually be the mistake.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Runs kube-prometheus-stack and the GPU Operator's dcgm-exporter over the mock
GPUs and ships the Mokka dashboard in-tree, so the real, unmodified exporter is
scraped and rendered on a cluster with no GPUs.

Two manual triggers inject a temperature pin or an Xid through nvml-mock-ctl and
fail if the fault never reaches Prometheus, which makes the scrape path asserted
rather than eyeballed. Neither can pass on residue: the thermal run resets every
GPU on the node and waits for a reading that differs before asserting exact
equality, and the Xid run rotates between two codes because DCGM latches the last
Xid per device and never retracts it.

kube-prometheus-stack installs before the GPU Operator because it ships the
ServiceMonitor CRD the Operator's chart needs, and the Operator's ServiceMonitor
must carry release=kube-prometheus-stack or Prometheus silently ignores the
target and every panel is empty with no error anywhere.

gpu_operator.tiltfile grows extra_values/extra_resource_deps to carry that
overlay and ordering; its default behavior is unchanged.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The demo composes with the GPU Operator rather than replacing it, so Tilt is a
better home than a self-contained run.sh that built its own cluster: the script
duplicated the per-worker nvidia-container-toolkit provisioning and CDI wiring
that the shared kind-node image already bakes in.

Drops docs/demo/observability/ and points the demo index and root README at
`tilt up -- --observability`, documented in local/README.md.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
local/README.md documents the flag; this carries the reference detail a reader
needs once the stack is up: what lands in the cluster, what each file is for, how
to read the six dashboard panels, the scenario tunables, and the behaviours that
look like bugs and are not — the empty Xid panel, the alternating Xid code, and
the temperature clamp.

It also names the two couplings that fail silently, the ServiceMonitor release
label and the dashboard sidecar label, because neither produces an error when it
drifts; and the gotchas that were duplicated in local/README.md now live here
only, so the two cannot disagree.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The demo index only said where observability moved. Say what it demonstrates and
how to start it, in the what-it-does / requirements / commands form the sibling
entries use, so a reader browsing the index can decide whether to follow it
without leaving the page.

Points at local/observability/README.md for the walkthrough, which is where the
detail now lives.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Every other resource the Tiltfiles create carries a label, so the two unlabeled
nvml-mock helm_resource calls were the only ones landing in Tilt's catch-all
group. Labelling them mokka-core makes the core stack read as one group beside
the consumers, which matters more now that --observability adds four more
resources to the UI.

Refs NVIDIA#597

Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
@giuliocalzo
giuliocalzo force-pushed the feat/issue-597-observability-demo branch from 34ae65e to f4080e6 Compare August 20, 2026 14:59
@giuliocalzo giuliocalzo self-assigned this Aug 20, 2026
@giuliocalzo giuliocalzo added kind/test issue/PR to fix/edit/create/enhance a project unit/e2e test kind/feature Feature request or enhancement. labels Aug 20, 2026

@ArangoGutierrez ArangoGutierrez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing here blocks merge.

The part I like here is that both scenarios are real assertions rather than eyeball demos, and the non-vacuity arguments hold up: the thermal reset-then-wait-for-a-differing-reading genuinely stops a re-run passing on the previous pin, and rotating Xid codes is the right answer to DCGM latching the last one. I checked the HOT_TEMP_C guard band against the source rather than the comment, and it is correct: 73 is exactly base 55 + ramp 15 + variance 3 from the dynamic-metrics simulator, 92 is the lowest shutdown_threshold_c (a100/h100), and no profile overrides the temperature defaults. The err_code ambiguity hard-error and the local-dev Grafana password both read as deliberate calls, so I left them alone. What follows is one assertion I think went missing in the port, plus some comment drift.

  1. This comment describes two split-out port constants, but it sits above _GRAFANA_URL (a dashboard URL) and the port-forward hardcodes 3000:80 inline in _serve_grafana. Reads like leftover from a refactor that removed the constants. (local/observability/observability.tiltfile:55)
  2. The "Ported from docs/demo/observability/run.sh" header points at a path this branch deletes, so it resolves to nothing for anyone reading after the merge. Citing #597 or the commit would survive. (local/observability/scenarios/inject-thermal.sh:18; also local/observability/scenarios/inject-xid.sh:21)
  3. _install_dashboard() hardcodes grafana_dashboard: "1" rather than reading these values, so "observability.tiltfile builds the ConfigMap's label from them" overstates it. The sync is manual, which is what the tiltfile's own comment already says correctly. (The enabled/label/labelValue/searchNamespace values themselves do match the 88.3.0 chart defaults.) (local/observability/kube-prometheus-stack.values.yaml:37)

"${sibling_count}" "${target_node}" \
"$(jq -r '[.[] | "gpu" + .metric.gpu + "=" + .value[1] + "C"] | sort | join(" ")' <<<"${siblings}")"

printf '\n==> inject-thermal passed: gpu %s on %s stepped %sC -> %sC and Prometheus recorded it.\n' \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The port drops the pod-churn check run.sh carried: it fingerprinted the nvml-mock and dcgm-exporter pods (name, startTime, per-container restartCount) before and after injection and failed if either set changed, so "no pod was recycled, so the series has no gap" was asserted rather than stated. Both script headers and the README still make that claim, but nothing verifies it now. If the Operator replaces dcgm-exporter mid-run, the pinned value still reaches Prometheus on the new pod and the scenario prints passed over exactly the gap it advertises against.

@roma-glushko roma-glushko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 🥇

env:
# The exporter loads the mock libnvidia-ml.so from the driver root the
# nvml-mock DaemonSet stages on each node.
- name: NVIDIA_DRIVER_ROOT

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to be careful with what we put into these YAML overrides because that's our end user experience. To me, env vars like this leaks what NRI was promised to help us with - hiding how we plug in mokka into consumer workloads.

Can we make NRI do this injection for us instead of asking our users to apply overrides themselves?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two sh files make me curious what's the best way to organize that.
Right now they look like one off tests that should not be even committed.

@giuliocalzo Let's think together why we are doing that & what we might be missing to do it in cleaner way if at all.

The shared Kind config carries no extraPortMappings, and adding them for
one optional consumer would change the cluster shape for every other one.
"""
local_resource(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tilt allows to do portforwarding natively not need to hack it:

https://docs.tilt.dev/accessing_resource_endpoints.html#creating-a-kubectl-port-forward-tunnel

create configmap` so Tilt watches the JSON: editing a panel and saving
re-applies the ConfigMap and the sidecar re-imports it, with no re-install.
"""
k8s_yaml(encode_yaml({

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine to create a CM/Secret from tilt but I would even consider having a custom, slim helm chart that depends on grafana chart and adds this configmap + required values overrides to it.

This might be even a cleaner solution. Worth trying out.

Then tilt would just deploy that helm chart. The only inconvenience could be the need to do helm build before tilt up (but tilt can handle that as well).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature Feature request or enhancement. kind/test issue/PR to fix/edit/create/enhance a project unit/e2e test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants