Skip to content

Commit 4942d9b

Browse files
DiaphteirosDiaphteiros
committed
add more tests for status updater and fix bugs
1 parent 2718c8f commit 4942d9b

File tree

4 files changed

+169
-15
lines changed

4 files changed

+169
-15
lines changed

‎pkg/controller/status_updater.go‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -215,32 +215,42 @@ func (s *statusUpdater[Obj, PhType, ConType]) UpdateStatus(ctx context.Context,
215215
SetField(status, s.fieldNames[STATUS_FIELD_REASON], reason)
216216
}
217217
if s.fieldNames[STATUS_FIELD_CONDITIONS] != "" && s.conConstruct != nil {
218+
conType := reflect.TypeOf(s.conConstruct())
219+
targetType := reflect.TypeOf(GetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], false))
220+
pointerMagic := false
221+
if !reflect.SliceOf(conType).AssignableTo(targetType) {
222+
// this can happen if the constructor returns a *ConditionImplementation,
223+
// but the condition list field is a []ConditionImplementation
224+
if conType.Kind() == reflect.Ptr {
225+
pointerMagic = true
226+
conType = conType.Elem()
227+
}
228+
}
218229
oldConsRaw := GetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], false)
219230
var oldCons []conditions.Condition[ConType]
220231
if !IsNil(oldConsRaw) {
221-
oldCons = oldConsRaw.([]conditions.Condition[ConType])
232+
val := reflect.ValueOf(oldConsRaw)
233+
oldCons := make([]conditions.Condition[ConType], val.Len())
234+
for i := 0; i < val.Len(); i++ {
235+
eVal := val.Index(i)
236+
if pointerMagic {
237+
ptrValue := reflect.New(conType)
238+
reflect.Indirect(ptrValue).Set(eVal)
239+
eVal = ptrValue
240+
}
241+
oldCons[i] = eVal.Interface().(conditions.Condition[ConType])
242+
}
222243
}
223244
cu := conditions.ConditionUpdater(s.conConstruct, oldCons, s.removeUntouchedConditions)
224245
cu.Now = now
225246
for _, con := range rr.Conditions {
226247
cu.UpdateConditionFromTemplate(con)
227248
}
228249
newConsRaw, _ := cu.Conditions()
229-
conType := reflect.TypeOf(s.conConstruct())
230-
targetType := reflect.TypeOf(GetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], false))
231-
depointerize := false
232-
if !reflect.SliceOf(conType).AssignableTo(targetType) {
233-
// this can happen if the constructor returns a *ConditionImplementation, which creates a []*ConditionImplementation below,
234-
// but the condition list field is a []ConditionImplementation
235-
if conType.Kind() == reflect.Ptr {
236-
depointerize = true
237-
conType = conType.Elem()
238-
}
239-
}
240250
newCons := reflect.MakeSlice(reflect.SliceOf(conType), len(newConsRaw), len(newConsRaw)).Interface()
241251
for i := range newConsRaw {
242252
val := reflect.ValueOf(newConsRaw[i])
243-
if depointerize {
253+
if pointerMagic {
244254
val = val.Elem()
245255
}
246256
reflect.ValueOf(newCons).Index(i).Set(val.Convert(conType))

‎pkg/controller/status_updater_test.go‎

Lines changed: 115 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
. "github.com/onsi/ginkgo/v2"
99
. "github.com/onsi/gomega"
10+
1011
. "github.com/openmcp-project/controller-utils/pkg/testing/matchers"
1112

1213
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -28,7 +29,7 @@ var _ = Describe("Status Updater", func() {
2829
It("should update an empty status", func() {
2930
env := testutils.NewEnvironmentBuilder().WithFakeClient(coScheme).WithInitObjectPath("testdata", "test-02").WithDynamicObjectsWithStatus(&CustomObject{}).Build()
3031
obj := &CustomObject{}
31-
Expect(env.Client().Get(env.Ctx, testutils.ObjectKey("nostatus", "default"), obj)).To(Succeed())
32+
Expect(env.Client().Get(env.Ctx, controller.ObjectKey("nostatus", "default"), obj)).To(Succeed())
3233
rr := controller.ReconcileResult[*CustomObject, ConditionStatus]{
3334
Object: obj,
3435
ReconcileError: errors.WithReason(fmt.Errorf("test error"), "TestError"),
@@ -63,6 +64,119 @@ var _ = Describe("Status Updater", func() {
6364
))
6465
})
6566

67+
It("should update an existing status", func() {
68+
env := testutils.NewEnvironmentBuilder().WithFakeClient(coScheme).WithInitObjectPath("testdata", "test-02").WithDynamicObjectsWithStatus(&CustomObject{}).Build()
69+
obj := &CustomObject{}
70+
Expect(env.Client().Get(env.Ctx, controller.ObjectKey("status", "default"), obj)).To(Succeed())
71+
rr := controller.ReconcileResult[*CustomObject, ConditionStatus]{
72+
Object: obj,
73+
Conditions: dummyConditions(),
74+
}
75+
su := preconfiguredStatusUpdaterBuilder().WithPhaseUpdateFunc(func(obj *CustomObject, rr controller.ReconcileResult[*CustomObject, ConditionStatus]) (CustomObjectPhase, error) {
76+
return PhaseSucceeded, nil
77+
}).Build()
78+
now := time.Now()
79+
res, err := su.UpdateStatus(env.Ctx, env.Client(), rr)
80+
Expect(res).To(Equal(rr.Result))
81+
Expect(err).ToNot(HaveOccurred())
82+
Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(obj), obj)).To(Succeed())
83+
84+
Expect(obj.Status.Phase).To(Equal(PhaseSucceeded))
85+
Expect(obj.Status.ObservedGeneration).To(Equal(obj.GetGeneration()))
86+
Expect(obj.Status.Reason).To(BeEmpty())
87+
Expect(obj.Status.Message).To(BeEmpty())
88+
Expect(obj.Status.LastReconcileTime.Time).To(BeTemporally("~", now, 1*time.Second))
89+
Expect(obj.Status.Conditions).To(ConsistOf(
90+
MatchCondition(NewConditionImpl[ConditionStatus]().
91+
WithType("TestConditionTrue").
92+
WithStatus(ConditionStatusTrue).
93+
WithReason("TestReasonTrue").
94+
WithMessage("TestMessageTrue").
95+
WithLastTransitionTime(obj.Status.LastReconcileTime.Time)),
96+
MatchCondition(NewConditionImpl[ConditionStatus]().
97+
WithType("TestConditionFalse").
98+
WithStatus(ConditionStatusFalse).
99+
WithReason("TestReasonFalse").
100+
WithMessage("TestMessageFalse").
101+
WithLastTransitionTime(obj.Status.LastReconcileTime.Time)),
102+
))
103+
})
104+
105+
It("should not update disabled fields", func() {
106+
env := testutils.NewEnvironmentBuilder().WithFakeClient(coScheme).WithInitObjectPath("testdata", "test-02").WithDynamicObjectsWithStatus(&CustomObject{}).Build()
107+
obj := &CustomObject{}
108+
Expect(env.Client().Get(env.Ctx, controller.ObjectKey("status", "default"), obj)).To(Succeed())
109+
before := obj.DeepCopy()
110+
for _, disabledField := range controller.AllStatusFields() {
111+
By(fmt.Sprintf("Testing disabled field %s", disabledField))
112+
// reset object to remove changes from previous loop executions
113+
modified := obj.DeepCopy()
114+
obj.Status = before.Status
115+
Expect(env.Client().Status().Patch(env.Ctx, obj, client.MergeFrom(modified))).To(Succeed())
116+
rr := controller.ReconcileResult[*CustomObject, ConditionStatus]{
117+
Object: obj,
118+
Conditions: dummyConditions(),
119+
Reason: "TestReason",
120+
Message: "TestMessage",
121+
}
122+
su := preconfiguredStatusUpdaterBuilder().WithPhaseUpdateFunc(func(obj *CustomObject, rr controller.ReconcileResult[*CustomObject, ConditionStatus]) (CustomObjectPhase, error) {
123+
return PhaseSucceeded, nil
124+
}).WithoutFields(disabledField).Build()
125+
now := time.Now()
126+
res, err := su.UpdateStatus(env.Ctx, env.Client(), rr)
127+
Expect(res).To(Equal(rr.Result))
128+
129+
Expect(err).ToNot(HaveOccurred())
130+
Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(obj), obj)).To(Succeed())
131+
132+
if disabledField == controller.STATUS_FIELD_PHASE {
133+
Expect(obj.Status.Phase).To(Equal(before.Status.Phase))
134+
} else {
135+
Expect(obj.Status.Phase).To(Equal(PhaseSucceeded))
136+
}
137+
if disabledField == controller.STATUS_FIELD_OBSERVED_GENERATION {
138+
Expect(obj.Status.ObservedGeneration).To(Equal(before.Status.ObservedGeneration))
139+
} else {
140+
Expect(obj.Status.ObservedGeneration).To(Equal(obj.GetGeneration()))
141+
}
142+
if disabledField == controller.STATUS_FIELD_REASON {
143+
Expect(obj.Status.Reason).To(Equal(before.Status.Reason))
144+
} else {
145+
Expect(obj.Status.Reason).To(Equal(rr.Reason))
146+
}
147+
if disabledField == controller.STATUS_FIELD_MESSAGE {
148+
Expect(obj.Status.Message).To(Equal(before.Status.Message))
149+
} else {
150+
Expect(obj.Status.Message).To(Equal(rr.Message))
151+
}
152+
if disabledField == controller.STATUS_FIELD_LAST_RECONCILE_TIME {
153+
Expect(obj.Status.LastReconcileTime.Time).To(Equal(before.Status.LastReconcileTime.Time))
154+
} else {
155+
Expect(obj.Status.LastReconcileTime.Time).To(BeTemporally("~", now, 1*time.Second))
156+
}
157+
if disabledField == controller.STATUS_FIELD_CONDITIONS {
158+
Expect(obj.Status.Conditions).To(Equal(before.Status.Conditions))
159+
} else {
160+
Expect(obj.Status.Conditions).To(ConsistOf(
161+
MatchCondition(NewConditionImpl[ConditionStatus]().
162+
WithType("TestConditionTrue").
163+
WithStatus(ConditionStatusTrue).
164+
WithReason("TestReasonTrue").
165+
WithMessage("TestMessageTrue").
166+
WithLastTransitionTime(now).
167+
WithTimestampTolerance(1*time.Second)),
168+
MatchCondition(NewConditionImpl[ConditionStatus]().
169+
WithType("TestConditionFalse").
170+
WithStatus(ConditionStatusFalse).
171+
WithReason("TestReasonFalse").
172+
WithMessage("TestMessageFalse").
173+
WithLastTransitionTime(now).
174+
WithTimestampTolerance(1*time.Second)),
175+
))
176+
}
177+
}
178+
})
179+
66180
})
67181

68182
func preconfiguredStatusUpdaterBuilder() *controller.StatusUpdaterBuilder[*CustomObject, CustomObjectPhase, ConditionStatus] {

‎pkg/controller/testdata/test-02/co_status.yaml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
apiVersion: testing.openmcp.cloud/v1alpha1
2+
kind: CustomObject
3+
metadata:
4+
name: status
5+
namespace: default
6+
spec: {}
7+
status:
8+
conditions:
9+
- type: "TestConditionTrue"
10+
status: "True"
11+
lastTransitionTime: "2023-10-01T00:00:00Z"
12+
- type: "TestConditionFalse"
13+
status: "Unknown"
14+
lastTransitionTime: "2023-10-01T00:00:00Z"
15+
- type: "AdditionalCondition"
16+
status: "True"
17+
lastTransitionTime: "2023-10-01T00:00:00Z"
18+
observedGeneration: 0
19+
phase: "Failed"
20+
reason: "OldReason"
21+
message: "This is the old message"
22+
lastReconcileTime: "2023-10-01T00:00:00Z"
23+

‎pkg/testing/matchers/conditions.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"time"
77

88
"github.com/onsi/gomega/types"
9+
910
"github.com/openmcp-project/controller-utils/pkg/conditions"
1011
)
1112

@@ -59,7 +60,7 @@ func (c *conditionMatcher[T]) Match(actualRaw any) (success bool, err error) {
5960
if c.expected.HasMessage() && c.expected.GetMessage() != actual.GetMessage() {
6061
return false, nil
6162
}
62-
if c.expected.HasLastTransitionTime() && !c.expected.GetLastTransitionTime().Equal(actual.GetLastTransitionTime()) {
63+
if c.expected.HasLastTransitionTime() && c.expected.GetLastTransitionTime().Sub(actual.GetLastTransitionTime()) > c.expected.timestampTolerance {
6364
return false, nil
6465
}
6566
return true, nil
@@ -103,6 +104,7 @@ type ConditionImpl[T comparable] struct {
103104
reason *string
104105
message *string
105106
lastTransitionTime *time.Time
107+
timestampTolerance time.Duration
106108
}
107109

108110
func (c *ConditionImpl[T]) String() string {
@@ -225,6 +227,11 @@ func (c *ConditionImpl[T]) WithMessage(message string) *ConditionImpl[T] {
225227
return c
226228
}
227229

230+
func (c *ConditionImpl[T]) WithTimestampTolerance(t time.Duration) *ConditionImpl[T] {
231+
c.timestampTolerance = t
232+
return c
233+
}
234+
228235
func Ptr[T any](v T) *T {
229236
return &v
230237
}

0 commit comments

Comments
 (0)