Updated to include some risk/mitigations

Author: annelau21

enhancements/sig-architecture/179-work-feedback-scrape-type/README.md

@@ -84,18 +84,18 @@ be a good place to talk about core concepts and how they relate.
 
 ### Risks and Mitigation
 
-What are the risks of this proposal and how do we mitigate. Think broadly. For
-example, consider both security and how this will impact the larger OKD
-ecosystem.
+#### Risk #1: Too many watch events and update calls to the ManifestWork. 
 
-How will security be reviewed and by whom? How will UX be reviewed and by whom?
+This will be mitigated by limiting when the ManifestWork controller sends updates to the ManifestWork and to stop processing WATCH events once the ManifestWork is "Ready". 
 
-Consider including folks that also work outside your immediate sub-project.
+ManifestWork Controller will parse the response from the informer watch and only send an update to the ManifestWork if there is an update on the feedbackRules. 
 
-## Design Details
+ManifestWork Controller that registers informers per resource, will unregister the informer once the ManifestWork is Ready.
 
-#### ManifestWork CRD Changes 
-```
+
+## Design Details
+### ManifestWorkReplicaSet CRD changes
+```yaml
 apiVersion: work.open-cluster-management.io/v1alpha1
 kind: ManifestWorkReplicaSet
 ...
@@ -111,6 +111,22 @@ spec:
     ....
 ```
 
+#### ManifestWork CRD changes 
+```yaml
+apiVersion: work.open-cluster-management.io/v1alpha1
+kind: ManifestWork
+...
+spec:
+  manifestConfigs:
+    - resourceIdentifier:
+        group: apps
+        resource: deployments
+        namespace: default
+        name: hello
+      feedbackScrapeType: WATCH (Options: WATCH | POLL, POLL is default behavior)
+    ....
+```
+
 This change will also be populated upwards to the ManifestWorkReplicaSet level of 
 `spec.manifestWorkTemplate.manifestConfigs`. 
 
@@ -127,8 +143,8 @@ sequenceDiagram
 
  opt if feedbackScrapeType == WATCH
     A->>B: (WATCH) event for resource changes
-    B->>B: feedbackRules & conditionRule Updates 
-    B->>C: (WATCH) for feedbackRule updates
+    B->>B: feedbackRules & conditionRule Updates (if there is any update)
+    B->>C: (WATCH) for feedbackRule updates only (if Progressing = True & Degraded = False|not set)
     C->>C: Update status.Conditions 
  end 
 ```
@@ -143,19 +159,22 @@ Poll remains as a safety net.
 Create a FeedbackScrape interface that polls or watches resources. 
 
 In the ManifestWork Controller (pkg/work/spoke/controllers/statuscontroller/availablestatus_controller.go)
-introduce a watch-based path alongside the existing poll loop. When syncing the ManifestWork, register a informer for the resource if `feedbackScrapeType`  WATCH. 
+introduce a watch-based path alongside the existing poll loop. 
+
+When syncing the ManifestWork, register a informer for the resource if `feedbackScrapeType`  WATCH. If the `feedbackScrapeType` is no longer WATCH, unregister the informer. If the ManifestWork is in Ready state (Progressing == False and Degraded == False / NotSet), unregister the informer. This will prevent long running WATCH from triggering too many ManifestWork changes. If there is new rollout, and the status changes to Progressing, then register the informer again. 
 
-When there is a change seen on the WATCH type, patch the status conditions for that resource. 
+When the informer recieves an event (create, update, delete), verify that there is a change on the fields defined in the `feedbackRules` and patch the status conditions for that resource. If there is no change on one of the `feedbackRules` field, ignore the WATCH event.
 
 #### Hub Level ManifestWorkReplicaSet Controller Reconciler 
 
-The update to the hub level will be the same for the new feature of `feedbackScrapeType` WATCH | POLL. When the ManifestWork has an update in the status feedback, trigger a resync of the MWRS to get the status feedback details. Once the MWRS is `Ready` status which has completed the rollout, stop the reconciler to prevent overloading the KubeAPI. (`Ready` status Proposal is TBD)
+There should be no updates on the MWRS reconciler. The expected behavior is that when the ManifestWork has an update in the status feedback, trigger a resync of the MWRS to get the status feedback details.
 
+In the current model, the MWRS controller is reconciled if there are changes to the ManifestWork (that are managed by manifestworkreplicaset), PlacementDecisions and Placements. Therefore any update to the status.Feedback on the ManifestWork should trigger the reconcile. Standard factory used [here](https://github.com/open-cluster-management-io/ocm/blob/main/pkg/work/hub/manager.go#L97-L98)
 
-### Open Questions [optional]
 
-> 1. Is there a limit that should be set on the `WATCH` type to prevent overloading of the KubeAPI? 
+### Open Questions [optional]
 
+TBD
 
 ### Upgrade / Downgrade Strategy