Repository: clusternet/clusternet
Pull request #437 (2022-07-29 12:21:59)

using patch instead of UpdateStatus to prevent conflict
Signed-off-by: Garrybest <garrybest@foxmail.com>

<!--  Thanks for sending a pull request!

-->

#### What type of PR is this?

#### What this PR does / why we need it:

#### Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes #

#### Special notes for your reviewer:


codecov-commenter: # [Codecov](https://codecov.io/gh/clusternet/clusternet/pull/437?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) Report
> Merging [#437](https://codecov.io/gh/clusternet/clusternet/pull/437?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) (2530d9d) into [main](https://codecov.io/gh/clusternet/clusternet/commit/73325a49db0175bbc557e435a40c0cd71f33dab7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) (73325a4) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.

```diff
@@            Coverage Diff             @@
##             main     #437      +/-   ##
==========================================
+ Coverage   13.72%   13.74%   +0.02%     
==========================================
  Files          61       61              
  Lines        6595     6607      +12     
==========================================
+ Hits          905      908       +3     
- Misses       5623     5629       +6     
- Partials       67       70       +3     
```


| [Impacted Files](https://codecov.io/gh/clusternet/clusternet/pull/437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) | Coverage Δ | |
|---|---|---|
| [.../framework/plugins/defaultbinder/default\_binder.go](https://codecov.io/gh/clusternet/clusternet/pull/437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet#diff-c3JjL2dpdGh1Yi5jb20vY2x1c3Rlcm5ldC9jbHVzdGVybmV0L3BrZy9zY2hlZHVsZXIvZnJhbWV3b3JrL3BsdWdpbnMvZGVmYXVsdGJpbmRlci9kZWZhdWx0X2JpbmRlci5nbw==) | `61.76% <0.00%> (-20.06%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/clusternet/clusternet/pull/437?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/clusternet/clusternet/pull/437?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet). Last update [73325a4...2530d9d](https://codecov.io/gh/clusternet/clusternet/pull/437?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet).


Patch:
diff --git a/pkg/scheduler/framework/plugins/defaultbinder/default_binder.go b/pkg/scheduler/framework/plugins/defaultbinder/default_binder.go
index 5bb5770..d209fc0 100644
--- a/pkg/scheduler/framework/plugins/defaultbinder/default_binder.go
+++ b/pkg/scheduler/framework/plugins/defaultbinder/default_binder.go
@@ -18,10 +18,14 @@ package defaultbinder
 
 import (
 	"context"
+	"encoding/json"
+	"fmt"
 	"sort"
 
+	jsonpatch "github.com/evanphx/json-patch"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog/v2"
 
 	appsapi "github.com/clusternet/clusternet/pkg/apis/apps/v1alpha1"
@@ -61,7 +65,20 @@ func (pl *DefaultBinder) Bind(ctx context.Context, state *framework.CycleState,
 	subCopy.Status.SpecHash = utils.HashSubscriptionSpec(&subCopy.Spec)
 	subCopy.Status.DesiredReleases = len(targetClusters.BindingClusters)
 
-	_, err := pl.handle.ClientSet().AppsV1alpha1().Subscriptions(sub.Namespace).UpdateStatus(ctx, subCopy, metav1.UpdateOptions{})
+	oldData, err := json.Marshal(sub)
+	if err != nil {
+		return framework.AsStatus(fmt.Errorf("failed to marshal original subscription: %v", err))
+	}
+	newData, err := json.Marshal(subCopy)
+	if err != nil {
+		return framework.AsStatus(fmt.Errorf("failed to marshal new subscription: %v", err))
+	}
+	patchBytes, err := jsonpatch.CreateMergePatch(oldData, newData)
+	if err != nil {
+		return framework.AsStatus(fmt.Errorf("failed to create a merge patch: %v", err))
+	}
+
+	_, err = pl.handle.ClientSet().AppsV1alpha1().Subscriptions(sub.Namespace).Patch(ctx, sub.Name, types.MergePatchType, patchBytes, metav1.PatchOptions{}, "status")
 	if err != nil {
 		return framework.AsStatus(err)
 	}


Test patch:
diff --git a/pkg/scheduler/framework/plugins/defaultbinder/default_binder_test.go b/pkg/scheduler/framework/plugins/defaultbinder/default_binder_test.go
index e17ace7..fb92f8f 100644
--- a/pkg/scheduler/framework/plugins/defaultbinder/default_binder_test.go
+++ b/pkg/scheduler/framework/plugins/defaultbinder/default_binder_test.go
@@ -60,21 +60,16 @@ func TestDefaultBinder(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			var bindedClusters []string
+			var boundClusters []string
 			client := fake.NewSimpleClientset(testSubscription)
-			client.PrependReactor("update", "subscriptions", func(action clienttesting.Action) (bool, runtime.Object, error) {
+			client.PrependReactor("patch", "subscriptions", func(action clienttesting.Action) (bool, runtime.Object, error) {
 				if action.GetSubresource() != "status" {
 					return false, nil, nil
 				}
 				if tt.injectErr != nil {
 					return true, nil, tt.injectErr
 				}
-
-				bindedSubscription := action.(clienttesting.UpdateAction).GetObject().(*appsapi.Subscription)
-				if bindedSubscription != nil {
-					bindedClusters = bindedSubscription.Status.BindingClusters
-				}
-				return true, bindedSubscription, nil
+				return false, nil, nil
 			})
 
 			fh, err := frameworkruntime.NewFramework(nil, nil, frameworkruntime.WithClientSet(client))
@@ -83,10 +78,15 @@ func TestDefaultBinder(t *testing.T) {
 			}
 			binder := &DefaultBinder{handle: fh}
 			status := binder.Bind(context.Background(), nil, testSubscription, testClusters)
+			gotSubscription, err := client.AppsV1alpha1().Subscriptions(testSubscription.Namespace).Get(context.Background(), testSubscription.Name, metav1.GetOptions{})
+			if err != nil {
+				t.Errorf("failed to get subscription: %v", err)
+			}
+			boundClusters = gotSubscription.Status.BindingClusters
 			if got := status.AsError(); (tt.injectErr != nil) != (got != nil) {
 				t.Errorf("got error %q, want %q", got, tt.injectErr)
 			}
-			if diff := cmp.Diff(tt.wantedBindings, bindedClusters); diff != "" {
+			if diff := cmp.Diff(tt.wantedBindings, boundClusters); diff != "" {
 				t.Errorf("got different binding (-want, +got): %s", diff)
 			}
 		})