-
Notifications
You must be signed in to change notification settings - Fork 90
MAISTRA-2234: Seed MemberRoll listeners with system namespace #308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I am not 100% convinced we should do this for the 2.0 branch. The issue on 2.1 is that changes in istiod mean this bug prevents things from starting at all in some cases. In my testing on the latest 2.0 builds, that is not the case, regardless of whether you've created the initial SMMR or not. I do think this has the potential to introduce a race between listeners seeing the cache has synced and looking for the SMMR, and one being created. I think it's possible, but maybe unlikely, that we then send add events out-of-order with this initial "seed" event. If that's the case, is it worth introducing that if we haven't seen this cause any actual issues on this branch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think just adding a mutex around the cacheWarmed
variable will be sufficient here. IMHO we should not fix a problem nobody has (ie 'no SMMR present causes strange behaviour') if it adds complexity - if a customer runs into an issue here we can still tackle it then
We know the problem exists, and while we may not currently be sure it has any impact it is much better to fix it now and not have it impact any customers. |
The MemberRoll controller was changed in MAISTRA-2197 to not send the initial update with just the system namespace to all listeners. This can be a problem when the user hasn't created an SMMR resource yet, because any listeners registered before the caches sync may never get an update informing them to watch the system namespace. This attempts to work around the issue by having all listeners individually wait on the caches to sync, then seed just the system namespace if no SMMR resource is found.
Updated to use a channel in a similar way to #312 to be sure we don't process events from the informer until we've sent the initial update when necessary. |
Alright, that's fine with me, too |
The MemberRoll controller was changed in MAISTRA-2197 to not send the
initial update with just the system namespace to all listeners. This
can be a problem when the user hasn't created an SMMR resource yet,
because any listeners registered before the caches sync may never get
an update informing them to watch the system namespace.
This attempts to work around the issue by having all listeners
individually wait on the caches to sync, then seed just the system
namespace if no SMMR resource is found. Unfortunately, it's possible
for this to race a user creating an SMMR, in which case the state may
be inconsistent until the next update or full sync.