HYPERFLEET-706 - fix: lastUpdateTime for Ready

Author: rh-amarin

docs/api-resources.md

@@ -456,7 +456,7 @@ The status object contains synthesized conditions computed from adapter reports:
 - All above fields plus:
 - `observed_generation` - Generation this condition reflects
 - `created_time` - When condition was first created (API-managed)
-- `last_updated_time` - When adapter last reported (API-managed, from AdapterStatus.last_report_time)
+- `last_updated_time` - When this condition was last refreshed (API-managed). For **Available**, always the evaluation time. For **Ready**: when Ready=True, the minimum of `last_report_time` across all required adapters that report Available=True at the current generation; when Ready=False, the evaluation time (so consumers can detect staleness).
 - `last_transition_time` - When status last changed (API-managed)
 
 ## Parameter Restrictions

pkg/services/CLAUDE.md

@@ -25,6 +25,8 @@ func NewClusterService(dao, adapterStatusDao, config) ClusterService
 - **Available**: True if all required adapters report `Available=True` (any generation)
 - **Ready**: True if all adapters report `Available=True` AND `observed_generation` matches current generation
 
+Ready's `LastUpdatedTime` is computed in `status_aggregation.computeReadyLastUpdated`: when Ready=False it is the evaluation time (so Sentinel can apply a freshness threshold); when Ready=True it is the minimum of `LastReportTime` across required adapters that have Available=True at the current generation.
+
 `ProcessAdapterStatus()` validates mandatory conditions (`Available`, `Applied`, `Health`) before persisting. Rejects `Available=Unknown` on subsequent reports (only allowed on first report).
 
 ## GenericService

pkg/services/cluster_test.go

@@ -767,7 +767,9 @@ func TestClusterSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
 		StatusConditions: initialConditionsJSON,
 	}
 	cluster.ID = clusterID
+	beforeCreate := time.Now()
 	created, svcErr := service.Create(ctx, cluster)
+	afterCreate := time.Now()
 	Expect(svcErr).To(BeNil())
 
 	var createdConds []api.ResourceCondition
@@ -787,12 +789,17 @@ func TestClusterSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
 	Expect(createdReady).ToNot(BeNil())
 	Expect(createdAvailable.CreatedTime).To(Equal(fixedNow))
 	Expect(createdAvailable.LastTransitionTime).To(Equal(fixedNow))
+	// Available.LastUpdatedTime is preserved from the initial conditions when status and generation are unchanged.
 	Expect(createdAvailable.LastUpdatedTime).To(Equal(fixedNow))
 	Expect(createdReady.CreatedTime).To(Equal(fixedNow))
 	Expect(createdReady.LastTransitionTime).To(Equal(fixedNow))
-	Expect(createdReady.LastUpdatedTime).To(Equal(fixedNow))
+	// Ready.LastUpdatedTime is refreshed to the evaluation time when isReady=false; assert it lies in the Create() window.
+	Expect(createdReady.LastUpdatedTime).To(BeTemporally(">=", beforeCreate))
+	Expect(createdReady.LastUpdatedTime).To(BeTemporally("<=", afterCreate))
 
+	beforeUpdate := time.Now()
 	updated, err := service.UpdateClusterStatusFromAdapters(ctx, clusterID)
+	afterUpdate := time.Now()
 	Expect(err).To(BeNil())
 
 	var updatedConds []api.ResourceCondition
@@ -812,10 +819,14 @@ func TestClusterSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
 	Expect(updatedReady).ToNot(BeNil())
 	Expect(updatedAvailable.CreatedTime).To(Equal(fixedNow))
 	Expect(updatedAvailable.LastTransitionTime).To(Equal(fixedNow))
+	// Available.LastUpdatedTime is stable when status and generation are unchanged.
 	Expect(updatedAvailable.LastUpdatedTime).To(Equal(fixedNow))
 	Expect(updatedReady.CreatedTime).To(Equal(fixedNow))
 	Expect(updatedReady.LastTransitionTime).To(Equal(fixedNow))
-	Expect(updatedReady.LastUpdatedTime).To(Equal(fixedNow))
+	// Ready.LastUpdatedTime is refreshed to the evaluation time when isReady=false;
+	// assert it lies in the UpdateClusterStatusFromAdapters() window.
+	Expect(updatedReady.LastUpdatedTime).To(BeTemporally(">=", beforeUpdate))
+	Expect(updatedReady.LastUpdatedTime).To(BeTemporally("<=", afterUpdate))
 }
 
 // TestProcessAdapterStatus_MissingMandatoryCondition_Available tests that updates missing Available are rejected

pkg/services/node_pool_test.go

@@ -627,7 +627,9 @@ func TestNodePoolSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
 		StatusConditions: initialConditionsJSON,
 	}
 	nodePool.ID = nodePoolID
+	beforeCreate := time.Now()
 	created, svcErr := service.Create(ctx, nodePool)
+	afterCreate := time.Now()
 	Expect(svcErr).To(BeNil())
 
 	var createdConds []api.ResourceCondition
@@ -647,12 +649,17 @@ func TestNodePoolSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
 	Expect(createdReady).ToNot(BeNil())
 	Expect(createdAvailable.CreatedTime).To(Equal(fixedNow))
 	Expect(createdAvailable.LastTransitionTime).To(Equal(fixedNow))
+	// Available.LastUpdatedTime is preserved from the initial conditions when status and generation are unchanged.
 	Expect(createdAvailable.LastUpdatedTime).To(Equal(fixedNow))
 	Expect(createdReady.CreatedTime).To(Equal(fixedNow))
 	Expect(createdReady.LastTransitionTime).To(Equal(fixedNow))
-	Expect(createdReady.LastUpdatedTime).To(Equal(fixedNow))
+	// Ready.LastUpdatedTime is refreshed to the evaluation time when isReady=false; assert it lies in the Create() window.
+	Expect(createdReady.LastUpdatedTime).To(BeTemporally(">=", beforeCreate))
+	Expect(createdReady.LastUpdatedTime).To(BeTemporally("<=", afterCreate))
 
+	beforeUpdate := time.Now()
 	updated, err := service.UpdateNodePoolStatusFromAdapters(ctx, nodePoolID)
+	afterUpdate := time.Now()
 	Expect(err).To(BeNil())
 
 	var updatedConds []api.ResourceCondition
@@ -672,8 +679,12 @@ func TestNodePoolSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
 	Expect(updatedReady).ToNot(BeNil())
 	Expect(updatedAvailable.CreatedTime).To(Equal(fixedNow))
 	Expect(updatedAvailable.LastTransitionTime).To(Equal(fixedNow))
+	// Available.LastUpdatedTime is stable when status and generation are unchanged.
 	Expect(updatedAvailable.LastUpdatedTime).To(Equal(fixedNow))
 	Expect(updatedReady.CreatedTime).To(Equal(fixedNow))
 	Expect(updatedReady.LastTransitionTime).To(Equal(fixedNow))
-	Expect(updatedReady.LastUpdatedTime).To(Equal(fixedNow))
+	// Ready.LastUpdatedTime is refreshed to the evaluation time when isReady=false;
+	// assert it lies in the UpdateNodePoolStatusFromAdapters() window.
+	Expect(updatedReady.LastUpdatedTime).To(BeTemporally(">=", beforeUpdate))
+	Expect(updatedReady.LastUpdatedTime).To(BeTemporally("<=", afterUpdate))
 }

pkg/services/status_aggregation.go

@@ -1,13 +1,16 @@
 package services
 
 import (
+	"context"
 	"encoding/json"
+	"fmt"
 	"math"
 	"strings"
 	"time"
 	"unicode"
 
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/api"
+	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger"
 )
 
 // Mandatory condition types that must be present in all adapter status updates
@@ -19,6 +22,9 @@ const (
 	ConditionValidationErrorMissing   = "missing"
 )
 
+// adapterConditionStatusTrue is the string value for a True adapter condition status.
+const adapterConditionStatusTrue = "True"
+
 // Required adapter lists configured via pkg/config/adapter.go (see AdapterRequirementsConfig)
 
 // adapterConditionSuffixMap allows overriding the default suffix for specific adapters
@@ -118,18 +124,21 @@ func ComputeAvailableCondition(adapterStatuses api.AdapterStatusList, requiredAd
 			Status string `json:"status"`
 		}
 		if len(adapterStatus.Conditions) > 0 {
-			if err := json.Unmarshal(adapterStatus.Conditions, &conditions); err == nil {
-				for _, cond := range conditions {
-					if cond.Type == api.ConditionTypeAvailable {
-						adapterMap[adapterStatus.Adapter] = struct {
-							available          string
-							observedGeneration int32
-						}{
-							available:          cond.Status,
-							observedGeneration: adapterStatus.ObservedGeneration,
-						}
-						break
+			if err := json.Unmarshal(adapterStatus.Conditions, &conditions); err != nil {
+				logger.WithError(context.Background(), err).Warn(
+					fmt.Sprintf("failed to parse adapter conditions for adapter %s", adapterStatus.Adapter))
+				continue
+			}
+			for _, cond := range conditions {
+				if cond.Type == api.ConditionTypeAvailable {
+					adapterMap[adapterStatus.Adapter] = struct {
+						available          string
+						observedGeneration int32
+					}{
+						available:          cond.Status,
+						observedGeneration: adapterStatus.ObservedGeneration,
 					}
+					break
 				}
 			}
 		}
@@ -149,7 +158,7 @@ func ComputeAvailableCondition(adapterStatuses api.AdapterStatusList, requiredAd
 
 		// For Available condition, we don't check generation matching
 		// We just need Available=True at ANY generation
-		if adapterInfo.available == "True" {
+		if adapterInfo.available == adapterConditionStatusTrue {
 			numAvailable++
 			if adapterInfo.observedGeneration < minObservedGeneration {
 				minObservedGeneration = adapterInfo.observedGeneration
@@ -189,18 +198,21 @@ func ComputeReadyCondition(
 			Status string `json:"status"`
 		}
 		if len(adapterStatus.Conditions) > 0 {
-			if err := json.Unmarshal(adapterStatus.Conditions, &conditions); err == nil {
-				for _, cond := range conditions {
-					if cond.Type == api.ConditionTypeAvailable {
-						adapterMap[adapterStatus.Adapter] = struct {
-							available          string
-							observedGeneration int32
-						}{
-							available:          cond.Status,
-							observedGeneration: adapterStatus.ObservedGeneration,
-						}
-						break
+			if err := json.Unmarshal(adapterStatus.Conditions, &conditions); err != nil {
+				logger.WithError(context.Background(), err).Warn(
+					fmt.Sprintf("failed to parse adapter conditions for adapter %s", adapterStatus.Adapter))
+				continue
+			}
+			for _, cond := range conditions {
+				if cond.Type == api.ConditionTypeAvailable {
+					adapterMap[adapterStatus.Adapter] = struct {
+						available          string
+						observedGeneration int32
+					}{
+						available:          cond.Status,
+						observedGeneration: adapterStatus.ObservedGeneration,
 					}
+					break
 				}
 			}
 		}
@@ -224,7 +236,7 @@ func ComputeReadyCondition(
 		}
 
 		// Check available status
-		if adapterInfo.available == "True" {
+		if adapterInfo.available == adapterConditionStatusTrue {
 			numReady++
 		}
 	}
@@ -234,6 +246,79 @@ func ComputeReadyCondition(
 	return numReady == numRequired
 }
 
+// findAdapterStatus returns the first adapter status in the list with the given adapter name, or (nil, false).
+func findAdapterStatus(adapterStatuses api.AdapterStatusList, adapterName string) (*api.AdapterStatus, bool) {
+	for _, s := range adapterStatuses {
+		if s.Adapter == adapterName {
+			return s, true
+		}
+	}
+	return nil, false
+}
+
+// adapterConditionsHasAvailableTrue returns true if the adapter conditions JSON
+// contains a condition with type Available and status True.
+func adapterConditionsHasAvailableTrue(conditions []byte) bool {
+	if len(conditions) == 0 {
+		return false
+	}
+	var conds []struct {
+		Type   string `json:"type"`
+		Status string `json:"status"`
+	}
+	if err := json.Unmarshal(conditions, &conds); err != nil {
+		return false
+	}
+	for _, c := range conds {
+		if c.Type == api.ConditionTypeAvailable && c.Status == adapterConditionStatusTrue {
+			return true
+		}
+	}
+	return false
+}
+
+// computeReadyLastUpdated returns the timestamp to use for the Ready condition's LastUpdatedTime.
+// When isReady is false, it returns now (Ready=False changes frequently; 10s threshold applies).
+// When isReady is true, it returns the minimum LastReportTime across all required adapters
+// that have Available=True at the current generation. Falls back to now if no timestamps found.
+func computeReadyLastUpdated(
+	adapterStatuses api.AdapterStatusList,
+	requiredAdapters []string,
+	resourceGeneration int32,
+	now time.Time,
+	isReady bool,
+) time.Time {
+	if !isReady {
+		return now
+	}
+
+	var minTime *time.Time
+	for _, adapterName := range requiredAdapters {
+		status, ok := findAdapterStatus(adapterStatuses, adapterName)
+		if !ok {
+			return now // safety: required adapter missing
+		}
+		if status.LastReportTime == nil {
+			return now // safety: no timestamp
+		}
+		if status.ObservedGeneration != resourceGeneration {
+			continue // not at current gen, skip
+		}
+		if !adapterConditionsHasAvailableTrue(status.Conditions) {
+			continue
+		}
+		if minTime == nil || status.LastReportTime.Before(*minTime) {
+			t := *status.LastReportTime
+			minTime = &t
+		}
+	}
+
+	if minTime == nil {
+		return now // safety fallback
+	}
+	return *minTime
+}
+
 func BuildSyntheticConditions(
 	existingConditionsJSON []byte,
 	adapterStatuses api.AdapterStatusList,
@@ -271,7 +356,14 @@ func BuildSyntheticConditions(
 		CreatedTime:        now,
 		LastUpdatedTime:    now,
 	}
-	preserveSyntheticCondition(&availableCondition, existingAvailable, now)
+	availableLastUpdated := now
+	if existingAvailable != nil &&
+		existingAvailable.Status == availableStatus &&
+		existingAvailable.ObservedGeneration == minObservedGeneration &&
+		!existingAvailable.LastUpdatedTime.IsZero() {
+		availableLastUpdated = existingAvailable.LastUpdatedTime
+	}
+	applyConditionHistory(&availableCondition, existingAvailable, now, availableLastUpdated)
 
 	isReady := ComputeReadyCondition(adapterStatuses, requiredAdapters, resourceGeneration)
 	readyStatus := api.ConditionFalse
@@ -286,13 +378,25 @@ func BuildSyntheticConditions(
 		CreatedTime:        now,
 		LastUpdatedTime:    now,
 	}
-	preserveSyntheticCondition(&readyCondition, existingReady, now)
+	readyLastUpdated := computeReadyLastUpdated(
+		adapterStatuses, requiredAdapters, resourceGeneration, now, isReady,
+	)
+	applyConditionHistory(&readyCondition, existingReady, now, readyLastUpdated)
 
 	return availableCondition, readyCondition
 }
 
-func preserveSyntheticCondition(target *api.ResourceCondition, existing *api.ResourceCondition, now time.Time) {
+// applyConditionHistory copies stable timestamps and metadata from an existing condition.
+// lastUpdatedTime is used unconditionally for LastUpdatedTime — the caller is responsible
+// for computing the correct value (e.g. now, computeReadyLastUpdated(...)).
+func applyConditionHistory(
+	target *api.ResourceCondition,
+	existing *api.ResourceCondition,
+	now time.Time,
+	lastUpdatedTime time.Time,
+) {
 	if existing == nil {
+		target.LastUpdatedTime = lastUpdatedTime
 		return
 	}
 
@@ -303,9 +407,7 @@ func preserveSyntheticCondition(target *api.ResourceCondition, existing *api.Res
 		if !existing.LastTransitionTime.IsZero() {
 			target.LastTransitionTime = existing.LastTransitionTime
 		}
-		if !existing.LastUpdatedTime.IsZero() {
-			target.LastUpdatedTime = existing.LastUpdatedTime
-		}
+		target.LastUpdatedTime = lastUpdatedTime
 		if target.Reason == nil && existing.Reason != nil {
 			target.Reason = existing.Reason
 		}
@@ -319,5 +421,5 @@ func preserveSyntheticCondition(target *api.ResourceCondition, existing *api.Res
 		target.CreatedTime = existing.CreatedTime
 	}
 	target.LastTransitionTime = now
-	target.LastUpdatedTime = now
+	target.LastUpdatedTime = lastUpdatedTime
 }