Merge pull request #1798 from deads2k/apply-04-lasttransitiontime

API-1835: set the last transition time for conditions

Author: openshift-merge-bot[bot]

pkg/operator/genericoperatorclient/dynamic_operator_client.go

@@ -282,7 +282,11 @@ func (c dynamicOperatorClient) applyOperatorStatus(ctx context.Context, fieldMan
 	uncastOriginal, err := c.informer.Lister().Get(c.configName)
 	switch {
 	case apierrors.IsNotFound(err):
-		// do nothing and proceed with the apply
+		// set last transitionTimes and then apply
+		// If our cache improperly 404's (the lister wasn't synchronized), then we will improperly reset all the last transition times.
+		// This isn't ideal, but we shouldn't hit this case unless a loop isn't waiting for HasSynced.
+		v1helpers.SetApplyConditionsLastTransitionTime(&desiredConfiguration.Conditions, nil)
+
 	case err != nil:
 		return fmt.Errorf("unable to read existing %q: %w", c.configName, err)
 	default:
@@ -295,6 +299,24 @@ func (c dynamicOperatorClient) applyOperatorStatus(ctx context.Context, fieldMan
 			return fmt.Errorf("unable to extract status for %q: %w", fieldManager, err)
 		}
 
+		// set last transitionTimes to properly calculate a difference
+		// It is possible for last transition time to shift a couple times until the cache updates to have the condition[*].status match,
+		// but it will eventually settle.  The failing sequence looks like
+		/*
+			1. type=foo, status=false, time=t0.Now
+			2. type=foo, status=true, time=t1.Now
+			3. rapid update happens and the cache still indicates #1
+			4. type=foo, status=true, time=t2.Now (this *should* be t1.Now)
+		*/
+		// Eventually the cache updates to see at #2 and we stop applying new times.
+		// This only becomes pathological if the condition is also flapping, but if that happens the time should also update.
+		switch {
+		case desiredConfiguration != nil && desiredConfiguration.Conditions != nil && previouslyDesiredConfiguration != nil:
+			v1helpers.SetApplyConditionsLastTransitionTime(&desiredConfiguration.Conditions, previouslyDesiredConfiguration.Conditions)
+		case desiredConfiguration != nil && desiredConfiguration.Conditions != nil && previouslyDesiredConfiguration == nil:
+			v1helpers.SetApplyConditionsLastTransitionTime(&desiredConfiguration.Conditions, nil)
+		}
+
 		// canonicalize so the DeepEqual works consistently
 		v1helpers.CanonicalizeStaticPodOperatorStatus(previouslyDesiredConfiguration)
 		v1helpers.CanonicalizeStaticPodOperatorStatus(desiredConfiguration)

pkg/operator/v1helpers/canonicalize.go

@@ -2,13 +2,12 @@ package v1helpers
 
 import (
 	"fmt"
-	"slices"
-	"strings"
-
 	operatorv1 "github.com/openshift/api/operator/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/json"
-
 	"k8s.io/utils/ptr"
+	"slices"
+	"strings"
 
 	applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
 )
@@ -33,6 +32,40 @@ func ToStaticPodOperator(in *applyoperatorv1.StaticPodOperatorStatusApplyConfigu
 	return ret, nil
 }
 
+func SetApplyConditionsLastTransitionTime(newConditions *[]applyoperatorv1.OperatorConditionApplyConfiguration, oldConditions []applyoperatorv1.OperatorConditionApplyConfiguration) {
+	if newConditions == nil {
+		return
+	}
+
+	now := metav1.Now()
+	for i := range *newConditions {
+		newCondition := (*newConditions)[i]
+
+		// if the condition status is the same, then the lastTransitionTime doesn't change
+		if existingCondition := FindApplyCondition(oldConditions, newCondition.Type); existingCondition != nil && ptr.Equal(existingCondition.Status, newCondition.Status) {
+			newCondition.LastTransitionTime = existingCondition.LastTransitionTime
+		}
+
+		// backstop to handle upgrade case too.  If the newCondition doesn't have a lastTransitionTime it needs something
+		if newCondition.LastTransitionTime == nil {
+			newCondition.LastTransitionTime = &now
+		}
+
+		(*newConditions)[i] = newCondition
+	}
+}
+
+func FindApplyCondition(haystack []applyoperatorv1.OperatorConditionApplyConfiguration, conditionType *string) *applyoperatorv1.OperatorConditionApplyConfiguration {
+	for i := range haystack {
+		curr := haystack[i]
+		if ptr.Equal(curr.Type, conditionType) {
+			return &curr
+		}
+	}
+
+	return nil
+}
+
 func CanonicalizeStaticPodOperatorStatus(obj *applyoperatorv1.StaticPodOperatorStatusApplyConfiguration) {
 	if obj == nil {
 		return