Skip to content

Commit

Permalink
Merge pull request #3382 from vincepri/fix-cmp-log
Browse files Browse the repository at this point in the history
🌱 Fix logging comparison between conditions patch
  • Loading branch information
k8s-ci-robot authored Jul 22, 2020
2 parents c4a7420 + e458d62 commit 823941b
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions util/conditions/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ type Patch []PatchOperation

// PatchOperation define an operation that changes a single condition.
type PatchOperation struct {
After *clusterv1.Condition
Before *clusterv1.Condition
After *clusterv1.Condition
Op PatchOperationType
}

Expand Down Expand Up @@ -129,7 +129,7 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
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) {
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(conditionPatch.Before, 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.
// NOTE: We are preserving LastTransitionTime from the latest in order to avoid altering the existing value.
Expand All @@ -149,14 +149,14 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {

// If the condition does not exist anymore on the latest, this is a conflict.
if latestCondition == nil {
return errors.Errorf("error patching conditions: The condition %q was deleted by a different process and this caused a merge/ChangeCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(conditionPatch.Before, conditionPatch.After))
return errors.Errorf("error patching conditions: The condition %q was deleted by a different process and this caused a merge/ChangeCondition conflict", conditionPatch.After.Type)
}

// 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) {
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(conditionPatch.Before, 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.
// NOTE: We are preserving LastTransitionTime from the latest in order to avoid altering the existing value.
Expand All @@ -176,7 +176,7 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
// if so then this is a conflict.
if latestCondition := Get(latest, conditionPatch.Before.Type); latestCondition != nil {
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(conditionPatch.Before, conditionPatch.After))
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))
}
}
// Otherwise the latest and after agreed on the delete operation, so there's nothing to change.
Expand Down

0 comments on commit 823941b

Please sign in to comment.