Restore LastTransitionTime after setting Ready Condition

instance.Status.Conditions.MarkTrue() always sets the current time as the LastTransitionTime and this results in infinite loop as the LastTransitionTime changes after every reconcile. I think this issue is there in many other operator controllers, but we don't see the issue as the next reconcile is probably too quick to not change the LastTransitionTime(precision for which is in seconds). The issue showed up in dataplane controllers as we reconcile a number of services in a loop for every reconcile of nodeset. Jira: OSPRH-8811 Signed-off-by: rabi <[email protected]>
stuggi · Jul 29, 2024 · 7add4dc · 7add4dc
1 parent 8f7cb45
commit 7add4dc
Show file tree

Hide file tree

Showing 3 changed files with 3 additions and 3 deletions.
diff --git a/controllers/ovncontroller_controller.go b/controllers/ovncontroller_controller.go
@@ -136,7 +136,6 @@ func (r *OVNControllerReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	// Always patch the instance status when exiting this function so we can persist any changes.
 	defer func() {
-		condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
 		// update the Ready condition based on the sub conditions
 		if instance.Status.Conditions.AllSubConditionIsTrue() {
 			instance.Status.Conditions.MarkTrue(
@@ -149,6 +148,7 @@ func (r *OVNControllerReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 			instance.Status.Conditions.Set(
 				instance.Status.Conditions.Mirror(condition.ReadyCondition))
 		}
+		condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
 		err := helper.PatchInstance(ctx, instance)
 		if err != nil {
 			_err = err

diff --git a/controllers/ovndbcluster_controller.go b/controllers/ovndbcluster_controller.go
@@ -169,7 +169,6 @@ func (r *OVNDBClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request
 
 	// Always patch the instance status when exiting this function so we can persist any changes.
 	defer func() {
-		condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
 		// update the Ready condition based on the sub conditions
 		if instance.Status.Conditions.AllSubConditionIsTrue() {
 			instance.Status.Conditions.MarkTrue(
@@ -182,6 +181,7 @@ func (r *OVNDBClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request
 			instance.Status.Conditions.Set(
 				instance.Status.Conditions.Mirror(condition.ReadyCondition))
 		}
+		condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
 		err := helper.PatchInstance(ctx, instance)
 		if err != nil {
 			_err = err

diff --git a/controllers/ovnnorthd_controller.go b/controllers/ovnnorthd_controller.go
@@ -145,7 +145,6 @@ func (r *OVNNorthdReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	// Always patch the instance status when exiting this function so we can persist any changes.
 	defer func() {
-		condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
 		// update the Ready condition based on the sub conditions
 		if instance.Status.Conditions.AllSubConditionIsTrue() {
 			instance.Status.Conditions.MarkTrue(
@@ -158,6 +157,7 @@ func (r *OVNNorthdReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			instance.Status.Conditions.Set(
 				instance.Status.Conditions.Mirror(condition.ReadyCondition))
 		}
+		condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
 		err := helper.PatchInstance(ctx, instance)
 		if err != nil {
 			_err = err