mgr: Allow more than two mgrs #12895
Conversation
| // +kubebuilder:validation:Minimum=0 | ||
| // +kubebuilder:validation:Maximum=2 | ||
| // +kubebuilder:validation:Maximum=5 |
There was a problem hiding this comment.
if we have a validation by CRD, why do we need secendory validation in the code?
There was a problem hiding this comment.
The code validation must be there from before the crd schema validation. If you're referring to the maxMgrCount in code, that's still needed for deleting extra mgrs. But I'm going to look at the implementation again to see if we can have a better way to remove extras without duplicating that in code.
There was a problem hiding this comment.
Now the maxMgrCount is removed from code and am querying for the existing mgrs to see if there are any extra to remove.
While two mgrs is considered sufficient, more mgr daemons are now allowed in case the admin wants even more fault tolerance to the active mgr going down, in case multiple mgrs go down. Up to five mgr pods will be allowed. All mgrs will be in standby mode except one active mgr. All mgr pods have a sidecar that will update their respective pod specs with the active or passive label. Signed-off-by: travisn <tnielsen@redhat.com>
travisn
force-pushed
the
multiple-mgr
branch
from
f01b90b
to
66547d2
Compare
mgr: Allow more than two mgrs (backport #12895)
| logger.Warningf("failed to check for extra mgrs. %v", err) | ||
| return | ||
| } | ||
| if len(mgrDeployments.Items) == len(daemonIDs) { |
There was a problem hiding this comment.
what if the deployments are less than daemons?
There was a problem hiding this comment.
If there are fewer deployments, then the loop below will just not find any to remove. But I don't expect that to happen actually because this is called right after all the expected mgr daemons are reconciled.
| continue | ||
| } | ||
| found := false | ||
| for _, daemonID := range daemonIDs { |
There was a problem hiding this comment.
if one of the mgr is deleted,
For ex: List of mgr{a,b,c}
b got deleted, new list {a,c}
Now a new mgr will try to get an added {a,c,d} to keep up the replica count but as per the daemonIDs array is implemented it will keep the daemonIDS list as {a,b,c} only
And later replica of mgr count become = 4
New list {a,c,d,e}
So now removing the mgr will work on
mgr list->{a,c,d,e} and hopefully daemon list should be {a,b,c,d}
By which this algo will remove e intentionally.
I can discuss more on this
There was a problem hiding this comment.
PS: No it won't be a problem sorry for the confusion,
It would have applied to mon algorithm
Description of your changes:
While two mgrs is considered sufficient, more mgr daemons are now allowed in case the admin wants even more fault tolerance to the active mgr going down, in case multiple mgrs go down. Up to five mgr pods will be allowed. All mgrs will be in standby mode except one active mgr. All mgr pods have a sidecar that will update their respective pod specs with the active or passive label.
Which issue is resolved by this Pull Request:
Resolves #12812
Checklist: