From c4f83e1ec187a68072cf9f8779a36ce2d607c95f Mon Sep 17 00:00:00 2001 From: Abu Kashem Date: Wed, 1 May 2019 08:46:49 -0400 Subject: [PATCH] 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 --- .../v1alpha1/clusterserviceversion.go | 44 ++++--- .../v1alpha1/clusterserviceversion_test.go | 112 +++++++++++++++++- 2 files changed, 141 insertions(+), 15 deletions(-) diff --git a/pkg/api/apis/operators/v1alpha1/clusterserviceversion.go b/pkg/api/apis/operators/v1alpha1/clusterserviceversion.go index 6819ff5ef20..9dd6f56f8da 100644 --- a/pkg/api/apis/operators/v1alpha1/clusterserviceversion.go +++ b/pkg/api/apis/operators/v1alpha1/clusterserviceversion.go @@ -10,6 +10,11 @@ import ( const ( CopiedLabelKey = "olm.copiedFrom" + + // ConditionsLengthLimit is the maximum length of Status.Conditions of a + // given ClusterServiceVersion object. The oldest condition(s) are removed + // from the list as it grows over time to keep it at limit. + ConditionsLengthLimit = 20 ) // 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 // SetPhase sets the current phase and adds a condition if necessary func (c *ClusterServiceVersion) SetPhase(phase ClusterServiceVersionPhase, reason ConditionReason, message string, now metav1.Time) { + newCondition := func() ClusterServiceVersionCondition { + return ClusterServiceVersionCondition{ + Phase: c.Status.Phase, + LastTransitionTime: c.Status.LastTransitionTime, + LastUpdateTime: c.Status.LastUpdateTime, + Message: message, + Reason: reason, + } + } + + defer c.TrimConditionsIfLimitExceeded() + c.Status.LastUpdateTime = now if c.Status.Phase != phase { c.Status.Phase = phase @@ -75,23 +92,13 @@ func (c *ClusterServiceVersion) SetPhase(phase ClusterServiceVersionPhase, reaso c.Status.Message = message c.Status.Reason = reason if len(c.Status.Conditions) == 0 { - c.Status.Conditions = append(c.Status.Conditions, ClusterServiceVersionCondition{ - Phase: c.Status.Phase, - LastTransitionTime: c.Status.LastTransitionTime, - LastUpdateTime: c.Status.LastUpdateTime, - Message: message, - Reason: reason, - }) + c.Status.Conditions = append(c.Status.Conditions, newCondition()) + return } + previousCondition := c.Status.Conditions[len(c.Status.Conditions)-1] if previousCondition.Phase != c.Status.Phase || previousCondition.Reason != c.Status.Reason { - c.Status.Conditions = append(c.Status.Conditions, ClusterServiceVersionCondition{ - Phase: c.Status.Phase, - LastTransitionTime: c.Status.LastTransitionTime, - LastUpdateTime: c.Status.LastUpdateTime, - Message: message, - Reason: reason, - }) + c.Status.Conditions = append(c.Status.Conditions, newCondition()) } } @@ -190,3 +197,12 @@ func (set InstallModeSet) Supports(operatorNamespace string, namespaces []string return nil } + +func (c *ClusterServiceVersion) TrimConditionsIfLimitExceeded() { + if len(c.Status.Conditions) <= ConditionsLengthLimit { + return + } + + firstIndex := len(c.Status.Conditions) - ConditionsLengthLimit + c.Status.Conditions = c.Status.Conditions[firstIndex:len(c.Status.Conditions)] +} diff --git a/pkg/api/apis/operators/v1alpha1/clusterserviceversion_test.go b/pkg/api/apis/operators/v1alpha1/clusterserviceversion_test.go index c9bb8e44800..3c545e014b2 100644 --- a/pkg/api/apis/operators/v1alpha1/clusterserviceversion_test.go +++ b/pkg/api/apis/operators/v1alpha1/clusterserviceversion_test.go @@ -4,9 +4,11 @@ import ( "fmt" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestSetRequirementStatus(t *testing.T) { @@ -282,3 +284,111 @@ func TestSupports(t *testing.T) { }) } } + +func TestSetPhaseWithConditionsLengthAtLimit(t *testing.T) { + csv := ClusterServiceVersion{} + + limit := ConditionsLengthLimit + csv.Status.Conditions = helperNewConditions(limit) + + now := metav1.Now() + + // We expect the oldest condition to be removed. + oldConditionsWant := csv.Status.Conditions[1:] + lastAddedConditionWant := ClusterServiceVersionCondition{ + Phase: ClusterServiceVersionPhase("Pending"), + LastTransitionTime: now, + LastUpdateTime: now, + Message: "message", + Reason: ConditionReason("reason"), + } + + csv.SetPhase("Pending", "reason", "message", now) + + conditionsGot := csv.Status.Conditions + assert.Equal(t, limit, len(conditionsGot)) + + oldConditionsGot := conditionsGot[0 : len(conditionsGot)-1] + assert.EqualValues(t, oldConditionsWant, oldConditionsGot) + + lastAddedConditionGot := conditionsGot[len(conditionsGot)-1] + assert.Equal(t, lastAddedConditionWant, lastAddedConditionGot) +} + +func TestSetPhaseWithConditionsLengthBelowLimit(t *testing.T) { + csv := ClusterServiceVersion{} + + limit := ConditionsLengthLimit + csv.Status.Conditions = helperNewConditions(limit - 1) + + now := metav1.Now() + + // We expect no condition to be removed from the list. + oldConditionsWant := csv.Status.Conditions + lastAddedConditionWant := ClusterServiceVersionCondition{ + Phase: ClusterServiceVersionPhase("Pending"), + LastTransitionTime: now, + LastUpdateTime: now, + Message: "message", + Reason: ConditionReason("reason"), + } + + csv.SetPhase("Pending", "reason", "message", now) + + conditionsGot := csv.Status.Conditions + assert.Equal(t, limit, len(conditionsGot)) + + oldConditionsGot := conditionsGot[0 : len(conditionsGot)-1] + assert.EqualValues(t, oldConditionsWant, oldConditionsGot) + + lastAddedConditionGot := conditionsGot[len(conditionsGot)-1] + assert.Equal(t, lastAddedConditionWant, lastAddedConditionGot) +} + +func TestSetPhaseWithLimitExceeded(t *testing.T) { + csv := ClusterServiceVersion{} + + limit, extra := ConditionsLengthLimit, 10 + csv.Status.Conditions = helperNewConditions(limit + extra) + + now := metav1.Now() + + // We expect extra plus 1 condition(s) to be removed from the list. + oldConditionsWant := csv.Status.Conditions[extra+1:] + lastAddedConditionWant := ClusterServiceVersionCondition{ + Phase: ClusterServiceVersionPhase("Pending"), + LastTransitionTime: now, + LastUpdateTime: now, + Message: "message", + Reason: ConditionReason("reason"), + } + + csv.SetPhase("Pending", "reason", "message", now) + + conditionsGot := csv.Status.Conditions + assert.Equal(t, limit, len(conditionsGot)) + + oldConditionsGot := conditionsGot[0 : len(conditionsGot)-1] + assert.EqualValues(t, oldConditionsWant, oldConditionsGot) + + lastAddedConditionGot := conditionsGot[len(conditionsGot)-1] + assert.Equal(t, lastAddedConditionWant, lastAddedConditionGot) +} + +func helperNewConditions(count int) []ClusterServiceVersionCondition { + conditions := make([]ClusterServiceVersionCondition, 0) + + for i := 1; i <= count; i++ { + now := metav1.Now() + condition := ClusterServiceVersionCondition{ + Phase: ClusterServiceVersionPhase(fmt.Sprintf("phase-%d", i)), + LastTransitionTime: now, + LastUpdateTime: now, + Message: fmt.Sprintf("message-%d", i), + Reason: ConditionReason(fmt.Sprintf("reason-%d", i)), + } + conditions = append(conditions, condition) + } + + return conditions +}