Skip to content

Commit 0f2c0a6

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

10 files changed

Lines changed: 69 additions & 74 deletions

File tree

‎api/nvidia/v1alpha1/gpucluster_types.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,6 @@ const (
2626
GPUClusterCRDName = "GPUCluster"
2727
)
2828

29-
const (
30-
// Ignored marks a duplicate GPUCluster that the singleton controller does not reconcile.
31-
Ignored State = "ignored"
32-
)
33-
3429
// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.
3530

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

171166
// GPUClusterStatus defines the observed state of GPUCluster
172167
type GPUClusterStatus struct {
173-
// +kubebuilder:validation:Enum=ignored;ready;notReady;disabled
168+
// +kubebuilder:validation:Enum=ready;notReady;disabled
174169
// State indicates the status of the GPUCluster instance
175170
State State `json:"state"`
176171
// Namespace indicates the namespace in which the operator and operands are installed
@@ -186,6 +181,7 @@ type GPUClusterStatus struct {
186181
//+kubebuilder:resource:scope=Cluster,shortName={"gc"}
187182
//+kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.state`,priority=0
188183
//+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`,priority=0
184+
//+kubebuilder:validation:XValidation:rule="self.metadata.name == 'gpu-cluster'",message="GPUCluster is a singleton, metadata.name must be 'gpu-cluster'"
189185

190186
// GPUCluster is the Schema for the gpuclusters API
191187
type GPUCluster struct {

‎bundle/manifests/nvidia.com_gpuclusters.yaml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1027,7 +1027,6 @@ spec:
10271027
state:
10281028
description: State indicates the status of the GPUCluster instance
10291029
enum:
1030-
- ignored
10311030
- ready
10321031
- notReady
10331032
- disabled
@@ -1036,6 +1035,9 @@ spec:
10361035
- state
10371036
type: object
10381037
type: object
1038+
x-kubernetes-validations:
1039+
- message: GPUCluster is a singleton, metadata.name must be 'gpu-cluster'
1040+
rule: self.metadata.name == 'gpu-cluster'
10391041
served: true
10401042
storage: true
10411043
subresources:

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1027,7 +1027,6 @@ spec:
10271027
state:
10281028
description: State indicates the status of the GPUCluster instance
10291029
enum:
1030-
- ignored
10311030
- ready
10321031
- notReady
10331032
- disabled
@@ -1036,6 +1035,9 @@ spec:
10361035
- state
10371036
type: object
10381037
type: object
1038+
x-kubernetes-validations:
1039+
- message: GPUCluster is a singleton, metadata.name must be 'gpu-cluster'
1040+
rule: self.metadata.name == 'gpu-cluster'
10391041
served: true
10401042
storage: true
10411043
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: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,36 @@ import (
2727
"github.com/NVIDIA/gpu-operator/internal/consts"
2828
)
2929

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
30+
// getSingleton returns the ClusterPolicy that wins the cluster-wide singleton
31+
// election: oldest creationTimestamp first, lowest name as the tie-breaker. Unlike a
32+
// first-reconciled-wins claim held in controller memory, the result is stable across
33+
// operator restarts and derivable by every controller. Returns nil for an empty list.
34+
// GPUCluster needs no election: its CRD pins metadata.name, enforcing the singleton
35+
// at admission.
36+
func getSingleton(items []gpuv1.ClusterPolicy) *gpuv1.ClusterPolicy {
37+
var active *gpuv1.ClusterPolicy
38+
for i := range items {
39+
candidate := &items[i]
40+
if active == nil {
41+
active = candidate
42+
continue
43+
}
44+
candidateCreated, activeCreated := candidate.CreationTimestamp, active.CreationTimestamp
45+
if candidateCreated.Before(&activeCreated) ||
46+
(candidateCreated.Equal(&activeCreated) && candidate.Name < active.Name) {
47+
active = candidate
48+
}
49+
}
50+
return active
51+
}
52+
53+
// resolveActiveConfig returns the active ClusterPolicy, elected with the same
54+
// getSingleton rule the ClusterPolicy controller uses, and the GPUCluster.
55+
func resolveActiveConfig(ctx context.Context, c client.Reader) (*gpuv1.ClusterPolicy, *nvidiav1alpha1.GPUCluster, error) {
3456
clusterPolicies := &gpuv1.ClusterPolicyList{}
3557
if err := c.List(ctx, clusterPolicies); err != nil {
3658
return nil, nil, fmt.Errorf("failed to list ClusterPolicy: %w", err)
3759
}
38-
if len(clusterPolicies.Items) > 0 {
39-
clusterPolicy = &clusterPolicies.Items[0]
40-
}
4160

4261
var gpuCluster *nvidiav1alpha1.GPUCluster
4362
gpuClusters := &nvidiav1alpha1.GPUClusterList{}
@@ -48,7 +67,7 @@ func resolveActiveConfig(ctx context.Context, c client.Client) (*gpuv1.ClusterPo
4867
gpuCluster = &gpuClusters.Items[0]
4968
}
5069

51-
return clusterPolicy, gpuCluster, nil
70+
return getSingleton(clusterPolicies.Items), gpuCluster, nil
5271
}
5372

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

‎controllers/active_config_test.go‎

Lines changed: 16 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,21 @@ func TestResolveActiveConfig(t *testing.T) {
7677
assert.Equal(t, "cluster-config", gc.Name)
7778
})
7879

80+
t.Run("oldest ClusterPolicy wins, name breaks ties", func(t *testing.T) {
81+
oldTime := metav1.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
82+
zOlder := &gpuv1.ClusterPolicy{ObjectMeta: metav1.ObjectMeta{Name: "z-older", CreationTimestamp: oldTime}}
83+
bOlder := &gpuv1.ClusterPolicy{ObjectMeta: metav1.ObjectMeta{Name: "b-older", CreationTimestamp: oldTime}}
84+
aNewer := &gpuv1.ClusterPolicy{ObjectMeta: metav1.ObjectMeta{
85+
Name: "a-newer", CreationTimestamp: metav1.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC)}}
86+
c := fake.NewClientBuilder().WithScheme(scheme).
87+
WithObjects(zOlder, bOlder, aNewer).Build()
88+
89+
cp, _, err := resolveActiveConfig(context.Background(), c)
90+
require.NoError(t, err)
91+
require.NotNil(t, cp)
92+
assert.Equal(t, "b-older", cp.Name)
93+
})
94+
7995
t.Run("neither CR present returns all nil", func(t *testing.T) {
8096
c := fake.NewClientBuilder().WithScheme(scheme).Build()
8197

‎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 getSingleton).
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 := getSingleton(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: 6 additions & 22 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,8 @@ 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 {
115-
logger.V(consts.LogLevelWarning).Info("Multiple GPUCluster instances found, ignoring this one",
116-
"name", instance.Name, "owner", r.singleton.Name)
117-
if err := r.updateCRStatus(ctx, instance, nvidiav1alpha1.Ignored); err != nil {
118-
return ctrl.Result{}, err
119-
}
120-
return ctrl.Result{}, nil
121-
}
122-
r.singleton = instance
107+
// No singleton claim is needed: the CRD's CEL rule pins metadata.name, so at most
108+
// one GPUCluster can exist.
123109

124110
// DRA requires all driver management through NVIDIADriver CRs: surface an unmet
125111
// prerequisite on this CR's status and hold off deploying operands until it is met.
@@ -185,12 +171,10 @@ func (r *GPUClusterReconciler) validatePrerequisites(ctx context.Context) (strin
185171
if err := r.List(ctx, clusterPolicies); err != nil {
186172
return "", fmt.Errorf("error listing ClusterPolicy objects: %w", err)
187173
}
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-
}
174+
// Only the active singleton ClusterPolicy matters here: an Ignored instance
175+
// deploys nothing, so it cannot own driver daemonsets.
176+
if active := getSingleton(clusterPolicies.Items); active != nil && !active.Spec.Driver.UseNvidiaDriverCRDType() {
177+
return fmt.Sprintf("ClusterPolicy %s does not have driver.useNvidiaDriverCRD enabled; migrate driver management to NVIDIADriver CRs before enabling DRA", active.Name), nil
194178
}
195179
return "", nil
196180
}

‎controllers/gpucluster_controller_test.go‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -267,37 +267,6 @@ 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.
272-
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)
276-
277-
gccReconcile(t, r, first.Name)
278-
require.Equal(t, nvidiav1alpha1.Ready, gccState(t, c, first.Name))
279-
280-
gccReconcile(t, r, second.Name)
281-
require.Equal(t, nvidiav1alpha1.Ignored, gccState(t, c, second.Name))
282-
}
283-
284-
// Matching ClusterPolicy, an ignored duplicate carries no status condition.
285-
func TestGPUClusterDuplicateNoCondition(t *testing.T) {
286-
owner := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "owner"}}
287-
duplicate := &nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "duplicate"}}
288-
r, c := newGPUClusterReconciler(t, owner, duplicate)
289-
r.conditionUpdater = conditions.NewGPUClusterUpdater(c)
290-
291-
gccReconcile(t, r, owner.Name) // owner reconciles first, claiming ownership
292-
gccReconcile(t, r, duplicate.Name) // duplicate is ignored
293-
294-
instance := &nvidiav1alpha1.GPUCluster{}
295-
require.NoError(t, c.Get(t.Context(), types.NamespacedName{Name: duplicate.Name}, instance))
296-
require.Equal(t, nvidiav1alpha1.Ignored, instance.Status.State)
297-
require.Nil(t, meta.FindStatusCondition(instance.Status.Conditions, conditions.Error))
298-
require.Nil(t, meta.FindStatusCondition(instance.Status.Conditions, conditions.Ready))
299-
}
300-
301270
func TestEnqueueAllGPUClusters(t *testing.T) {
302271
r, _ := newGPUClusterReconciler(t,
303272
&nvidiav1alpha1.GPUCluster{ObjectMeta: metav1.ObjectMeta{Name: "config-a"}},

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1027,7 +1027,6 @@ spec:
10271027
state:
10281028
description: State indicates the status of the GPUCluster instance
10291029
enum:
1030-
- ignored
10311030
- ready
10321031
- notReady
10331032
- disabled
@@ -1036,6 +1035,9 @@ spec:
10361035
- state
10371036
type: object
10381037
type: object
1038+
x-kubernetes-validations:
1039+
- message: GPUCluster is a singleton, metadata.name must be 'gpu-cluster'
1040+
rule: self.metadata.name == 'gpu-cluster'
10391041
served: true
10401042
storage: true
10411043
subresources:

0 commit comments

Comments
 (0)