Apimeta Set/RemoveStatusCondition: Indicate change

The SetStatusCondition and RemoveStatusCondition currently do not
indicate if they changed anything. In most cases that information is
necessary to determine if an Update of the object is needed. This change
adds a boolean return to them that indicate if they changed anything.

As the two functions had no return at all prior to this, this shouldn't
break anything.
This commit is contained in:
Alvaro Aleman 2023-09-09 15:41:58 -04:00
parent 33c5bd631d
commit 5d56f7cf86
2 changed files with 62 additions and 19 deletions

View File

@ -22,14 +22,15 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
// SetStatusCondition sets the corresponding condition in conditions to newCondition.
// SetStatusCondition sets the corresponding condition in conditions to newCondition and returns true
// if the conditions are changed by this call.
// conditions must be non-nil.
// 1. if the condition of the specified type already exists (all fields of the existing condition are updated to
// newCondition, LastTransitionTime is set to now if the new status differs from the old status)
// 2. if a condition of the specified type does not exist (LastTransitionTime is set to now() if unset, and newCondition is appended)
func SetStatusCondition(conditions *[]metav1.Condition, newCondition metav1.Condition) {
func SetStatusCondition(conditions *[]metav1.Condition, newCondition metav1.Condition) (changed bool) {
if conditions == nil {
return
return false
}
existingCondition := FindStatusCondition(*conditions, newCondition.Type)
if existingCondition == nil {
@ -37,7 +38,7 @@ func SetStatusCondition(conditions *[]metav1.Condition, newCondition metav1.Cond
newCondition.LastTransitionTime = metav1.NewTime(time.Now())
}
*conditions = append(*conditions, newCondition)
return
return true
}
if existingCondition.Status != newCondition.Status {
@ -47,18 +48,31 @@ func SetStatusCondition(conditions *[]metav1.Condition, newCondition metav1.Cond
} else {
existingCondition.LastTransitionTime = metav1.NewTime(time.Now())
}
changed = true
}
if existingCondition.Reason != newCondition.Reason {
existingCondition.Reason = newCondition.Reason
changed = true
}
if existingCondition.Message != newCondition.Message {
existingCondition.Message = newCondition.Message
changed = true
}
if existingCondition.ObservedGeneration != newCondition.ObservedGeneration {
existingCondition.ObservedGeneration = newCondition.ObservedGeneration
changed = true
}
// RemoveStatusCondition removes the corresponding conditionType from conditions.
return changed
}
// RemoveStatusCondition removes the corresponding conditionType from conditions if present. Returns
// true if it was present and got removed.
// conditions must be non-nil.
func RemoveStatusCondition(conditions *[]metav1.Condition, conditionType string) {
func RemoveStatusCondition(conditions *[]metav1.Condition, conditionType string) (removed bool) {
if conditions == nil || len(*conditions) == 0 {
return
return false
}
newConditions := make([]metav1.Condition, 0, len(*conditions)-1)
for _, condition := range *conditions {
@ -67,7 +81,10 @@ func RemoveStatusCondition(conditions *[]metav1.Condition, conditionType string)
}
}
removed = len(*conditions) != len(newConditions)
*conditions = newConditions
return removed
}
// FindStatusCondition finds the conditionType in conditions.

View File

@ -32,6 +32,7 @@ func TestSetStatusCondition(t *testing.T) {
name string
conditions []metav1.Condition
toAdd metav1.Condition
expectChanged bool
expected []metav1.Condition
}{
{
@ -41,6 +42,7 @@ func TestSetStatusCondition(t *testing.T) {
{Type: "third"},
},
toAdd: metav1.Condition{Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}, Reason: "reason", Message: "message"},
expectChanged: true,
expected: []metav1.Condition{
{Type: "first"},
{Type: "third"},
@ -55,6 +57,7 @@ func TestSetStatusCondition(t *testing.T) {
{Type: "third"},
},
toAdd: metav1.Condition{Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}, Reason: "reason", Message: "message"},
expectChanged: true,
expected: []metav1.Condition{
{Type: "first"},
{Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}, Reason: "reason", Message: "message"},
@ -69,17 +72,35 @@ func TestSetStatusCondition(t *testing.T) {
{Type: "third"},
},
toAdd: metav1.Condition{Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourAfter}, Reason: "reason", Message: "message", ObservedGeneration: 3},
expectChanged: true,
expected: []metav1.Condition{
{Type: "first"},
{Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}, Reason: "reason", Message: "message", ObservedGeneration: 3},
{Type: "third"},
},
},
{
name: "nothing changes",
conditions: []metav1.Condition{{
Type: "type",
Status: metav1.ConditionTrue,
LastTransitionTime: metav1.Time{Time: oneHourBefore},
}},
toAdd: metav1.Condition{Type: "type", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}},
expected: []metav1.Condition{{
Type: "type",
Status: metav1.ConditionTrue,
LastTransitionTime: metav1.Time{Time: oneHourBefore},
}},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
SetStatusCondition(&test.conditions, test.toAdd)
changed := SetStatusCondition(&test.conditions, test.toAdd)
if test.expectChanged != changed {
t.Errorf("expectChanged=%t != changed=%t", test.expectChanged, changed)
}
if !reflect.DeepEqual(test.conditions, test.expected) {
t.Error(test.conditions)
}
@ -92,6 +113,7 @@ func TestRemoveStatusCondition(t *testing.T) {
name string
conditions []metav1.Condition
conditionType string
expectRemoval bool
expected []metav1.Condition
}{
{
@ -102,6 +124,7 @@ func TestRemoveStatusCondition(t *testing.T) {
{Type: "third"},
},
conditionType: "second",
expectRemoval: true,
expected: []metav1.Condition{
{Type: "first"},
{Type: "third"},
@ -131,7 +154,10 @@ func TestRemoveStatusCondition(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
RemoveStatusCondition(&test.conditions, test.conditionType)
removed := RemoveStatusCondition(&test.conditions, test.conditionType)
if test.expectRemoval != removed {
t.Errorf("expectRemoval=%t != removal=%t", test.expectRemoval, removed)
}
if !reflect.DeepEqual(test.conditions, test.expected) {
t.Error(test.conditions)
}