Skip to content

Commit bfcb598

Browse files
Meiri28clauderunatom-ai
committed
Wait for MPS daemon .ready before advertising shared resources
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>
1 parent c648e14 commit bfcb598

5 files changed

Lines changed: 109 additions & 6 deletions

File tree

cmd/mps-control-daemon/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ func startDaemons(c *cli.Context, cfg *Config) ([]*mps.Daemon, bool, error) {
213213
return mpsDaemons, true, nil
214214
}
215215
}
216-
readyFile, err := os.Create("/mps/.ready")
216+
readyFile, err := os.Create(mps.ContainerRoot.ReadyFilePath())
217217
if err != nil {
218218
return mpsDaemons, true, fmt.Errorf("failed to create .ready file")
219219
}
@@ -223,7 +223,7 @@ func startDaemons(c *cli.Context, cfg *Config) ([]*mps.Daemon, bool, error) {
223223
}
224224

225225
func stopDaemons(mpsDaemons ...*mps.Daemon) error {
226-
if err := os.Remove("/mps/.ready"); err != nil {
226+
if err := os.Remove(mps.ContainerRoot.ReadyFilePath()); err != nil {
227227
klog.Warningf("Failed to remove .ready file: %v", err)
228228
}
229229
klog.Info("Stopping MPS daemons.")

cmd/mps-control-daemon/mps/daemon.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,16 @@ func (d *Daemon) AssertHealthy() error {
206206
return err
207207
}
208208

209+
// Ready returns true once the MPS daemons have signalled that initialization
210+
// has completed by creating the node-global .ready file under the MPS root.
211+
// AssertHealthy only proves the control pipe is responsive, which happens
212+
// before per-device memory limits and thread percentages are applied; Ready
213+
// gates on the full configuration being in place.
214+
func (d *Daemon) Ready() bool {
215+
_, err := os.Stat(d.root.ReadyFilePath())
216+
return err == nil
217+
}
218+
209219
// EchoPipeToControl sends the specified command to the MPS control daemon.
210220
func (d *Daemon) EchoPipeToControl(command string) (string, error) {
211221
var out bytes.Buffer
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
# Copyright 2026 NVIDIA CORPORATION
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
**/
16+
17+
package mps
18+
19+
import (
20+
"os"
21+
"path/filepath"
22+
"testing"
23+
24+
"github.com/stretchr/testify/require"
25+
)
26+
27+
func TestReadyFilePath(t *testing.T) {
28+
require.Equal(t, "/mps/.ready", ContainerRoot.ReadyFilePath())
29+
require.Equal(t, "/custom/root/.ready", Root("/custom/root").ReadyFilePath())
30+
}
31+
32+
func TestDaemonReady(t *testing.T) {
33+
root := t.TempDir()
34+
d := &Daemon{root: Root(root)}
35+
36+
// No .ready file yet: the daemon must not report ready. This is the window
37+
// in which AssertHealthy can already succeed (control pipe responsive)
38+
// while per-device memory/thread configuration is not yet applied.
39+
require.False(t, d.Ready(), "daemon must not be ready before the .ready file exists")
40+
41+
// Once the MPS control daemon has finished initialization it creates the
42+
// .ready file; the daemon must then report ready.
43+
require.NoError(t, os.WriteFile(filepath.Join(root, ".ready"), nil, 0o644))
44+
require.True(t, d.Ready(), "daemon must be ready once the .ready file exists")
45+
46+
// Removing the file (e.g. on daemon stop) flips readiness back to false.
47+
require.NoError(t, os.Remove(filepath.Join(root, ".ready")))
48+
require.False(t, d.Ready(), "daemon must not be ready after the .ready file is removed")
49+
}

cmd/mps-control-daemon/mps/root.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,16 @@ func (r Root) startedFile(resourceName spec.ResourceName) string {
5252
return r.Path(string(resourceName), ".started")
5353
}
5454

55+
// ReadyFilePath returns the path to the node-global .ready file. Unlike the
56+
// per-resource .started file, this single marker is created only after all MPS
57+
// daemons have completed initialization (compute mode, pinned memory limits,
58+
// and active thread percentage). Consumers such as the device plugin use it to
59+
// avoid advertising MPS-shared resources before the daemons are fully
60+
// configured.
61+
func (r Root) ReadyFilePath() string {
62+
return r.Path(".ready")
63+
}
64+
5565
// Path returns a path relative to the MPS root.
5666
func (r Root) Path(parts ...string) string {
5767
pathparts := append([]string{string(r)}, parts...)

internal/plugin/mps.go

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package plugin
1919
import (
2020
"errors"
2121
"fmt"
22+
"time"
2223

2324
"k8s.io/klog/v2"
2425
pluginapi "k8s.io/kubelet/pkg/apis/deviceplugin/v1beta1"
@@ -28,6 +29,15 @@ import (
2829
"github.com/NVIDIA/k8s-device-plugin/internal/rm"
2930
)
3031

32+
const (
33+
// mpsReadyCheckInterval is how often we poll for MPS daemon readiness.
34+
mpsReadyCheckInterval = 5 * time.Second
35+
// mpsReadyCheckTimeout bounds how long we wait for the MPS daemon to become
36+
// ready before giving up. On timeout the caller (plugin startup) fails and
37+
// is retried by the plugin manager, so this is an upper bound per attempt.
38+
mpsReadyCheckTimeout = 5 * time.Minute
39+
)
40+
3141
type mpsOptions struct {
3242
enabled bool
3343
resourceName spec.ResourceName
@@ -62,12 +72,36 @@ func (m *mpsOptions) waitForDaemon() error {
6272
if m == nil || !m.enabled {
6373
return nil
6474
}
65-
// TODO: Check the .ready file here.
66-
// TODO: Have some retry strategy here.
75+
76+
deadline := time.Now().Add(mpsReadyCheckTimeout)
77+
for {
78+
err := m.checkDaemonReady()
79+
if err == nil {
80+
klog.InfoS("MPS daemon is ready", "resource", m.resourceName)
81+
return nil
82+
}
83+
if time.Now().After(deadline) {
84+
return fmt.Errorf("timed out waiting for MPS daemon for %v: %w", m.resourceName, err)
85+
}
86+
klog.InfoS("Waiting for MPS daemon to be ready", "resource", m.resourceName, "reason", err)
87+
time.Sleep(mpsReadyCheckInterval)
88+
}
89+
}
90+
91+
// checkDaemonReady reports whether the MPS daemon has finished initialization.
92+
// It requires both that the daemon has signalled readiness via its .ready file
93+
// (created only after compute mode, pinned memory limits, and thread
94+
// percentages are applied) and that the control pipe is responsive. Checking
95+
// only AssertHealthy is insufficient: the pipe becomes responsive before the
96+
// per-device configuration is in place, so a pod scheduled in that window
97+
// could run without the configured MPS memory/thread limits.
98+
func (m *mpsOptions) checkDaemonReady() error {
99+
if !m.daemon.Ready() {
100+
return fmt.Errorf("MPS daemon has not signalled readiness")
101+
}
67102
if err := m.daemon.AssertHealthy(); err != nil {
68-
return fmt.Errorf("error checking MPS daemon health: %w", err)
103+
return fmt.Errorf("MPS daemon is not healthy: %w", err)
69104
}
70-
klog.InfoS("MPS daemon is healthy", "resource", m.resourceName)
71105
return nil
72106
}
73107

0 commit comments

Comments
 (0)