pkg/status: use conditions map instead of slice

Author: joelanford

pkg/status/conditions.go

@@ -73,13 +73,19 @@ func (c Condition) IsUnknown() bool {
 	return c.Status == corev1.ConditionUnknown
 }
 
-// DeepCopyInto copies in into out.
-func (in *Condition) DeepCopyInto(out *Condition) {
-	*out = *in
+// DeepCopy returns a deep copy of the condition
+func (c *Condition) DeepCopy() *Condition {
+	if c == nil {
+		return nil
+	}
+	out := *c
+	return &out
 }
 
 // Conditions is a set of Condition instances.
-type Conditions []Condition
+//
+// +kubebuilder:validation:Type=array
+type Conditions map[ConditionType]Condition
 
 // NewConditions initializes a set of conditions with the given list of
 // conditions.
@@ -95,10 +101,8 @@ func NewConditions(conds ...Condition) Conditions {
 // ConditionType. If found, it returns `condition.IsTrue()`. If not found,
 // it returns false.
 func (conditions Conditions) IsTrue(t ConditionType) bool {
-	for _, condition := range conditions {
-		if condition.Type == t {
-			return condition.IsTrue()
-		}
+	if condition, ok := conditions[t]; ok {
+		return condition.IsTrue()
 	}
 	return false
 }
@@ -107,10 +111,8 @@ func (conditions Conditions) IsTrue(t ConditionType) bool {
 // ConditionType. If found, it returns `condition.IsFalse()`. If not found,
 // it returns false.
 func (conditions Conditions) IsFalse(t ConditionType) bool {
-	for _, condition := range conditions {
-		if condition.Type == t {
-			return condition.IsFalse()
-		}
+	if condition, ok := conditions[t]; ok {
+		return condition.IsFalse()
 	}
 	return false
 }
@@ -119,10 +121,8 @@ func (conditions Conditions) IsFalse(t ConditionType) bool {
 // ConditionType. If found, it returns `condition.IsUnknown()`. If not found,
 // it returns true.
 func (conditions Conditions) IsUnknown(t ConditionType) bool {
-	for _, condition := range conditions {
-		if condition.Type == t {
-			return condition.IsUnknown()
-		}
+	if condition, ok := conditions[t]; ok {
+		return condition.IsUnknown()
 	}
 	return true
 }
@@ -131,34 +131,31 @@ func (conditions Conditions) IsUnknown(t ConditionType) bool {
 // condition. It returns a boolean value indicating whether the set condition
 // is new or was a change to the existing condition with the same type.
 func (conditions *Conditions) SetCondition(newCond Condition) bool {
-	if conditions == nil {
-		*conditions = make([]Condition, 0)
+	if conditions == nil || *conditions == nil {
+		*conditions = make(map[ConditionType]Condition)
 	}
 	newCond.LastTransitionTime = metav1.Time{Time: clock.Now()}
-	for i, condition := range *conditions {
-		if condition.Type == newCond.Type {
-			if condition.Status == newCond.Status {
-				newCond.LastTransitionTime = condition.LastTransitionTime
-			}
-			changed := condition.Status != newCond.Status ||
-				condition.Reason != newCond.Reason ||
-				condition.Message != newCond.Message
-			(*conditions)[i] = newCond
-			return changed
+
+	if condition, ok := (*conditions)[newCond.Type]; ok {
+		if condition.Status == newCond.Status {
+			newCond.LastTransitionTime = condition.LastTransitionTime
 		}
+		changed := condition.Status != newCond.Status ||
+			condition.Reason != newCond.Reason ||
+			condition.Message != newCond.Message
+		(*conditions)[newCond.Type] = newCond
+		return changed
 	}
-	*conditions = append(*conditions, newCond)
+	(*conditions)[newCond.Type] = newCond
 	return true
 }
 
 // GetCondition searches the set of conditions for the condition with the given
 // ConditionType and returns it. If the matching condition is not found,
 // GetCondition returns nil.
 func (conditions Conditions) GetCondition(t ConditionType) *Condition {
-	for _, condition := range conditions {
-		if condition.Type == t {
-			return &condition
-		}
+	if condition, ok := conditions[t]; ok {
+		return &condition
 	}
 	return nil
 }
@@ -167,21 +164,37 @@ func (conditions Conditions) GetCondition(t ConditionType) *Condition {
 // the conditions set. If no condition with that type is found, RemoveCondition
 // returns without performing any action.
 func (conditions *Conditions) RemoveCondition(t ConditionType) bool {
-	for i, condition := range *conditions {
-		if condition.Type == t {
-			*conditions = append((*conditions)[:i], (*conditions)[i+1:]...)
-			return true
-		}
+	if conditions == nil || *conditions == nil {
+		return false
+	}
+	if _, ok := (*conditions)[t]; ok {
+		delete(*conditions, t)
+		return true
 	}
 	return false
 }
 
 // MarshalJSON marshals the set of conditions as a JSON array, sorted by
 // condition type.
 func (conditions Conditions) MarshalJSON() ([]byte, error) {
-	conds := []Condition(conditions)
+	conds := []Condition{}
+	for _, condition := range conditions {
+		conds = append(conds, condition)
+	}
 	sort.Slice(conds, func(a, b int) bool {
 		return conds[a].Type < conds[b].Type
 	})
 	return json.Marshal(conds)
 }
+
+func (conditions *Conditions) UnmarshalJSON(data []byte) error {
+	*conditions = make(map[ConditionType]Condition)
+	conds := []Condition{}
+	if err := json.Unmarshal(data, &conds); err != nil {
+		return err
+	}
+	for _, condition := range conds {
+		(*conditions)[condition.Type] = condition
+	}
+	return nil
+}

pkg/status/conditions_test.go

@@ -69,6 +69,26 @@ func withLastTransitionTime(c Condition, t time.Time) Condition {
 	return c
 }
 
+func TestConditionDeepCopy(t *testing.T) {
+	var n *Condition
+	assert.Nil(t, n.DeepCopy())
+
+	a := generateCondition("A", corev1.ConditionTrue)
+	aCopy := a.DeepCopy()
+	if &a == aCopy {
+		t.Errorf("Expected and actual point to the same object: %p %#v", &a, &a)
+	}
+	if &a.Status == &aCopy.Status {
+		t.Errorf("Expected and actual point to the same object: %p %#v", &a.Status, &a.Status)
+	}
+	if &a.Reason == &aCopy.Reason {
+		t.Errorf("Expected and actual point to the same object: %p %#v", &a.Reason, &a.Reason)
+	}
+	if &a.Message == &aCopy.Message {
+		t.Errorf("Expected and actual point to the same object: %p %#v", &a.Message, &a.Message)
+	}
+}
+
 func TestConditionsSetEmpty(t *testing.T) {
 	conditions := initConditions()