Skip to content

Commit 98a34f7

Browse files
authored
fix: surface ValkeyNode reconcile errors in status conditions (#257)
## Summary When `ensureConfigMap`, `ensurePersistentVolumeClaim`, or `ensureWorkload` fails during ValkeyNode reconciliation, the controller returns the error to controller-runtime without updating the ValkeyNode status. This leaves the resource with no visible indication of why it is stuck other than the operator logs. Found during testing where a PVC quota exceeded error left a ValkeyNode with a completely empty status section. ## Implementation Add a `setReadyCondition` helper that patches the ValkeyNode Ready condition to False with the error reason before returning. Reasons are specific to the failing stage: `ConfigMapError`, `PersistentVolumeClaimError`, `StatefulSetError`, or `DeploymentError`. The condition is self-clearing: once the error resolves, the existing `updateStatus` path overwrites it with the healthy state. ## Limitations Does not cover the `reconcilePersistenceFinalizer` early-return path, ok to me for now. ## Testing Unit tests pass. Verified manually by triggering a PVC quota error on an ENG cluster: ``` ༼ つ ▀_▀ ༽つ  ~  src  valkey-ope  kubectl describe valkeynode eng-test-3-0 -n valkey-operator | tail -15 Tolerations: Effect: NoSchedule Key: dedicated Value: openebs Users ACL Secret Name: internal-eng-test-acl Workload Type: StatefulSet Status: Conditions: Last Transition Time: 2026-06-17T13:21:49Z Message: persistentvolumeclaims "valkey-eng-test-3-0-data" is forbidden: exceeded quota: fuze-quota, requested: persistentvolumeclaims=1, used: persistentvolumeclaims=12, limited: persistentvolumeclaims=12 Observed Generation: 1 Reason: PersistentVolumeClaimError Status: False Type: Ready ``` Signed-off-by: Daan Vinken <daanvinken@tythus.com>
1 parent 3a01229 commit 98a34f7

1 file changed

Lines changed: 36 additions & 0 deletions

File tree

internal/controller/valkeynode_controller.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,23 @@ func (r *ValkeyNodeReconciler) Reconcile(ctx context.Context, req ctrl.Request)
122122
return ctrl.Result{RequeueAfter: time.Second}, nil
123123
}
124124
if err := r.ensureConfigMap(ctx, node); err != nil {
125+
r.setReadyCondition(ctx, node, "ConfigMapError", err.Error())
125126
return ctrl.Result{}, err
126127
}
127128
if err := r.ensurePersistentVolumeClaim(ctx, node); err != nil {
129+
r.setReadyCondition(ctx, node, "PersistentVolumeClaimError", err.Error())
128130
return ctrl.Result{}, err
129131
}
130132

131133
if err := r.ensureWorkload(ctx, node); err != nil {
134+
workloadReason := "WorkloadError"
135+
switch node.Spec.WorkloadType {
136+
case valkeyiov1alpha1.WorkloadTypeStatefulSet:
137+
workloadReason = "StatefulSetError"
138+
case valkeyiov1alpha1.WorkloadTypeDeployment:
139+
workloadReason = "DeploymentError"
140+
}
141+
r.setReadyCondition(ctx, node, workloadReason, err.Error())
132142
return ctrl.Result{}, err
133143
}
134144

@@ -212,6 +222,32 @@ func (r *ValkeyNodeReconciler) clearLiveConfigCondition(ctx context.Context, nod
212222
return nil
213223
}
214224

225+
// setReadyCondition sets the Ready condition to False on the ValkeyNode status
226+
// so that errors from early reconcile stages (ConfigMap, PVC, workload creation)
227+
// are visible on the resource.
228+
func (r *ValkeyNodeReconciler) setReadyCondition(ctx context.Context, node *valkeyiov1alpha1.ValkeyNode, reason, message string) {
229+
log := logf.FromContext(ctx)
230+
current := &valkeyiov1alpha1.ValkeyNode{}
231+
if err := r.Get(ctx, client.ObjectKeyFromObject(node), current); err != nil {
232+
log.Error(err, "failed to get ValkeyNode for status update")
233+
return
234+
}
235+
patchBase := current.DeepCopy()
236+
if !meta.SetStatusCondition(&current.Status.Conditions, metav1.Condition{
237+
Type: valkeyiov1alpha1.ValkeyNodeConditionReady,
238+
Status: metav1.ConditionFalse,
239+
Reason: reason,
240+
Message: message,
241+
ObservedGeneration: current.Generation,
242+
}) {
243+
return
244+
}
245+
current.Status.Ready = false
246+
if err := r.Status().Patch(ctx, current, client.MergeFrom(patchBase)); err != nil {
247+
log.Error(err, "failed to patch ValkeyNode Ready condition")
248+
}
249+
}
250+
215251
func (r *ValkeyNodeReconciler) ensureWorkload(ctx context.Context, node *valkeyiov1alpha1.ValkeyNode) error {
216252
switch node.Spec.WorkloadType {
217253
case valkeyiov1alpha1.WorkloadTypeStatefulSet:

0 commit comments

Comments
 (0)