Skip to content

Commit 6310a8a

Browse files
apelisseapelisse
committed
Do not use Compare for Equality checks
Currently, we use the Compare function to compare objects, but this doesn't look at the actual differences between objects, it collects the fields from each object using smd logic and then compares that. This has a couple of drawbacks: 1. Order is not preserved, which means anything that depends on order is wrong! 2. When we decide to allow invalid duplicates, we won't be able to compare such objects. Use value.Equals to compare objects so that we have proper comparison, and cmp.Diff in tests to highlight the differences. As a consequence of 1, this has identified an interesting bug: we used the comparison to see if two objects (before and after) were equal to return nil as an output of `Apply`, but that logic doesn't check if the order has changed, which caused some tests to fail (since only the order had changed, and that change was ignored, the object wasn't updated). Using Equals instead of re-using the compare result in applying has an extra cost that can be noticed on ApplyTwice test-suite, as shown below: ``` name old time/op new time/op delta DeducedSimple-8 60.7µs ± 0% 62.1µs ± 0% +2.21% (p=0.008 n=5+5) DeducedNested-8 131µs ± 0% 131µs ± 0% -0.19% (p=0.008 n=5+5) DeducedNestedAcrossVersion-8 171µs ± 0% 174µs ± 0% +1.89% (p=0.008 n=5+5) LeafConflictAcrossVersion-8 83.2µs ± 0% 83.2µs ± 0% +0.11% (p=0.008 n=5+5) MultipleApplierRecursiveRealConversion-8 1.15ms ± 0% 1.17ms ± 0% +1.59% (p=0.008 n=5+5) Operations/Pod/Create-8 72.5µs ± 0% 70.9µs ± 0% -2.29% (p=0.008 n=5+5) Operations/Pod/Apply-8 184µs ± 0% 182µs ± 0% -0.81% (p=0.008 n=5+5) Operations/Pod/ApplyTwice-8 416µs ± 0% 457µs ± 0% +10.03% (p=0.008 n=5+5) Operations/Pod/Update-8 181µs ± 0% 178µs ± 0% -1.62% (p=0.008 n=5+5) Operations/Pod/UpdateVersion-8 259µs ± 0% 256µs ± 0% -0.86% (p=0.008 n=5+5) Operations/Node/Create-8 110µs ± 0% 111µs ± 0% +1.18% (p=0.008 n=5+5) Operations/Node/Apply-8 283µs ± 0% 275µs ± 0% -2.97% (p=0.008 n=5+5) Operations/Node/ApplyTwice-8 661µs ± 0% 685µs ± 0% +3.64% (p=0.008 n=5+5) Operations/Node/Update-8 344µs ± 0% 275µs ± 0% -20.20% (p=0.008 n=5+5) Operations/Node/UpdateVersion-8 383µs ± 0% 401µs ± 0% +4.60% (p=0.008 n=5+5) Operations/Endpoints/Create-8 7.46µs ± 0% 9.98µs ± 0% +33.75% (p=0.008 n=5+5) Operations/Endpoints/Apply-8 16.6µs ± 0% 19.0µs ± 0% +14.22% (p=0.008 n=5+5) Operations/Endpoints/ApplyTwice-8 930µs ± 0% 4300µs ± 0% +362.33% (p=0.008 n=5+5) Operations/Endpoints/Update-8 882µs ± 0% 1241µs ± 0% +40.70% (p=0.008 n=5+5) Operations/Endpoints/UpdateVersion-8 1.90ms ± 0% 2.24ms ± 0% +17.77% (p=0.008 n=5+5) Operations/Node100%override/Create-8 108µs ± 0% 130µs ± 0% +20.77% (p=0.008 n=5+5) Operations/Node100%override/Apply-8 273µs ± 0% 307µs ± 0% +12.56% (p=0.008 n=5+5) Operations/Node100%override/ApplyTwice-8 630µs ± 0% 688µs ± 0% +9.26% (p=0.008 n=5+5) Operations/Node100%override/Update-8 281µs ± 0% 290µs ± 0% +3.23% (p=0.008 n=5+5) Operations/Node100%override/UpdateVersion-8 402µs ± 0% 416µs ± 0% +3.40% (p=0.008 n=5+5) Operations/Node10%override/Create-8 110µs ± 0% 119µs ± 0% +7.80% (p=0.008 n=5+5) Operations/Node10%override/Apply-8 272µs ± 0% 307µs ± 0% +12.74% (p=0.008 n=5+5) Operations/Node10%override/ApplyTwice-8 640µs ± 0% 711µs ± 0% +11.19% (p=0.008 n=5+5) Operations/Node10%override/Update-8 272µs ± 0% 281µs ± 0% +3.12% (p=0.008 n=5+5) Operations/Node10%override/UpdateVersion-8 407µs ± 0% 472µs ± 0% +16.00% (p=0.008 n=5+5) Operations/Endpoints100%override/Create-8 7.66µs ± 0% 8.93µs ± 0% +16.56% (p=0.008 n=5+5) Operations/Endpoints100%override/Apply-8 16.4µs ± 0% 22.3µs ± 0% +35.78% (p=0.008 n=5+5) Operations/Endpoints100%override/ApplyTwice-8 889µs ± 0% 2445µs ± 0% +175.06% (p=0.008 n=5+5) Operations/Endpoints100%override/Update-8 872µs ± 0% 1009µs ± 0% +15.68% (p=0.008 n=5+5) Operations/Endpoints100%override/UpdateVersion-8 1.97ms ± 0% 2.28ms ± 0% +15.72% (p=0.008 n=5+5) Operations/Endpoints10%override/Create-8 7.62µs ± 0% 11.83µs ± 0% +55.25% (p=0.008 n=5+5) Operations/Endpoints10%override/Apply-8 16.5µs ± 0% 26.4µs ± 0% +59.77% (p=0.008 n=5+5) Operations/Endpoints10%override/ApplyTwice-8 901µs ± 0% 4497µs ± 0% +399.05% (p=0.008 n=5+5) Operations/Endpoints10%override/Update-8 879µs ± 0% 1034µs ± 0% +17.57% (p=0.008 n=5+5) Operations/Endpoints10%override/UpdateVersion-8 1.96ms ± 0% 2.48ms ± 0% +26.24% (p=0.008 n=5+5) Operations/PrometheusCRD/Create-8 646µs ± 0% 625µs ± 0% -3.21% (p=0.008 n=5+5) Operations/PrometheusCRD/Apply-8 1.54ms ± 0% 1.57ms ± 0% +2.12% (p=0.008 n=5+5) Operations/PrometheusCRD/ApplyTwice-8 3.61ms ± 0% 4.23ms ± 0% +16.93% (p=0.008 n=5+5) Operations/PrometheusCRD/Update-8 1.56ms ± 0% 1.68ms ± 0% +7.56% (p=0.008 n=5+5) Operations/PrometheusCRD/UpdateVersion-8 2.29ms ± 0% 2.48ms ± 0% +8.42% (p=0.008 n=5+5) Operations/apiresourceimport/Create-8 3.61ms ± 0% 3.05ms ± 0% -15.32% (p=0.008 n=5+5) Operations/apiresourceimport/Apply-8 8.12ms ± 0% 8.41ms ± 0% +3.64% (p=0.008 n=5+5) Operations/apiresourceimport/ApplyTwice-8 16.3ms ± 0% 17.1ms ± 0% +4.42% (p=0.008 n=5+5) Operations/apiresourceimport/Update-8 4.88ms ± 0% 5.00ms ± 0% +2.57% (p=0.008 n=5+5) Operations/apiresourceimport/UpdateVersion-8 6.50ms ± 0% 6.87ms ± 0% +5.80% (p=0.008 n=5+5) name old alloc/op new alloc/op delta DeducedSimple-8 29.7kB ± 0% 30.4kB ± 0% +2.35% (p=0.008 n=5+5) DeducedNested-8 58.3kB ± 0% 59.1kB ± 0% +1.39% (p=0.008 n=5+5) DeducedNestedAcrossVersion-8 78.2kB ± 0% 79.1kB ± 0% +1.16% (p=0.008 n=5+5) LeafConflictAcrossVersion-8 42.3kB ± 0% 43.0kB ± 0% +1.69% (p=0.008 n=5+5) MultipleApplierRecursiveRealConversion-8 714kB ± 0% 716kB ± 0% +0.29% (p=0.008 n=5+5) Operations/Pod/Create-8 21.3kB ± 0% 21.3kB ± 0% +0.02% (p=0.008 n=5+5) Operations/Pod/Apply-8 52.8kB ± 0% 53.1kB ± 0% +0.56% (p=0.008 n=5+5) Operations/Pod/ApplyTwice-8 107kB ± 0% 110kB ± 0% +3.60% (p=0.008 n=5+5) Operations/Pod/Update-8 40.0kB ± 0% 40.0kB ± 0% -0.01% (p=0.008 n=5+5) Operations/Pod/UpdateVersion-8 52.6kB ± 0% 52.6kB ± 0% +0.03% (p=0.008 n=5+5) Operations/Node/Create-8 30.2kB ± 0% 30.2kB ± 0% -0.01% (p=0.008 n=5+5) Operations/Node/Apply-8 73.8kB ± 0% 74.1kB ± 0% +0.38% (p=0.008 n=5+5) Operations/Node/ApplyTwice-8 149kB ± 0% 155kB ± 0% +4.24% (p=0.008 n=5+5) Operations/Node/Update-8 57.4kB ± 0% 57.4kB ± 0% -0.03% (p=0.008 n=5+5) Operations/Node/UpdateVersion-8 75.9kB ± 0% 75.9kB ± 0% +0.01% (p=0.008 n=5+5) Operations/Endpoints/Create-8 3.84kB ± 0% 3.84kB ± 0% ~ (all equal) Operations/Endpoints/Apply-8 6.70kB ± 0% 6.99kB ± 0% +4.28% (p=0.008 n=5+5) Operations/Endpoints/ApplyTwice-8 110kB ± 0% 208kB ± 0% +88.69% (p=0.008 n=5+5) Operations/Endpoints/Update-8 104kB ± 0% 104kB ± 0% +0.00% (p=0.008 n=5+5) Operations/Endpoints/UpdateVersion-8 203kB ± 0% 202kB ± 0% -0.05% (p=0.008 n=5+5) Operations/Node100%override/Create-8 30.2kB ± 0% 30.2kB ± 0% -0.01% (p=0.008 n=5+5) Operations/Node100%override/Apply-8 73.8kB ± 0% 74.1kB ± 0% +0.43% (p=0.008 n=5+5) Operations/Node100%override/ApplyTwice-8 149kB ± 0% 155kB ± 0% +4.19% (p=0.008 n=5+5) Operations/Node100%override/Update-8 57.4kB ± 0% 57.4kB ± 0% -0.05% (p=0.008 n=5+5) Operations/Node100%override/UpdateVersion-8 75.9kB ± 0% 75.9kB ± 0% -0.01% (p=0.008 n=5+5) Operations/Node10%override/Create-8 30.2kB ± 0% 30.2kB ± 0% -0.01% (p=0.008 n=5+5) Operations/Node10%override/Apply-8 73.8kB ± 0% 74.1kB ± 0% +0.36% (p=0.008 n=5+5) Operations/Node10%override/ApplyTwice-8 149kB ± 0% 155kB ± 0% +4.17% (p=0.008 n=5+5) Operations/Node10%override/Update-8 57.4kB ± 0% 57.4kB ± 0% -0.02% (p=0.008 n=5+5) Operations/Node10%override/UpdateVersion-8 75.9kB ± 0% 75.9kB ± 0% +0.01% (p=0.008 n=5+5) Operations/Endpoints100%override/Create-8 3.84kB ± 0% 3.84kB ± 0% ~ (all equal) Operations/Endpoints100%override/Apply-8 6.70kB ± 0% 6.99kB ± 0% +4.30% (p=0.008 n=5+5) Operations/Endpoints100%override/ApplyTwice-8 110kB ± 0% 208kB ± 0% +88.60% (p=0.008 n=5+5) Operations/Endpoints100%override/Update-8 104kB ± 0% 104kB ± 0% -0.01% (p=0.008 n=5+5) Operations/Endpoints100%override/UpdateVersion-8 203kB ± 0% 203kB ± 0% -0.03% (p=0.008 n=5+5) Operations/Endpoints10%override/Create-8 3.84kB ± 0% 3.84kB ± 0% +0.03% (p=0.008 n=5+5) Operations/Endpoints10%override/Apply-8 6.70kB ± 0% 6.99kB ± 0% +4.32% (p=0.008 n=5+5) Operations/Endpoints10%override/ApplyTwice-8 110kB ± 0% 208kB ± 0% +88.58% (p=0.008 n=5+5) Operations/Endpoints10%override/Update-8 104kB ± 0% 104kB ± 0% +0.01% (p=0.008 n=5+5) Operations/Endpoints10%override/UpdateVersion-8 203kB ± 0% 203kB ± 0% -0.02% (p=0.008 n=5+5) Operations/PrometheusCRD/Create-8 165kB ± 0% 165kB ± 0% ~ (all equal) Operations/PrometheusCRD/Apply-8 471kB ± 0% 471kB ± 0% +0.13% (p=0.008 n=5+5) Operations/PrometheusCRD/ApplyTwice-8 935kB ± 0% 983kB ± 0% +5.11% (p=0.008 n=5+5) Operations/PrometheusCRD/Update-8 318kB ± 0% 318kB ± 0% -0.01% (p=0.008 n=5+5) Operations/PrometheusCRD/UpdateVersion-8 425kB ± 0% 424kB ± 0% -0.11% (p=0.008 n=5+5) Operations/apiresourceimport/Create-8 679kB ± 0% 679kB ± 0% -0.00% (p=0.008 n=5+5) Operations/apiresourceimport/Apply-8 1.94MB ± 0% 1.94MB ± 0% +0.11% (p=0.008 n=5+5) Operations/apiresourceimport/ApplyTwice-8 3.56MB ± 0% 3.65MB ± 0% +2.42% (p=0.008 n=5+5) Operations/apiresourceimport/Update-8 1.02MB ± 0% 1.02MB ± 0% +0.00% (p=0.008 n=5+5) Operations/apiresourceimport/UpdateVersion-8 1.35MB ± 0% 1.35MB ± 0% +0.01% (p=0.008 n=5+5) name old allocs/op new allocs/op delta DeducedSimple-8 703 ± 0% 724 ± 0% +2.99% (p=0.008 n=5+5) DeducedNested-8 1.34k ± 0% 1.36k ± 0% +1.72% (p=0.008 n=5+5) DeducedNestedAcrossVersion-8 1.83k ± 0% 1.85k ± 0% +1.37% (p=0.008 n=5+5) LeafConflictAcrossVersion-8 943 ± 0% 964 ± 0% +2.23% (p=0.008 n=5+5) MultipleApplierRecursiveRealConversion-8 7.57k ± 0% 7.62k ± 0% +0.67% (p=0.008 n=5+5) Operations/Pod/Create-8 481 ± 0% 481 ± 0% ~ (all equal) Operations/Pod/Apply-8 1.26k ± 0% 1.27k ± 0% +0.63% (p=0.008 n=5+5) Operations/Pod/ApplyTwice-8 2.62k ± 0% 2.72k ± 0% +3.97% (p=0.008 n=5+5) Operations/Pod/Update-8 976 ± 0% 976 ± 0% ~ (all equal) Operations/Pod/UpdateVersion-8 1.40k ± 0% 1.40k ± 0% ~ (all equal) Operations/Node/Create-8 565 ± 0% 565 ± 0% ~ (all equal) Operations/Node/Apply-8 1.57k ± 0% 1.58k ± 0% +0.51% (p=0.008 n=5+5) Operations/Node/ApplyTwice-8 3.37k ± 0% 3.56k ± 0% +5.58% (p=0.008 n=5+5) Operations/Node/Update-8 1.26k ± 0% 1.26k ± 0% -0.08% (p=0.008 n=5+5) Operations/Node/UpdateVersion-8 1.87k ± 0% 1.87k ± 0% ~ (all equal) Operations/Endpoints/Create-8 84.0 ± 0% 84.0 ± 0% ~ (all equal) Operations/Endpoints/Apply-8 156 ± 0% 164 ± 0% +5.13% (p=0.008 n=5+5) Operations/Endpoints/ApplyTwice-8 2.35k ± 0% 4.40k ± 0% +87.52% (p=0.008 n=5+5) Operations/Endpoints/Update-8 2.20k ± 0% 2.20k ± 0% ~ (all equal) Operations/Endpoints/UpdateVersion-8 4.27k ± 0% 4.27k ± 0% -0.02% (p=0.008 n=5+5) Operations/Node100%override/Create-8 565 ± 0% 565 ± 0% ~ (all equal) Operations/Node100%override/Apply-8 1.57k ± 0% 1.58k ± 0% +0.57% (p=0.008 n=5+5) Operations/Node100%override/ApplyTwice-8 3.37k ± 0% 3.56k ± 0% +5.55% (p=0.008 n=5+5) Operations/Node100%override/Update-8 1.26k ± 0% 1.26k ± 0% -0.08% (p=0.008 n=5+5) Operations/Node100%override/UpdateVersion-8 1.87k ± 0% 1.87k ± 0% ~ (all equal) Operations/Node10%override/Create-8 565 ± 0% 565 ± 0% ~ (all equal) Operations/Node10%override/Apply-8 1.57k ± 0% 1.58k ± 0% +0.51% (p=0.008 n=5+5) Operations/Node10%override/ApplyTwice-8 3.37k ± 0% 3.56k ± 0% +5.52% (p=0.008 n=5+5) Operations/Node10%override/Update-8 1.26k ± 0% 1.26k ± 0% -0.08% (p=0.008 n=5+5) Operations/Node10%override/UpdateVersion-8 1.87k ± 0% 1.87k ± 0% +0.05% (p=0.008 n=5+5) Operations/Endpoints100%override/Create-8 84.0 ± 0% 84.0 ± 0% ~ (all equal) Operations/Endpoints100%override/Apply-8 156 ± 0% 164 ± 0% +5.13% (p=0.008 n=5+5) Operations/Endpoints100%override/ApplyTwice-8 2.35k ± 0% 4.40k ± 0% +87.44% (p=0.008 n=5+5) Operations/Endpoints100%override/Update-8 2.20k ± 0% 2.20k ± 0% ~ (all equal) Operations/Endpoints100%override/UpdateVersion-8 4.27k ± 0% 4.27k ± 0% ~ (all equal) Operations/Endpoints10%override/Create-8 84.0 ± 0% 84.0 ± 0% ~ (all equal) Operations/Endpoints10%override/Apply-8 156 ± 0% 164 ± 0% +5.13% (p=0.008 n=5+5) Operations/Endpoints10%override/ApplyTwice-8 2.35k ± 0% 4.40k ± 0% +87.47% (p=0.008 n=5+5) Operations/Endpoints10%override/Update-8 2.20k ± 0% 2.20k ± 0% ~ (all equal) Operations/Endpoints10%override/UpdateVersion-8 4.27k ± 0% 4.27k ± 0% ~ (all equal) Operations/PrometheusCRD/Create-8 3.68k ± 0% 3.68k ± 0% ~ (all equal) Operations/PrometheusCRD/Apply-8 10.2k ± 0% 10.2k ± 0% +0.10% (p=0.008 n=5+5) Operations/PrometheusCRD/ApplyTwice-8 20.0k ± 0% 21.1k ± 0% +5.56% (p=0.008 n=5+5) Operations/PrometheusCRD/Update-8 6.98k ± 0% 6.98k ± 0% ~ (all equal) Operations/PrometheusCRD/UpdateVersion-8 10.2k ± 0% 10.2k ± 0% -0.05% (p=0.008 n=5+5) Operations/apiresourceimport/Create-8 15.4k ± 0% 15.4k ± 0% ~ (all equal) Operations/apiresourceimport/Apply-8 43.0k ± 0% 43.0k ± 0% +0.06% (p=0.008 n=5+5) Operations/apiresourceimport/ApplyTwice-8 81.7k ± 0% 83.8k ± 0% +2.56% (p=0.008 n=5+5) Operations/apiresourceimport/Update-8 26.3k ± 0% 26.3k ± 0% ~ (all equal) Operations/apiresourceimport/UpdateVersion-8 37.3k ± 0% 37.3k ± 0% ~ (all equal) ```
1 parent ab402c1 commit 6310a8a

File tree

6 files changed

+48
-17
lines changed

6 files changed

+48
-17
lines changed

‎go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module sigs.k8s.io/structured-merge-diff/v4
33
require gopkg.in/yaml.v2 v2.2.8
44

55
require (
6+
github.com/google/go-cmp v0.5.9 // indirect
67
github.com/google/gofuzz v1.0.0
78
github.com/json-iterator/go v1.1.12
89
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect

‎go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
22
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
33
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
4+
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
5+
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
46
github.com/google/gofuzz v1.0.0 h1:A8PeW59pxE9IoFRqBp37U+mSNaQoZ46F1f0f863XSXw=
57
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
68
github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=

‎internal/fixture/state.go

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@ import (
2020
"bytes"
2121
"fmt"
2222

23+
"github.com/google/go-cmp/cmp"
24+
2325
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
2426
"sigs.k8s.io/structured-merge-diff/v4/merge"
2527
"sigs.k8s.io/structured-merge-diff/v4/typed"
26-
"sigs.k8s.io/structured-merge-diff/v4/value"
2728
)
2829

2930
// For the sake of tests, a parser is something that can retrieve a
@@ -168,20 +169,47 @@ func (s *State) Apply(obj typed.YAMLObject, version fieldpath.APIVersion, manage
168169

169170
// CompareLive takes a YAML string and returns the comparison with the
170171
// current live object or an error.
171-
func (s *State) CompareLive(obj typed.YAMLObject, version fieldpath.APIVersion) (*typed.Comparison, error) {
172+
func (s *State) CompareLive(obj typed.YAMLObject, version fieldpath.APIVersion) (string, error) {
172173
obj = FixTabsOrDie(obj)
173174
if err := s.checkInit(version); err != nil {
174-
return nil, err
175+
return "", err
175176
}
176177
tv, err := s.Parser.Type(string(version)).FromYAML(obj)
177178
if err != nil {
178-
return nil, err
179+
return "", err
179180
}
180181
live, err := s.Updater.Converter.Convert(s.Live, version)
181182
if err != nil {
182-
return nil, err
183+
return "", err
184+
}
185+
tvu := convertMapAnyToMapString(tv.AsValue().Unstructured())
186+
liveu := convertMapAnyToMapString(live.AsValue().Unstructured())
187+
return cmp.Diff(tvu, liveu), nil
188+
}
189+
190+
func convertMapAnyToMapString(obj interface{}) interface{} {
191+
switch m := obj.(type) {
192+
case map[string]interface{}:
193+
out := map[string]interface{}{}
194+
for key, value := range m {
195+
out[key] = convertMapAnyToMapString(value)
196+
}
197+
return out
198+
case map[interface{}]interface{}:
199+
out := map[string]interface{}{}
200+
for key, value := range m {
201+
out[key.(string)] = convertMapAnyToMapString(value)
202+
}
203+
return out
204+
case []interface{}:
205+
out := []interface{}{}
206+
for _, value := range m {
207+
out = append(out, convertMapAnyToMapString(value))
208+
}
209+
return out
210+
default:
211+
return obj
183212
}
184-
return live.Compare(tv)
185213
}
186214

187215
// dummyConverter doesn't convert, it just returns the same exact object, as long as a version is provided.
@@ -581,8 +609,8 @@ func (tc TestCase) TestWithConverter(parser Parser, converter merge.Converter) e
581609
if err != nil {
582610
return fmt.Errorf("failed to compare live with config: %v", err)
583611
}
584-
if !comparison.IsSame() {
585-
return fmt.Errorf("expected live and config to be the same:\n%v\nConfig: %v\n", comparison, value.ToString(state.Live.AsValue()))
612+
if comparison != "" {
613+
return fmt.Errorf("expected live and config to be the same:\n%v\n", comparison)
586614
}
587615
}
588616

‎merge/obsolete_versions_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func TestApplyObsoleteVersion(t *testing.T) {
127127
if err != nil {
128128
t.Fatalf("Failed to compare live object: %v", err)
129129
}
130-
if !comparison.IsSame() {
130+
if comparison != "" {
131131
t.Fatalf("Unexpected object:\n%v", comparison)
132132
}
133133
}

‎merge/set_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,11 @@ func TestUpdateSet(t *testing.T) {
124124
Object: `
125125
list:
126126
- a
127-
- aprime
128127
- b
128+
- aprime
129129
- c
130-
- cprime
131130
- d
131+
- cprime
132132
`,
133133
APIVersion: "v1",
134134
Managed: fieldpath.ManagedFields{
@@ -189,11 +189,11 @@ func TestUpdateSet(t *testing.T) {
189189
Object: `
190190
list:
191191
- a
192-
- aprime
193192
- b
193+
- aprime
194194
- c
195-
- cprime
196195
- d
196+
- cprime
197197
`,
198198
APIVersion: "v1",
199199
Managed: fieldpath.ManagedFields{

‎merge/update.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
2020
"sigs.k8s.io/structured-merge-diff/v4/typed"
21+
"sigs.k8s.io/structured-merge-diff/v4/value"
2122
)
2223

2324
// Converter is an interface to the conversion logic. The converter
@@ -157,8 +158,7 @@ func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldp
157158

158159
// Apply should be called when Apply is run, given the current object as
159160
// well as the configuration that is applied. This will merge the object
160-
// and return it. If the object hasn't changed, nil is returned (the
161-
// managers can still have changed though).
161+
// and return it.
162162
func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string, force bool) (*typed.TypedValue, fieldpath.ManagedFields, error) {
163163
var err error
164164
managers, err = s.reconcileManagedFieldsWithSchemaChanges(liveObject, managers)
@@ -200,11 +200,11 @@ func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fiel
200200
if err != nil {
201201
return nil, fieldpath.ManagedFields{}, fmt.Errorf("failed to prune fields: %v", err)
202202
}
203-
managers, compare, err := s.update(liveObject, newObject, version, managers, manager, force)
203+
managers, _, err = s.update(liveObject, newObject, version, managers, manager, force)
204204
if err != nil {
205205
return nil, fieldpath.ManagedFields{}, err
206206
}
207-
if compare.IsSame() {
207+
if value.EqualsUsing(value.NewFreelistAllocator(), liveObject.AsValue(), newObject.AsValue()) {
208208
newObject = nil
209209
}
210210
return newObject, managers, nil

0 commit comments

Comments
 (0)