Skip to content

Commit 3177a01

Browse files
Enforce singleton GPUCluster name and elect ClusterPolicy by creation time
Signed-off-by: Karthik Vetrivel <kvetrivel@nvidia.com>
1 parent edfdd7a commit 3177a01

10 files changed

Lines changed: 96 additions & 44 deletions

File tree

‎api/nvidia/v1alpha1/gpucluster_types.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ type GPUClusterStatus struct {
186186
//+kubebuilder:resource:scope=Cluster,shortName={"gc"}
187187
//+kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.state`,priority=0
188188
//+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`,priority=0
189+
//+kubebuilder:validation:XValidation:rule="self.metadata.name == 'gpu-cluster'",message="GPUCluster is a singleton, metadata.name must be 'gpu-cluster'"
189190

190191
// GPUCluster is the Schema for the gpuclusters API
191192
type GPUCluster struct {

‎bundle/manifests/nvidia.com_gpuclusters.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,9 @@ spec:
10361036
- state
10371037
type: object
10381038
type: object
1039+
x-kubernetes-validations:
1040+
- message: GPUCluster is a singleton, metadata.name must be 'gpu-cluster'
1041+
rule: self.metadata.name == 'gpu-cluster'
10391042
served: true
10401043
storage: true
10411044
subresources:

‎config/crd/bases/nvidia.com_gpuclusters.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,9 @@ spec:
10361036
- state
10371037
type: object
10381038
type: object
1039+
x-kubernetes-validations:
1040+
- message: GPUCluster is a singleton, metadata.name must be 'gpu-cluster'
1041+
rule: self.metadata.name == 'gpu-cluster'
10391042
served: true
10401043
storage: true
10411044
subresources:

‎config/samples/nvidia_v1alpha1_gpucluster.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
apiVersion: nvidia.com/v1alpha1
22
kind: GPUCluster
33
metadata:
4-
name: gpucluster-sample
4+
name: gpu-cluster
55
spec:
66
draDriver:
77
repository: registry.k8s.io/dra-driver-nvidia

‎controllers/active_config.go‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,35 +20,52 @@ import (
2020
"context"
2121
"fmt"
2222

23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2324
"sigs.k8s.io/controller-runtime/pkg/client"
2425

2526
gpuv1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1"
2627
nvidiav1alpha1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1alpha1"
2728
"github.com/NVIDIA/gpu-operator/internal/consts"
2829
)
2930

30-
// TODO: with multiple CRs of a kind, the tie-breaker is list order, which is not
31-
// guaranteed. Resolve the singleton the reconcilers actually selected instead.
32-
func resolveActiveConfig(ctx context.Context, c client.Client) (*gpuv1.ClusterPolicy, *nvidiav1alpha1.GPUCluster, error) {
33-
var clusterPolicy *gpuv1.ClusterPolicy
31+
// electSingleton returns the instance that wins the cluster-wide singleton election:
32+
// oldest creationTimestamp first, lowest name as the tie-breaker. Unlike a
33+
// first-reconciled-wins claim held in controller memory, the result is stable across
34+
// operator restarts and derivable by every controller. Returns nil for an empty list.
35+
func electSingleton[T any, PT interface {
36+
*T
37+
metav1.Object
38+
}](items []T) PT {
39+
var active PT
40+
for i := range items {
41+
candidate := PT(&items[i])
42+
if active == nil {
43+
active = candidate
44+
continue
45+
}
46+
candidateCreated, activeCreated := candidate.GetCreationTimestamp(), active.GetCreationTimestamp()
47+
if candidateCreated.Before(&activeCreated) ||
48+
(candidateCreated.Equal(&activeCreated) && candidate.GetName() < active.GetName()) {
49+
active = candidate
50+
}
51+
}
52+
return active
53+
}
54+
55+
// resolveActiveConfig returns the active ClusterPolicy and GPUCluster, elected with
56+
// the same electSingleton rule the owning controllers use.
57+
func resolveActiveConfig(ctx context.Context, c client.Reader) (*gpuv1.ClusterPolicy, *nvidiav1alpha1.GPUCluster, error) {
3458
clusterPolicies := &gpuv1.ClusterPolicyList{}
3559
if err := c.List(ctx, clusterPolicies); err != nil {
3660
return nil, nil, fmt.Errorf("failed to list ClusterPolicy: %w", err)
3761
}
38-
if len(clusterPolicies.Items) > 0 {
39-
clusterPolicy = &clusterPolicies.Items[0]
40-
}
4162

42-
var gpuCluster *nvidiav1alpha1.GPUCluster
4363
gpuClusters := &nvidiav1alpha1.GPUClusterList{}
4464
if err := c.List(ctx, gpuClusters); err != nil {
4565
return nil, nil, fmt.Errorf("failed to list GPUCluster: %w", err)
4666
}
47-
if len(gpuClusters.Items) > 0 {
48-
gpuCluster = &gpuClusters.Items[0]
49-
}
5067

51-
return clusterPolicy, gpuCluster, nil
68+
return electSingleton(clusterPolicies.Items), electSingleton(gpuClusters.Items), nil
5269
}
5370

5471
// resolveDefaultMode returns the nvidia.com/gpu-operator.resource-allocation.mode value for a GPU node that

‎controllers/active_config_test.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"errors"
2222
"testing"
23+
"time"
2324

2425
"github.com/stretchr/testify/assert"
2526
"github.com/stretchr/testify/require"
@@ -76,6 +77,25 @@ func TestResolveActiveConfig(t *testing.T) {
7677
assert.Equal(t, "cluster-config", gc.Name)
7778
})
7879

80+
t.Run("oldest creationTimestamp wins, name breaks ties", func(t *testing.T) {
81+
older := &gpuv1.ClusterPolicy{ObjectMeta: metav1.ObjectMeta{
82+
Name: "z-older", CreationTimestamp: metav1.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}}
83+
newer := &gpuv1.ClusterPolicy{ObjectMeta: metav1.ObjectMeta{
84+
Name: "a-newer", CreationTimestamp: metav1.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC)}}
85+
sameTime := metav1.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
86+
gcB := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "b-cluster", CreationTimestamp: sameTime}}
87+
gcA := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "a-cluster", CreationTimestamp: sameTime}}
88+
c := fake.NewClientBuilder().WithScheme(scheme).
89+
WithObjects(older, newer, gcB, gcA).Build()
90+
91+
cp, gc, err := resolveActiveConfig(context.Background(), c)
92+
require.NoError(t, err)
93+
require.NotNil(t, cp)
94+
assert.Equal(t, "z-older", cp.Name)
95+
require.NotNil(t, gc)
96+
assert.Equal(t, "a-cluster", gc.Name)
97+
})
98+
7999
t.Run("neither CR present returns all nil", func(t *testing.T) {
80100
c := fake.NewClientBuilder().WithScheme(scheme).Build()
81101

‎controllers/clusterpolicy_controller.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,14 @@ func (r *ClusterPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reques
117117
return reconcile.Result{}, err
118118
}
119119

120-
// TODO: Handle deletion of the main ClusterPolicy and cycle to the next one.
121-
// We already have a main Clusterpolicy
122-
if clusterPolicyCtrl.singleton != nil && clusterPolicyCtrl.singleton.Name != instance.Name {
120+
// Singleton election: the ClusterPolicy with the oldest creationTimestamp (name
121+
// as tie-breaker) is the active instance, so the choice is stable across
122+
// operator restarts and derivable by other controllers (see electSingleton).
123+
clusterPolicies := &gpuv1.ClusterPolicyList{}
124+
if err := r.List(ctx, clusterPolicies); err != nil {
125+
return ctrl.Result{}, fmt.Errorf("error listing ClusterPolicy objects: %w", err)
126+
}
127+
if active := electSingleton(clusterPolicies.Items); active != nil && active.Name != instance.Name {
123128
instance.SetStatus(gpuv1.Ignored, clusterPolicyCtrl.operatorNamespace)
124129
// do not change `clusterPolicyCtrl.operatorMetrics.reconciliationStatus` here,
125130
// spurious reconciliation

‎controllers/gpucluster_controller.go‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,6 @@ type GPUClusterReconciler struct {
6666
stateManager state.Manager
6767
conditionUpdater conditions.Updater
6868
recorder events.EventRecorder
69-
70-
// singleton is the GPUCluster that owns reconciliation; the first instance to
71-
// reconcile claims it (first-wins), mirroring ClusterPolicy.
72-
singleton *nvidiav1alpha1.GPUCluster
7369
}
7470

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

111-
// Singleton, first-wins (mirroring ClusterPolicy): the first instance to reconcile
112-
// claims ownership; any other instance is marked Ignored and skipped. The owner is
113-
// held in memory, so the choice resets on operator restart.
114-
if r.singleton != nil && r.singleton.Name != instance.Name {
107+
// Singleton election (mirroring ClusterPolicy): the GPUCluster with the oldest
108+
// creationTimestamp (name as tie-breaker) is the active instance. The CRD's CEL
109+
// rule pins metadata.name, so multiple instances can only predate that rule.
110+
gpuClusters := &nvidiav1alpha1.GPUClusterList{}
111+
if err := r.List(ctx, gpuClusters); err != nil {
112+
return ctrl.Result{}, fmt.Errorf("error listing GPUCluster objects: %w", err)
113+
}
114+
if active := electSingleton(gpuClusters.Items); active != nil && active.Name != instance.Name {
115115
logger.V(consts.LogLevelWarning).Info("Multiple GPUCluster instances found, ignoring this one",
116-
"name", instance.Name, "owner", r.singleton.Name)
116+
"name", instance.Name, "owner", active.Name)
117117
if err := r.updateCRStatus(ctx, instance, nvidiav1alpha1.Ignored); err != nil {
118118
return ctrl.Result{}, err
119119
}
120120
return ctrl.Result{}, nil
121121
}
122-
r.singleton = instance
123122

124123
// DRA requires all driver management through NVIDIADriver CRs: surface an unmet
125124
// prerequisite on this CR's status and hold off deploying operands until it is met.
@@ -185,12 +184,10 @@ func (r *GPUClusterReconciler) validatePrerequisites(ctx context.Context) (strin
185184
if err := r.List(ctx, clusterPolicies); err != nil {
186185
return "", fmt.Errorf("error listing ClusterPolicy objects: %w", err)
187186
}
188-
// TODO: check only the active singleton ClusterPolicy once the singleton
189-
// selection is resolvable across controllers (see resolveActiveConfig).
190-
for _, clusterPolicy := range clusterPolicies.Items {
191-
if !clusterPolicy.Spec.Driver.UseNvidiaDriverCRDType() {
192-
return fmt.Sprintf("ClusterPolicy %s does not have driver.useNvidiaDriverCRD enabled; migrate driver management to NVIDIADriver CRs before enabling DRA", clusterPolicy.Name), nil
193-
}
187+
// Only the active singleton ClusterPolicy matters here: an Ignored instance
188+
// deploys nothing, so it cannot own driver daemonsets.
189+
if active := electSingleton(clusterPolicies.Items); active != nil && !active.Spec.Driver.UseNvidiaDriverCRDType() {
190+
return fmt.Sprintf("ClusterPolicy %s does not have driver.useNvidiaDriverCRD enabled; migrate driver management to NVIDIADriver CRs before enabling DRA", active.Name), nil
194191
}
195192
return "", nil
196193
}

‎controllers/gpucluster_controller_test.go‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -267,28 +267,31 @@ func TestGPUClusterClusterPolicyDriverPrerequisite(t *testing.T) {
267267
require.Equal(t, nvidiav1alpha1.Ready, gccState(t, c, cfg.Name))
268268
}
269269

270-
// First-reconciled wins (mirroring ClusterPolicy): whichever instance reconciles first
271-
// claims ownership, regardless of name or creationTimestamp.
270+
// Oldest creationTimestamp wins the singleton election, regardless of reconcile order.
272271
func TestGPUClusterSingleton(t *testing.T) {
273-
first := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "first"}}
274-
second := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "second"}}
275-
r, c := newGPUClusterReconciler(t, first, second)
272+
older := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{
273+
Name: "older", CreationTimestamp: metav1.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}}
274+
newer := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{
275+
Name: "newer", CreationTimestamp: metav1.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC)}}
276+
r, c := newGPUClusterReconciler(t, older, newer)
276277

277-
gccReconcile(t, r, first.Name)
278-
require.Equal(t, nvidiav1alpha1.Ready, gccState(t, c, first.Name))
278+
gccReconcile(t, r, newer.Name)
279+
require.Equal(t, nvidiav1alpha1.Ignored, gccState(t, c, newer.Name))
279280

280-
gccReconcile(t, r, second.Name)
281-
require.Equal(t, nvidiav1alpha1.Ignored, gccState(t, c, second.Name))
281+
gccReconcile(t, r, older.Name)
282+
require.Equal(t, nvidiav1alpha1.Ready, gccState(t, c, older.Name))
282283
}
283284

284285
// Matching ClusterPolicy, an ignored duplicate carries no status condition.
285286
func TestGPUClusterDuplicateNoCondition(t *testing.T) {
286-
owner := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "owner"}}
287-
duplicate := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "duplicate"}}
287+
owner := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{
288+
Name: "owner", CreationTimestamp: metav1.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}}
289+
duplicate := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{
290+
Name: "duplicate", CreationTimestamp: metav1.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC)}}
288291
r, c := newGPUClusterReconciler(t, owner, duplicate)
289292
r.conditionUpdater = conditions.NewGPUClusterUpdater(c)
290293

291-
gccReconcile(t, r, owner.Name) // owner reconciles first, claiming ownership
294+
gccReconcile(t, r, owner.Name) // owner is the older instance and wins the election
292295
gccReconcile(t, r, duplicate.Name) // duplicate is ignored
293296

294297
instance := &nvidiav1alpha1.GPUCluster{}

‎deployments/gpu-operator/crds/nvidia.com_gpuclusters.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,9 @@ spec:
10361036
- state
10371037
type: object
10381038
type: object
1039+
x-kubernetes-validations:
1040+
- message: GPUCluster is a singleton, metadata.name must be 'gpu-cluster'
1041+
rule: self.metadata.name == 'gpu-cluster'
10391042
served: true
10401043
storage: true
10411044
subresources:

0 commit comments

Comments
 (0)