From 0d628b3bff2fafa334b76e322f60cc095cac3192 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 5 Jan 2015 13:38:39 -0800 Subject: [PATCH] Make semantic deep equal public feature * Use semantic deep equal when validating * More test cases for deep equal --- pkg/api/helpers.go | 22 ++++++++++++++++++++++ pkg/api/serialization_test.go | 15 +++------------ pkg/api/validation/validation.go | 7 +++---- pkg/conversion/deep_equal_test.go | 17 +++++++++++++++++ 4 files changed, 45 insertions(+), 16 deletions(-) diff --git a/pkg/api/helpers.go b/pkg/api/helpers.go index a593dde3d90..5ee4fa1b010 100644 --- a/pkg/api/helpers.go +++ b/pkg/api/helpers.go @@ -18,6 +18,28 @@ package api import ( "strings" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource" + "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" +) + +// Semantic can do semantic deep equality checks for api objects. +// Example: api.Semantic.DeepEqual(aPod, aPodWithNonNilButEmptyMaps) == true +var Semantic = conversion.EqualitiesOrDie( + func(a, b resource.Quantity) bool { + // Ignore formatting, only care that numeric value stayed the same. + // TODO: if we decide it's important, after we drop v1beta1/2, we + // could start comparing format. + // + // Uninitialized quantities are equivilent to 0 quantities. + if a.Amount == nil && b.MilliValue() == 0 { + return true + } + if b.Amount == nil && a.MilliValue() == 0 { + return true + } + return a.Amount.Cmp(b.Amount) == 0 + }, ) // TODO: Address these per #1502 diff --git a/pkg/api/serialization_test.go b/pkg/api/serialization_test.go index 156e0119eaf..46330c4e10d 100644 --- a/pkg/api/serialization_test.go +++ b/pkg/api/serialization_test.go @@ -30,7 +30,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta2" - "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" @@ -41,14 +40,6 @@ import ( var fuzzIters = flag.Int("fuzz_iters", 40, "How many fuzzing iterations to do.") -// apiObjectComparer can do semantic deep equality checks for api objects. -var apiObjectComparer = conversion.EqualitiesOrDie( - func(a, b resource.Quantity) bool { - // Ignore formatting, only care that numeric value stayed the same. - return a.Amount.Cmp(b.Amount) == 0 - }, -) - // apiObjectFuzzer can randomly populate api objects. var apiObjectFuzzer = fuzz.New().NilChance(.5).NumElements(1, 1).Funcs( func(j *runtime.PluginBase, c fuzz.Continue) { @@ -181,7 +172,7 @@ func runTest(t *testing.T, codec runtime.Codec, source runtime.Object) { t.Errorf("0: %v: %v\nCodec: %v\nData: %s\nSource: %#v", name, err, codec, string(data), source) return } - if !apiObjectComparer.DeepEqual(source, obj2) { + if !api.Semantic.DeepEqual(source, obj2) { t.Errorf("1: %v: diff: %v\nCodec: %v\nData: %s\nSource: %#v", name, util.ObjectGoPrintDiff(source, obj2), codec, string(data), source) return } @@ -192,7 +183,7 @@ func runTest(t *testing.T, codec runtime.Codec, source runtime.Object) { t.Errorf("2: %v: %v", name, err) return } - if !apiObjectComparer.DeepEqual(source, obj3) { + if !api.Semantic.DeepEqual(source, obj3) { t.Errorf("3: %v: diff: %v\nCodec: %v", name, util.ObjectDiff(source, obj3), codec) return } @@ -266,7 +257,7 @@ func TestEncode_Ptr(t *testing.T) { if _, ok := obj2.(*api.Pod); !ok { t.Fatalf("Got wrong type") } - if !apiObjectComparer.DeepEqual(obj2, pod) { + if !api.Semantic.DeepEqual(obj2, pod) { t.Errorf("Expected:\n %#v,\n Got:\n %#v", &pod, obj2) } } diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 4731c48b886..8566d8012c1 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -18,7 +18,6 @@ package validation import ( "fmt" - "reflect" "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -429,7 +428,7 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) errs.ValidationErrorList { newContainers = append(newContainers, container) } pod.Spec.Containers = newContainers - if !reflect.DeepEqual(pod.Spec, oldPod.Spec) { + if !api.Semantic.DeepEqual(pod.Spec, oldPod.Spec) { // TODO: a better error would include all immutable fields explicitly. allErrs = append(allErrs, errs.NewFieldInvalid("spec.containers", newPod.Spec.Containers, "some fields are immutable")) } @@ -586,7 +585,7 @@ func ValidateMinion(minion *api.Node) errs.ValidationErrorList { func ValidateMinionUpdate(oldMinion *api.Node, minion *api.Node) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - if !reflect.DeepEqual(minion.Status, api.NodeStatus{}) { + if !api.Semantic.DeepEqual(minion.Status, api.NodeStatus{}) { allErrs = append(allErrs, errs.NewFieldInvalid("status", minion.Status, "status must be empty")) } @@ -596,7 +595,7 @@ func ValidateMinionUpdate(oldMinion *api.Node, minion *api.Node) errs.Validation // Clear status oldMinion.Status = minion.Status - if !reflect.DeepEqual(oldMinion, minion) { + if !api.Semantic.DeepEqual(oldMinion, minion) { glog.V(4).Infof("Update failed validation %#v vs %#v", oldMinion, minion) allErrs = append(allErrs, fmt.Errorf("update contains more than labels or capacity changes")) } diff --git a/pkg/conversion/deep_equal_test.go b/pkg/conversion/deep_equal_test.go index 5d4a5dabbb7..d89ab53e5cf 100644 --- a/pkg/conversion/deep_equal_test.go +++ b/pkg/conversion/deep_equal_test.go @@ -22,15 +22,28 @@ import ( func TestEqualities(t *testing.T) { e := Equalities{} + type Bar struct { + X int + } + type Baz struct { + Y Bar + } err := e.AddFuncs( func(a, b int) bool { return a+1 == b }, + func(a, b Bar) bool { + return a.X*10 == b.X + }, ) if err != nil { t.Fatalf("Unexpected: %v", err) } + type Foo struct { + X int + } + table := []struct { a, b interface{} equal bool @@ -38,6 +51,10 @@ func TestEqualities(t *testing.T) { {1, 2, true}, {2, 1, false}, {"foo", "foo", true}, + {Foo{1}, Foo{2}, true}, + {Bar{1}, Bar{10}, true}, + {&Bar{1}, &Bar{10}, true}, + {Baz{Bar{1}}, Baz{Bar{10}}, true}, {map[string]int{"foo": 1}, map[string]int{"foo": 2}, true}, {map[string]int{}, map[string]int(nil), true}, {[]int{}, []int(nil), true},