Fix race in EtcdWatcher #18956
Fix race in EtcdWatcher #18956
Conversation
|
Labelling this PR as size/S |
k8s-github-robot
added
the
size/S
|
GCE e2e test build/test passed for commit 8573ef0cfb861abe9e38f84e663010cd1813d4dc. |
|
It will not fix the OIDC test - this one just failed. @k8s-bot unit test this please |
|
@kubernetes/goog-testing |
|
@krousey @caesarxuchao - can one of you please take a look? |
|
|
||
| // We need to be prepared, that Stop() can be called at any time. | ||
| // It can potentially also be called, even before this function is called. | ||
| // If that is the case, we simply skip all the code here. |
There was a problem hiding this comment.
It would be a good idea to link to the issue numbers in the comments.
|
Minor comments, but I've still never repo'd locally. It would be good to list down the reproduction conditions. |
|
I also didn't repro'd it locally. This is just based on my intuition and some logs. |
wojtek-t
force-pushed
the
debug_etcd_generic
branch
from
8573ef0 to
327aa94
Compare
|
GCE e2e test build/test passed for commit 327aa94df805aa3d0dcf04715cc6fe8568fbc13c. |
| w.stopLock.Lock() | ||
| if w.stopped { | ||
| // Watcher has already been stopped - don't event initiate it here. | ||
| w.stopLock.Unlock() |
There was a problem hiding this comment.
Can you use an anonymous function or a separate function to encapsulate this logic before the infinite for loop so you can just defer Unlock? I'd hate for someone to come add another error condition with an early return and forget to unlock.
wojtek-t
force-pushed
the
debug_etcd_generic
branch
from
327aa94 to
6297232
Compare
|
@krousey - PTAL |
krousey
added
the
lgtm
|
@krousey - thanks! |
|
Labelling this PR as size/M |
k8s-github-robot
added
size/M
|
GCE e2e test build/test passed for commit 6297232. |
|
Merging manually to check if that helps. |
Automatic merge from submit-queue. make registry installation a component Demonstrate a simple way of making a component and installing it. This builds on previous pulls and shows how we can start define an interface. I think after we switch pieces over, we'll find points of commonality as their entry points. I suspect they will include: 1. cluster-admin.kubeconfig 2. docker helper? We should try to switch this to actually installing with pods. 3. uninstall API. Something to remove it anyway. 4. idempotent. We will call it *on every cluster up* /assign @mfojtik /assign @soltysh Origin-commit: eb016b358623b0d6c672ba1c3cfcfa1b20f1970b
Ref #18928
@timothysc