Wait for MPS daemon readiness before advertising shared resources by jonathan-meiri · Pull Request #1946 · NVIDIA/k8s-device-plugin

jonathan-meiri · 2026-08-02T13:21:28Z

Summary

The device plugin can advertise MPS-shared resources to kubelet before the MPS daemon has finished configuring per-device memory limits and thread percentages. A pod scheduled in that window runs against MPS with the daemon defaults (no pinned memory limit, 100% threads) instead of the configured limits — silently bypassing the intended isolation.

This gates readiness on the MPS daemon's .ready file, closing the two existing TODOs in waitForDaemon.

Contributed by @Meiri28 on behalf of @runatom-ai.

The race

internal/plugin/server.go:Start() calls plugin.mps.waitForDaemon() before Serve() + Register(). Previously waitForDaemon ran a single AssertHealthy():

// TODO: Check the .ready file here.
// TODO: Have some retry strategy here.
if err := m.daemon.AssertHealthy(); err != nil { ... }

AssertHealthy() issues get_default_active_thread_percentage, which only proves the control pipe is responsive. In Daemon.Start() the ordering is:

mpsControlBin -d starts → pipe becomes responsive (AssertHealthy passes here)
per-device pinned memory limits applied
active thread percentage applied
.ready file created (after all daemons' Start() return)

If the single check lands between (1) and (4), the plugin registers as ready while the configured MPS limits are not yet in place.

The MPS control daemon already creates a node-global .ready file only after full initialization, but nothing consumed it — the two TODOs noted exactly this gap. Both components share the same /mps hostPath mount, so the file is visible to the device plugin.

Changes

mps/root.go: add Root.ReadyFilePath() so the marker path has a single definition.
mps-control-daemon/main.go: create/remove .ready via ReadyFilePath() instead of the hardcoded /mps/.ready, keeping writer and reader in sync.
mps/daemon.go: add Daemon.Ready(), reporting whether the .ready file exists.
internal/plugin/mps.go: rewrite waitForDaemon to poll checkDaemonReady (Ready() and AssertHealthy()) every 5s up to a 5m bound. On timeout the caller fails and is retried by the plugin manager, so the bound is per-attempt.

Test plan

go test ./... — full suite passes; new TestReadyFilePath and TestDaemonReady cover the readiness primitive (readiness reflects .ready file existence).
make build, make check-modules, gofmt — clean.

Commits are DCO-signed.

copy-pr-bot · 2026-08-02T13:21:31Z

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.

myeolenv · 2026-08-06T16:03:46Z

Thanks for the contribution. We will review this soon.

jonathan-meiri · 2026-09-14T20:12:50Z

Trimmed all four, comments now ≤2 lines. Thanks @tariq1890!

tariq1890 · 2026-09-14T21:18:41Z

@@ -0,0 +1,43 @@
+/**
+# Copyright 2026 NVIDIA CORPORATION


Suggested change

# Copyright 2026 NVIDIA CORPORATION

# Copyright (c) NVIDIA CORPORATION. All rights reserved.

tariq1890 · 2026-09-14T21:20:03Z

-	// TODO: Check the .ready file here.
-	// TODO: Have some retry strategy here.
+
+	return wait.PollUntilContextTimeout(context.Background(), mpsReadyCheckInterval, mpsReadyCheckTimeout, true, func(context.Context) (bool, error) {


let's propagate a context from the caller instead of intitialising a new one here.

Done, copyright header fixed, and context is now propagated from the caller
(Start(ctx, ...) → waitForDaemon(ctx)). Thanks!

tariq1890 · 2026-09-14T21:20:17Z

Suggested change

func (m *mpsOptions) waitForDaemon(ctx context.Context) error {

abrarshivani · 2026-09-14T22:17:44Z

 		}
 	}
-	readyFile, err := os.Create("/mps/.ready")
+	readyFile, err := os.Create(mps.ContainerRoot.ReadyFilePath())


Can we remove .ready before starting the mps daemons? /mps is a hostPath, so the file can survive a container restart. A stale .ready can make the device plugin think mps is ready before the new daemon has finished applying the new config.

abrarshivani · 2026-09-14T22:22:16Z

+// finished initialization (compute mode, memory limits, thread percentages).
+func (d *Daemon) Ready() bool {
+	_, err := os.Stat(d.root.ReadyFilePath())
+	return err == nil


Can Ready() return an error as well? Right now any os.Stat error is treated as not ready, so a missing file looks the same as a permission or filesystem error.

abrarshivani · 2026-09-14T22:34:20Z

+	require.False(t, d.Ready(), "not ready before the .ready file exists")
+
+	require.NoError(t, os.WriteFile(filepath.Join(root, ".ready"), nil, 0o644))
+	require.True(t, d.Ready(), "ready once the .ready file exists")


Can we add regression tests for this change? The current tests only check .ready file presence. They don't cover the race this PR fixes, such as the mps pipe being healthy before .ready is created.

All three addressed:

Clear stale .ready at daemon startup (hostPath can survive restarts).

Ready() now returns (bool, error) so real stat errors surface.

Added regression tests: readiness is withheld until .ready exists (before the pipe is checked), plus a stat-error case.
Thanks for the careful review!

Thanks, these address the earlier comments. I left one more comment about a possible config update race.

abrarshivani · 2026-09-22T02:59:51Z

+// checkDaemonReady requires the .ready file (written after full configuration)
+// and a responsive pipe; AssertHealthy alone responds before config is applied.
+func (m *mpsOptions) checkDaemonReady() error {
+	ready, err := m.daemon.Ready()


One case I think is still possible: say the current config is A and it changes to B. The device plugin can observe B before the MPS daemon does. In that window, .ready may still be from A and the MPS pipe may still be healthy, so this check can succeed before MPS has applied B.

This should converge once the MPS daemon processes the update, so I don't think it needs to block this PR, but ideally readiness should also confirm that MPS is running the expected config

Good catch, you're right this gates startup but not a config change. Opened #2055 to track it; I'll follow up there once this PR merges.
Thanks!

The device plugin's waitForDaemon only ran a single AssertHealthy check before serving and registering the resource with kubelet. AssertHealthy issues get_default_active_thread_percentage, which only proves the MPS control pipe is responsive — and the pipe becomes responsive at Daemon.Start (mpsControlBin -d) before the per-device pinned memory limits and active thread percentage are applied. A pod scheduled in that window starts against MPS with the daemon defaults (no pinned memory limit, 100% threads) rather than the configured limits, silently bypassing the intended isolation. The MPS control daemon already creates a node-global .ready file, but only after every daemon's full initialization completes. Nothing consumed it (the two TODOs in waitForDaemon noted exactly this), so the readiness signal was unused. Gate readiness on that file: - Add Root.ReadyFilePath so the marker path has a single definition, and use it in the MPS control daemon for both create and remove instead of the hardcoded "/mps/.ready". - Add Daemon.Ready, which reports whether the .ready file exists. - Rewrite waitForDaemon to poll checkDaemonReady (Ready AND AssertHealthy) every 5s up to a 5m bound, replacing the single unconditional AssertHealthy. On timeout the caller fails and is retried by the plugin manager, so the bound is per-attempt. This closes both TODOs and ensures shared MPS resources are not advertised until the daemon is fully configured. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: runatom-ai <258621014+runatom-ai@users.noreply.github.com> Signed-off-by: Jonathan Meiri <33288957+Meiri28@users.noreply.github.com>

jonathan-meiri force-pushed the mps-wait-for-ready branch 2 times, most recently from 6966a94 to bfcb598 Compare August 6, 2026 13:13

jonathan-meiri force-pushed the mps-wait-for-ready branch from bfcb598 to 827e00b Compare August 12, 2026 17:03

myeolenv assigned tariq1890 Aug 26, 2026

jonathan-meiri force-pushed the mps-wait-for-ready branch 4 times, most recently from ed8adcd to 13985c2 Compare September 5, 2026 09:03

jonathan-meiri force-pushed the mps-wait-for-ready branch from 13985c2 to cc570b7 Compare September 14, 2026 18:49