ensure we have lastTransitionTimes on status conditions

openshift · Oct 24, 2024 · 99e03c8 · 99e03c8
1 parent db83b67
commit 99e03c8
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 12 deletions.
diff --git a/pkg/operator/status/condition.go b/pkg/operator/status/condition.go
@@ -3,12 +3,11 @@ package status
 import (
 	"fmt"
 	applyconfigv1 "github.com/openshift/client-go/config/applyconfigurations/config/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/clock"
 	"k8s.io/utils/ptr"
 	"sort"
 	"strings"
-	"time"
-
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	configv1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
@@ -20,7 +19,7 @@ import (
 // on true, but Available merges on false.  Thing of it like an anti-default.
 //
 // If inertia is non-nil, then resist returning a condition with a status opposite the defaultConditionStatus.
-func UnionCondition(conditionType string, defaultConditionStatus operatorv1.ConditionStatus, inertia Inertia, allConditions ...operatorv1.OperatorCondition) operatorv1.OperatorCondition {
+func UnionCondition(clock clock.PassiveClock, conditionType string, defaultConditionStatus operatorv1.ConditionStatus, inertia Inertia, allConditions ...operatorv1.OperatorCondition) operatorv1.OperatorCondition {
 	var oppositeConditionStatus operatorv1.ConditionStatus
 	if defaultConditionStatus == operatorv1.ConditionTrue {
 		oppositeConditionStatus = operatorv1.ConditionFalse
@@ -56,7 +55,7 @@ func UnionCondition(conditionType string, defaultConditionStatus operatorv1.Cond
 	if inertia == nil {
 		elderBadConditions = badConditions
 	} else {
-		now := time.Now()
+		now := clock.Now()
 		for _, condition := range badConditions {
 			if condition.LastTransitionTime.Time.Before(now.Add(-inertia(condition))) {
 				elderBadConditions = append(elderBadConditions, condition)
@@ -91,8 +90,8 @@ func UnionCondition(conditionType string, defaultConditionStatus operatorv1.Cond
 // on true, but Available merges on false.  Thing of it like an anti-default.
 //
 // If inertia is non-nil, then resist returning a condition with a status opposite the defaultConditionStatus.
-func UnionClusterCondition(conditionType configv1.ClusterStatusConditionType, defaultConditionStatus operatorv1.ConditionStatus, inertia Inertia, allConditions ...operatorv1.OperatorCondition) *applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration {
-	cnd := UnionCondition(string(conditionType), defaultConditionStatus, inertia, allConditions...)
+func UnionClusterCondition(clock clock.PassiveClock, conditionType configv1.ClusterStatusConditionType, defaultConditionStatus operatorv1.ConditionStatus, inertia Inertia, allConditions ...operatorv1.OperatorCondition) *applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration {
+	cnd := UnionCondition(clock, string(conditionType), defaultConditionStatus, inertia, allConditions...)
 	return OperatorConditionToClusterOperatorCondition(cnd)
 }
 

diff --git a/pkg/operator/status/status_controller.go b/pkg/operator/status/status_controller.go
@@ -266,12 +266,17 @@ func (c StatusSyncer) Sync(ctx context.Context, syncCtx factory.SyncContext) err
 	}
 
 	desiredStatus.WithConditions(
-		UnionClusterCondition(configv1.OperatorDegraded, operatorv1.ConditionFalse, c.degradedInertia, currentDetailedStatus.Conditions...),
-		UnionClusterCondition(configv1.OperatorProgressing, operatorv1.ConditionFalse, nil, currentDetailedStatus.Conditions...),
-		UnionClusterCondition(configv1.OperatorAvailable, operatorv1.ConditionTrue, nil, currentDetailedStatus.Conditions...),
-		UnionClusterCondition(configv1.OperatorUpgradeable, operatorv1.ConditionTrue, nil, currentDetailedStatus.Conditions...),
-		UnionClusterCondition(configv1.EvaluationConditionsDetected, operatorv1.ConditionFalse, nil, currentDetailedStatus.Conditions...),
+		UnionClusterCondition(c.clock, configv1.OperatorDegraded, operatorv1.ConditionFalse, c.degradedInertia, currentDetailedStatus.Conditions...),
+		UnionClusterCondition(c.clock, configv1.OperatorProgressing, operatorv1.ConditionFalse, nil, currentDetailedStatus.Conditions...),
+		UnionClusterCondition(c.clock, configv1.OperatorAvailable, operatorv1.ConditionTrue, nil, currentDetailedStatus.Conditions...),
+		UnionClusterCondition(c.clock, configv1.OperatorUpgradeable, operatorv1.ConditionTrue, nil, currentDetailedStatus.Conditions...),
+		UnionClusterCondition(c.clock, configv1.EvaluationConditionsDetected, operatorv1.ConditionFalse, nil, currentDetailedStatus.Conditions...),
 	)
+	var previousConditions []applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration
+	if previouslyDesiredStatus != nil && previouslyDesiredStatus.Status != nil {
+		previousConditions = previouslyDesiredStatus.Status.Conditions
+	}
+	operatorv1helpers.SetClusterOperatorApplyConditionsLastTransitionTime(c.clock, &desiredStatus.Conditions, previousConditions)
 
 	desiredStatus.WithVersions(c.createNewOperatorVersions(previouslyDesiredStatus.Status, syncCtx)...)
 

diff --git a/pkg/operator/status/status_controller_test.go b/pkg/operator/status/status_controller_test.go
@@ -491,6 +491,7 @@ func TestRelatedObjects(t *testing.T) {
 			controller := &StatusSyncer{
 				controllerInstanceName: factory.ControllerInstanceName("OPERATOR_NAME", "ClusterOperatorStatus"),
 				clusterOperatorName:    "OPERATOR_NAME",
+				clock:                  clocktesting.NewFakePassiveClock(time.Now()),
 				clusterOperatorClient:  clusterOperatorClient.ConfigV1(),
 				clusterOperatorLister:  configv1listers.NewClusterOperatorLister(indexer),
 				operatorClient:         statusClient,
@@ -643,6 +644,7 @@ func TestVersions(t *testing.T) {
 			controller := &StatusSyncer{
 				controllerInstanceName: factory.ControllerInstanceName("OPERATOR_NAME", "ClusterOperatorStatus"),
 				clusterOperatorName:    "OPERATOR_NAME",
+				clock:                  clocktesting.NewFakePassiveClock(time.Now()),
 				clusterOperatorClient:  clusterOperatorClient.ConfigV1(),
 				clusterOperatorLister:  configv1listers.NewClusterOperatorLister(indexer),
 				operatorClient:         statusClient,