From 99e03c8a85b16368d217ffeb15411f05a75cde65 Mon Sep 17 00:00:00 2001 From: David Eads Date: Thu, 24 Oct 2024 19:34:53 -0400 Subject: [PATCH] ensure we have lastTransitionTimes on status conditions --- pkg/operator/status/condition.go | 13 ++++++------- pkg/operator/status/status_controller.go | 15 ++++++++++----- pkg/operator/status/status_controller_test.go | 2 ++ 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/pkg/operator/status/condition.go b/pkg/operator/status/condition.go index edcafa0b62..aeeec191c9 100644 --- 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 index 9a08a4e1b8..3b79e3c96d 100644 --- 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 index 2355e07516..856b03c6a0 100644 --- 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,