Skip to content

Commit 0f274fc

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

File tree

4 files changed

+250
-15
lines changed

4 files changed

+250
-15
lines changed

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: 85 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,74 @@ func ComputeReadyCondition(
234234
return numReady == numRequired
235235
}
236236

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

276344
isReady := ComputeReadyCondition(adapterStatuses, requiredAdapters, resourceGeneration)
277345
readyStatus := api.ConditionFalse
@@ -286,13 +354,25 @@ func BuildSyntheticConditions(
286354
CreatedTime: now,
287355
LastUpdatedTime: now,
288356
}
289-
preserveSyntheticCondition(&readyCondition, existingReady, now)
357+
readyLastUpdated := computeReadyLastUpdated(
358+
adapterStatuses, requiredAdapters, resourceGeneration, now, isReady,
359+
)
360+
applyConditionHistory(&readyCondition, existingReady, now, readyLastUpdated)
290361

291362
return availableCondition, readyCondition
292363
}
293364

294-
func preserveSyntheticCondition(target *api.ResourceCondition, existing *api.ResourceCondition, now time.Time) {
365+
// applyConditionHistory copies stable timestamps and metadata from an existing condition.
366+
// lastUpdatedTime is used unconditionally for LastUpdatedTime — the caller is responsible
367+
// for computing the correct value (e.g. now, computeReadyLastUpdated(...)).
368+
func applyConditionHistory(
369+
target *api.ResourceCondition,
370+
existing *api.ResourceCondition,
371+
now time.Time,
372+
lastUpdatedTime time.Time,
373+
) {
295374
if existing == nil {
375+
target.LastUpdatedTime = lastUpdatedTime
296376
return
297377
}
298378

@@ -303,9 +383,7 @@ func preserveSyntheticCondition(target *api.ResourceCondition, existing *api.Res
303383
if !existing.LastTransitionTime.IsZero() {
304384
target.LastTransitionTime = existing.LastTransitionTime
305385
}
306-
if !existing.LastUpdatedTime.IsZero() {
307-
target.LastUpdatedTime = existing.LastUpdatedTime
308-
}
386+
target.LastUpdatedTime = lastUpdatedTime
309387
if target.Reason == nil && existing.Reason != nil {
310388
target.Reason = existing.Reason
311389
}
@@ -319,5 +397,5 @@ func preserveSyntheticCondition(target *api.ResourceCondition, existing *api.Res
319397
target.CreatedTime = existing.CreatedTime
320398
}
321399
target.LastTransitionTime = now
322-
target.LastUpdatedTime = now
400+
target.LastUpdatedTime = lastUpdatedTime
323401
}

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)