Release workload on a non-MNNVL node in a CD

[jgehrcke]

Resolves #625.

Slightly less trivial than I thought.

• Needs a special case maxNodesPerIMEXDomain for the group of non-MNNVL nodes (can become large)
• Always needs injection of CD details into CD daemon container (via CDI)

The group of nodes that does not perform MNNVL may be large. With this change, all of these nodes will contribute to the update-with-retry-on-conflict race, which is important to consider. I added a few code comments along the way.

I tested this manually, with this patch to simulate the scenario:

--- a/cmd/compute-domain-kubelet-plugin/nvlib.go
+++ b/cmd/compute-domain-kubelet-plugin/nvlib.go
@@ -197,6 +197,9 @@ func (l deviceLib) enumerateComputeDomainDaemons(config *Config) (AllocatableDev
 }
 
 func (l deviceLib) getCliqueID() (string, error) {
+
+       return "", nil
+

CD daemon log excerpt, from start-to-shutdown:

± kubectl logs -n nvidia-dra-driver-gpu   imex-channel-injection-p7m8n-2ghhj
I1007 11:24:04.713698       1 main.go:200] config: &{gb-nvl-043-compute06 49426eaf-63f5-4610-8928-306a99de8777 imex-channel-injection default  192.168.35.115 imex-channel-injection-p7m8n-2ghhj nvidia-dra-driver-gpu 18}
I1007 11:24:04.713783       1 main.go:209] no cliqueID: register with ComputeDomain, but do not run IMEX daemon
I1007 11:24:04.714026       1 dnsnames.go:199] Created static nodes config file with 18 DNS names using format compute-domain-daemon-%d
[...]
I1007 11:24:04.715152       1 process.go:175] Start watchdog
I1007 11:24:04.715185       1 main.go:338] wait for nodes update
I1007 11:24:04.715330       1 reflector.go:358] "Starting reflector" type="*v1beta1.ComputeDomain" resyncPeriod="10m0s" reflector="pkg/nvidia.com/informers/externalversions/factory.go:141"
I1007 11:24:04.715346       1 reflector.go:404] "Listing and watching" type="*v1beta1.ComputeDomain" reflector="pkg/nvidia.com/informers/externalversions/factory.go:141"
[...]
I1007 11:24:04.916398       1 computedomain.go:266] CD status does not contain node name 'gb-nvl-043-compute06' yet, try to insert myself: &{gb-nvl-043-compute06   0 NotReady}
[...]
I1007 11:24:04.919998       1 computedomain.go:288] Successfully updated CD
[...]
I1007 11:24:04.920268       1 main.go:350] empty cliqueID: do not start IMEX daemon
[...]
I1007 11:24:06.092166       1 podmanager.go:194] Successfully updated node gb-nvl-043-compute06 status to Ready
[...]
I1007 11:58:36.795592       1 main.go:89] Received SIGTERM, initiate shutdown
I1007 11:58:36.795671       1 main.go:341] shutdown: stop IMEXDaemonUpdateLoopWithDNSNames
I1007 11:58:36.795696       1 main.go:278] Terminated: IMEX daemon update task
I1007 11:58:36.795690       1 process.go:179] Watchdog: context canceled, attempt to stop child process
E1007 11:58:36.795727       1 main.go:288] watch failed, initiate shutdown: watchdog: stop failed: pm: stop failed: not started
I1007 11:58:36.795737       1 main.go:291] Terminated: process manager
[...]
I1007 11:58:36.803227       1 main.go:257] Terminated: controller task
I1007 11:58:36.803241       1 main.go:297] Exiting

With the getCliqueID() patch removed, I saw a passing test suite:

15 tests, 0 failures in 118 seconds

[copy-pr-bot]

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.

[jgehrcke]

I think a question I had here really is:

If everyone only sends their own update after receiving an update -- who performs the initial update to start the cascade?

[klueska]

onAddOrUpdate() gets called on any addition / update of the CD. When the informer first comes online it triggers an "add" on all of the existing objects that it receives from the API server. This is the intial callback that allows this component to insert its NodeInfo in the Status.

[jgehrcke]

When the informer first comes online it triggers an "add" on all of the existing objects that it receives from the API server.

Thank you! (I now remember that you've described this before)

[jgehrcke]

added commit, removed comment (clarified this in the comment for onAddOrUpdate though that I won't forget again so quickly)

[jgehrcke]

How many update attempts overall does it take along the happy path for say N=1000 nodes each trying to insert themselves at about the same time? The individual node has to expect a large number of conflict errors E before its own update succeeds. If E=100 (just making something up, this could be larger) then we make N*E=100000 update attempts in total. Is that a correct consideration? If it is, we should think about finding a smarter strategy that scales better with N.

[klueska]

After each attempt there is an exponential backoff with some random jitter to stagger attempts from different nodes. We also (re)read the CD object just before attempting to write, in order to make the window as small as possible where a conflict might slip in. We would need to measure it, but I don't think the number of average conflicts would be nearly as large as you are suggesting.

[jgehrcke]

We would need to measure it

Yes, that's the only productive approach!

We also (re)read the CD object just before attempting to write

Yes! But we understand this doesn't necessarily yield the latest -- we read it from the informer stream/cache (which might be subject to lag, right?).

After each attempt there is an exponential backoff with some random jitter to stagger attempts from different nodes.

I understand :)

I also do think we agree, and I am not planning to change anything here or criticize what we have.

I am merely trying to rationally reason through current scaling behavior.

In terms of that scaling behavior, we want to understand not only how the number of global write attempts scales with the number of nodes N (that might be a measure for API server load generated).

But (maybe even more importantly) we also want to understand how long the overall convergence duration D scales with N. And we probably eventually need to set ourselves a goal (as in: an upper bound that we find tolerable), and assess on that basis if we need to consider a more sophisticated approach.

nearly as large as you are suggesting.

I want to say: I did not suggest E=100, I was mainly suggesting somewhat linear scaling behavior with N. That might even be best case; the convergence duration might after all scale non-linearly (say, with O(N^2) or so) -- precisely because of the applied back-off during retrying.

We will see.

Supporting "Uber-sized Compute Domains" will be fun in that regard!

[jgehrcke]

removed the comment as part of rebasing on top of head of main -- thanks for discussion

[jgehrcke]

@klueska thanks for review. I added responded to comments and added commits. Can you have another look? Before merge, I would squash commits.

[jgehrcke]

I have rebased this branch on current head of main, resolved conficts, and also squashed commits.

To be sure the merge-to-main wouldn't be bad, I just tested this again manually with

--- a/cmd/compute-domain-kubelet-plugin/nvlib.go
+++ b/cmd/compute-domain-kubelet-plugin/nvlib.go
@@ -197,6 +197,7 @@ func (l deviceLib) enumerateComputeDomainDaemons(config *Config) (AllocatableDev
 }
 
 func (l deviceLib) getCliqueID() (string, error) {
+       return "", nil

Works (CD daemon log):

...
15:32:24 ± kubectl logs -n nvidia-dra-driver-gpu   imex-channel-injection-rdx24-k9qg5
I1008 15:32:22.570420       1 main.go:200] config: &{gb-nvl-043-compute07 91d00090-848c-4953-8a25-ffe23d409e90 imex-channel-injection default  192.168.34.186 imex-channel-injection-rdx24-k9qg5 nvidia-dra-driver-gpu 18}
I1008 15:32:22.570486       1 main.go:209] no cliqueID: register with ComputeDomain, but do not run IMEX daemon
...
I1008 15:32:24.173219       1 podmanager.go:201] Successfully updated node gb-nvl-043-compute07 status to Ready
...

Workload got released:

$ kubectl get pods
NAME                     READY   STATUS    RESTARTS   AGE
imex-channel-injection   1/1     Running   0          3m20s

Is of course half-broken because we didn't perform CDI-based injection:

$ kubectl  logs imex-channel-injection 
ls: cannot access '/dev/nvidia-caps-imex-channels': No such file or directory

Let's land this now, (unless there's a rough problem in here). Let's do all non-functional changes in a follow-up. Specifically, let us sort out finding a better encapsulation for the non-imex-daemon-mode later.

[klueska]

Add this to the function to avoid this else statement (and just always do this unconditionally):

// writeIMEXConfig renders the config template with the pod IP and writes it to the final config file.
func writeIMEXConfig(podIP string) error {
        // Ensure the directory exists
        dir := filepath.Dir(nodesConfigPath)
        if err := os.MkdirAll(dir, 0755); err != nil {
                return fmt.Errorf("failed to create directory %s: %w", dir, err)
        }
...
}

[jgehrcke]

Done, now calling os.MkdirAll() in writeIMEXConfig().

[jgehrcke]

After these changes, I once again needed to provoke & test that scenario. Due diligence. By mocking the cliqueID acquisition function (returning an empty string).

Glad I did, because the above's change resulted in:

Error: writeIMEXConfig failed: error parsing template file: open /etc/nvidia-imex/config.tmpl.cfg: no such file or directory

The problem with not mounting in /etc/nvidia-imex is not just "oh this dir doesn't exist".

The problem is: not mounting in the template to render the config file from.

But mounting that in requires writing it to the host filesystem in the first place. Otherwise:

Error: failed to create containerd task: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error mounting "/var/lib/kubelet/plugins/compute-domain.nvidia.com/domains/885e0f1b-f319-40dd-81c6-15ae6c690130" to rootfs at "/etc/nvidia-imex": create mount destination for /etc/nvidia-imex mount: bind mount source stat: stat /var/lib/kubelet/plugins/compute-domain.nvidia.com/domains/885e0f1b-f319-40dd-81c6-15ae6c690130: no such file or directory: unknown

I have now added more changes to this PR to always write this IMEX daemon configuration directory tree to the host, and to always mount it in (to the CD daemon). I think it makes sense!

Now, we only do not inject any IMEX-related dev nodes in non-IMEX mode.

So, with this change, we've created even more symmetry between non-IMEX and IMEX mode.

Specifically, we now always create

/var/lib/kubelet/plugins/compute-domain.nvidia.com/domains/<domainID>

on the host (that's the mount source).

One might argue that this host filesystem interaction is superfluous and only introduces more failure paths -- because if this fails, we can't even launch the CD daemon in 'pretend mode'. But it's very unlikely to ever hurt in practice.

I do believe that for now we should cherish the simplicity of "let's do everything except for actually spawning the IMEX daemon process".

[klueska]

Looking good, just a few small changes

[jgehrcke]

  Error: cmd/compute-domain-kubelet-plugin/device_state.go:496:14: `emtpy` is a misspelling of `empty` (misspell)
  	// ID being emtpy or not).
  	            ^
  1 issues:
  * misspell: 1

Of course. Time to get a spell check into my IDE, too.

[jgehrcke]

I've updated the PR with the changes discussed, and made more adjustments described here to further increase symmetry between "non-IMEX" and "actual-IMEX" modes (for a lack of better words). I've also worked on cleaning up the commit history a bit (squashed what made sense). I performed another manual test for cliqueID == "" and saw the desired behavior. I think I'd still like to land this tonight or on the weekend so that we can do a bit of QAing before next week.

[jgehrcke]

note to self: we could have made the diff smaller by moving Env up again -- but that's okay.

[jgehrcke]

done: #646 (comment)

[jgehrcke]

nit for symmetry, this could be callled GetCDIContainerEditsCommon instead of GetCommonCDIContainerEdits so that we have:

GetCDIContainerEditsForImex
GetCDIContainerEditsCommon

Personally, I like grouping by prefix.

[jgehrcke]

done: #646 (comment)

[jgehrcke]

Great discussion and feedback.

Post-approval, I took liberty to add this cosmetic cleanup patch (squashed into the last commit on this branch):

From de09926e03d39322c7de47ecafcc2da460671283 Mon Sep 17 00:00:00 2001
From: "Dr. Jan-Philip Gehrcke" <jgehrcke@nvidia.com>
Date: Sat, 11 Oct 2025 11:25:07 +0000
Subject: [PATCH] make diff smaller, rename func

Signed-off-by: Dr. Jan-Philip Gehrcke <jgehrcke@nvidia.com>
---
 .../computedomain.go                             | 16 ++++++++--------
 .../device_state.go                              |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/cmd/compute-domain-kubelet-plugin/computedomain.go b/cmd/compute-domain-kubelet-plugin/computedomain.go
index 76baba8d..5ea94db1 100644
--- a/cmd/compute-domain-kubelet-plugin/computedomain.go
+++ b/cmd/compute-domain-kubelet-plugin/computedomain.go
@@ -154,10 +154,10 @@ func (m *ComputeDomainManager) GetComputeDomainChannelContainerEdits(devRoot str
 	}
 }
 
-// GetCommonCDIContainerEdits() returns the CDI spec edits always required for
+// GetCDIContainerEditsCommon() returns the CDI spec edits always required for
 // launching the CD Daemon (whether or not it tries to launch an IMEX daemon
 // internally).
-func (s *ComputeDomainDaemonSettings) GetCommonCDIContainerEdits(ctx context.Context) (*cdiapi.ContainerEdits, error) {
+func (s *ComputeDomainDaemonSettings) GetCDIContainerEditsCommon(ctx context.Context) (*cdiapi.ContainerEdits, error) {
 	cd, err := s.manager.GetComputeDomain(ctx, s.domainID)
 	if err != nil {
 		return nil, fmt.Errorf("error getting compute domain %s: %w", s.domainID, err)
@@ -168,6 +168,12 @@ func (s *ComputeDomainDaemonSettings) GetCommonCDIContainerEdits(ctx context.Con
 
 	edits := &cdiapi.ContainerEdits{
 		ContainerEdits: &cdispec.ContainerEdits{
+			Env: []string{
+				fmt.Sprintf("CLIQUE_ID=%s", s.manager.cliqueID),
+				fmt.Sprintf("COMPUTE_DOMAIN_UUID=%s", cd.UID),
+				fmt.Sprintf("COMPUTE_DOMAIN_NAME=%s", cd.Name),
+				fmt.Sprintf("COMPUTE_DOMAIN_NAMESPACE=%s", cd.Namespace),
+			},
 			Mounts: []*cdispec.Mount{
 				{
 					ContainerPath: "/etc/nvidia-imex",
@@ -175,12 +181,6 @@ func (s *ComputeDomainDaemonSettings) GetCommonCDIContainerEdits(ctx context.Con
 					Options:       []string{"rw", "nosuid", "nodev", "bind"},
 				},
 			},
-			Env: []string{
-				fmt.Sprintf("CLIQUE_ID=%s", s.manager.cliqueID),
-				fmt.Sprintf("COMPUTE_DOMAIN_UUID=%s", cd.UID),
-				fmt.Sprintf("COMPUTE_DOMAIN_NAME=%s", cd.Name),
-				fmt.Sprintf("COMPUTE_DOMAIN_NAMESPACE=%s", cd.Namespace),
-			},
 		},
 	}
 	return edits, nil
diff --git a/cmd/compute-domain-kubelet-plugin/device_state.go b/cmd/compute-domain-kubelet-plugin/device_state.go
index 9f82e866..d6acf3e4 100644
--- a/cmd/compute-domain-kubelet-plugin/device_state.go
+++ b/cmd/compute-domain-kubelet-plugin/device_state.go
@@ -506,7 +506,7 @@ func (s *DeviceState) applyComputeDomainDaemonConfig(ctx context.Context, config
 
 	// Always inject CD config details into the CD daemon (regardless of clique
 	// ID being empty or not).
-	edits, err := computeDomainDaemonSettings.GetCommonCDIContainerEdits(ctx)
+	edits, err := computeDomainDaemonSettings.GetCDIContainerEditsCommon(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("error getting common container edits for ComputeDomain daemon '%s': %w", config.DomainID, err)
 	}