Skip to content
This repository was archived by the owner on Aug 11, 2025. It is now read-only.

Commit d03f704

Browse files
authored
keep status value after completion (#275)
Deleting the status value that a controller uses to track its work can lead to that controller continuing to work after the orchestrator's reconciler goroutine has marked the condition as failed. In a situation where the controller attempts a status update and the orchestrator has already failed the condition, the only acceptable response is for the controller to stop work.
1 parent dd89460 commit d03f704

File tree

1 file changed

+3
-23
lines changed

1 file changed

+3
-23
lines changed

internal/orchestrator/updates.go

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -425,10 +425,9 @@ func (o *Orchestrator) eventUpdate(ctx context.Context, evt *v1types.ConditionUp
425425
}
426426

427427
func (o *Orchestrator) finalizeCondition(ctx context.Context, cond *rctypes.Condition) error {
428-
// if we fail to update the event history or to delete this event from the KV,
429-
// the reconciler will catch it later and walk this code, so return early. It
430-
// is kosher to replay event history iff the contents of that history (id,
431-
// condition kind, target, parameters, state and status) are identical.
428+
// if we fail to update the event history the reconciler will catch it later and walk
429+
// this code, so return early. It is kosher to replay event history iff the contents of
430+
// that history (id, condition kind, target, parameters, state and status) are identical.
432431
if err := o.db.WriteEventHistory(ctx, cond); err != nil {
433432
o.logger.WithError(err).WithFields(logrus.Fields{
434433
"condition.id": cond.ID.String(),
@@ -441,19 +440,6 @@ func (o *Orchestrator) finalizeCondition(ctx context.Context, cond *rctypes.Cond
441440
return errors.Wrap(errCompleteEvent, err.Error())
442441
}
443442

444-
delErr := status.DeleteCondition(cond.Kind, o.facility, cond.ID.String())
445-
if delErr != nil {
446-
o.logger.WithError(delErr).WithFields(logrus.Fields{
447-
"condition.id": cond.ID.String(),
448-
"server.id": cond.Target.String(),
449-
"condition.kind": cond.Kind,
450-
}).Warn("removing completed condition data")
451-
452-
metrics.DependencyError("nats", "remove completed condition condition")
453-
454-
return errors.Wrap(errCompleteEvent, delErr.Error())
455-
}
456-
457443
metrics.ConditionCompleted.With(
458444
prometheus.Labels{
459445
"conditionKind": string(cond.Kind),
@@ -609,12 +595,6 @@ func (o *Orchestrator) reconcileStatusKVEntries(ctx context.Context) {
609595
// arguably dependencies are in a weird state, maybe return?
610596
continue
611597
}
612-
// if we're here there has been a terminal error trying to reconcile this
613-
// status value. Get rid of it now.
614-
if err = status.DeleteCondition(evt.Kind, o.facility, evt.ConditionID.String()); err != nil {
615-
le.WithError(err).Warn("deleting condition on reconciliation")
616-
continue
617-
}
618598
}
619599

620600
le.Info("condition reconciled")

0 commit comments

Comments
 (0)