Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions api/nvidia/v1alpha1/gpucluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ const (
GPUClusterCRDName = "GPUCluster"
)

const (
// Ignored marks a duplicate GPUCluster that the singleton controller does not reconcile.
Ignored State = "ignored"
)

// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.

// GPUClusterSpec defines the desired state of GPUCluster, the DRA-based
Expand Down Expand Up @@ -170,7 +165,7 @@ type HostPathsSpec struct {

// GPUClusterStatus defines the observed state of GPUCluster
type GPUClusterStatus struct {
// +kubebuilder:validation:Enum=ignored;ready;notReady;disabled
// +kubebuilder:validation:Enum=ready;notReady;disabled
// State indicates the status of the GPUCluster instance
State State `json:"state"`
// Namespace indicates the namespace in which the operator and operands are installed
Expand All @@ -186,6 +181,7 @@ type GPUClusterStatus struct {
//+kubebuilder:resource:scope=Cluster,shortName={"gc"}
//+kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.state`,priority=0
//+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`,priority=0
//+kubebuilder:validation:XValidation:rule="self.metadata.name == 'gpu-cluster'",message="GPUCluster is a singleton, metadata.name must be 'gpu-cluster'"
Comment thread
tariq1890 marked this conversation as resolved.

// GPUCluster is the Schema for the gpuclusters API
type GPUCluster struct {
Expand Down
4 changes: 3 additions & 1 deletion bundle/manifests/nvidia.com_gpuclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,6 @@ spec:
state:
description: State indicates the status of the GPUCluster instance
enum:
- ignored
- ready
- notReady
- disabled
Expand All @@ -1036,6 +1035,9 @@ spec:
- state
type: object
type: object
x-kubernetes-validations:
- message: GPUCluster is a singleton, metadata.name must be 'gpu-cluster'
rule: self.metadata.name == 'gpu-cluster'
served: true
storage: true
subresources:
Expand Down
4 changes: 3 additions & 1 deletion config/crd/bases/nvidia.com_gpuclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,6 @@ spec:
state:
description: State indicates the status of the GPUCluster instance
enum:
- ignored
- ready
- notReady
- disabled
Expand All @@ -1036,6 +1035,9 @@ spec:
- state
type: object
type: object
x-kubernetes-validations:
- message: GPUCluster is a singleton, metadata.name must be 'gpu-cluster'
rule: self.metadata.name == 'gpu-cluster'
served: true
storage: true
subresources:
Expand Down
2 changes: 1 addition & 1 deletion config/samples/nvidia_v1alpha1_gpucluster.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apiVersion: nvidia.com/v1alpha1
kind: GPUCluster
metadata:
name: gpucluster-sample
name: gpu-cluster
spec:
draDriver:
repository: registry.k8s.io/dra-driver-nvidia
Expand Down
38 changes: 30 additions & 8 deletions controllers/active_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,39 @@ import (
"github.com/NVIDIA/gpu-operator/internal/consts"
)

// TODO: with multiple CRs of a kind, the tie-breaker is list order, which is not
// guaranteed. Resolve the singleton the reconcilers actually selected instead.
func resolveActiveConfig(ctx context.Context, c client.Client) (*gpuv1.ClusterPolicy, *nvidiav1alpha1.GPUCluster, error) {
var clusterPolicy *gpuv1.ClusterPolicy
// getSingletonClusterPolicy returns the ClusterPolicy treated as the cluster-wide
// singleton: oldest creationTimestamp first, lowest name as the tie-breaker. This
// selection is a heuristic: ideally the operator would hold a reference to the
// singleton ClusterPolicy globally throughout its lifetime, and until it does,
// picking the oldest is the next-best approach. Unlike a first-reconciled-wins claim
// held in controller memory, the result is stable across operator restarts and
// derivable by every controller. Returns nil for an empty list.
// GPUCluster needs no selection: its CRD pins metadata.name, enforcing the singleton
// at admission.
func getSingletonClusterPolicy(items []gpuv1.ClusterPolicy) *gpuv1.ClusterPolicy {
var active *gpuv1.ClusterPolicy
for i := range items {
candidate := &items[i]
if active == nil {
active = candidate
continue
}
candidateCreated, activeCreated := candidate.CreationTimestamp, active.CreationTimestamp
if candidateCreated.Before(&activeCreated) ||
(candidateCreated.Equal(&activeCreated) && candidate.Name < active.Name) {
active = candidate
}
}
return active
}

// resolveActiveConfig returns the active ClusterPolicy, selected with the same
// getSingletonClusterPolicy rule the ClusterPolicy controller uses, and the GPUCluster.
func resolveActiveConfig(ctx context.Context, c client.Reader) (*gpuv1.ClusterPolicy, *nvidiav1alpha1.GPUCluster, error) {
clusterPolicies := &gpuv1.ClusterPolicyList{}
if err := c.List(ctx, clusterPolicies); err != nil {
return nil, nil, fmt.Errorf("failed to list ClusterPolicy: %w", err)
}
if len(clusterPolicies.Items) > 0 {
clusterPolicy = &clusterPolicies.Items[0]
}

var gpuCluster *nvidiav1alpha1.GPUCluster
gpuClusters := &nvidiav1alpha1.GPUClusterList{}
Expand All @@ -48,7 +70,7 @@ func resolveActiveConfig(ctx context.Context, c client.Client) (*gpuv1.ClusterPo
gpuCluster = &gpuClusters.Items[0]
}

return clusterPolicy, gpuCluster, nil
return getSingletonClusterPolicy(clusterPolicies.Items), gpuCluster, nil
}

// resolveDefaultMode returns the nvidia.com/gpu-operator.resource-allocation.mode value for a GPU node that
Expand Down
16 changes: 16 additions & 0 deletions controllers/active_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"errors"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -76,6 +77,21 @@ func TestResolveActiveConfig(t *testing.T) {
assert.Equal(t, "cluster-config", gc.Name)
})

t.Run("oldest ClusterPolicy is the singleton, name breaks ties", func(t *testing.T) {
oldTime := metav1.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
zOlder := &gpuv1.ClusterPolicy{ObjectMeta: metav1.ObjectMeta{Name: "z-older", CreationTimestamp: oldTime}}
bOlder := &gpuv1.ClusterPolicy{ObjectMeta: metav1.ObjectMeta{Name: "b-older", CreationTimestamp: oldTime}}
aNewer := &gpuv1.ClusterPolicy{ObjectMeta: metav1.ObjectMeta{
Name: "a-newer", CreationTimestamp: metav1.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC)}}
c := fake.NewClientBuilder().WithScheme(scheme).
WithObjects(zOlder, bOlder, aNewer).Build()

cp, _, err := resolveActiveConfig(context.Background(), c)
require.NoError(t, err)
require.NotNil(t, cp)
assert.Equal(t, "b-older", cp.Name)
})

t.Run("neither CR present returns all nil", func(t *testing.T) {
c := fake.NewClientBuilder().WithScheme(scheme).Build()

Expand Down
11 changes: 8 additions & 3 deletions controllers/clusterpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,14 @@ func (r *ClusterPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reques
return reconcile.Result{}, err
}

// TODO: Handle deletion of the main ClusterPolicy and cycle to the next one.
// We already have a main Clusterpolicy
if clusterPolicyCtrl.singleton != nil && clusterPolicyCtrl.singleton.Name != instance.Name {
// The ClusterPolicy with the oldest creationTimestamp (name as tie-breaker) is
// treated as the singleton, so the choice is stable across operator restarts and
// derivable by other controllers (see getSingletonClusterPolicy).
clusterPolicies := &gpuv1.ClusterPolicyList{}
if err := r.List(ctx, clusterPolicies); err != nil {
return ctrl.Result{}, fmt.Errorf("error listing ClusterPolicy objects: %w", err)
}
if active := getSingletonClusterPolicy(clusterPolicies.Items); active != nil && active.Name != instance.Name {
instance.SetStatus(gpuv1.Ignored, clusterPolicyCtrl.operatorNamespace)
// do not change `clusterPolicyCtrl.operatorMetrics.reconciliationStatus` here,
// spurious reconciliation
Expand Down
25 changes: 25 additions & 0 deletions controllers/clusterpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,31 @@ func clusterPolicyForUpgradeTest(useNvidiaDriverCRD bool) *gpuv1.ClusterPolicy {
}
}

// The ClusterPolicy with the oldest creationTimestamp is the singleton regardless of
// reconcile order; any other instance is skipped without running any states.
func TestClusterPolicyReconcileSkipsNonSingleton(t *testing.T) {
older := clusterPolicyForUpgradeTest(true)
older.Name = "older"
older.CreationTimestamp = metav1.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
r, c, _ := newClusterPolicyUpgradeTestReconciler(t, older)

newer := clusterPolicyForUpgradeTest(true)
newer.Name = "newer"
newer.CreationTimestamp = metav1.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC)
require.NoError(t, c.Create(t.Context(), newer))

// The newer instance reconciles first but is not the singleton: no error, no
// requeue, and no state is persisted since its states never run.
result, err := r.Reconcile(t.Context(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(newer)})
require.NoError(t, err)
require.Zero(t, result)
require.Empty(t, clusterPolicyState(t, c, newer.Name))

_, err = r.Reconcile(t.Context(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(older)})
require.NoError(t, err)
require.Equal(t, gpuv1.Ready, clusterPolicyState(t, c, older.Name))
}

func newClusterPolicyUpgradeTestReconciler(t *testing.T, cp *gpuv1.ClusterPolicy, nodes ...*corev1.Node) (*ClusterPolicyReconciler, client.Client, *OperatorMetrics) {
t.Helper()
scheme := runtime.NewScheme()
Expand Down
28 changes: 6 additions & 22 deletions controllers/gpucluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@ type GPUClusterReconciler struct {
stateManager state.Manager
conditionUpdater conditions.Updater
recorder events.EventRecorder

// singleton is the GPUCluster that owns reconciliation; the first instance to
// reconcile claims it (first-wins), mirroring ClusterPolicy.
singleton *nvidiav1alpha1.GPUCluster
}

//+kubebuilder:rbac:groups=nvidia.com,resources=gpuclusters,verbs=get;list;watch;create;update;patch;delete
Expand Down Expand Up @@ -108,18 +104,8 @@ func (r *GPUClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
// stack): every operand DaemonSet of both stacks gates on the per-node
// nvidia.com/gpu-operator.resource-allocation.mode label, so each node is served by exactly one stack.

// Singleton, first-wins (mirroring ClusterPolicy): the first instance to reconcile
// claims ownership; any other instance is marked Ignored and skipped. The owner is
// held in memory, so the choice resets on operator restart.
if r.singleton != nil && r.singleton.Name != instance.Name {
logger.V(consts.LogLevelWarning).Info("Multiple GPUCluster instances found, ignoring this one",
"name", instance.Name, "owner", r.singleton.Name)
if err := r.updateCRStatus(ctx, instance, nvidiav1alpha1.Ignored); err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
r.singleton = instance
// No singleton claim is needed: the CRD's CEL rule pins metadata.name, so at most
// one GPUCluster can exist.

// DRA requires all driver management through NVIDIADriver CRs: surface an unmet
// prerequisite on this CR's status and hold off deploying operands until it is met.
Expand Down Expand Up @@ -185,12 +171,10 @@ func (r *GPUClusterReconciler) validatePrerequisites(ctx context.Context) (strin
if err := r.List(ctx, clusterPolicies); err != nil {
return "", fmt.Errorf("error listing ClusterPolicy objects: %w", err)
}
// TODO: check only the active singleton ClusterPolicy once the singleton
// selection is resolvable across controllers (see resolveActiveConfig).
for _, clusterPolicy := range clusterPolicies.Items {
if !clusterPolicy.Spec.Driver.UseNvidiaDriverCRDType() {
return fmt.Sprintf("ClusterPolicy %s does not have driver.useNvidiaDriverCRD enabled; migrate driver management to NVIDIADriver CRs before enabling DRA", clusterPolicy.Name), nil
}
// Only the active singleton ClusterPolicy matters here: an Ignored instance
// deploys nothing, so it cannot own driver daemonsets.
if active := getSingletonClusterPolicy(clusterPolicies.Items); active != nil && !active.Spec.Driver.UseNvidiaDriverCRDType() {
return fmt.Sprintf("ClusterPolicy %s does not have driver.useNvidiaDriverCRD enabled; migrate driver management to NVIDIADriver CRs before enabling DRA", active.Name), nil
}
return "", nil
}
Expand Down
31 changes: 0 additions & 31 deletions controllers/gpucluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,37 +267,6 @@ func TestGPUClusterClusterPolicyDriverPrerequisite(t *testing.T) {
require.Equal(t, nvidiav1alpha1.Ready, gccState(t, c, cfg.Name))
}

// First-reconciled wins (mirroring ClusterPolicy): whichever instance reconciles first
// claims ownership, regardless of name or creationTimestamp.
func TestGPUClusterSingleton(t *testing.T) {
first := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "first"}}
second := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "second"}}
r, c := newGPUClusterReconciler(t, first, second)

gccReconcile(t, r, first.Name)
require.Equal(t, nvidiav1alpha1.Ready, gccState(t, c, first.Name))

gccReconcile(t, r, second.Name)
require.Equal(t, nvidiav1alpha1.Ignored, gccState(t, c, second.Name))
}

// Matching ClusterPolicy, an ignored duplicate carries no status condition.
func TestGPUClusterDuplicateNoCondition(t *testing.T) {
owner := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "owner"}}
duplicate := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "duplicate"}}
r, c := newGPUClusterReconciler(t, owner, duplicate)
r.conditionUpdater = conditions.NewGPUClusterUpdater(c)

gccReconcile(t, r, owner.Name) // owner reconciles first, claiming ownership
gccReconcile(t, r, duplicate.Name) // duplicate is ignored

instance := &nvidiav1alpha1.GPUCluster{}
require.NoError(t, c.Get(t.Context(), types.NamespacedName{Name: duplicate.Name}, instance))
require.Equal(t, nvidiav1alpha1.Ignored, instance.Status.State)
require.Nil(t, meta.FindStatusCondition(instance.Status.Conditions, conditions.Error))
require.Nil(t, meta.FindStatusCondition(instance.Status.Conditions, conditions.Ready))
}

func TestEnqueueAllGPUClusters(t *testing.T) {
r, _ := newGPUClusterReconciler(t,
&nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "config-a"}},
Expand Down
4 changes: 3 additions & 1 deletion deployments/gpu-operator/crds/nvidia.com_gpuclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,6 @@ spec:
state:
description: State indicates the status of the GPUCluster instance
enum:
- ignored
- ready
- notReady
- disabled
Expand All @@ -1036,6 +1035,9 @@ spec:
- state
type: object
type: object
x-kubernetes-validations:
- message: GPUCluster is a singleton, metadata.name must be 'gpu-cluster'
rule: self.metadata.name == 'gpu-cluster'
served: true
storage: true
subresources:
Expand Down
Loading