Skip to content

Commit

Permalink
Merge pull request #11253 from m-messiah/export-compare
Browse files Browse the repository at this point in the history
🌱Export conditions.HasSameState method
  • Loading branch information
k8s-ci-robot authored Oct 7, 2024
2 parents 4bb8fd5 + 491860c commit a3711b4
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 26 deletions.
12 changes: 6 additions & 6 deletions util/conditions/getter.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type Getter interface {
GetConditions() clusterv1.Conditions
}

// Get returns the condition with the given type, if the condition does not exists,
// Get returns the condition with the given type, if the condition does not exist,
// it returns nil.
func Get(from Getter, t clusterv1.ConditionType) *clusterv1.Condition {
conditions := from.GetConditions()
Expand All @@ -54,7 +54,7 @@ func Has(from Getter, t clusterv1.ConditionType) bool {
return Get(from, t) != nil
}

// IsTrue is true if the condition with the given type is True, otherwise it return false
// IsTrue is true if the condition with the given type is True, otherwise it returns false
// if the condition is not True or if the condition does not exist (is nil).
func IsTrue(from Getter, t clusterv1.ConditionType) bool {
if c := Get(from, t); c != nil {
Expand All @@ -63,7 +63,7 @@ func IsTrue(from Getter, t clusterv1.ConditionType) bool {
return false
}

// IsFalse is true if the condition with the given type is False, otherwise it return false
// IsFalse is true if the condition with the given type is False, otherwise it returns false
// if the condition is not False or if the condition does not exist (is nil).
func IsFalse(from Getter, t clusterv1.ConditionType) bool {
if c := Get(from, t); c != nil {
Expand Down Expand Up @@ -208,7 +208,7 @@ type mirrorOptions struct {
// MirrorOptions defines an option for mirroring conditions.
type MirrorOptions func(*mirrorOptions)

// WithFallbackValue specify a fallback value to use in case the mirrored condition does not exists;
// WithFallbackValue specify a fallback value to use in case the mirrored condition does not exist;
// in case the fallbackValue is false, given values for reason, severity and message will be used.
func WithFallbackValue(fallbackValue bool, reason string, severity clusterv1.ConditionSeverity, message string) MirrorOptions {
return func(c *mirrorOptions) {
Expand All @@ -220,7 +220,7 @@ func WithFallbackValue(fallbackValue bool, reason string, severity clusterv1.Con
}

// mirror mirrors the Ready condition from a dependent object into the target condition;
// if the Ready condition does not exists in the source object, no target conditions is generated.
// if the Ready condition does not exist in the source object, no target conditions is generated.
// NOTE: Considering that we are mirroring Ready conditions with positive polarity, also the resulting condition will have positive polarity.
func mirror(from Getter, targetCondition clusterv1.ConditionType, options ...MirrorOptions) *clusterv1.Condition {
mirrorOpt := &mirrorOptions{}
Expand All @@ -247,7 +247,7 @@ func mirror(from Getter, targetCondition clusterv1.ConditionType, options ...Mir
}

// Aggregates all the Ready condition from a list of dependent objects into the target object;
// if the Ready condition does not exists in one of the source object, the object is excluded from
// if the Ready condition does not exist in one of the source object, the object is excluded from
// the aggregation; if none of the source object have ready condition, no target conditions is generated.
// NOTE: Considering that we are aggregating Ready conditions with positive polarity, also the resulting condition will have positive polarity.
func aggregate(from []Getter, targetCondition clusterv1.ConditionType, options ...MergeOption) *clusterv1.Condition {
Expand Down
2 changes: 1 addition & 1 deletion util/conditions/matchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (matcher *conditionMatcher) Match(actual interface{}) (success bool, err er
return false, errors.New("value should be a condition")
}

return hasSameState(actualCondition, matcher.Expected), nil
return HasSameState(actualCondition, matcher.Expected), nil
}

func (matcher *conditionMatcher) FailureMessage(actual interface{}) (message string) {
Expand Down
2 changes: 1 addition & 1 deletion util/conditions/merge_strategies.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type mergeOptions struct {
type MergeOption func(*mergeOptions)

// WithConditions instructs merge about the condition types to consider when doing a merge operation;
// if this option is not specified, all the conditions (excepts Ready) will be considered. This is required
// if this option is not specified, all the conditions (excepts Ready) will be considered. This is required,
// so we can provide some guarantees about the semantic of the target condition without worrying about
// side effects if someone or something adds custom conditions to the objects.
//
Expand Down
6 changes: 3 additions & 3 deletions util/conditions/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
// If the condition is already on latest, check if latest and after agree on the change; if not, this is a conflict.
if latestCondition := Get(latest, conditionPatch.After.Type); latestCondition != nil {
// If latest and after agree on the change, then it is a conflict.
if !hasSameState(latestCondition, conditionPatch.After) {
if !HasSameState(latestCondition, conditionPatch.After) {
return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/AddCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(latestCondition, conditionPatch.After))
}
// otherwise, the latest is already as intended.
Expand All @@ -179,7 +179,7 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
// If the condition on the latest is different from the base condition, check if
// the after state corresponds to the desired value. If not this is a conflict (unless we should ignore conflicts for this condition type).
if !reflect.DeepEqual(latestCondition, conditionPatch.Before) {
if !hasSameState(latestCondition, conditionPatch.After) {
if !HasSameState(latestCondition, conditionPatch.After) {
return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/ChangeCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(latestCondition, conditionPatch.After))
}
// Otherwise the latest is already as intended.
Expand All @@ -199,7 +199,7 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
// If the condition is still on the latest, check if it is changed in the meantime;
// if so then this is a conflict.
if latestCondition := Get(latest, conditionPatch.Before.Type); latestCondition != nil {
if !hasSameState(latestCondition, conditionPatch.Before) {
if !HasSameState(latestCondition, conditionPatch.Before) {
return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/RemoveCondition conflict: %v", conditionPatch.Before.Type, cmp.Diff(latestCondition, conditionPatch.Before))
}
}
Expand Down
14 changes: 7 additions & 7 deletions util/conditions/setter.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func Set(to Setter, condition *clusterv1.Condition) {
existingCondition := conditions[i]
if existingCondition.Type == condition.Type {
exists = true
if !hasSameState(&existingCondition, condition) {
if !HasSameState(&existingCondition, condition) {
condition.LastTransitionTime = metav1.NewTime(time.Now().UTC().Truncate(time.Second))
conditions[i] = *condition
break
Expand Down Expand Up @@ -157,13 +157,13 @@ func SetSummary(to Setter, options ...MergeOption) {
}

// SetMirror creates a new condition by mirroring the Ready condition from a dependent object;
// if the Ready condition does not exists in the source object, no target conditions is generated.
// if the Ready condition does not exist in the source object, no target conditions is generated.
func SetMirror(to Setter, targetCondition clusterv1.ConditionType, from Getter, options ...MirrorOptions) {
Set(to, mirror(from, targetCondition, options...))
}

// SetAggregate creates a new condition with the aggregation of all the Ready condition
// from a list of dependent objects; if the Ready condition does not exists in one of the source object,
// from a list of dependent objects; if the Ready condition does not exist in one of the source object,
// the object is excluded from the aggregation; if none of the source object have ready condition,
// no target conditions is generated.
func SetAggregate(to Setter, targetCondition clusterv1.ConditionType, from []Getter, options ...MergeOption) {
Expand All @@ -186,17 +186,17 @@ func Delete(to Setter, t clusterv1.ConditionType) {
to.SetConditions(newConditions)
}

// lexicographicLess returns true if a condition is less than another with regards to the
// to order of conditions designed for convenience of the consumer, i.e. kubectl.
// lexicographicLess returns true if a condition is less than another in regard to
// the order of conditions designed for convenience of the consumer, i.e. kubectl.
// According to this order the Ready condition always goes first, followed by all the other
// conditions sorted by Type.
func lexicographicLess(i, j *clusterv1.Condition) bool {
return (i.Type == clusterv1.ReadyCondition || i.Type < j.Type) && j.Type != clusterv1.ReadyCondition
}

// hasSameState returns true if a condition has the same state of another; state is defined
// HasSameState returns true if a condition has the same state of another; state is defined
// by the union of following fields: Type, Status, Reason, Severity and Message (it excludes LastTransitionTime).
func hasSameState(i, j *clusterv1.Condition) bool {
func HasSameState(i, j *clusterv1.Condition) bool {
return i.Type == j.Type &&
i.Status == j.Status &&
i.Reason == j.Reason &&
Expand Down
16 changes: 8 additions & 8 deletions util/conditions/setter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,33 +35,33 @@ func TestHasSameState(t *testing.T) {

// same condition
falseInfo2 := falseInfo1.DeepCopy()
g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeTrue())
g.Expect(HasSameState(falseInfo1, falseInfo2)).To(BeTrue())

// different LastTransitionTime does not impact state
falseInfo2 = falseInfo1.DeepCopy()
falseInfo2.LastTransitionTime = metav1.NewTime(time.Date(1900, time.November, 10, 23, 0, 0, 0, time.UTC))
g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeTrue())
g.Expect(HasSameState(falseInfo1, falseInfo2)).To(BeTrue())

// different Type, Status, Reason, Severity and Message determine different state
falseInfo2 = falseInfo1.DeepCopy()
falseInfo2.Type = "another type"
g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeFalse())
g.Expect(HasSameState(falseInfo1, falseInfo2)).To(BeFalse())

falseInfo2 = falseInfo1.DeepCopy()
falseInfo2.Status = corev1.ConditionTrue
g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeFalse())
g.Expect(HasSameState(falseInfo1, falseInfo2)).To(BeFalse())

falseInfo2 = falseInfo1.DeepCopy()
falseInfo2.Severity = clusterv1.ConditionSeverityWarning
g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeFalse())
g.Expect(HasSameState(falseInfo1, falseInfo2)).To(BeFalse())

falseInfo2 = falseInfo1.DeepCopy()
falseInfo2.Reason = "another severity"
g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeFalse())
g.Expect(HasSameState(falseInfo1, falseInfo2)).To(BeFalse())

falseInfo2 = falseInfo1.DeepCopy()
falseInfo2.Message = "another message"
g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeFalse())
g.Expect(HasSameState(falseInfo1, falseInfo2)).To(BeFalse())
}

func TestLexicographicLess(t *testing.T) {
Expand Down Expand Up @@ -304,7 +304,7 @@ func (matcher *ConditionsMatcher) Match(actual interface{}) (success bool, err e
}

for i := range actualConditions {
if !hasSameState(&actualConditions[i], &matcher.Expected[i]) {
if !HasSameState(&actualConditions[i], &matcher.Expected[i]) {
return false, nil
}
}
Expand Down

0 comments on commit a3711b4

Please sign in to comment.