Merge pull request #4094 from ctripcloud/fix-binding-status-error-rate
Use Patch() when binding-status controller update workload's status
This commit is contained in:
karmada-bot
GitHub
commit
33fdf5225f
|
|
@ -24,6 +24,7 @@ import (
|
|||
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/klog/v2"
|
||||
"sigs.k8s.io/controller-runtime/pkg/builder"
|
||||
|
|
@ -34,6 +35,7 @@ import (
|
|||
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/resourceinterpreter"
|
||||
"github.com/karmada-io/karmada/pkg/util/helper"
|
||||
"github.com/karmada-io/karmada/pkg/util/restmapper"
|
||||
)
|
||||
|
||||
|
|
@ -119,13 +121,24 @@ func updateResourceStatus(
|
|||
klog.Errorf("Failed to aggregate status for resource(%s/%s/%s, Error: %v", gvr, resource.GetNamespace(), resource.GetName(), err)
|
||||
return err
|
||||
}
|
||||
if reflect.DeepEqual(resource, newObj) {
|
||||
|
||||
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(context.TODO(), newObj, metav1.UpdateOptions{}); err != nil {
|
||||
klog.Errorf("Failed to update resource(%s/%s/%s), Error: %v", gvr, resource.GetNamespace(), resource.GetName(), err)
|
||||
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)
|
||||
return err
|
||||
}
|
||||
|
||||
_, err = dynamicClient.Resource(gvr).Namespace(resource.GetNamespace()).
|
||||
Patch(context.TODO(), resource.GetName(), types.JSONPatchType, patchBytes, metav1.PatchOptions{}, "status")
|
||||
if err != nil {
|
||||
klog.Error("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())
|
||||
|
|
|
|||
|
|
@ -19,10 +19,28 @@ 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
|
||||
|
|
@ -48,3 +66,40 @@ func GenMergePatch(originalObj interface{}, modifiedObj interface{}) ([]byte, er
|
|||
|
||||
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,8 +17,12 @@ 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"
|
||||
|
|
@ -122,3 +126,277 @@ 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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue