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,18 @@ func TestClusterSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
 	Expect(createdReady).ToNot(BeNil())
 	Expect(createdAvailable.CreatedTime).To(Equal(fixedNow))
 	Expect(createdAvailable.LastTransitionTime).To(Equal(fixedNow))
-	Expect(createdAvailable.LastUpdatedTime).To(Equal(fixedNow))
+	// Available.LastUpdatedTime is refreshed to the evaluation time; assert it lies in the Create() window.
+	Expect(createdAvailable.LastUpdatedTime).To(BeTemporally(">=", beforeCreate))
+	Expect(createdAvailable.LastUpdatedTime).To(BeTemporally("<=", afterCreate))
 	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 +820,14 @@ func TestClusterSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
 	Expect(updatedReady).ToNot(BeNil())
 	Expect(updatedAvailable.CreatedTime).To(Equal(fixedNow))
 	Expect(updatedAvailable.LastTransitionTime).To(Equal(fixedNow))
-	Expect(updatedAvailable.LastUpdatedTime).To(Equal(fixedNow))
+	// Available.LastUpdatedTime is refreshed to the evaluation time; assert it lies in the UpdateClusterStatusFromAdapters() window.
+	Expect(updatedAvailable.LastUpdatedTime).To(BeTemporally(">=", beforeUpdate))
+	Expect(updatedAvailable.LastUpdatedTime).To(BeTemporally("<=", afterUpdate))
 	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,18 @@ func TestNodePoolSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
 	Expect(createdReady).ToNot(BeNil())
 	Expect(createdAvailable.CreatedTime).To(Equal(fixedNow))
 	Expect(createdAvailable.LastTransitionTime).To(Equal(fixedNow))
-	Expect(createdAvailable.LastUpdatedTime).To(Equal(fixedNow))
+	// Available.LastUpdatedTime is refreshed to the evaluation time; assert it lies in the Create() window.
+	Expect(createdAvailable.LastUpdatedTime).To(BeTemporally(">=", beforeCreate))
+	Expect(createdAvailable.LastUpdatedTime).To(BeTemporally("<=", afterCreate))
 	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 +680,12 @@ func TestNodePoolSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
 	Expect(updatedReady).ToNot(BeNil())
 	Expect(updatedAvailable.CreatedTime).To(Equal(fixedNow))
 	Expect(updatedAvailable.LastTransitionTime).To(Equal(fixedNow))
-	Expect(updatedAvailable.LastUpdatedTime).To(Equal(fixedNow))
+	// Available.LastUpdatedTime is refreshed to the evaluation time; assert it lies in the UpdateNodePoolStatusFromAdapters() window.
+	Expect(updatedAvailable.LastUpdatedTime).To(BeTemporally(">=", beforeUpdate))
+	Expect(updatedAvailable.LastUpdatedTime).To(BeTemporally("<=", afterUpdate))
 	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

@@ -234,6 +234,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 == "True" {
+			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 +344,7 @@ func BuildSyntheticConditions(
 		CreatedTime:        now,
 		LastUpdatedTime:    now,
 	}
-	preserveSyntheticCondition(&availableCondition, existingAvailable, now)
+	applyConditionHistory(&availableCondition, existingAvailable, now, now)
 
 	isReady := ComputeReadyCondition(adapterStatuses, requiredAdapters, resourceGeneration)
 	readyStatus := api.ConditionFalse
@@ -286,13 +359,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 +388,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 +402,5 @@ func preserveSyntheticCondition(target *api.ResourceCondition, existing *api.Res
 		target.CreatedTime = existing.CreatedTime
 	}
 	target.LastTransitionTime = now
-	target.LastUpdatedTime = now
+	target.LastUpdatedTime = lastUpdatedTime
 }