Skip to content

Commit 0465f91

Browse files
committed
HYPERFLEET-706 - fix: lastUpdateTime for Ready
1 parent 89c8376 commit 0465f91

6 files changed

Lines changed: 258 additions & 16 deletions

File tree

docs/api-resources.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ The status object contains synthesized conditions computed from adapter reports:
456456
- All above fields plus:
457457
- `observed_generation` - Generation this condition reflects
458458
- `created_time` - When condition was first created (API-managed)
459-
- `last_updated_time` - When adapter last reported (API-managed, from AdapterStatus.last_report_time)
459+
- `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).
460460
- `last_transition_time` - When status last changed (API-managed)
461461

462462
## Parameter Restrictions

pkg/services/CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ func NewClusterService(dao, adapterStatusDao, config) ClusterService
2525
- **Available**: True if all required adapters report `Available=True` (any generation)
2626
- **Ready**: True if all adapters report `Available=True` AND `observed_generation` matches current generation
2727

28+
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.
29+
2830
`ProcessAdapterStatus()` validates mandatory conditions (`Available`, `Applied`, `Health`) before persisting. Rejects `Available=Unknown` on subsequent reports (only allowed on first report).
2931

3032
## GenericService

pkg/services/cluster_test.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -787,10 +787,12 @@ func TestClusterSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
787787
Expect(createdReady).ToNot(BeNil())
788788
Expect(createdAvailable.CreatedTime).To(Equal(fixedNow))
789789
Expect(createdAvailable.LastTransitionTime).To(Equal(fixedNow))
790-
Expect(createdAvailable.LastUpdatedTime).To(Equal(fixedNow))
790+
// Available.LastUpdatedTime is always refreshed to the evaluation time (not preserved from DB).
791+
Expect(createdAvailable.LastUpdatedTime).To(BeTemporally(">", fixedNow))
791792
Expect(createdReady.CreatedTime).To(Equal(fixedNow))
792793
Expect(createdReady.LastTransitionTime).To(Equal(fixedNow))
793-
Expect(createdReady.LastUpdatedTime).To(Equal(fixedNow))
794+
// Ready.LastUpdatedTime is always refreshed to the evaluation time when isReady=false.
795+
Expect(createdReady.LastUpdatedTime).To(BeTemporally(">", fixedNow))
794796

795797
updated, err := service.UpdateClusterStatusFromAdapters(ctx, clusterID)
796798
Expect(err).To(BeNil())
@@ -812,10 +814,13 @@ func TestClusterSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
812814
Expect(updatedReady).ToNot(BeNil())
813815
Expect(updatedAvailable.CreatedTime).To(Equal(fixedNow))
814816
Expect(updatedAvailable.LastTransitionTime).To(Equal(fixedNow))
815-
Expect(updatedAvailable.LastUpdatedTime).To(Equal(fixedNow))
817+
// Available.LastUpdatedTime is always refreshed to the evaluation time (not preserved from DB).
818+
Expect(updatedAvailable.LastUpdatedTime).To(BeTemporally(">", fixedNow))
816819
Expect(updatedReady.CreatedTime).To(Equal(fixedNow))
817820
Expect(updatedReady.LastTransitionTime).To(Equal(fixedNow))
818-
Expect(updatedReady.LastUpdatedTime).To(Equal(fixedNow))
821+
// Ready.LastUpdatedTime is always refreshed to the evaluation time when isReady=false,
822+
// so Sentinel can detect stale conditions via its freshness threshold.
823+
Expect(updatedReady.LastUpdatedTime).To(BeTemporally(">", fixedNow))
819824
}
820825

821826
// TestProcessAdapterStatus_MissingMandatoryCondition_Available tests that updates missing Available are rejected

pkg/services/node_pool_test.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -647,10 +647,12 @@ func TestNodePoolSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
647647
Expect(createdReady).ToNot(BeNil())
648648
Expect(createdAvailable.CreatedTime).To(Equal(fixedNow))
649649
Expect(createdAvailable.LastTransitionTime).To(Equal(fixedNow))
650-
Expect(createdAvailable.LastUpdatedTime).To(Equal(fixedNow))
650+
// Available.LastUpdatedTime is always refreshed to the evaluation time (not preserved from DB).
651+
Expect(createdAvailable.LastUpdatedTime).To(BeTemporally(">", fixedNow))
651652
Expect(createdReady.CreatedTime).To(Equal(fixedNow))
652653
Expect(createdReady.LastTransitionTime).To(Equal(fixedNow))
653-
Expect(createdReady.LastUpdatedTime).To(Equal(fixedNow))
654+
// Ready.LastUpdatedTime is always refreshed to the evaluation time when isReady=false.
655+
Expect(createdReady.LastUpdatedTime).To(BeTemporally(">", fixedNow))
654656

655657
updated, err := service.UpdateNodePoolStatusFromAdapters(ctx, nodePoolID)
656658
Expect(err).To(BeNil())
@@ -672,8 +674,11 @@ func TestNodePoolSyntheticTimestampsStableWithoutAdapterStatus(t *testing.T) {
672674
Expect(updatedReady).ToNot(BeNil())
673675
Expect(updatedAvailable.CreatedTime).To(Equal(fixedNow))
674676
Expect(updatedAvailable.LastTransitionTime).To(Equal(fixedNow))
675-
Expect(updatedAvailable.LastUpdatedTime).To(Equal(fixedNow))
677+
// Available.LastUpdatedTime is always refreshed to the evaluation time (not preserved from DB).
678+
Expect(updatedAvailable.LastUpdatedTime).To(BeTemporally(">", fixedNow))
676679
Expect(updatedReady.CreatedTime).To(Equal(fixedNow))
677680
Expect(updatedReady.LastTransitionTime).To(Equal(fixedNow))
678-
Expect(updatedReady.LastUpdatedTime).To(Equal(fixedNow))
681+
// Ready.LastUpdatedTime is always refreshed to the evaluation time when isReady=false,
682+
// so Sentinel can detect stale conditions via its freshness threshold.
683+
Expect(updatedReady.LastUpdatedTime).To(BeTemporally(">", fixedNow))
679684
}

pkg/services/status_aggregation.go

Lines changed: 90 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,79 @@ func ComputeReadyCondition(
234234
return numReady == numRequired
235235
}
236236

237+
// findAdapterStatus returns the first adapter status in the list with the given adapter name, or (nil, false).
238+
func findAdapterStatus(adapterStatuses api.AdapterStatusList, adapterName string) (*api.AdapterStatus, bool) {
239+
for _, s := range adapterStatuses {
240+
if s.Adapter == adapterName {
241+
return s, true
242+
}
243+
}
244+
return nil, false
245+
}
246+
247+
// adapterConditionsHasAvailableTrue returns true if the adapter conditions JSON
248+
// contains a condition with type Available and status True.
249+
func adapterConditionsHasAvailableTrue(conditions []byte) bool {
250+
if len(conditions) == 0 {
251+
return false
252+
}
253+
var conds []struct {
254+
Type string `json:"type"`
255+
Status string `json:"status"`
256+
}
257+
if err := json.Unmarshal(conditions, &conds); err != nil {
258+
return false
259+
}
260+
for _, c := range conds {
261+
if c.Type == api.ConditionTypeAvailable && c.Status == "True" {
262+
return true
263+
}
264+
}
265+
return false
266+
}
267+
268+
// computeReadyLastUpdated returns the timestamp to use for the Ready condition's LastUpdatedTime.
269+
// When isReady is false, it returns now (Ready=False changes frequently; 10s threshold applies).
270+
// When isReady is true, it returns the minimum LastReportTime across all required adapters
271+
// that have Available=True at the current generation. Falls back to now if no timestamps found.
272+
func computeReadyLastUpdated(
273+
adapterStatuses api.AdapterStatusList,
274+
requiredAdapters []string,
275+
resourceGeneration int32,
276+
now time.Time,
277+
isReady bool,
278+
) time.Time {
279+
if !isReady {
280+
return now
281+
}
282+
283+
var minTime *time.Time
284+
for _, adapterName := range requiredAdapters {
285+
status, ok := findAdapterStatus(adapterStatuses, adapterName)
286+
if !ok {
287+
return now // safety: required adapter missing
288+
}
289+
if status.LastReportTime == nil {
290+
return now // safety: no timestamp
291+
}
292+
if status.ObservedGeneration != resourceGeneration {
293+
continue // not at current gen, skip
294+
}
295+
if !adapterConditionsHasAvailableTrue(status.Conditions) {
296+
continue
297+
}
298+
if minTime == nil || status.LastReportTime.Before(*minTime) {
299+
t := *status.LastReportTime
300+
minTime = &t
301+
}
302+
}
303+
304+
if minTime == nil {
305+
return now // safety fallback
306+
}
307+
return *minTime
308+
}
309+
237310
func BuildSyntheticConditions(
238311
existingConditionsJSON []byte,
239312
adapterStatuses api.AdapterStatusList,
@@ -271,7 +344,7 @@ func BuildSyntheticConditions(
271344
CreatedTime: now,
272345
LastUpdatedTime: now,
273346
}
274-
preserveSyntheticCondition(&availableCondition, existingAvailable, now)
347+
applyConditionHistory(&availableCondition, existingAvailable, now, now)
275348

276349
isReady := ComputeReadyCondition(adapterStatuses, requiredAdapters, resourceGeneration)
277350
readyStatus := api.ConditionFalse
@@ -286,13 +359,25 @@ func BuildSyntheticConditions(
286359
CreatedTime: now,
287360
LastUpdatedTime: now,
288361
}
289-
preserveSyntheticCondition(&readyCondition, existingReady, now)
362+
readyLastUpdated := computeReadyLastUpdated(
363+
adapterStatuses, requiredAdapters, resourceGeneration, now, isReady,
364+
)
365+
applyConditionHistory(&readyCondition, existingReady, now, readyLastUpdated)
290366

291367
return availableCondition, readyCondition
292368
}
293369

294-
func preserveSyntheticCondition(target *api.ResourceCondition, existing *api.ResourceCondition, now time.Time) {
370+
// applyConditionHistory copies stable timestamps and metadata from an existing condition.
371+
// lastUpdatedTime is used unconditionally for LastUpdatedTime — the caller is responsible
372+
// for computing the correct value (e.g. now, computeReadyLastUpdated(...)).
373+
func applyConditionHistory(
374+
target *api.ResourceCondition,
375+
existing *api.ResourceCondition,
376+
now time.Time,
377+
lastUpdatedTime time.Time,
378+
) {
295379
if existing == nil {
380+
target.LastUpdatedTime = lastUpdatedTime
296381
return
297382
}
298383

@@ -303,9 +388,7 @@ func preserveSyntheticCondition(target *api.ResourceCondition, existing *api.Res
303388
if !existing.LastTransitionTime.IsZero() {
304389
target.LastTransitionTime = existing.LastTransitionTime
305390
}
306-
if !existing.LastUpdatedTime.IsZero() {
307-
target.LastUpdatedTime = existing.LastUpdatedTime
308-
}
391+
target.LastUpdatedTime = lastUpdatedTime
309392
if target.Reason == nil && existing.Reason != nil {
310393
target.Reason = existing.Reason
311394
}
@@ -319,5 +402,5 @@ func preserveSyntheticCondition(target *api.ResourceCondition, existing *api.Res
319402
target.CreatedTime = existing.CreatedTime
320403
}
321404
target.LastTransitionTime = now
322-
target.LastUpdatedTime = now
405+
target.LastUpdatedTime = lastUpdatedTime
323406
}

pkg/services/status_aggregation_test.go

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,159 @@
11
package services
22

33
import (
4+
"encoding/json"
45
"testing"
56
"time"
67

8+
"gorm.io/datatypes"
9+
710
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/api"
811
)
912

13+
// makeConditionsJSON marshals a slice of {Type, Status} pairs into datatypes.JSON.
14+
func makeConditionsJSON(t *testing.T, conditions []struct{ Type, Status string }) datatypes.JSON {
15+
t.Helper()
16+
b, err := json.Marshal(conditions)
17+
if err != nil {
18+
t.Fatalf("failed to marshal conditions: %v", err)
19+
}
20+
return datatypes.JSON(b)
21+
}
22+
23+
// makeAdapterStatus builds an AdapterStatus with the given fields.
24+
func makeAdapterStatus(adapter string, gen int32, lastReportTime *time.Time, conditionsJSON datatypes.JSON) *api.AdapterStatus {
25+
return &api.AdapterStatus{
26+
Adapter: adapter,
27+
ObservedGeneration: gen,
28+
LastReportTime: lastReportTime,
29+
Conditions: conditionsJSON,
30+
}
31+
}
32+
33+
func ptr(t time.Time) *time.Time { return &t }
34+
35+
func TestComputeReadyLastUpdated_NotReady(t *testing.T) {
36+
now := time.Now()
37+
// When isReady=false the function must return now regardless of adapter state.
38+
result := computeReadyLastUpdated(nil, []string{"dns"}, 1, now, false)
39+
if !result.Equal(now) {
40+
t.Errorf("expected now, got %v", result)
41+
}
42+
}
43+
44+
func TestComputeReadyLastUpdated_MissingAdapter(t *testing.T) {
45+
now := time.Now()
46+
statuses := api.AdapterStatusList{
47+
makeAdapterStatus("validator", 1, ptr(now.Add(-5*time.Second)), makeConditionsJSON(t, []struct{ Type, Status string }{
48+
{api.ConditionTypeAvailable, "True"},
49+
})),
50+
}
51+
// "dns" is required but not in the list → safety fallback to now.
52+
result := computeReadyLastUpdated(statuses, []string{"validator", "dns"}, 1, now, true)
53+
if !result.Equal(now) {
54+
t.Errorf("expected now (missing adapter), got %v", result)
55+
}
56+
}
57+
58+
func TestComputeReadyLastUpdated_NilLastReportTime(t *testing.T) {
59+
now := time.Now()
60+
statuses := api.AdapterStatusList{
61+
makeAdapterStatus("dns", 1, nil, makeConditionsJSON(t, []struct{ Type, Status string }{
62+
{api.ConditionTypeAvailable, "True"},
63+
})),
64+
}
65+
result := computeReadyLastUpdated(statuses, []string{"dns"}, 1, now, true)
66+
if !result.Equal(now) {
67+
t.Errorf("expected now (nil LastReportTime), got %v", result)
68+
}
69+
}
70+
71+
func TestComputeReadyLastUpdated_WrongGeneration(t *testing.T) {
72+
now := time.Now()
73+
reportTime := now.Add(-10 * time.Second)
74+
statuses := api.AdapterStatusList{
75+
// ObservedGeneration=1 but resourceGeneration=2 — skipped.
76+
makeAdapterStatus("dns", 1, ptr(reportTime), makeConditionsJSON(t, []struct{ Type, Status string }{
77+
{api.ConditionTypeAvailable, "True"},
78+
})),
79+
}
80+
// All adapters skipped → minTime is nil → fallback to now.
81+
result := computeReadyLastUpdated(statuses, []string{"dns"}, 2, now, true)
82+
if !result.Equal(now) {
83+
t.Errorf("expected now (wrong generation), got %v", result)
84+
}
85+
}
86+
87+
func TestComputeReadyLastUpdated_AvailableFalse(t *testing.T) {
88+
now := time.Now()
89+
reportTime := now.Add(-10 * time.Second)
90+
statuses := api.AdapterStatusList{
91+
makeAdapterStatus("dns", 1, ptr(reportTime), makeConditionsJSON(t, []struct{ Type, Status string }{
92+
{api.ConditionTypeAvailable, "False"},
93+
})),
94+
}
95+
// Available=False → skipped → fallback to now.
96+
result := computeReadyLastUpdated(statuses, []string{"dns"}, 1, now, true)
97+
if !result.Equal(now) {
98+
t.Errorf("expected now (Available=False), got %v", result)
99+
}
100+
}
101+
102+
func TestComputeReadyLastUpdated_SingleQualifyingAdapter(t *testing.T) {
103+
now := time.Now()
104+
reportTime := now.Add(-30 * time.Second)
105+
statuses := api.AdapterStatusList{
106+
makeAdapterStatus("dns", 1, ptr(reportTime), makeConditionsJSON(t, []struct{ Type, Status string }{
107+
{api.ConditionTypeAvailable, "True"},
108+
})),
109+
}
110+
result := computeReadyLastUpdated(statuses, []string{"dns"}, 1, now, true)
111+
if !result.Equal(reportTime) {
112+
t.Errorf("expected %v, got %v", reportTime, result)
113+
}
114+
}
115+
116+
func TestComputeReadyLastUpdated_MultipleAdapters_ReturnsMinimum(t *testing.T) {
117+
now := time.Now()
118+
older := now.Add(-60 * time.Second)
119+
newer := now.Add(-10 * time.Second)
120+
121+
statuses := api.AdapterStatusList{
122+
makeAdapterStatus("validator", 2, ptr(newer), makeConditionsJSON(t, []struct{ Type, Status string }{
123+
{api.ConditionTypeAvailable, "True"},
124+
})),
125+
makeAdapterStatus("dns", 2, ptr(older), makeConditionsJSON(t, []struct{ Type, Status string }{
126+
{api.ConditionTypeAvailable, "True"},
127+
})),
128+
}
129+
result := computeReadyLastUpdated(statuses, []string{"validator", "dns"}, 2, now, true)
130+
if !result.Equal(older) {
131+
t.Errorf("expected minimum timestamp %v, got %v", older, result)
132+
}
133+
}
134+
135+
func TestComputeReadyLastUpdated_MixedGenerations_OnlyCurrentGenCounts(t *testing.T) {
136+
now := time.Now()
137+
oldGenTime := now.Add(-120 * time.Second)
138+
newGenTime := now.Add(-5 * time.Second)
139+
140+
statuses := api.AdapterStatusList{
141+
// validator is at gen 3 (current) — qualifies.
142+
makeAdapterStatus("validator", 3, ptr(newGenTime), makeConditionsJSON(t, []struct{ Type, Status string }{
143+
{api.ConditionTypeAvailable, "True"},
144+
})),
145+
// dns is at gen 2 (stale) — skipped.
146+
makeAdapterStatus("dns", 2, ptr(oldGenTime), makeConditionsJSON(t, []struct{ Type, Status string }{
147+
{api.ConditionTypeAvailable, "True"},
148+
})),
149+
}
150+
// Only validator qualifies; dns is skipped (wrong gen), so minTime = newGenTime.
151+
result := computeReadyLastUpdated(statuses, []string{"validator", "dns"}, 3, now, true)
152+
if !result.Equal(newGenTime) {
153+
t.Errorf("expected %v (only current-gen adapter), got %v", newGenTime, result)
154+
}
155+
}
156+
10157
func TestMapAdapterToConditionType(t *testing.T) {
11158
tests := []struct {
12159
adapter string

0 commit comments

Comments
 (0)