feat: add rollout controller skeleton #378
Conversation
zhiying-lin
force-pushed
the
new-rollout-controller
branch
from
4b249f3 to
9106e7f
Compare
| ResourceSnapshotName string `json:"resourceSnapshotName"` | ||
|
|
||
| // PolicySnapshotName is the name of the policy snapshot that this resource binding & resource snapshot points to. | ||
| // It is required to decide whether the binding needs to be deleted or not without talking to API server. |
There was a problem hiding this comment.
Ah, I remember that ResourceSnapshot has a reference to PolicySnapshot, correct?
Additionally, since we cannot tell whether a PolicySnapshot is the latest from looking at the name, we have to talk to the API server (or actually, cache) any way.
There was a problem hiding this comment.
yeah, but we only store the resourceSnapshot name here, but not include the policySnapshot name. :(
We will get the latest policySnapshot name by calling API server.
This field is to quickly figure out which policySnapshot the resourceSnapsht of the binding points to.
otherwise, we need to call API server by GET resourceSnapshot.
There was a problem hiding this comment.
Ah, but the client in use will query the cache for the GET ResourceSnapshot call, right? It will never reach API server. Besides, doesn't the rollout controller have to retrieve ResourceSnapshot any way?
There was a problem hiding this comment.
Does the controller needs to get the ResourceSnapshot? If so, then I am not sure we need this field as the policySnapshotName is in the ResourceSnapshot.
There was a problem hiding this comment.
nope, it does not need to query the resourceSnapshot for each binding.
The PolicySnapshotName will be used in two places:
- use to check if the current binding is old or not and then delete the old bindings for the ReCreate rollout strategy, in line 144, https://github.com/zhiying-lin/fleet/blob/rollout-controller/pkg/controllers/clusterresourcerollout/clusterresourcerollout_controller.go#L144
- use to check if the the current binding is using latest policy, then update the resource of the bindings, in line 160 https://github.com/zhiying-lin/fleet/blob/rollout-controller/pkg/controllers/clusterresourcerollout/clusterresourcerollout_controller.go#L160
There was a problem hiding this comment.
Please see the message in chat. We don't need this field for saving trips to API server (the trip will not happen because of caches), but we do need it for a different reason.
|
The rollout controller is no longer needed. Closing this PR |
Signed-off-by: Zhiying Lin <zhiyingl456@gmail.com>
3 participants
Description of your changes
add rollout controller skeleton
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Will add the integration test to validate Reconcile func in a separate PR.