enable force cluster upgrade #231
Conversation
f788973 to
d326690
Compare
|
@Jeffwan you can take a look at this and I believe we still need to use filter functions in the near future. I am thinking comparing different revisions on RayParams is the best solution since we are parsing and giving them as arguments to pods which needs a lot of efforts to compare if we want to do it in the reconcile logic. |
|
BTW, What does the |
| } | ||
| updatedWorkerPods := false | ||
| for _, item := range workerPods.Items { | ||
| if r.needRemoveOldPod(worker.Template, item) { |
There was a problem hiding this comment.
we actually has the worker group template. In that case, the template would be same. Do we need to compare pod with template one by one?
There was a problem hiding this comment.
In this case, the group template length would be 1. So anyway we will not have extra overhead here, right?
There was a problem hiding this comment.
Yes. the workingGroupTemplate would be 1. I mean the pods created from the template should be same. Do we want to compare each of them with the template or just randomly pick one? I don't have strong opinion on this because technically, the pods could be different if there're some manual modification.
yes |
28d6e97 to
433d693
Compare
|
Overall looks good to me. Since this is a tricky pattern, let's wait for more feedbacks. |
|
I've been meaning to take a look at this PR but haven't gotten the chance yet. @wilsonwang371 @Jeffwan how's this PR going? Can we merge an initial version across the finish line? |
|
i will clean it up and make it ready soon |
|
Thanks @wilsonwang371 !! |
8607fba to
19d9043
Compare
19d9043 to
626bb33
Compare
|
It's safe to merge especially protected by the feature flag. Let's merge it and give it a try. |
Why are these changes needed?
This is a patch to bug #155. I added reconcile logic so that in case of following fields updated, we will rebuild the cluster.
Related issue number
#155
Checks