Skip to content

Commit 46cb7c2

Browse files
ansdansd
committed
Fix endless reconcile issue
Fixes #250 Before this commit, the LastTransitionTime was updated in every reconcile. This is wrong because LastTransitionTime should only be updated if the status transitions from one to the other. This led to endless reconciliations because updating the status leads to a new resourceVersion resulting in a new reoncile. With sufficient objects (>100), it takes more than one second between reconciling the same object. Since the granularity of lastTransitionTime is 1 second, this ended up in endless updates of lastTranistionTime and therefore resourceVersion.
1 parent 1431754 commit 46cb7c2

24 files changed

+184
-55
lines changed

‎api/v1beta1/conditions.go‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ type Condition struct {
1414
Type ConditionType `json:"type"`
1515
// True, False, or Unknown
1616
Status corev1.ConditionStatus `json:"status"`
17-
// The last time this Condition type changed.
17+
// The last time this Condition status changed.
1818
LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
1919
// One word, camel-case reason for current status of the condition.
2020
Reason string `json:"reason,omitempty"`
@@ -23,22 +23,33 @@ type Condition struct {
2323
}
2424

2525
// Ready indicates that the last Create/Update operator on the CR was successful.
26-
func Ready() Condition {
26+
func Ready(lastConditions []Condition) Condition {
27+
time := lastTransitionTime(corev1.ConditionTrue, lastConditions)
2728
return Condition{
2829
Type: ready,
2930
Status: corev1.ConditionTrue,
30-
LastTransitionTime: metav1.Now(),
31+
LastTransitionTime: time,
3132
Reason: "SuccessfulCreateOrUpdate",
3233
}
3334
}
3435

3536
// NotReady indicates that the last Create/Update operator on the CR failed.
36-
func NotReady(msg string) Condition {
37+
func NotReady(msg string, lastConditions []Condition) Condition {
38+
time := lastTransitionTime(corev1.ConditionFalse, lastConditions)
3739
return Condition{
3840
Type: ready,
3941
Status: corev1.ConditionFalse,
40-
LastTransitionTime: metav1.Now(),
42+
LastTransitionTime: time,
4143
Reason: "FailedCreateOrUpdate",
4244
Message: msg,
4345
}
4446
}
47+
48+
func lastTransitionTime(newStatus corev1.ConditionStatus, lastConditions []Condition) metav1.Time {
49+
for _, lastCondition := range lastConditions {
50+
if lastCondition.Type == ready && lastCondition.Status == newStatus {
51+
return lastCondition.LastTransitionTime
52+
}
53+
}
54+
return metav1.Now()
55+
}

‎api/v1beta1/conditions_test.go‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,39 @@ import (
44
. "github.com/onsi/ginkgo"
55
. "github.com/onsi/gomega"
66
corev1 "k8s.io/api/core/v1"
7-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
87
)
98

109
var _ = Describe("Conditions", func() {
11-
Context("Ready", func() {
10+
Describe("Ready", func() {
1211
It("returns 'Ready' condition set to true", func() {
13-
c := Ready()
12+
c := Ready(nil)
1413
Expect(string(c.Type)).To(Equal("Ready"))
1514
Expect(c.Status).To(Equal(corev1.ConditionTrue))
1615
Expect(c.Reason).To(Equal("SuccessfulCreateOrUpdate"))
17-
Expect(c.LastTransitionTime).NotTo(Equal(metav1.Time{})) // has been set; not empty
16+
Expect(c.LastTransitionTime.IsZero()).To(BeFalse())
1817
})
19-
2018
})
21-
22-
Context("NotReady", func() {
19+
Describe("NotReady", func() {
2320
It("returns 'Ready' condition set to false", func() {
24-
c := NotReady("fail to declare queue")
21+
c := NotReady("fail to declare queue", nil)
2522
Expect(string(c.Type)).To(Equal("Ready"))
2623
Expect(c.Status).To(Equal(corev1.ConditionFalse))
2724
Expect(c.Reason).To(Equal("FailedCreateOrUpdate"))
2825
Expect(c.Message).To(Equal("fail to declare queue"))
29-
Expect(c.LastTransitionTime).NotTo(Equal(metav1.Time{})) // has been set; not empty
26+
Expect(c.LastTransitionTime.IsZero()).To(BeFalse())
27+
})
28+
})
29+
Context("LastTransitionTime", func() {
30+
It("changes only if status changes", func() {
31+
c1 := Ready(nil)
32+
Expect(c1.LastTransitionTime.IsZero()).To(BeFalse())
33+
c2 := Ready([]Condition{
34+
Condition{Type: "I'm some other type"},
35+
c1,
36+
})
37+
Expect(c2.LastTransitionTime.Time).To(BeTemporally("==", c1.LastTransitionTime.Time))
38+
c3 := NotReady("some message", []Condition{c2})
39+
Expect(c3.LastTransitionTime.Time).To(BeTemporally(">", c2.LastTransitionTime.Time))
3040
})
3141
})
3242
})

‎config/crd/bases/rabbitmq.com_bindings.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ spec:
8585
items:
8686
properties:
8787
lastTransitionTime:
88-
description: The last time this Condition type changed.
88+
description: The last time this Condition status changed.
8989
format: date-time
9090
type: string
9191
message:

‎config/crd/bases/rabbitmq.com_exchanges.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ spec:
8383
items:
8484
properties:
8585
lastTransitionTime:
86-
description: The last time this Condition type changed.
86+
description: The last time this Condition status changed.
8787
format: date-time
8888
type: string
8989
message:

‎config/crd/bases/rabbitmq.com_federations.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ spec:
104104
items:
105105
properties:
106106
lastTransitionTime:
107-
description: The last time this Condition type changed.
107+
description: The last time this Condition status changed.
108108
format: date-time
109109
type: string
110110
message:

‎config/crd/bases/rabbitmq.com_permissions.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ spec:
9393
items:
9494
properties:
9595
lastTransitionTime:
96-
description: The last time this Condition type changed.
96+
description: The last time this Condition status changed.
9797
format: date-time
9898
type: string
9999
message:

‎config/crd/bases/rabbitmq.com_policies.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ spec:
9595
items:
9696
properties:
9797
lastTransitionTime:
98-
description: The last time this Condition type changed.
98+
description: The last time this Condition status changed.
9999
format: date-time
100100
type: string
101101
message:

‎config/crd/bases/rabbitmq.com_queues.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ spec:
8686
items:
8787
properties:
8888
lastTransitionTime:
89-
description: The last time this Condition type changed.
89+
description: The last time this Condition status changed.
9090
format: date-time
9191
type: string
9292
message:

‎config/crd/bases/rabbitmq.com_schemareplications.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ spec:
7878
items:
7979
properties:
8080
lastTransitionTime:
81-
description: The last time this Condition type changed.
81+
description: The last time this Condition status changed.
8282
format: date-time
8383
type: string
8484
message:

‎config/crd/bases/rabbitmq.com_shovels.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ spec:
131131
items:
132132
properties:
133133
lastTransitionTime:
134-
description: The last time this Condition type changed.
134+
description: The last time this Condition status changed.
135135
format: date-time
136136
type: string
137137
message:

0 commit comments

Comments
 (0)