Skip to content

Commit 7dde09c

Browse files
atd9876Copilot
andcommitted
More controller condition setting
Co-authored-by: Copilot <copilot@github.com> Signed-off-by: Andrew Dodds <andrew.dodds@sap.com>
1 parent 0f61190 commit 7dde09c

2 files changed

Lines changed: 30 additions & 9 deletions

File tree

internal/controller/bmcsettings_controller.go

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -533,8 +533,10 @@ func (r *BMCSettingsReconciler) updateSettingsAndVerify(ctx context.Context, set
533533
}
534534

535535
// persistApplyCycleConditions resets later-phase conditions (verified, reset) and
536-
// persists all phase conditions after a successful settings apply in Phase 1.
536+
// persists all phase conditions atomically after a successful settings apply in Phase 1.
537537
func (r *BMCSettingsReconciler) persistApplyCycleConditions(ctx context.Context, settings *metalv1alpha1.BMCSettings, changesIssued *metav1.Condition, resetBMCReq bool) error {
538+
log := ctrl.LoggerFrom(ctx)
539+
538540
resetCond, err := GetCondition(r.Conditions, settings.Status.Conditions, ConditionBMCResetPostSettingApply)
539541
if err != nil {
540542
return fmt.Errorf("failed to get condition for reset of BMC: %w", err)
@@ -565,20 +567,37 @@ func (r *BMCSettingsReconciler) persistApplyCycleConditions(ctx context.Context,
565567
if err := r.Conditions.Update(
566568
resetCond,
567569
conditionutils.UpdateStatus(corev1.ConditionTrue),
568-
conditionutils.UpdateReason(ReasonResetIssued),
570+
conditionutils.UpdateReason(ReasonNoResetRequired),
569571
conditionutils.UpdateMessage("No BMC reset required for this apply cycle"),
570572
); err != nil {
571573
return fmt.Errorf("failed to update reset not required condition: %w", err)
572574
}
573575
}
574576

575-
if err := r.updateBMCSettingsStatus(ctx, settings, settings.Status.State, changesIssued); err != nil {
576-
return fmt.Errorf("failed to persist settings changes issued condition: %w", err)
577+
// Persist all three conditions in a single atomic status patch.
578+
BMCSettingsBase := settings.DeepCopy()
579+
for _, cond := range []*metav1.Condition{changesIssued, verifiedCond, resetCond} {
580+
updateOpts := []conditionutils.UpdateOption{
581+
conditionutils.UpdateStatus(cond.Status),
582+
conditionutils.UpdateReason(cond.Reason),
583+
conditionutils.UpdateMessage(cond.Message),
584+
}
585+
if cond.ObservedGeneration > 0 {
586+
updateOpts = append(updateOpts, conditionutils.UpdateObservedGeneration(cond.ObservedGeneration))
587+
}
588+
if err := r.Conditions.UpdateSlice(
589+
&settings.Status.Conditions,
590+
cond.Type,
591+
updateOpts...,
592+
); err != nil {
593+
return fmt.Errorf("failed to update condition %s in slice: %w", cond.Type, err)
594+
}
577595
}
578-
if err := r.updateBMCSettingsStatus(ctx, settings, settings.Status.State, verifiedCond); err != nil {
579-
return fmt.Errorf("failed to persist settings changes verified condition: %w", err)
596+
if err := r.Status().Patch(ctx, settings, client.MergeFrom(BMCSettingsBase)); err != nil {
597+
return fmt.Errorf("failed to patch settings status: %w", err)
580598
}
581-
return r.updateBMCSettingsStatus(ctx, settings, settings.Status.State, resetCond)
599+
log.V(1).Info("Persisted apply-cycle conditions atomically")
600+
return nil
582601
}
583602

584603
func (r *BMCSettingsReconciler) handleSettingAppliedState(ctx context.Context, settings *metalv1alpha1.BMCSettings, bmcObj *metalv1alpha1.BMC, bmcClient bmc.BMC) error {
@@ -661,8 +680,8 @@ func (r *BMCSettingsReconciler) handleBMCReset(
661680
return false, fmt.Errorf("failed to get condition for reset of BMC of server %v", err)
662681
}
663682

664-
// No reset requested — nothing to do
665-
if resetBMC.Reason != ReasonResetRequired && resetBMC.Reason != ReasonResetIssued {
683+
// The post-apply reset phase can be skipped explicitly when no reset was needed.
684+
if conditionType == ConditionBMCResetPostSettingApply && resetBMC.Status == metav1.ConditionTrue && resetBMC.Reason == ReasonNoResetRequired {
666685
return true, nil
667686
}
668687

internal/controller/conditions.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ const (
5353
ReasonResetIssued = "ResetIssued"
5454
// ReasonResetRequired indicates a BMC reset is needed but not yet issued.
5555
ReasonResetRequired = "ResetRequired"
56+
// ReasonNoResetRequired indicates no BMC reset is needed for this apply cycle.
57+
ReasonNoResetRequired = "NoResetRequired"
5658
// ReasonAuthenticationFailed indicates authentication has failed.
5759
ReasonAuthenticationFailed = "AuthenticationFailed"
5860
// ReasonInternalError indicates an internal server error.

0 commit comments

Comments
 (0)