interface: updated ResourceBinding API #400
Conversation
|
|
||
| // PolicySnapshotName is the name of the policy snapshot that this resource snapshot is pointing to. | ||
| // +required | ||
| PolicySnapshotName string `json:"policySnapshotName"` |
There was a problem hiding this comment.
just wonder why we remove this field?
For example, CRP has two policySnapshots & two resourceSnapshots. policySnapshot A selects resources A and policy Snapshot B selects resource B. policySnapshot B and resourceSnapshot B are active.
Scheduler has not created the new bindings for policyA and marked the old bindings as deleted yet.
In this case, should rollout controller avoid updating the these old bindings by checking the policySnapshotName in the resourceSnapshot?
If we remove this field, we cannot distinguish whether the resourceSnapshot is created for policyA or policyB.
For the future rollback feature, should we also rollback the policy?
There was a problem hiding this comment.
The main reason we removed this field is to completely decouple the schedulingPolicy from resource.
Scheduler has not created the new bindings for policyA and marked the old bindings as deleted yet.
In this case, should rollout controller avoid updating the these old bindings by checking the policySnapshotName in the resourceSnapshot?
I am not sure if I follow this case. The policyA is old so why is scheduler creating new bindings for it? What are the old bindings for?
If we remove this field, we cannot distinguish whether the resourceSnapshot is created for policyA or policyB.
Isn't the latest resourceSnapshot always point to the latest policy?
For the future rollback feature, should we also rollback the policy?
Given now that we have two revisions. It seems that the future rollback policy has to contain both of them?
Description of your changes
This PR updates the ResourceBinding API.
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
N/A
Special notes for your reviewer