|
| 1 | +--- |
| 2 | +title: Remove Detector and Add PropagationPolicy and ClusterPropagationPolicy Controller |
| 3 | +authors: |
| 4 | +- "@pigletfly" |
| 5 | +reviewers: |
| 6 | +- "@RainbowMango" |
| 7 | +- "@kevin-wangzefeng" |
| 8 | +- "@Garrybest" |
| 9 | +approvers: |
| 10 | +- "@RainbowMango" |
| 11 | +- "@kevin-wangzefeng" |
| 12 | + |
| 13 | +creation-date: 2022-02-14 |
| 14 | + |
| 15 | +--- |
| 16 | + |
| 17 | +# Remove Detector and Add PropagationPolicy and ClusterPropagationPolicy Controller |
| 18 | + |
| 19 | + |
| 20 | +## Summary |
| 21 | + |
| 22 | +In karmada controller, we have common controllers like, ResourceBinding Controller, ClusterController.And there is also a unordinary controller called detector, which is in charge of detecting resource changes in host cluster and finding the matching propagation policy, if the matching policy is found, it will add resource to `waitingObjects`.But there are some flaws : |
| 23 | + |
| 24 | +1. The detector has to watch all resource changes in host cluster, which is not efficient, and can be easily be reach the limit of `--http2-max-streams-per-connection`, which is set to 250 by default(https://github.com/kubernetes/kubernetes/pull/60054). |
| 25 | + |
| 26 | +2. There is a chance that the detector will miss the policy matching, then ResourceBinding will not be created, thus the resource will not be propagated.Chances will be greater if we increase the worker number of the detector, which is hardcoded to 1 by default.But the worker number should be configurable by user, for example, we have tons of resources in host cluster, and they have lots of events, the detector can't handle the events in time. |
| 27 | + |
| 28 | +3. When a PropagationPolicy is created, the correspondind ResourceBinding object is getting reconciled many times, which is not efficient. |
| 29 | + |
| 30 | +there are some issues ralated with detector or resourcebinding controller: |
| 31 | + |
| 32 | +https://github.com/karmada-io/karmada/issues/1353 |
| 33 | +https://github.com/karmada-io/karmada/issues/1309 |
| 34 | +https://github.com/karmada-io/karmada/issues/1195 |
| 35 | +https://github.com/karmada-io/karmada/issues/679 |
| 36 | +https://github.com/karmada-io/karmada/issues/670 |
| 37 | + |
| 38 | +So this KEP recommends to remove the detector controller and add propagation policy controller and cluster propagation policy controller. |
| 39 | + |
| 40 | +## Motivation |
| 41 | + |
| 42 | +Detector has chances to miss the policy matching, and is not suitable for large-scale cluster. |
| 43 | + |
| 44 | + |
| 45 | +### Goals |
| 46 | + |
| 47 | +* separate PropagationPolicy Controller from detector |
| 48 | +* separate ClusterPropagationPolicy Controller from detector |
| 49 | +* separate ResourceBinding reconcile from detector, put it back to ResourceBinding Controller |
| 50 | +* add Resource Controller and its lifecycle is controller by PropagationPolicy or ClusterPropagationPolicy Controller |
| 51 | + |
| 52 | +### Non-Goals |
| 53 | + |
| 54 | + |
| 55 | + |
| 56 | +## Proposal |
| 57 | + |
| 58 | + |
| 59 | + |
| 60 | +### User Stories (Optional) |
| 61 | + |
| 62 | +<!-- |
| 63 | +Detail the things that people will be able to do if this KEP is implemented. |
| 64 | +Include as much detail as possible so that people can understand the "how" of |
| 65 | +the system. The goal here is to make this feel real for users without getting |
| 66 | +bogged down. |
| 67 | +--> |
| 68 | + |
| 69 | +#### Story 1 |
| 70 | + |
| 71 | + |
| 72 | + |
| 73 | +#### Story 2 |
| 74 | + |
| 75 | +### Notes/Constraints/Caveats (Optional) |
| 76 | + |
| 77 | +<!-- |
| 78 | +What are the caveats to the proposal? |
| 79 | +What are some important details that didn't come across above? |
| 80 | +Go in to as much detail as necessary here. |
| 81 | +This might be a good place to talk about core concepts and how they relate. |
| 82 | +--> |
| 83 | + |
| 84 | +### Risks and Mitigations |
| 85 | + |
| 86 | +<!-- |
| 87 | +What are the risks of this proposal, and how do we mitigate? |
| 88 | +
|
| 89 | +How will security be reviewed, and by whom? |
| 90 | +
|
| 91 | +How will UX be reviewed, and by whom? |
| 92 | +
|
| 93 | +Consider including folks who also work outside the SIG or subproject. |
| 94 | +--> |
| 95 | + |
| 96 | +## Design Details |
| 97 | + |
| 98 | +The KEP is composed of two parts: |
| 99 | + |
| 100 | +1. ResourceBinding Controller |
| 101 | +2. PropagationPolicy(ClusterPropagationPolicy) Controller and ResourceBinding Controller |
| 102 | + |
| 103 | +in ResourceBinding Controller part, we should move `ReconcileResourceBinding` in detector back to ResourceBinding Controller. |
| 104 | + |
| 105 | +Here is the workflow of ResourceBinding Controller: |
| 106 | + |
| 107 | + |
| 108 | + |
| 109 | +in the new workflow, resource status is aggregated in ResourceBinding Controller in a single ResourceBinding reconcile. |
| 110 | + |
| 111 | +in the second part, the new PropagationPolicy Controller is in charge of reconciling the PropagationPolicy, and do the PropagationPolicy and resource template matching, if the matching fails, then put the PropagationPolicy back to PropagationPolicy Controller queue to be reconciled again.Then the detector will be removed.When a PropagationPolicy is getting reconciled, the `ResourceSelectors` in the PropagationPolicy will determine which Resource Controller will be started, so PropagationPolicy will not watch all resources in host cluster, if we have many different resources(GVKs) in host cluster, it can easily reach the limit of `--http2-max-streams-per-connection`.And when resource gets any updates, Resource Controller will do the resource reconcile, and do the policy matching if the resource is not matched by any policy.In Resource Controller, we can increase the worker number of the Resource Controller by GVK.When a PropagatePolicy is deleted, it will check whether to stop the corresponding Resource Controller or not, which is shared by GVK, if all `ResourceSelectors` in all PropagationPolicy don't have the corresponding GVK, then the Resource Controller will be stopped.Here is the workflow. |
| 112 | + |
| 113 | + |
| 114 | + |
| 115 | + |
| 116 | + |
| 117 | +### Test Plan |
| 118 | + |
| 119 | +<!-- |
| 120 | +**Note:** *Not required until targeted at a release.* |
| 121 | +
|
| 122 | +Consider the following in developing a test plan for this enhancement: |
| 123 | +- Will there be e2e and integration tests, in addition to unit tests? |
| 124 | +- How will it be tested in isolation vs with other components? |
| 125 | +
|
| 126 | +No need to outline all of the test cases, just the general strategy. Anything |
| 127 | +that would count as tricky in the implementation, and anything particularly |
| 128 | +challenging to test, should be called out. |
| 129 | +
|
| 130 | +--> |
| 131 | + |
| 132 | +## Alternatives |
| 133 | + |
| 134 | +<!-- |
| 135 | +What other approaches did you consider, and why did you rule them out? These do |
| 136 | +not need to be as detailed as the proposal, but should include enough |
| 137 | +information to express the idea and why it was not acceptable. |
| 138 | +--> |
| 139 | + |
| 140 | +<!-- |
| 141 | +Note: This is a simplified version of kubernetes enhancement proposal template. |
| 142 | +https://github.com/kubernetes/enhancements/tree/3317d4cb548c396a430d1c1ac6625226018adf6a/keps/NNNN-kep-template |
| 143 | +--> |
0 commit comments