Merge pull request #5318 from chaosi-zju/fixpatch
fix expected patch operations may be missed when AggregateStatus
This commit is contained in:
karmada-bot
GitHub
commit
e7cb1332c5
|
|
@ -20,12 +20,15 @@ import (
|
|||
"context"
|
||||
"reflect"
|
||||
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
apierrors "k8s.io/apimachinery/pkg/api/errors"
|
||||
"k8s.io/apimachinery/pkg/api/meta"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
|
||||
"k8s.io/apimachinery/pkg/runtime/schema"
|
||||
"k8s.io/apimachinery/pkg/types"
|
||||
"k8s.io/client-go/dynamic"
|
||||
"k8s.io/client-go/tools/record"
|
||||
"k8s.io/client-go/util/retry"
|
||||
"k8s.io/klog/v2"
|
||||
"sigs.k8s.io/controller-runtime/pkg/builder"
|
||||
"sigs.k8s.io/controller-runtime/pkg/event"
|
||||
|
|
@ -34,8 +37,8 @@ import (
|
|||
configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1"
|
||||
workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1"
|
||||
workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
|
||||
"github.com/karmada-io/karmada/pkg/events"
|
||||
"github.com/karmada-io/karmada/pkg/resourceinterpreter"
|
||||
"github.com/karmada-io/karmada/pkg/util/helper"
|
||||
"github.com/karmada-io/karmada/pkg/util/restmapper"
|
||||
)
|
||||
|
||||
|
|
@ -105,43 +108,67 @@ func updateResourceStatus(
|
|||
dynamicClient dynamic.Interface,
|
||||
restMapper meta.RESTMapper,
|
||||
interpreter resourceinterpreter.ResourceInterpreter,
|
||||
resource *unstructured.Unstructured,
|
||||
eventRecorder record.EventRecorder,
|
||||
objRef workv1alpha2.ObjectReference,
|
||||
bindingStatus workv1alpha2.ResourceBindingStatus,
|
||||
) error {
|
||||
gvr, err := restmapper.GetGroupVersionResource(restMapper, schema.FromAPIVersionAndKind(resource.GetAPIVersion(), resource.GetKind()))
|
||||
gvr, err := restmapper.GetGroupVersionResource(restMapper, schema.FromAPIVersionAndKind(objRef.APIVersion, objRef.Kind))
|
||||
if err != nil {
|
||||
klog.Errorf("Failed to get GVR from GVK(%s/%s), Error: %v", resource.GetAPIVersion(), resource.GetKind(), err)
|
||||
klog.Errorf("Failed to get GVR from GVK(%s/%s), Error: %v", objRef.APIVersion, objRef.Kind, err)
|
||||
return err
|
||||
}
|
||||
|
||||
if !interpreter.HookEnabled(resource.GroupVersionKind(), configv1alpha1.InterpreterOperationAggregateStatus) {
|
||||
return nil
|
||||
}
|
||||
newObj, err := interpreter.AggregateStatus(resource, bindingStatus.AggregatedStatus)
|
||||
if err != nil {
|
||||
klog.Errorf("Failed to aggregate status for resource(%s/%s/%s, Error: %v", gvr, resource.GetNamespace(), resource.GetName(), err)
|
||||
return err
|
||||
}
|
||||
|
||||
oldStatus, _, _ := unstructured.NestedFieldNoCopy(resource.Object, "status")
|
||||
newStatus, _, _ := unstructured.NestedFieldNoCopy(newObj.Object, "status")
|
||||
if reflect.DeepEqual(oldStatus, newStatus) {
|
||||
klog.V(3).Infof("Ignore update resource(%s/%s/%s) status as up to date.", gvr, resource.GetNamespace(), resource.GetName())
|
||||
gvk := schema.GroupVersionKind{Group: gvr.Group, Version: gvr.Version, Kind: objRef.Kind}
|
||||
if !interpreter.HookEnabled(gvk, configv1alpha1.InterpreterOperationAggregateStatus) {
|
||||
return nil
|
||||
}
|
||||
|
||||
patchBytes, err := helper.GenReplaceFieldJSONPatch("/status", oldStatus, newStatus)
|
||||
if err != nil {
|
||||
klog.Errorf("Failed to gen patch bytes for resource(%s/%s/%s, Error: %v", gvr, resource.GetNamespace(), resource.GetName(), err)
|
||||
var resource *unstructured.Unstructured
|
||||
if err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
|
||||
// Fetch resource template from karmada-apiserver instead of informer cache, to avoid retry due to
|
||||
// resource conflict which often happens, especially with a huge amount of resource templates and
|
||||
// the informer cache doesn't sync quickly enough.
|
||||
// For more details refer to https://github.com/karmada-io/karmada/issues/5285.
|
||||
resource, err = dynamicClient.Resource(gvr).Namespace(objRef.Namespace).Get(ctx, objRef.Name, metav1.GetOptions{})
|
||||
if err != nil {
|
||||
if apierrors.IsNotFound(err) {
|
||||
// It might happen when the resource template has been removed but the garbage collector hasn't removed
|
||||
// the ResourceBinding which dependent on resource template.
|
||||
// So, just return without retry(requeue) would save unnecessary loop.
|
||||
return nil
|
||||
}
|
||||
klog.Errorf("Failed to fetch resource template(%s/%s/%s), Error: %v.", gvr, objRef.Namespace, objRef.Name, err)
|
||||
return err
|
||||
}
|
||||
|
||||
newObj, err := interpreter.AggregateStatus(resource, bindingStatus.AggregatedStatus)
|
||||
if err != nil {
|
||||
klog.Errorf("Failed to aggregate status for resource template(%s/%s/%s), Error: %v", gvr, resource.GetNamespace(), resource.GetName(), err)
|
||||
return err
|
||||
}
|
||||
|
||||
oldStatus, _, _ := unstructured.NestedFieldNoCopy(resource.Object, "status")
|
||||
newStatus, _, _ := unstructured.NestedFieldNoCopy(newObj.Object, "status")
|
||||
if reflect.DeepEqual(oldStatus, newStatus) {
|
||||
klog.V(3).Infof("Ignore update resource(%s/%s/%s) status as up to date.", gvr, resource.GetNamespace(), resource.GetName())
|
||||
return nil
|
||||
}
|
||||
|
||||
if _, err = dynamicClient.Resource(gvr).Namespace(resource.GetNamespace()).UpdateStatus(ctx, newObj, metav1.UpdateOptions{}); err != nil {
|
||||
klog.Errorf("Failed to update resource(%s/%s/%s), Error: %v", gvr, resource.GetNamespace(), resource.GetName(), err)
|
||||
return err
|
||||
}
|
||||
|
||||
eventRecorder.Event(resource, corev1.EventTypeNormal, events.EventReasonAggregateStatusSucceed, "Update Resource with AggregatedStatus successfully.")
|
||||
klog.V(3).Infof("Update resource(%s/%s/%s) status successfully.", gvr, resource.GetNamespace(), resource.GetName())
|
||||
|
||||
return nil
|
||||
}); err != nil {
|
||||
if resource != nil {
|
||||
eventRecorder.Event(resource, corev1.EventTypeWarning, events.EventReasonAggregateStatusFailed, err.Error())
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
_, err = dynamicClient.Resource(gvr).Namespace(resource.GetNamespace()).
|
||||
Patch(ctx, resource.GetName(), types.JSONPatchType, patchBytes, metav1.PatchOptions{}, "status")
|
||||
if err != nil {
|
||||
klog.Errorf("Failed to update resource(%s/%s/%s), Error: %v", gvr, resource.GetNamespace(), resource.GetName(), err)
|
||||
return err
|
||||
}
|
||||
klog.V(3).Infof("Update resource(%s/%s/%s) status successfully.", gvr, resource.GetNamespace(), resource.GetName())
|
||||
return nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -109,27 +109,14 @@ func (c *CRBStatusController) SetupWithManager(mgr controllerruntime.Manager) er
|
|||
}
|
||||
|
||||
func (c *CRBStatusController) syncBindingStatus(ctx context.Context, binding *workv1alpha2.ClusterResourceBinding) error {
|
||||
resource, err := helper.FetchResourceTemplate(ctx, c.DynamicClient, c.InformerManager, c.RESTMapper, binding.Spec.Resource)
|
||||
if err != nil {
|
||||
if apierrors.IsNotFound(err) {
|
||||
// It might happen when the resource template has been removed but the garbage collector hasn't removed
|
||||
// the ResourceBinding which dependent on resource template.
|
||||
// So, just return without retry(requeue) would save unnecessary loop.
|
||||
return nil
|
||||
}
|
||||
klog.Errorf("Failed to fetch workload for clusterResourceBinding(%s). Error: %v",
|
||||
binding.GetName(), err)
|
||||
return err
|
||||
}
|
||||
|
||||
err = helper.AggregateClusterResourceBindingWorkStatus(ctx, c.Client, binding, resource, c.EventRecorder)
|
||||
err := helper.AggregateClusterResourceBindingWorkStatus(ctx, c.Client, binding, c.EventRecorder)
|
||||
if err != nil {
|
||||
klog.Errorf("Failed to aggregate workStatues to clusterResourceBinding(%s), Error: %v",
|
||||
binding.Name, err)
|
||||
return err
|
||||
}
|
||||
|
||||
err = updateResourceStatus(ctx, c.DynamicClient, c.RESTMapper, c.ResourceInterpreter, resource, binding.Status)
|
||||
err = updateResourceStatus(ctx, c.DynamicClient, c.RESTMapper, c.ResourceInterpreter, c.EventRecorder, binding.Spec.Resource, binding.Status)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
|
|||
|
|
@ -111,29 +111,17 @@ func (c *RBStatusController) SetupWithManager(mgr controllerruntime.Manager) err
|
|||
}
|
||||
|
||||
func (c *RBStatusController) syncBindingStatus(ctx context.Context, binding *workv1alpha2.ResourceBinding) error {
|
||||
resourceTemplate, err := helper.FetchResourceTemplate(ctx, c.DynamicClient, c.InformerManager, c.RESTMapper, binding.Spec.Resource)
|
||||
err := helper.AggregateResourceBindingWorkStatus(ctx, c.Client, binding, c.EventRecorder)
|
||||
if err != nil {
|
||||
if apierrors.IsNotFound(err) {
|
||||
// It might happen when the resource template has been removed but the garbage collector hasn't removed
|
||||
// the ResourceBinding which dependent on resource template.
|
||||
// So, just return without retry(requeue) would save unnecessary loop.
|
||||
return nil
|
||||
}
|
||||
klog.Errorf("Failed to fetch workload for resourceBinding(%s/%s). Error: %v.",
|
||||
binding.GetNamespace(), binding.GetName(), err)
|
||||
return err
|
||||
}
|
||||
|
||||
err = helper.AggregateResourceBindingWorkStatus(ctx, c.Client, binding, resourceTemplate, c.EventRecorder)
|
||||
if err != nil {
|
||||
klog.Errorf("Failed to aggregate workStatues to resourceBinding(%s/%s), Error: %v",
|
||||
klog.Errorf("Failed to aggregate workStatus to resourceBinding(%s/%s), Error: %v",
|
||||
binding.Namespace, binding.Name, err)
|
||||
return err
|
||||
}
|
||||
|
||||
err = updateResourceStatus(ctx, c.DynamicClient, c.RESTMapper, c.ResourceInterpreter, resourceTemplate, binding.Status)
|
||||
err = updateResourceStatus(ctx, c.DynamicClient, c.RESTMapper, c.ResourceInterpreter, c.EventRecorder, binding.Spec.Resource, binding.Status)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -390,7 +390,9 @@ func (c *WorkStatusController) reflectStatus(ctx context.Context, work *workv1al
|
|||
}
|
||||
|
||||
func (c *WorkStatusController) buildStatusIdentifier(work *workv1alpha1.Work, clusterObj *unstructured.Unstructured) (*workv1alpha1.ResourceIdentifier, error) {
|
||||
ordinal, err := helper.GetManifestIndex(work.Spec.Workload.Manifests, clusterObj)
|
||||
manifestRef := helper.ManifestReference{APIVersion: clusterObj.GetAPIVersion(), Kind: clusterObj.GetKind(),
|
||||
Namespace: clusterObj.GetNamespace(), Name: clusterObj.GetName()}
|
||||
ordinal, err := helper.GetManifestIndex(work.Spec.Workload.Manifests, &manifestRef)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
|
|
|||
|
|
@ -19,28 +19,10 @@ package helper
|
|||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"reflect"
|
||||
|
||||
jsonpatch "github.com/evanphx/json-patch/v5"
|
||||
)
|
||||
|
||||
// RFC6902 JSONPatch operations
|
||||
const (
|
||||
JSONPatchOPAdd = "add"
|
||||
JSONPatchOPReplace = "replace"
|
||||
JSONPatchOPRemove = "remove"
|
||||
JSONPatchOPMove = "move"
|
||||
JSONPatchOPCopy = "copy"
|
||||
JSONPatchOPTest = "test"
|
||||
)
|
||||
|
||||
type jsonPatch struct {
|
||||
OP string `json:"op"`
|
||||
From string `json:"from,omitempty"`
|
||||
Path string `json:"path"`
|
||||
Value interface{} `json:"value,omitempty"`
|
||||
}
|
||||
|
||||
// GenMergePatch will return a merge patch document capable of converting the
|
||||
// original object to the modified object.
|
||||
// The merge patch format is primarily intended for use with the HTTP PATCH method
|
||||
|
|
@ -84,40 +66,3 @@ func GenFieldMergePatch(fieldName string, originField interface{}, modifiedField
|
|||
patchBytes = append([]byte(`{"`+fieldName+`":`), patchBytes...)
|
||||
return patchBytes, nil
|
||||
}
|
||||
|
||||
// GenReplaceFieldJSONPatch returns the RFC6902 JSONPatch array as []byte, which is used to simply
|
||||
// add/replace/delete certain JSON **Object** field.
|
||||
func GenReplaceFieldJSONPatch(path string, originalFieldValue, newFieldValue interface{}) ([]byte, error) {
|
||||
if reflect.DeepEqual(originalFieldValue, newFieldValue) {
|
||||
return nil, nil
|
||||
}
|
||||
if newFieldValue == nil {
|
||||
return GenJSONPatch(JSONPatchOPRemove, "", path, nil)
|
||||
}
|
||||
// The implementation of "add" and "replace" for JSON objects is actually the same
|
||||
// in "github.com/evanphx/json-patch/v5", which is used by Karmada and K8s.
|
||||
// We implemented it here just to follow the RFC6902.
|
||||
if originalFieldValue == nil {
|
||||
return GenJSONPatch(JSONPatchOPAdd, "", path, newFieldValue)
|
||||
}
|
||||
return GenJSONPatch(JSONPatchOPReplace, "", path, newFieldValue)
|
||||
}
|
||||
|
||||
// GenJSONPatch return JSONPatch array as []byte according to RFC6902
|
||||
func GenJSONPatch(op, from, path string, value interface{}) ([]byte, error) {
|
||||
jp := jsonPatch{
|
||||
OP: op,
|
||||
Path: path,
|
||||
}
|
||||
switch op {
|
||||
case JSONPatchOPAdd, JSONPatchOPReplace, JSONPatchOPTest:
|
||||
jp.Value = value
|
||||
case JSONPatchOPMove, JSONPatchOPCopy:
|
||||
jp.From = from
|
||||
case JSONPatchOPRemove:
|
||||
default:
|
||||
return nil, fmt.Errorf("unrecognized JSONPatch OP: %s", op)
|
||||
}
|
||||
|
||||
return json.Marshal([]jsonPatch{jp})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,12 +17,8 @@ limitations under the License.
|
|||
package helper
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"math"
|
||||
"testing"
|
||||
|
||||
appsv1 "k8s.io/api/apps/v1"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
|
||||
workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
|
||||
|
|
@ -126,277 +122,3 @@ func TestGenMergePatch(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGenJSONPatch(t *testing.T) {
|
||||
type args struct {
|
||||
op string
|
||||
from string
|
||||
path string
|
||||
value interface{}
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
args args
|
||||
want string
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "add object field",
|
||||
args: args{
|
||||
op: "add",
|
||||
path: "/abc",
|
||||
value: 1,
|
||||
},
|
||||
want: `[{"op":"add","path":"/abc","value":1}]`,
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "replace object field",
|
||||
args: args{
|
||||
op: "replace",
|
||||
path: "/abc",
|
||||
value: 1,
|
||||
},
|
||||
want: `[{"op":"replace","path":"/abc","value":1}]`,
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "remove object field, redundant args will be ignored",
|
||||
args: args{
|
||||
op: "remove",
|
||||
from: "123",
|
||||
path: "/abc",
|
||||
value: 1,
|
||||
},
|
||||
want: `[{"op":"remove","path":"/abc"}]`,
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "move object field",
|
||||
args: args{
|
||||
op: "move",
|
||||
from: "/abc",
|
||||
path: "/123",
|
||||
},
|
||||
want: `[{"op":"move","from":"/abc","path":"/123"}]`,
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "copy object field, redundant array value will be ignored",
|
||||
args: args{
|
||||
op: "copy",
|
||||
from: "/123",
|
||||
path: "/abc",
|
||||
value: []interface{}{1, "a", false, 4.5},
|
||||
},
|
||||
want: `[{"op":"copy","from":"/123","path":"/abc"}]`,
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "replace object field, input string typed number",
|
||||
args: args{
|
||||
op: "replace",
|
||||
path: "/abc",
|
||||
value: "1",
|
||||
},
|
||||
want: `[{"op":"replace","path":"/abc","value":"1"}]`,
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "replace object field, input invalid type",
|
||||
args: args{
|
||||
op: "replace",
|
||||
path: "/abc",
|
||||
value: make(chan int),
|
||||
},
|
||||
want: "",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "replace object field, input invalid value",
|
||||
args: args{
|
||||
op: "replace",
|
||||
path: "/abc",
|
||||
value: math.Inf(1),
|
||||
},
|
||||
want: "",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "replace object field, input struct value",
|
||||
args: args{
|
||||
op: "replace",
|
||||
path: "/abc",
|
||||
value: struct {
|
||||
A string
|
||||
B int
|
||||
C float64
|
||||
D bool
|
||||
}{"a", 1, 1.2, true},
|
||||
},
|
||||
want: `[{"op":"replace","path":"/abc","value":{"A":"a","B":1,"C":1.2,"D":true}}]`,
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "test object field, input array value",
|
||||
args: args{
|
||||
op: "test",
|
||||
path: "/abc",
|
||||
value: []interface{}{1, "a", false, 4.5},
|
||||
},
|
||||
want: `[{"op":"test","path":"/abc","value":[1,"a",false,4.5]}]`,
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "move object field, input invalid path, but we won't verify it",
|
||||
args: args{
|
||||
op: "move",
|
||||
from: "123",
|
||||
path: "abc",
|
||||
},
|
||||
want: `[{"op":"move","from":"123","path":"abc"}]`,
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "input invalid op",
|
||||
args: args{
|
||||
op: "whatever",
|
||||
path: "/abc",
|
||||
},
|
||||
want: "",
|
||||
wantErr: true,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
patchBytes, err := GenJSONPatch(tt.args.op, tt.args.from, tt.args.path, tt.args.value)
|
||||
if tt.wantErr != (err != nil) {
|
||||
t.Errorf("wantErr: %v, but got err: %v", tt.wantErr, err)
|
||||
}
|
||||
if tt.want != string(patchBytes) {
|
||||
t.Errorf("want: %s, but got: %s", tt.want, patchBytes)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGenReplaceFieldJSONPatch(t *testing.T) {
|
||||
originalObj := &appsv1.Deployment{Status: appsv1.DeploymentStatus{
|
||||
ObservedGeneration: 1,
|
||||
Replicas: 1,
|
||||
UpdatedReplicas: 1,
|
||||
ReadyReplicas: 1,
|
||||
AvailableReplicas: 1,
|
||||
}}
|
||||
newObj := originalObj.DeepCopy()
|
||||
newObj.Status = appsv1.DeploymentStatus{
|
||||
ObservedGeneration: 2,
|
||||
Replicas: 2,
|
||||
UpdatedReplicas: 2,
|
||||
ReadyReplicas: 2,
|
||||
AvailableReplicas: 2,
|
||||
}
|
||||
newStatusJSON, _ := json.Marshal(newObj.Status)
|
||||
pathStatus := "/status"
|
||||
type args struct {
|
||||
path string
|
||||
originalFieldValue interface{}
|
||||
newFieldValue interface{}
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
args args
|
||||
want func() ([]byte, error)
|
||||
}{
|
||||
{
|
||||
name: "return nil when no patch is needed",
|
||||
args: args{
|
||||
path: pathStatus,
|
||||
originalFieldValue: originalObj.Status,
|
||||
newFieldValue: originalObj.Status,
|
||||
},
|
||||
want: func() ([]byte, error) {
|
||||
return nil, nil
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "return add JSONPatch when field in original obj is nil",
|
||||
args: args{
|
||||
path: pathStatus,
|
||||
originalFieldValue: nil,
|
||||
newFieldValue: newObj.Status,
|
||||
},
|
||||
want: func() ([]byte, error) {
|
||||
return GenJSONPatch(JSONPatchOPAdd, "", pathStatus, newObj.Status)
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "e2e return add JSONPatch when field in original obj is nil",
|
||||
args: args{
|
||||
path: pathStatus,
|
||||
originalFieldValue: nil,
|
||||
newFieldValue: newObj.Status,
|
||||
},
|
||||
want: func() ([]byte, error) {
|
||||
return []byte(fmt.Sprintf(`[{"op":"add","path":"%s","value":%s}]`, pathStatus, newStatusJSON)), nil
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "return replace JSONPatch when field in original obj in non-nil, whatever what's in the original field",
|
||||
args: args{
|
||||
path: pathStatus,
|
||||
originalFieldValue: originalObj.Status,
|
||||
newFieldValue: newObj.Status,
|
||||
},
|
||||
want: func() ([]byte, error) {
|
||||
return GenJSONPatch(JSONPatchOPReplace, "", pathStatus, newObj.Status)
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "e2e return replace JSONPatch when field in original obj in non-nil, whatever what's in the original field",
|
||||
args: args{
|
||||
path: pathStatus,
|
||||
originalFieldValue: originalObj.Status,
|
||||
newFieldValue: newObj.Status,
|
||||
},
|
||||
want: func() ([]byte, error) {
|
||||
return []byte(fmt.Sprintf(`[{"op":"replace","path":"%s","value":%s}]`, pathStatus, newStatusJSON)), nil
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "return remove JSONPatch when field in new obj is nil",
|
||||
args: args{
|
||||
path: pathStatus,
|
||||
originalFieldValue: originalObj.Status,
|
||||
newFieldValue: nil,
|
||||
},
|
||||
want: func() ([]byte, error) {
|
||||
return GenJSONPatch(JSONPatchOPRemove, "", pathStatus, nil)
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "e2e return remove JSONPatch when field in new obj is nil",
|
||||
args: args{
|
||||
path: pathStatus,
|
||||
originalFieldValue: originalObj.Status,
|
||||
newFieldValue: nil,
|
||||
},
|
||||
want: func() ([]byte, error) {
|
||||
return []byte(fmt.Sprintf(`[{"op":"remove","path":"%s"}]`, pathStatus)), nil
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got, err := GenReplaceFieldJSONPatch(tt.args.path, tt.args.originalFieldValue, tt.args.newFieldValue)
|
||||
want, wantErr := tt.want()
|
||||
|
||||
if fmt.Sprint(wantErr) != fmt.Sprint(err) {
|
||||
t.Errorf("wantErr: %s, but got err: %s", fmt.Sprint(wantErr), fmt.Sprint(err))
|
||||
}
|
||||
if string(want) != string(got) {
|
||||
t.Errorf("\nwant: %s\nbut got: %s\n", want, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -59,7 +59,6 @@ func AggregateResourceBindingWorkStatus(
|
|||
ctx context.Context,
|
||||
c client.Client,
|
||||
binding *workv1alpha2.ResourceBinding,
|
||||
resourceTemplate *unstructured.Unstructured,
|
||||
eventRecorder record.EventRecorder,
|
||||
) error {
|
||||
workList, err := GetWorksByBindingID(ctx, c, binding.Labels[workv1alpha2.ResourceBindingPermanentIDLabel], true)
|
||||
|
|
@ -67,7 +66,7 @@ func AggregateResourceBindingWorkStatus(
|
|||
return err
|
||||
}
|
||||
|
||||
aggregatedStatuses, err := assembleWorkStatus(workList.Items, resourceTemplate)
|
||||
aggregatedStatuses, err := assembleWorkStatus(workList.Items, binding.Spec.Resource)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
@ -85,14 +84,12 @@ func AggregateResourceBindingWorkStatus(
|
|||
return err
|
||||
}); err != nil {
|
||||
eventRecorder.Event(binding, corev1.EventTypeWarning, events.EventReasonAggregateStatusFailed, err.Error())
|
||||
eventRecorder.Event(resourceTemplate, corev1.EventTypeWarning, events.EventReasonAggregateStatusFailed, err.Error())
|
||||
return err
|
||||
}
|
||||
|
||||
if operationResult == controllerutil.OperationResultUpdatedStatusOnly {
|
||||
msg := fmt.Sprintf("Update ResourceBinding(%s/%s) with AggregatedStatus successfully.", binding.Namespace, binding.Name)
|
||||
eventRecorder.Event(binding, corev1.EventTypeNormal, events.EventReasonAggregateStatusSucceed, msg)
|
||||
eventRecorder.Event(resourceTemplate, corev1.EventTypeNormal, events.EventReasonAggregateStatusSucceed, msg)
|
||||
} else {
|
||||
klog.Infof("New aggregatedStatuses are equal with old ResourceBinding(%s/%s) AggregatedStatus, no update required.", binding.Namespace, binding.Name)
|
||||
}
|
||||
|
|
@ -105,7 +102,6 @@ func AggregateClusterResourceBindingWorkStatus(
|
|||
ctx context.Context,
|
||||
c client.Client,
|
||||
binding *workv1alpha2.ClusterResourceBinding,
|
||||
resourceTemplate *unstructured.Unstructured,
|
||||
eventRecorder record.EventRecorder,
|
||||
) error {
|
||||
workList, err := GetWorksByBindingID(ctx, c, binding.Labels[workv1alpha2.ClusterResourceBindingPermanentIDLabel], false)
|
||||
|
|
@ -113,7 +109,7 @@ func AggregateClusterResourceBindingWorkStatus(
|
|||
return err
|
||||
}
|
||||
|
||||
aggregatedStatuses, err := assembleWorkStatus(workList.Items, resourceTemplate)
|
||||
aggregatedStatuses, err := assembleWorkStatus(workList.Items, binding.Spec.Resource)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
@ -131,14 +127,12 @@ func AggregateClusterResourceBindingWorkStatus(
|
|||
return err
|
||||
}); err != nil {
|
||||
eventRecorder.Event(binding, corev1.EventTypeWarning, events.EventReasonAggregateStatusFailed, err.Error())
|
||||
eventRecorder.Event(resourceTemplate, corev1.EventTypeWarning, events.EventReasonAggregateStatusFailed, err.Error())
|
||||
return err
|
||||
}
|
||||
|
||||
if operationResult == controllerutil.OperationResultUpdatedStatusOnly {
|
||||
msg := fmt.Sprintf("Update ClusterResourceBinding(%s) with AggregatedStatus successfully.", binding.Name)
|
||||
eventRecorder.Event(binding, corev1.EventTypeNormal, events.EventReasonAggregateStatusSucceed, msg)
|
||||
eventRecorder.Event(resourceTemplate, corev1.EventTypeNormal, events.EventReasonAggregateStatusSucceed, msg)
|
||||
} else {
|
||||
klog.Infof("New aggregatedStatuses are equal with old ClusterResourceBinding(%s) AggregatedStatus, no update required.", binding.Name)
|
||||
}
|
||||
|
|
@ -155,14 +149,15 @@ func generateFullyAppliedCondition(spec workv1alpha2.ResourceBindingSpec, aggreg
|
|||
}
|
||||
|
||||
// assemble workStatuses from workList which list by selector and match with workload.
|
||||
func assembleWorkStatus(works []workv1alpha1.Work, workload *unstructured.Unstructured) ([]workv1alpha2.AggregatedStatusItem, error) {
|
||||
func assembleWorkStatus(works []workv1alpha1.Work, objRef workv1alpha2.ObjectReference) ([]workv1alpha2.AggregatedStatusItem, error) {
|
||||
statuses := make([]workv1alpha2.AggregatedStatusItem, 0)
|
||||
for _, work := range works {
|
||||
if !work.DeletionTimestamp.IsZero() {
|
||||
continue
|
||||
}
|
||||
|
||||
identifierIndex, err := GetManifestIndex(work.Spec.Workload.Manifests, workload)
|
||||
manifestRef := ManifestReference{APIVersion: objRef.APIVersion, Kind: objRef.Kind, Namespace: objRef.Namespace, Name: objRef.Name}
|
||||
identifierIndex, err := GetManifestIndex(work.Spec.Workload.Manifests, &manifestRef)
|
||||
if err != nil {
|
||||
klog.Errorf("Failed to get manifestIndex of workload in work.Spec.Workload.Manifests. Error: %v.", err)
|
||||
return nil, err
|
||||
|
|
@ -208,7 +203,7 @@ func assembleWorkStatus(works []workv1alpha1.Work, workload *unstructured.Unstru
|
|||
}
|
||||
|
||||
for i := range work.Status.ManifestStatuses {
|
||||
equal, err := equalIdentifier(&work.Status.ManifestStatuses[i].Identifier, identifierIndex, workload)
|
||||
equal, err := equalIdentifier(&work.Status.ManifestStatuses[i].Identifier, identifierIndex, &manifestRef)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
|
@ -227,18 +222,25 @@ func assembleWorkStatus(works []workv1alpha1.Work, workload *unstructured.Unstru
|
|||
return statuses, nil
|
||||
}
|
||||
|
||||
// ManifestReference identifies an object in manifest list
|
||||
type ManifestReference struct {
|
||||
APIVersion string
|
||||
Kind string
|
||||
Namespace string
|
||||
Name string
|
||||
}
|
||||
|
||||
// GetManifestIndex gets the index of clusterObj in manifest list, if not exist return -1.
|
||||
func GetManifestIndex(manifests []workv1alpha1.Manifest, clusterObj *unstructured.Unstructured) (int, error) {
|
||||
func GetManifestIndex(manifests []workv1alpha1.Manifest, manifestRef *ManifestReference) (int, error) {
|
||||
for index, rawManifest := range manifests {
|
||||
manifest := &unstructured.Unstructured{}
|
||||
if err := manifest.UnmarshalJSON(rawManifest.Raw); err != nil {
|
||||
return -1, err
|
||||
}
|
||||
|
||||
if manifest.GetAPIVersion() == clusterObj.GetAPIVersion() &&
|
||||
manifest.GetKind() == clusterObj.GetKind() &&
|
||||
manifest.GetNamespace() == clusterObj.GetNamespace() &&
|
||||
manifest.GetName() == clusterObj.GetName() {
|
||||
if manifest.GetAPIVersion() == manifestRef.APIVersion &&
|
||||
manifest.GetKind() == manifestRef.Kind &&
|
||||
manifest.GetNamespace() == manifestRef.Namespace &&
|
||||
manifest.GetName() == manifestRef.Name {
|
||||
return index, nil
|
||||
}
|
||||
}
|
||||
|
|
@ -246,8 +248,8 @@ func GetManifestIndex(manifests []workv1alpha1.Manifest, clusterObj *unstructure
|
|||
return -1, fmt.Errorf("no such manifest exist")
|
||||
}
|
||||
|
||||
func equalIdentifier(targetIdentifier *workv1alpha1.ResourceIdentifier, ordinal int, workload *unstructured.Unstructured) (bool, error) {
|
||||
groupVersion, err := schema.ParseGroupVersion(workload.GetAPIVersion())
|
||||
func equalIdentifier(targetIdentifier *workv1alpha1.ResourceIdentifier, ordinal int, manifestRef *ManifestReference) (bool, error) {
|
||||
groupVersion, err := schema.ParseGroupVersion(manifestRef.APIVersion)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
|
@ -255,9 +257,9 @@ func equalIdentifier(targetIdentifier *workv1alpha1.ResourceIdentifier, ordinal
|
|||
if targetIdentifier.Ordinal == ordinal &&
|
||||
targetIdentifier.Group == groupVersion.Group &&
|
||||
targetIdentifier.Version == groupVersion.Version &&
|
||||
targetIdentifier.Kind == workload.GetKind() &&
|
||||
targetIdentifier.Namespace == workload.GetNamespace() &&
|
||||
targetIdentifier.Name == workload.GetName() {
|
||||
targetIdentifier.Kind == manifestRef.Kind &&
|
||||
targetIdentifier.Namespace == manifestRef.Namespace &&
|
||||
targetIdentifier.Name == manifestRef.Name {
|
||||
return true, nil
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -192,7 +192,9 @@ func TestGetManifestIndex(t *testing.T) {
|
|||
}
|
||||
|
||||
t.Run("Service", func(t *testing.T) {
|
||||
index, err := GetManifestIndex(manifests, service)
|
||||
manifestRef := ManifestReference{APIVersion: service.GetAPIVersion(), Kind: service.GetKind(),
|
||||
Namespace: service.GetNamespace(), Name: service.GetName()}
|
||||
index, err := GetManifestIndex(manifests, &manifestRef)
|
||||
if err != nil {
|
||||
t.Errorf("unexpected error: %v", err)
|
||||
}
|
||||
|
|
@ -202,7 +204,9 @@ func TestGetManifestIndex(t *testing.T) {
|
|||
})
|
||||
|
||||
t.Run("Deployment", func(t *testing.T) {
|
||||
index, err := GetManifestIndex(manifests, deployment)
|
||||
manifestRef := ManifestReference{APIVersion: deployment.GetAPIVersion(), Kind: deployment.GetKind(),
|
||||
Namespace: deployment.GetNamespace(), Name: deployment.GetName()}
|
||||
index, err := GetManifestIndex(manifests, &manifestRef)
|
||||
if err != nil {
|
||||
t.Errorf("unexpected error: %v", err)
|
||||
}
|
||||
|
|
@ -212,7 +216,7 @@ func TestGetManifestIndex(t *testing.T) {
|
|||
})
|
||||
|
||||
t.Run("No match", func(t *testing.T) {
|
||||
_, err := GetManifestIndex(manifests, &unstructured.Unstructured{})
|
||||
_, err := GetManifestIndex(manifests, &ManifestReference{})
|
||||
if err == nil {
|
||||
t.Errorf("expected error, got nil")
|
||||
}
|
||||
|
|
@ -224,7 +228,7 @@ func TestEqualIdentifier(t *testing.T) {
|
|||
name string
|
||||
target *workv1alpha1.ResourceIdentifier
|
||||
ordinal int
|
||||
workload *unstructured.Unstructured
|
||||
workload *ManifestReference
|
||||
expectedOutput bool
|
||||
}{
|
||||
{
|
||||
|
|
@ -238,15 +242,11 @@ func TestEqualIdentifier(t *testing.T) {
|
|||
Name: "test-deployment",
|
||||
},
|
||||
ordinal: 0,
|
||||
workload: &unstructured.Unstructured{
|
||||
Object: map[string]interface{}{
|
||||
"apiVersion": "apps/v1",
|
||||
"kind": "Deployment",
|
||||
"metadata": map[string]interface{}{
|
||||
"namespace": "default",
|
||||
"name": "test-deployment",
|
||||
},
|
||||
},
|
||||
workload: &ManifestReference{
|
||||
APIVersion: "apps/v1",
|
||||
Kind: "Deployment",
|
||||
Namespace: "default",
|
||||
Name: "test-deployment",
|
||||
},
|
||||
expectedOutput: true,
|
||||
},
|
||||
|
|
@ -261,15 +261,11 @@ func TestEqualIdentifier(t *testing.T) {
|
|||
Name: "test-deployment",
|
||||
},
|
||||
ordinal: 0,
|
||||
workload: &unstructured.Unstructured{
|
||||
Object: map[string]interface{}{
|
||||
"apiVersion": "apps/v1",
|
||||
"kind": "Deployment",
|
||||
"metadata": map[string]interface{}{
|
||||
"namespace": "default",
|
||||
"name": "test-deployment",
|
||||
},
|
||||
},
|
||||
workload: &ManifestReference{
|
||||
APIVersion: "apps/v1",
|
||||
Kind: "Deployment",
|
||||
Namespace: "default",
|
||||
Name: "test-deployment",
|
||||
},
|
||||
expectedOutput: false,
|
||||
},
|
||||
|
|
|
|||
Loading…
Reference in New Issue