Update with more comments

Signed-off-by: Jian Qiu <[email protected]>
open-cluster-management-io · Nov 25, 2024 · e0b390b · e0b390b
1 parent c9f8bdf
commit e0b390b
Showing 1 changed file with 66 additions and 52 deletions.
diff --git a/enhancements/sig-architecture/132-work-update-only-when-spec-change/README.md b/enhancements/sig-architecture/132-work-update-only-when-spec-change/README.md
@@ -38,76 +38,77 @@ set.
 
 ## Proposal
 
-We would like to introduce new options in `Update` and `ServerSideApply` update strategy to resolve the above issue.
-So user can set the option in the manifestwork, so when another actor on the spoke cluster update the resource. The
+We would like to introduce new options and `ServerSideApply` update strategy to resolve the above issue.
+The user can set the option in the manifestwork, so when another actor on the spoke cluster update the resource. The
 work-agent will ignore the change and not try to change the resource back. On the other hand, when the resource spec
 defined in the manifestwork is updated by the user, the work-agent will still update the resource accordingly. In
-summary, the option is to ignore the change triggered from spoke cluster only.
+summary, the option is to ignore the change triggered from spoke cluster. 
 
-In addition, in `ServerSideApplly` strategy, we would also introduce a new `ignoreDifferences` options similar as what
+We would also introduce a new `ignoreFields` options similar as what
 is defined in argoCD (https://argo-cd.readthedocs.io/en/stable/user-guide/diffing/). Such that user can choose to not
-let work-agent patch certain resource fields.
+let work-agent patch certain resource fields when it is changed on the managed cluster or in the manifestwork.
 
 ### Design Details
 
 #### API change
 
-The change will be added into the `updateStratey` field. For `Update` strategy, we will add an option
+The change will be added into the `updateStratey` field. For `ServerSideApply` strategy, we will add:
 
 ```go
-type UpdateConfig struct {
-    // OnSpokeChange defines whether the resource should be overridden by the manifestwork it is changed
-    // on the spoke by another actor.
-    // +kubebuilder:default=Override
-    // +kubebuilder:validation:Enum=Override;NoOverride
-    // +kubebuilder:validation:Required
-    // +required
-	OnSpokeChange string `json:"onSpokeChange,omitempty"`
+type ServerSideApplyConfig struct {
+	...
+    // IgnoreFields defines a list of json paths in the resource that will not be updated on the spoke.
+    // +listType:=map
+    // +listMapKey:=type
+    // +optional
+    IgnoreFields []IgnoreField `json:"ignoreFields,omitempty"`
 }
-```
 
-For existing `ServerSideApply` strategy, we will add:
+type IgnoreField struct {
+    // Condition defines the condition that the fields should be ignored when apply the resource. All the fields
+	// will be ignored if the condition is met, otherwise all fields will be updated.
+    // +kubebuilder:default=OnSpokePresent
+    // +kubebuilder:validation:Enum=OnSpokePresent;OnSpokeChange
+    // +kubebuilder:validation:Required
+    // +required
+    Condition string `json:"condition"`
 
-```go
-type ServerSideApplyConfig struct {
-	...
-	// IgnoreFields defines a list of json paths in the resource that will not be updated on the spoke.
-	IgnoreFields []string `json:"ignoreFields,omitempty"`
-
-	// OnSpokeChange defines whether the resource should be overriden by the manifestwork it is changed
-	// on the spoke by another actor.
-	// +kubebuilder:default=Override
-	// +kubebuilder:validation:Enum=Override;NoOverride
-	// +kubebuilder:validation:Required
-	// +required
-	OnSpokeChange string `json:"onSpokeChange,omitempty"`
+    // JSONPaths defines the list of json path in the resource to be ignored
+	JSONPaths []string `json:"jsonPaths"`
 }
 ```
 
-#### agent implemntation
+The `IgnoreFields` defines the setting that under certain situation, will not update the certain fields on the
+resource when the field is changed in the manifestwork or in the spoke cluster by another actor. It has two
+types:
+- `OnSpokeChange` means the agent only ignores field update when the field is changed on the managed cluster, the field
+will still be updated when it is updated in the manifestwork.
+- `OnSpokePresent` means the agent ignores the field update as long as the resource is present on the managed cluster, no
+matter the change is made on the managed cluster or in the manifestwork.
 
-When work-agent identify that the `OnSpokeChange` is set `NoOverride` for a certain resource in the `ManifestWork`, the agent
-when apply the resource to the spoke cluster will add an annotation with the key `open-cluster-management.io/object-hash`.
-The value of the annotation is the computed hash of the resource spec in the `ManifestWork`. Later when another actor
-updates the resource, the work-agent will at first check if the object-hash mismatches with the current resource spec
-in the `ManifestWork`. If it is the same, the resource will not be updated so the change from spoke is ignored. When
-the resource in the manifestwork is updated, the annotation will not match which then trigger the work-agent to update.
+#### agent implemntation
 
-To handle the `IgnoreFields` for `ServerSideApply`, we will remove the fields defined in the `IgnoreFields` and then
-generates the apply patch. The objec-hash will also be computed considering the `IgnoreFields`.
+To handle the `IgnoreFields` for `ServerSideApply`, we will remove the fields of the resource to be applied defined in 
+the `IgnoreFields` and then generates the apply patch. The agent when apply the resource to the spoke cluster will add
+an annotation with the key `open-cluster-management.io/object-hash`. The value of the annotation is the computed hash
+of the resource manifests in the `ManifestWork`, excluding the items in the `IgnoreFields` with the `OnSpokePresent`
+condition. Later when another actor updates the resource, the work-agent will at first check if the object-hash mismatches
+with the current resource spec in the `ManifestWork`. If it is the same, the resource will not be updated so the
+change from spoke is ignored. When the resource field not included in the `OnSpokePresent` in the manifestwork is
+updated, the annotation will not match which then trigger the work-agent to update.
 
 When the resource is deleted by another actor, the resource will be deleted with the annotation. So when work-agent
-recreates the resource using the latest spec. The `NoOverride` and `IgnoreFields` setting applies only when after 
+recreates the resource using the latest spec. The `IgnoreFields` setting applies only when 
 the resource is created or present already on the spoke cluster.
 
 When the `ManifestWork` is deleted, and the `DeleteOption` is `Orphan`, the resource will be kept on the spoke cluster
 together with the object-hash. It means if the user creates another `ManifestWork` with the same resource manifest and
-set `NoOverride` in the strategy, the resource will not be updated on the spoke cluster. 
+set `IgnoreFields` in the strategy, the resource might not be updated on the spoke cluster.
 
 #### examples
 
 To resolve [issue 631](https://github.com/open-cluster-management-io/ocm/issues/631), user can set the manifestwork 
-with `OnSpokeChange` set to `NoOverride`
+with `OnSpokeChange`
 
 ```yaml
 apiVersion: work.open-cluster-management.io/v1
@@ -119,13 +120,18 @@ spec:
   workload: ...
   manifestConfigs:
     - resourceIdentifier:
-        resource: secrets
+        resource: configmaps
         namespace: default
-        name: some-secret
+        name: some-configmap
       updateStrategy:
-        type: Update
-        update:
-          onSpokeChange: NoOverride
+        type: ServerSideApply
+        force: true
+        serverSideApply:
+          ignoreFields:
+            - condition: OnSpokeChange
+              jsonPaths:
+                - .data
+
 ```
 
 To resolve [issue 670](https://github.com/open-cluster-management-io/ocm/issues/670), user also can do the same for
@@ -146,9 +152,12 @@ spec:
         namespace: default
         name: application1
       updateStrategy:
-        type: Update
-        update:
-          onSpokeChange: NoOverride
+        type: ServerSideApply
+        serverSideApply:
+          ignoreFields:
+            - condition: OnSpokeChange
+              jsonPaths:
+                - .operation
 ```
 
 To resolve [issue 690](https://github.com/open-cluster-management-io/ocm/issues/690), user can set like:
@@ -171,13 +180,15 @@ spec:
         type: ServerSideApply
         serverSideApply:
           ignoreFields:
-          - ".spec.replicas"
+            - condition: OnSpokePresent
+              jsonPaths:
+                - .spec.replicas
 ```
 
 
 ### Test Plan
 
-- test on `OnSpokeChange` with `Overrid` and `NonOverride` option
+- test on `IgnoreFields` with `OnSpokePresent` and `OnSpokeChange` option.
 - test on `IgnoreFields` with a single field, a full strcutrue and an item in the list.
 
 ### Graduation Criteria
@@ -186,8 +197,11 @@ N/A
 ### Upgrade Strategy
 It will need upgrade on CRD of ManifestWork on hub cluster, and upgrade of work agent on managed cluster.
 
+When a user needs to use this feature with an existing `ManifestWork`, the user needs to update the `ManifestWork` to
+`ServerSideApply` strategy and enable force apply if the `ManifestWork` uses `Update` strategy before.
+
 ### Version Skew Strategy
 - The field is optional, and if it is not set, the manifestwork will be updated as is by the work agent with elder version
-- The elder version work agent will ignore the newly added field field.
+- The elder version work agent will ignore the newly added field.
 
 ## Alternatives