Skip to content

Add GPUCluster CRD and controller for DRA-based stack#2571

Open
karthikvetrivel wants to merge 7 commits into
mainfrom
kv-gpuclusterconfig-crd
Open

Add GPUCluster CRD and controller for DRA-based stack#2571
karthikvetrivel wants to merge 7 commits into
mainfrom
kv-gpuclusterconfig-crd

Conversation

@karthikvetrivel

@karthikvetrivel karthikvetrivel commented Jun 23, 2026

Copy link
Copy Markdown
Member

1. Overview

We introduce a new CRD named GPUCluster and a new controller for reconciling it. Like ClusterPolicy today, it is a singleton, cluster-scoped CRD that configures the operands needed to enable GPUs in Kubernetes. GPUCluster represents the new DRA-based software-enablement stack; it is an evolution of ClusterPolicy. Unlike ClusterPolicy, it does not manage the driver or a device plugin: the driver is either preinstalled on the host or managed by NVIDIADriver CRs, and GPUs are surfaced to workloads through DRA.

A GPUCluster may coexist with a ClusterPolicy in the same cluster, with every GPU node served by exactly one of the two stacks.

2. Testing

Unit tests cover the new API, operand rendering, controller status handling, and the node-labeling logic. I also tested this end-to-end on a 2-node heterogeneous cluster (A100 + T4, Kubernetes v1.34):

  • GPUCluster-only install via Helm: all operands reconcile to ready. DRA allocation was validated for full GPUs, MIG, and adminAccess. The dra-driver-validator was verified against both host-installed and NVIDIADriver-managed drivers.
  • Coexistence: ClusterPolicy and GPUCluster run in one cluster, with one node per plane. Each node runs only its plane's operands, with extended resources on the device-plugin node and ResourceSlices on the DRA node.
  • Mode switching: flipping nvidia.com/gpu-operator.mode in either direction swaps the operand set. A shared NVIDIADriver-managed driver survives the switch without a reload.

Quick author design note: the driver daemonset deployed by the NVIDIADriver CR is not gated on the deploy mode so that it is not evicted and re-scheduled when the user switches modes.

  • Label lifecycle: driver-manager pause values survive reconciles. A node's previous-plane labels are cleaned up, and unrecognized gpu.deploy.* labels are left untouched.

I have not tested ComputeDomain functionality yet.

3. Next Steps

PRs to land:

We also need minor changes to the upgrade controller, which I'm working on as a follow-up PR. Context: DRA nodes are auto-enrolled in driver upgrades (the shared driver DaemonSet has no mode gate) but can never pass the hard-coded nvidia-operator-validator validation gate, leaving them cordoned in validation-required indefinitely — until fixed, autoUpgrade should be disabled for NVIDIADriver CRs serving DRA nodes. The upgrade controller is also currently ClusterPolicy-gated and must change.

After we have a new validator daemonset, this PR can be marked as ready:


Moved from #2513 (re-created with the head branch on NVIDIA/gpu-operator instead of a fork, to enable stacked PRs). The earlier review discussion — including the GPUClusterConfigGPUCluster naming suggestion, since adopted — lives in #2513.

Comment thread deployments/gpu-operator/values.yaml Outdated
Comment thread deployments/gpu-operator/templates/cleanup_crd.yaml
# NVIDIADriver CR). GPUClusterConfig does not manage the driver or device plugin
# itself; it waits for driver readiness before deploying the DRA driver.
gpuClusterConfig:
enabled: false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should think a bit more on the right interface for this. A few questions come to mind:

  1. Is enabled the right name for this field? As currently implemented, setting gpuClusterConfig.enabled=true will create a default GPUClusterConfig CR and will NOT create a default ClusterPolicy CR when the helm chart gets rendered. This may not be clear to the user.
  2. Should the draDriver struct be embedded under the top-level gpuClusterConfig struct?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was thinking about this. The logic has changed since your comment and now a ClusterPolicy CR is always created.

If we remove this enabled flag, are we okay with a GPUCluster CR always being created? This will cause a issue on clusters where the API server doesn't serve DRA APIs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

and now a ClusterPolicy CR is always created.

IMO we should provide the ability to NOT create a ClusterPolicy CR (in favor of a GPUCluster CR) on helm chart install. I am not quite sure what the interface should be here.

Thinking out loud here, what about:

defaultCRs:
  clusterPolicy:
    enabled: true
  gpuCluster:
    enabled: false

E.g. to only deploy a default GPUCluster CR, one would set defaultCRs.clusterPolicy.enabled=false defaultCRs.gpuCluster.enabeld=true.

@tariq1890 @rahulait @rajathagasthya any thoughts here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What about making the booleans on the same level?

defaultCRsEnabled:
  clusterPolicy: true
  gpuCluster: false 

@tariq1890 tariq1890 Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am generally okay with this structure

defaultCRsEnabled:
  clusterPolicy: true
  gpuCluster: false 

Just wondering if we can come up with an alternative name to defaultCRs

Comment thread api/nvidia/v1alpha1/gpuclusterconfig_types.go Outdated
Comment thread api/nvidia/v1alpha1/gpuclusterconfig_types.go Outdated
Comment thread api/nvidia/v1alpha1/gpuclusterconfig_types.go Outdated
Comment thread internal/state/dra_driver.go Outdated
Comment thread manifests/state-dra-driver/0500_daemonset.yaml Outdated
Comment thread manifests/state-dra-driver/0500_daemonset.yaml
Comment thread manifests/state-dra-driver/0500_daemonset.yaml Outdated
Comment thread manifests/state-gfd/0400_resourceclaimtemplate.yaml
@karthikvetrivel
karthikvetrivel force-pushed the kv-gpuclusterconfig-crd branch from 5691bbc to e5dcecd Compare July 1, 2026 15:24
@coveralls

coveralls commented Jul 1, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 32.688% (+0.9%) from 31.812% — kv-gpuclusterconfig-crd into main

@karthikvetrivel
karthikvetrivel force-pushed the kv-gpuclusterconfig-crd branch 4 times, most recently from a4c09b7 to 6b5ac48 Compare July 2, 2026 19:50
@karthikvetrivel karthikvetrivel changed the title Add GPUClusterConfig CRD and controller for DRA-based stack Add GPUCluster CRD and controller for DRA-based stack Jul 2, 2026
@karthikvetrivel
karthikvetrivel marked this pull request as draft July 2, 2026 20:19
@karthikvetrivel
karthikvetrivel marked this pull request as ready for review July 6, 2026 16:48
Comment thread controllers/nodelabeling_controller.go
Comment thread controllers/nodelabeling_controller.go Outdated
Comment on lines +120 to +124
// The driver auto-upgrade annotation is derived from ClusterPolicy spec.
if nlc.clusterPolicy != nil {
if err := nlc.applyDriverAutoUpgradeAnnotation(ctx); err != nil {
return reconcile.Result{}, err
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The auto-upgrade annotation is used by the k8s-driver-manager init container in the driver daemonset. So we need to set it regardless if ClusterPolicy or GPUCluster is used. We need to decouple the applyDriverAutoUpgradeAnnotation method from clusterpolicy -- it should account for when clusterpolicy is nil.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updated.

Comment thread controllers/nodelabeling_controller.go Outdated
if _, ok := labels[consts.GPUOperatorModeLabelKey]; ok {
return false
}
if nlc.defaultMode == "" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question -- would it be better to always default to one stack? E.g. if the user misconfigured the PREFERRED_MODE env or left it empty, shouldn't we fallback to a default set in the code?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If the user misconfigured the env or left it empty, defaultMode would still resolve to device-plugin (see resolveDefaultMode).

I can see how this guard is a little confusing. This check exists in case neither GPUCluster CR or ClusterPolicy CR exists (in which case, the mode would be an empty string on every node). Right now, this shouldn't be a concern but is defensive against future changes in the node labeling controller.

I can add a concise comment to clarify, unless you prefer removing it. I have no strong preference.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No strong opinion from me either. I think I slightly prefer removing since A) this method should never be called if neither a ClusterPolicy or GPUCluster CR exist (as Reconcile will return early in this case), and B) a reasonable assumption to make is that defaultMode is resolved / set prior to reconcileModeLabel() being called.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Got it. Removed.

Comment thread controllers/nodelabeling_controller.go Outdated
modified = true
}
return modified
case consts.GPUOperatorModeDevicePlugin:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question -- shouldn't we be calling sweepOtherPlaneStateLabels() in this case as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Under the GPUOperatorModeDevicePlugin case, we use the legacy ClusterPolicy engine.

https://github.com/NVIDIA/gpu-operator/blob/main/controllers/state_manager.go#L396-L405

At some point, I think it makes sense to unify these sweeps into a single shared sync engine (i.e. pull removeGPUStateLabels/addGPUStateLabels out of gpuWorkloadConfiguration) but I think that is out of scope for this PR.

name: nvidia-dra-validator
namespace: {{ .Namespace }}
labels:
app.kubernetes.io/name: nvidia-dra-validator

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question -- what's the reason for naming this nvidia-dra-validator instead of nvidia-operator-validator? If we kept the same name (and node selector) as the validator deployed by ClusterPolicy, wouldn't that solve our issue with the driver upgrade-controller (and how it depends on the readiness of the nvidia-operator-validator pod)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point. One note: the upgrade controller keys on the pod label, not the name

WithValidationEnabled("app=nvidia-operator-validator")

So I changed the daemonset pod template to have the same app app: nvidia-operator-validator. I didn't make the names the same for collisions reasons (even if the pods land on different clusters, I believe this is still an issue).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note, there are a few other changes needed in this file. E.g. add a sample GPUCluster CR in .metadata.annotations.alm-examples, update the operator's ClusterRole and Role definitions, etc.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch, updated.

Comment thread assets/state-vfio-manager/0500_daemonset.yaml Outdated
Comment thread cmd/dra-driver-validator/find.go Outdated
@karthikvetrivel
karthikvetrivel force-pushed the kv-gpuclusterconfig-crd branch 2 times, most recently from 3b7cc55 to 4ad2c9d Compare July 8, 2026 18:48
karthikvetrivel added a commit to karthikvetrivel/k8s-driver-manager that referenced this pull request Jul 8, 2026
Pause the DRA validator with other claim-holding clients and keep the kubelet plugin available through targeted and full node drains. Stop the plugin only after those claims are released and before unloading the driver, then restore both deployment labels.

This consumes labels from NVIDIA/gpu-operator#2571. NVIDIA DRA user-pod discovery and eviction is handled separately by NVIDIA#204.

Signed-off-by: Karthik Vetrivel <kvetrivel@nvidia.com>
@karthikvetrivel
karthikvetrivel force-pushed the kv-gpuclusterconfig-crd branch from 4ad2c9d to b0803b4 Compare July 10, 2026 20:18

@cdesiniotis cdesiniotis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Made another pass. @karthikvetrivel thanks for the patience on this one.

Comment thread api/nvidia/v1alpha1/gpucluster_types.go Outdated
Comment on lines +54 to +56
// GFD defines the spec for the standalone GPU Feature Discovery operand. Enabled by default,
// but the reused enabled field carries no server-side default; the controller defaults nil enabled.
GFD *nvidiav1.GPUFeatureDiscoverySpec `json:"gfd,omitempty"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As discussed with @shivamerla, let's omit GFD for now. We can add it to the GPUCluster CR in a follow-up PR if we ever change our mind. But for now, my understanding is that kubernetes-sigs/dra-driver-nvidia-gpu#1247 will remove the hard dependency on GFD.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, lets omit from the new API as we know it has to be deprecated in the context of DRA. Please make a note to document the workaround to install a separate helm chart if customers depend on few common GFD labels.

Comment thread api/nvidia/v1alpha1/gpucluster_types.go Outdated
type DRADriverGPUsSpec struct {
// Enabled indicates if the gpus capability of the DRA driver is enabled.
// +kubebuilder:default=true
Enabled *bool `json:"enabled,omitempty"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was pointed out by @rajathagasthya in the design doc, but I believe we can remove this field. The GPUCluster CR assumes that GPUs are managed by the DRA Driver, right? I don't think I see a use case where one is using the GPUCluster CR but wants to disable the GPU portion of the DRA driver (e.g. only use compute domains).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Agreed, removed.

// DaemonSet; the controller is a separate Deployment.
type DRADriverComputeDomainsSpec struct {
// Enabled indicates if the computeDomains capability of the DRA driver is enabled.
// +kubebuilder:default=true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@shivamerla are you okay if we enable compute domains by default?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As discussed in Slack, we will enable compute domains by default.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@karthikvetrivel this is how it is deployed with the helm chart though there is no good justification of it running when there are no MNNVL systems in the cluster (atleast not yet!).

The idea is that customers can create ComputeDomain instances regardless of the cluster's topology and schedule their workloads without having to reason about the actual node layout. The CD plugin auto-detects fabric vs. non-fabric nodes and simply skips IMEX management on the non-fabric ones. Later, if a user introduces an MNNVL system into the cluster and some workload replicas happen to land there, the already-running CD plugin will pick that up automatically and form an IMEX domain around the new system, since the same resource claim template is shared across all the claims of the workload's replicas. Though AFAIK, for multi-node workloads (inference/training) it is always recommended to run with homogeneous GPU types. So need some real customer feedback on this use case.

For clarity, here are the pros and cons of enabling this when there's no MNNVL system in the cluster.

Pros:

  • As mentioned above, workloads can span multiple racks without topology constraints and seamlessly adopt any MNNVL system that gets added later. Otherwise, users would have to delete the workload and re-bind it to a ComputeDomain after the fact (using its auto-generated resource claim template).
  • Less operational overhead, admins don't have to go read the docs and explicitly turn this on whenever a new MNNVL system shows up in the cluster.

Cons:

  • An extra CD controller pod gets deployed, along with a sidecar plugin container. Depending on the cluster scale, this added footprint may or may not matter.
  • The controller pod sets up informers on ComputeDomain, CDClique, DaemonSets, Pods, and Nodes, and the kubelet plugin sets up an informer on ComputeDomain. That said, with no actual CR instances created, this should stay lightweight.
  • An extra resource slice is created to advertise channel and daemon devices, which could be confusing to users.
  • An extra device class is created for channel allocations.
  • Any change to the CD plugin config triggers a rollout of the GPU plugin too, since both ship within a single DaemonSet.
  • For OLM-based deployments, the webhook configuration has to be embedded in the CSV, so it ends up intercepting every ResourceClaim creation, even in cases where the CD plugin isn't actually being used.

I don't see any user feedback on this yet on the DRA side as well, so we have to make a judgement based on the above factors. The operator could also be smart and enable it (i.e actually create a deployment, plugin sidecar) only when it sees a MNNVL capable node in the cluster. There could be a sub-option in the CR under cdPlugin to control this behavior. If users really want to use ComputeDomain CR prior to adding capable systems, then they can use this sub-option to let it deploy always.

Comment thread assets/state-vfio-manager/0500_daemonset.yaml Outdated
- name: gpu-operator-image
image: ghcr.io/nvidia/gpu-operator:main-latest
- name: nvidia-dra-driver-image
image: registry.k8s.io/dra-driver-nvidia/dra-driver-nvidia-gpu:v0.4.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's append the image digest as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

- name: "VALIDATOR_IMAGE"
value: "ghcr.io/nvidia/gpu-operator:main-latest"
- name: "DRA_DRIVER_IMAGE"
value: "registry.k8s.io/dra-driver-nvidia/dra-driver-nvidia-gpu:v0.4.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's append the image digest as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.


// runValidation mirrors nvidia-validator's host-then-container probe, but validates
// with `nvidia-smi --version` only. It returns the env-contract driverInfo on success.
func runValidation() (driverInfo, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question -- how does this differ from the driver-validation code we have in cmd/nvidia-validator? IIUC the driver validation code is not specific to DRA, so why introduce a new executable, dra-driver-validator, in the first place?

Comment on lines +236 to +240
// writeDriverReady writes the driver-ready env contract sourced by the
// kubelet-plugin containers. Only NVIDIA_DRIVER_ROOT and DRIVER_ROOT_CTR_PATH are
// emitted: those are the only values the DRA kubelet plugins read (see
// cmd/gpu-kubelet-plugin/main.go in k8s-dra-driver-gpu). The plugin derives its
// dev root from DRIVER_ROOT_CTR_PATH itself.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this the primary motivation for creating a new dra-driver-validator executable instead of reusing nvidia-validator? If yes, I would argue we should just re-use the driver-validation code from the nvidia-validator binary as-is instead. (feel free to push back if there is another difference I am not seeing)

Note, exporting the DEV_ROOT specific envvars should have no negative side effects since they don't mean anything to the dra-driver-kubelet-plugin.

Just for reference, the DRA driver infers the dev root here: https://github.com/kubernetes-sigs/dra-driver-nvidia-gpu/blob/9001f17e513115a9366987bf5fd9f7850ac52368/cmd/gpu-kubelet-plugin/device_state.go#L88 (it will either resolve to DRIVER_ROOT_CTR_PATH or /, which aligns with what our nvidia-validator code does).

Comment thread controllers/nodelabeling_controller.go Outdated
if _, ok := labels[consts.GPUOperatorModeLabelKey]; ok {
return false
}
if nlc.defaultMode == "" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No strong opinion from me either. I think I slightly prefer removing since A) this method should never be called if neither a ClusterPolicy or GPUCluster CR exist (as Reconcile will return early in this case), and B) a reasonable assumption to make is that defaultMode is resolved / set prior to reconcileModeLabel() being called.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not a blocker -- should we consider adding validations in the helm chart to prevent users from deploying the GPUCluster CR on a k8s cluster that does not support DRA? See what I had previously proposed: https://github.com/NVIDIA/gpu-operator/pull/1541/changes#diff-b056279a4ae42adaeb1878776dd3d6e426565027c47ee47f7f8cc607708d5b16

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep, I put one on validations.yaml.

Comment thread controllers/gpucluster_controller.go Outdated
Comment thread controllers/nodelabeling_controller.go Outdated
Comment thread controllers/nodelabeling_controller.go Outdated
Comment thread controllers/nvidiadriver_controller_test.go Outdated
Comment thread api/nvidia/v1alpha1/gpucluster_types.go
@karthikvetrivel
karthikvetrivel force-pushed the kv-gpuclusterconfig-crd branch 2 times, most recently from 03a7a7a to 93cabf5 Compare July 13, 2026 19:13
Comment thread internal/consts/consts.go Outdated
Comment thread internal/consts/consts.go Outdated
Comment thread internal/consts/consts.go Outdated
Comment thread controllers/gpucluster_controller.go
Comment thread controllers/gpucluster_controller.go Outdated
Comment thread internal/state/dra_driver.go Outdated
Comment thread controllers/nodelabeling_controller.go Outdated
Comment thread controllers/active_config.go
Comment thread controllers/nodelabeling_controller.go Outdated
Comment thread internal/utils/utils.go Outdated
@karthikvetrivel
karthikvetrivel force-pushed the kv-gpuclusterconfig-crd branch 3 times, most recently from fc9345b to fd5f3b3 Compare July 14, 2026 20:09
Comment thread controllers/nodelabeling_controller.go Outdated
Comment on lines +308 to +311
modified := nlc.clearPreviousStackStateLabels(labels, devicePluginOnlyStateLabelKeys(), nodeName)
if updateGPUClusterStateLabels(labels) {
modified = true
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like these two method calls return a boolean and the end result of modified is "OR"ing of the two method's return values.

Can we modify this to match the style of this line? I find this readable.

		modified := gpuDiscoveryStateChanged || modeLabelModified || stateLabelsModified

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done, modified to match this style.

@tariq1890

Copy link
Copy Markdown
Contributor

@karthikvetrivel This PR will also need the YAML render test cases which executes the templates and compares the output with fixtures that represent the expected final YAML (like the ones here)

{{- if $dra.imagePullPolicy }}
imagePullPolicy: {{ $dra.imagePullPolicy }}
{{- end }}
{{- with $dra.imagePullSecrets }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of using the with and toYaml helm methods, can we just access the various helm values the same way we do in clusterpolicy.yaml?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, good point. Done.

Comment thread api/nvidia/v1alpha1/gpucluster_types.go Outdated
)

const (
GPUClusterCRDName = "GPUCluster"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, the GPUCluster controller passes it to state.NewManager. I can remove if preferred, no strong preference here

Comment thread assets/state-vfio-manager/0500_daemonset.yaml Outdated
type DRADriverComputeDomainsSpec struct {
// Enabled indicates if the computeDomains capability of the DRA driver is enabled.
// +kubebuilder:default=true
Enabled *bool `json:"enabled,omitempty"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this a *bool instead of just bool? Are we trying to differentiate between unset and set (true/false)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Exactly, yes.


// HealthcheckPort is the port running a gRPC health service checked by a livenessProbe.
// Set to a negative value to disable the service and the probe.
HealthcheckPort *int32 `json:"healthcheckPort,omitempty"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similarly, if this is set to nil, will it disable the probe?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I believe nil will use the default port, which is 51516 for the gpus container and 51515 from the compute-domains container.

@karthikvetrivel
karthikvetrivel force-pushed the kv-gpuclusterconfig-crd branch from fd5f3b3 to 1a06b01 Compare July 16, 2026 15:35
Signed-off-by: Karthik Vetrivel <kvetrivel@nvidia.com>
Signed-off-by: Karthik Vetrivel <kvetrivel@nvidia.com>
Signed-off-by: Karthik Vetrivel <kvetrivel@nvidia.com>
Signed-off-by: Karthik Vetrivel <kvetrivel@nvidia.com>
Signed-off-by: Karthik Vetrivel <kvetrivel@nvidia.com>
Signed-off-by: Karthik Vetrivel <kvetrivel@nvidia.com>
Signed-off-by: Karthik Vetrivel <kvetrivel@nvidia.com>
@karthikvetrivel
karthikvetrivel force-pushed the kv-gpuclusterconfig-crd branch from 1a06b01 to b101993 Compare July 16, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants