Skip to content

Commit

Permalink
fix deep equal check failure on objects with runtime.RawExtension
Browse files Browse the repository at this point in the history
Signed-off-by: zach593 <[email protected]>
  • Loading branch information
zach593 committed Dec 12, 2024
1 parent a6df137 commit 4a15b13
Show file tree
Hide file tree
Showing 4 changed files with 195 additions and 0 deletions.
6 changes: 6 additions & 0 deletions cmd/agent/app/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"

"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/dynamic"
Expand Down Expand Up @@ -235,6 +236,11 @@ func run(ctx context.Context, opts *options.Options) error {
crtlmetrics.Registry.MustRegister(metrics.ResourceCollectorsForAgent()...)
crtlmetrics.Registry.MustRegister(metrics.PoolCollectors()...)

if err := util.RegisterEqualityCheckFunctions(&equality.Semantic); err != nil {
klog.Errorf("Failed to register equality check functions: %v", err)
return err
}

if err = setupControllers(controllerManager, opts, ctx.Done()); err != nil {
return err
}
Expand Down
6 changes: 6 additions & 0 deletions cmd/controller-manager/app/controllermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"time"

"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/discovery"
Expand Down Expand Up @@ -192,6 +193,11 @@ func Run(ctx context.Context, opts *options.Options) error {
crtlmetrics.Registry.MustRegister(metrics.ResourceCollectors()...)
crtlmetrics.Registry.MustRegister(metrics.PoolCollectors()...)

if err := util.RegisterEqualityCheckFunctions(&equality.Semantic); err != nil {
klog.Errorf("Failed to register equality check functions: %v", err)
return err
}

if err := helper.IndexWork(ctx, controllerManager); err != nil {
klog.Fatalf("Failed to index Work: %v", err)
}
Expand Down
73 changes: 73 additions & 0 deletions pkg/util/equality.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
Copyright 2024 The Karmada Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package util

import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/conversion"
"k8s.io/apimachinery/pkg/runtime"

workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1"
workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
)

// RegisterEqualityCheckFunctions registers custom check functions to the equality checker.
// These functions help avoid performing deep-equality checks on workloads represented as byte slices.
func RegisterEqualityCheckFunctions(e *conversion.Equalities) error {
return e.AddFuncs(
func(a, b workv1alpha1.Manifest) bool {
return rawExtensionDeepEqual(&a.RawExtension, &b.RawExtension, e)
},
func(a, b workv1alpha1.ManifestStatus) bool {
return e.DeepEqual(a.Identifier, b.Identifier) &&
rawExtensionDeepEqual(a.Status, b.Status, e) &&
e.DeepEqual(a.Health, b.Health)
},
func(a, b workv1alpha1.AggregatedStatusItem) bool {
return e.DeepEqual(a.ClusterName, b.ClusterName) &&
rawExtensionDeepEqual(a.Status, b.Status, e) &&
e.DeepEqual(a.Applied, b.Applied) &&
e.DeepEqual(a.AppliedMessage, b.AppliedMessage)
},
func(a, b workv1alpha2.AggregatedStatusItem) bool {
return e.DeepEqual(a.ClusterName, b.ClusterName) &&
rawExtensionDeepEqual(a.Status, b.Status, e) &&
e.DeepEqual(a.Applied, b.Applied) &&
e.DeepEqual(a.AppliedMessage, b.AppliedMessage)
},
)
}

func rawExtensionDeepEqual(a, b *runtime.RawExtension, checker *conversion.Equalities) bool {
if a == nil && b == nil {
return true
}
if a == nil || b == nil {
return false
}

var aObj, bObj unstructured.Unstructured
err := aObj.UnmarshalJSON(a.Raw)
if err != nil {
return false
}
err = bObj.UnmarshalJSON(b.Raw)
if err != nil {
return false
}
return checker.DeepEqual(aObj, bObj) && checker.DeepEqual(a.Object, b.Object)
}
110 changes: 110 additions & 0 deletions pkg/util/equality_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
Copyright 2024 The Karmada Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package util

import (
"encoding/json"
"testing"

"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"

workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1"
)

func TestRegisterEqualityCheckFunctions(t *testing.T) {
obj := &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
},
}
workloadWithCR, _ := obj.MarshalJSON()
workloadWithoutCR, _ := json.Marshal(obj)

tests := []struct {
name string
obj1 runtime.Object
obj2 runtime.Object
addCheckFunc bool
wantEqual bool
wantErr bool
}{
{
name: "without custom check functions",
obj1: &workv1alpha1.Work{
Spec: workv1alpha1.WorkSpec{
Workload: workv1alpha1.WorkloadTemplate{Manifests: []workv1alpha1.Manifest{{RawExtension: runtime.RawExtension{Raw: workloadWithCR}}}},
},
},
obj2: &workv1alpha1.Work{
Spec: workv1alpha1.WorkSpec{
Workload: workv1alpha1.WorkloadTemplate{Manifests: []workv1alpha1.Manifest{{RawExtension: runtime.RawExtension{Raw: workloadWithoutCR}}}},
},
},
addCheckFunc: false,
wantEqual: false,
wantErr: false,
},
{
name: "with custom check functions",
obj1: &workv1alpha1.Work{
Spec: workv1alpha1.WorkSpec{
Workload: workv1alpha1.WorkloadTemplate{Manifests: []workv1alpha1.Manifest{{RawExtension: runtime.RawExtension{Raw: workloadWithCR}}}},
},
},
obj2: &workv1alpha1.Work{
Spec: workv1alpha1.WorkSpec{
Workload: workv1alpha1.WorkloadTemplate{Manifests: []workv1alpha1.Manifest{{RawExtension: runtime.RawExtension{Raw: workloadWithoutCR}}}},
},
},
addCheckFunc: true,
wantEqual: true,
wantErr: false,
},
{
name: "custom check functions should be able to notice the difference",
obj1: &workv1alpha1.Work{
Spec: workv1alpha1.WorkSpec{
Workload: workv1alpha1.WorkloadTemplate{Manifests: []workv1alpha1.Manifest{{RawExtension: runtime.RawExtension{Raw: workloadWithCR}}}},
},
},
obj2: &workv1alpha1.Work{
Spec: workv1alpha1.WorkSpec{
Workload: workv1alpha1.WorkloadTemplate{Manifests: []workv1alpha1.Manifest{{RawExtension: runtime.RawExtension{Raw: nil}}}},
},
},
addCheckFunc: true,
wantEqual: false,
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
checker := equality.Semantic.Copy()
if tt.addCheckFunc {
if err := RegisterEqualityCheckFunctions(&checker); (err != nil) != tt.wantErr {
t.Errorf("XXX() error = %v, wantErr %v", err, tt.wantErr)
}
}
if equal := checker.DeepEqual(tt.obj1, tt.obj2); equal != tt.wantEqual {
t.Errorf("XXX() = %v, want %v", equal, tt.wantEqual)
}
})
}
}

0 comments on commit 4a15b13

Please sign in to comment.