Repository: clusternet/clusternet
Pull request #417 (2022-07-13 04:26:23)

remove ignored fileds during appying overrides
Signed-off-by: Di Xu <stephenhsu90@gmail.com>

<!--  Thanks for sending a pull request!

-->

#### What type of PR is this?
kind/enhancement

#### 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/417?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) Report
> Merging [#417](https://codecov.io/gh/clusternet/clusternet/pull/417?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) (7f61ff2) into [main](https://codecov.io/gh/clusternet/clusternet/commit/0c16db7ec81d10ff1ea5cf3d13810d4f7dfe17c1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) (0c16db7) will **increase** coverage by `0.01%`.
> The diff coverage is `n/a`.

```diff
@@            Coverage Diff             @@
##             main     #417      +/-   ##
==========================================
+ Coverage   13.62%   13.64%   +0.01%     
==========================================
  Files          60       60              
  Lines        6576     6574       -2     
==========================================
+ Hits          896      897       +1     
+ Misses       5613     5610       -3     
  Partials       67       67              
```


| [Impacted Files](https://codecov.io/gh/clusternet/clusternet/pull/417?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) | Coverage Δ | |
|---|---|---|
| [...usternet/clusternet/pkg/hub/localizer/localizer.go](https://codecov.io/gh/clusternet/clusternet/pull/417/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet#diff-c3JjL2dpdGh1Yi5jb20vY2x1c3Rlcm5ldC9jbHVzdGVybmV0L3BrZy9odWIvbG9jYWxpemVyL2xvY2FsaXplci5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ub.com/clusternet/clusternet/pkg/utils/deployer.go](https://codecov.io/gh/clusternet/clusternet/pull/417/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet#diff-c3JjL2dpdGh1Yi5jb20vY2x1c3Rlcm5ldC9jbHVzdGVybmV0L3BrZy91dGlscy9kZXBsb3llci5nbw==) | `6.56% <0.00%> (+0.03%)` | :arrow_up: |
| [...m/clusternet/clusternet/pkg/hub/localizer/patch.go](https://codecov.io/gh/clusternet/clusternet/pull/417/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet#diff-c3JjL2dpdGh1Yi5jb20vY2x1c3Rlcm5ldC9jbHVzdGVybmV0L3BrZy9odWIvbG9jYWxpemVyL3BhdGNoLmdv) | `56.33% <0.00%> (+0.62%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/clusternet/clusternet/pull/417?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/417?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet). Last update [0c16db7...7f61ff2](https://codecov.io/gh/clusternet/clusternet/pull/417?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/hub/localizer/localizer.go b/pkg/hub/localizer/localizer.go
index 38e973b..5bc8d86 100644
--- a/pkg/hub/localizer/localizer.go
+++ b/pkg/hub/localizer/localizer.go
@@ -49,7 +49,7 @@ var (
 	chartKind = appsapi.SchemeGroupVersion.WithKind("HelmChart")
 
 	// cannot override chart name and targetNamespace, remove them from the override
-	defaultChartOverrideConfig = []appsapi.OverrideConfig{
+	defaultChartOverrideConfigs = []appsapi.OverrideConfig{
 		{
 			Name:  "skip-overriding-chart-name",
 			Value: `[{"path":"/spec/chart","op":"remove"}]`,
@@ -61,6 +61,26 @@ var (
 			Type:  appsapi.JSONPatchType,
 		},
 	}
+
+	// removing ignored fields from serialized bytes, which can help reduce the size of Description objects and improve
+	// decoding performance
+	defaultOverrideConfigs = []appsapi.OverrideConfig{
+		{
+			Name:  "ignore-metadata-managedFields",
+			Value: `[{"path":"/metadata/managedFields","op":"remove"}]`,
+			Type:  appsapi.JSONPatchType,
+		},
+		{
+			Name:  "ignore-metadata-uid",
+			Value: `[{"path":"/metadata/uid","op":"remove"}]`,
+			Type:  appsapi.JSONPatchType,
+		},
+		{
+			Name:  "ignore-status",
+			Value: `[{"path":"/status","op":"remove"}]`,
+			Type:  appsapi.JSONPatchType,
+		},
+	}
 )
 
 // Localizer defines configuration for the application localization
@@ -299,7 +319,7 @@ func (l *Localizer) ApplyOverridesToDescription(desc *appsapi.Description) error
 			desc.Spec.Raw[idx] = genericResult
 
 			// apply default overrides for helm charts
-			chartOverrideResult, _, err = applyOverrides(chartOverrideResult, []byte(" "), defaultChartOverrideConfig)
+			chartOverrideResult, _, err = applyOverrides(chartOverrideResult, []byte(" "), defaultChartOverrideConfigs)
 			if err != nil {
 				allErrs = append(allErrs, err)
 				continue
diff --git a/pkg/hub/localizer/patch.go b/pkg/hub/localizer/patch.go
index 2a40d3a..f48cbcd 100644
--- a/pkg/hub/localizer/patch.go
+++ b/pkg/hub/localizer/patch.go
@@ -38,6 +38,7 @@ const (
 // `genericResult` the generic result of the overrides.
 // `chartResult` the helmchart result after applying the overrides.
 func applyOverrides(genericOriginal []byte, chartOriginal []byte, overrides []appsapi.OverrideConfig) ([]byte, []byte, error) {
+	overrides = append(overrides, defaultOverrideConfigs...)
 	genericResult, chartResult := genericOriginal, chartOriginal
 	for _, overrideConfig := range overrides {
 		// validates override value first
diff --git a/pkg/utils/deployer.go b/pkg/utils/deployer.go
index 339757e..39c3bac 100644
--- a/pkg/utils/deployer.go
+++ b/pkg/utils/deployer.go
@@ -491,9 +491,6 @@ func OffloadDescription(ctx context.Context, clusternetClient *clusternetclients
 
 func ApplyResourceWithRetry(ctx context.Context, dynamicClient dynamic.Interface, restMapper meta.RESTMapper,
 	resource *unstructured.Unstructured, ignoreAdd bool) error {
-	// set UID as empty
-	resource.SetUID("")
-
 	var lastError error
 	err := wait.ExponentialBackoffWithContext(ctx, retry.DefaultBackoff, func() (bool, error) {
 		restMapping, err := restMapper.RESTMapping(resource.GroupVersionKind().GroupKind(), resource.GroupVersionKind().Version)


Test patch:
diff --git a/pkg/hub/localizer/patch_test.go b/pkg/hub/localizer/patch_test.go
index baaf9b6..59041b5 100644
--- a/pkg/hub/localizer/patch_test.go
+++ b/pkg/hub/localizer/patch_test.go
@@ -271,7 +271,7 @@ hole: black
 				}
 			}`),
 			originalChart: []byte(``),
-			overrides:     defaultChartOverrideConfig,
+			overrides:     defaultChartOverrideConfigs,
 			want: []byte(`{
 				"apiVersion": "apps.clusternet.io/v1alpha1",
 				"kind": "HelmChart",
@@ -296,13 +296,56 @@ hole: black
 				"kind": "Pod",
 				"metadata": {
 					"name": "pod",
-					"labels": {"app": "nginx"}
+					"labels": {"app": "nginx"},
+					"uid": "1234-678",
+					"managedFields": [{
+					  "manager": "kubectl-client-side-apply",
+					  "operation": "Update",
+					  "apiVersion": "apps.clusternet.io/v1alpha1",
+					  "time": "2022-07-12T09:18:34Z",
+					  "fieldsType": "FieldsV1",
+					  "fieldsV1": {
+						"f:metadata": {
+						  "f:annotations": {
+							".": {},
+							"f:kubectl.kubernetes.io/last-applied-configuration": {}
+						  }
+						},
+						"f:spec": {
+						  ".": {},
+						  "f:chart": {},
+						  "f:repo": {},
+						  "f:targetNamespace": {},
+						  "f:version": {}
+						}
+					  }
+					}, {
+					  "manager": "clusternet-hub",
+					  "operation": "Update",
+					  "apiVersion": "apps.clusternet.io/v1alpha1",
+					  "time": "2022-07-12T09:24:39Z",
+					  "fieldsType": "FieldsV1",
+					  "fieldsV1": {
+						"f:status": {
+						  ".": {},
+						  "f:phase": {}
+						}
+					  },
+					  "subresource": "status"
+					}]
 				},
 				"spec": {
 					"containers": [{
 						"name":  "nginx",
 						"image": "nginx:latest"
 					}]
+				},
+				"status": {
+					"a": "b",
+					"some-value": [{
+						"key1":  "value1",
+						"key2": "value2"
+					}]
 				}
 			}`),
 			overrides: []appsapi.OverrideConfig{
@@ -430,14 +473,14 @@ func TestApplyJSONPatch(t *testing.T) {
 		{
 			name:          "remove nonexistent key (/spec/chart)",
 			cur:           []byte(`{"metadata":{"labels":{"another-label":"another-value","some-label":"some-value"}},"spec":{"version":"1.8.0"}}`),
-			overrideBytes: []byte(defaultChartOverrideConfig[0].Value),
+			overrideBytes: []byte(defaultChartOverrideConfigs[0].Value),
 			want:          []byte(`{"metadata":{"labels":{"another-label":"another-value","some-label":"some-value"}},"spec":{"version":"1.8.0"}}`),
 			wantErr:       false,
 		},
 		{
 			name:          "remove nonexistent key (/spec/targetNamespace)",
 			cur:           []byte(`{"metadata":{"labels":{"another-label":"another-value","some-label":"some-value"}},"spec":{"version":"1.8.0"}}`),
-			overrideBytes: []byte(defaultChartOverrideConfig[1].Value),
+			overrideBytes: []byte(defaultChartOverrideConfigs[1].Value),
 			want:          []byte(`{"metadata":{"labels":{"another-label":"another-value","some-label":"some-value"}},"spec":{"version":"1.8.0"}}`),
 			wantErr:       false,
 		},