Skip to content

Commit af2a3b3

Browse files
committed
Set limit on length of Status.Conditions of a csv
Set a maximum length of Status.Conditions of ClusterServiceVersion object(s). The oldest condition(s) should be removed from the list as it grows over time to keep it at limit. The default limit is set to 20. Jira: https://jira.coreos.com/browse/OLM-963
1 parent 48a1875 commit af2a3b3

File tree

2 files changed

+123
-15
lines changed

2 files changed

+123
-15
lines changed

pkg/api/apis/operators/v1alpha1/clusterserviceversion.go

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ import (
1010

1111
const (
1212
CopiedLabelKey = "olm.copiedFrom"
13+
14+
// ConditionsLengthLimit is the maximum length of Status.Conditions of a
15+
// given ClusterServiceVersion object. The oldest condition(s) are removed
16+
// from the list as it grows over time to keep it at limit.
17+
ConditionsLengthLimit = 20
1318
)
1419

1520
// obsoleteReasons are the set of reasons that mean a CSV should no longer be processed as active
@@ -67,6 +72,18 @@ func (c *ClusterServiceVersion) SetPhaseWithEvent(phase ClusterServiceVersionPha
6772

6873
// SetPhase sets the current phase and adds a condition if necessary
6974
func (c *ClusterServiceVersion) SetPhase(phase ClusterServiceVersionPhase, reason ConditionReason, message string, now metav1.Time) {
75+
newCondition := func() ClusterServiceVersionCondition {
76+
return ClusterServiceVersionCondition{
77+
Phase: c.Status.Phase,
78+
LastTransitionTime: c.Status.LastTransitionTime,
79+
LastUpdateTime: c.Status.LastUpdateTime,
80+
Message: message,
81+
Reason: reason,
82+
}
83+
}
84+
85+
defer c.TrimConditionsIfLimitExceeded()
86+
7087
c.Status.LastUpdateTime = now
7188
if c.Status.Phase != phase {
7289
c.Status.Phase = phase
@@ -75,23 +92,13 @@ func (c *ClusterServiceVersion) SetPhase(phase ClusterServiceVersionPhase, reaso
7592
c.Status.Message = message
7693
c.Status.Reason = reason
7794
if len(c.Status.Conditions) == 0 {
78-
c.Status.Conditions = append(c.Status.Conditions, ClusterServiceVersionCondition{
79-
Phase: c.Status.Phase,
80-
LastTransitionTime: c.Status.LastTransitionTime,
81-
LastUpdateTime: c.Status.LastUpdateTime,
82-
Message: message,
83-
Reason: reason,
84-
})
95+
c.Status.Conditions = append(c.Status.Conditions, newCondition())
96+
return
8597
}
98+
8699
previousCondition := c.Status.Conditions[len(c.Status.Conditions)-1]
87100
if previousCondition.Phase != c.Status.Phase || previousCondition.Reason != c.Status.Reason {
88-
c.Status.Conditions = append(c.Status.Conditions, ClusterServiceVersionCondition{
89-
Phase: c.Status.Phase,
90-
LastTransitionTime: c.Status.LastTransitionTime,
91-
LastUpdateTime: c.Status.LastUpdateTime,
92-
Message: message,
93-
Reason: reason,
94-
})
101+
c.Status.Conditions = append(c.Status.Conditions, newCondition())
95102
}
96103
}
97104

@@ -190,3 +197,12 @@ func (set InstallModeSet) Supports(operatorNamespace string, namespaces []string
190197

191198
return nil
192199
}
200+
201+
func (c *ClusterServiceVersion) TrimConditionsIfLimitExceeded() {
202+
if len(c.Status.Conditions) <= ConditionsLengthLimit {
203+
return
204+
}
205+
206+
firstIndex := len(c.Status.Conditions) - ConditionsLengthLimit
207+
c.Status.Conditions = c.Status.Conditions[firstIndex:len(c.Status.Conditions)]
208+
}

pkg/api/apis/operators/v1alpha1/clusterserviceversion_test.go

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import (
44
"fmt"
55
"testing"
66

7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
9+
"github.com/stretchr/testify/assert"
710
"github.com/stretchr/testify/require"
811
corev1 "k8s.io/api/core/v1"
9-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1012
)
1113

1214
func TestSetRequirementStatus(t *testing.T) {
@@ -282,3 +284,93 @@ func TestSupports(t *testing.T) {
282284
})
283285
}
284286
}
287+
288+
func TestSetPhaseWithConditions(t *testing.T) {
289+
tests := []struct {
290+
description string
291+
limit int
292+
currentLength int
293+
startIndex int
294+
}{
295+
{
296+
// The original list is already at limit (length == limit).
297+
// We expect the oldest element ( item at 0 index) to be removed.
298+
description: "TestSetPhaseWithConditionsLengthAtLimit",
299+
limit: ConditionsLengthLimit,
300+
currentLength: ConditionsLengthLimit,
301+
302+
// The first element from the original list should be dropped from
303+
// the new list.
304+
startIndex: 1,
305+
},
306+
{
307+
// The original list is 1 length away from limit.
308+
// We don't expect the list to be trimmed.
309+
description: "TestSetPhaseWithConditionsLengthBelowLimit",
310+
limit: ConditionsLengthLimit,
311+
currentLength: ConditionsLengthLimit - 1,
312+
313+
// Everything in the original list should be preserved.
314+
startIndex: 0,
315+
},
316+
{
317+
// The original list has N more element(s) than allowed limit.
318+
// We expect (N + 1) oldest elements to be deleted to keep the list
319+
// at limit.
320+
description: "TestSetPhaseWithConditionsLimitExceeded",
321+
limit: ConditionsLengthLimit,
322+
currentLength: ConditionsLengthLimit + 10,
323+
324+
// The first 11 (N=10 plus 1 to make room for the newly added
325+
// condition) elements from the original list should be dropped.
326+
startIndex: 11,
327+
},
328+
}
329+
330+
for _, tt := range tests {
331+
t.Run(tt.description, func(t *testing.T) {
332+
csv := ClusterServiceVersion{}
333+
csv.Status.Conditions = helperNewConditions(tt.currentLength)
334+
335+
now := metav1.Now()
336+
337+
oldConditionsWant := csv.Status.Conditions[tt.startIndex:]
338+
lastAddedConditionWant := ClusterServiceVersionCondition{
339+
Phase: ClusterServiceVersionPhase("Pending"),
340+
LastTransitionTime: now,
341+
LastUpdateTime: now,
342+
Message: "message",
343+
Reason: ConditionReason("reason"),
344+
}
345+
346+
csv.SetPhase("Pending", "reason", "message", now)
347+
348+
conditionsGot := csv.Status.Conditions
349+
assert.Equal(t, tt.limit, len(conditionsGot))
350+
351+
oldConditionsGot := conditionsGot[0 : len(conditionsGot)-1]
352+
assert.EqualValues(t, oldConditionsWant, oldConditionsGot)
353+
354+
lastAddedConditionGot := conditionsGot[len(conditionsGot)-1]
355+
assert.Equal(t, lastAddedConditionWant, lastAddedConditionGot)
356+
})
357+
}
358+
}
359+
360+
func helperNewConditions(count int) []ClusterServiceVersionCondition {
361+
conditions := make([]ClusterServiceVersionCondition, 0)
362+
363+
for i := 1; i <= count; i++ {
364+
now := metav1.Now()
365+
condition := ClusterServiceVersionCondition{
366+
Phase: ClusterServiceVersionPhase(fmt.Sprintf("phase-%d", i)),
367+
LastTransitionTime: now,
368+
LastUpdateTime: now,
369+
Message: fmt.Sprintf("message-%d", i),
370+
Reason: ConditionReason(fmt.Sprintf("reason-%d", i)),
371+
}
372+
conditions = append(conditions, condition)
373+
}
374+
375+
return conditions
376+
}

0 commit comments

Comments
 (0)