-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Start Ingress cache Controller immediately #2269
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: antoineco Assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report
@@ Coverage Diff @@
## master #2269 +/- ##
==========================================
- Coverage 37.13% 37.12% -0.02%
==========================================
Files 71 71
Lines 5025 5021 -4
==========================================
- Hits 1866 1864 -2
+ Misses 2874 2873 -1
+ Partials 285 284 -1
Continue to review full report at Codecov.
|
/assign @aledbf |
This 😄 As you know the controllers are independent (secrets, configmap, endpoints, services, ingress). The issue here is that the Ingress cache controller requires (in some cases) content from the others controllers (listers actually) like secrets. That is the reason why we cannot start the controllers on the same block. If you checked the linked issue in 1891 you will see one of the problems when we start at the same time (503 when an Ingress cannot fetch a secret) |
But at that stage we only populate caches don't we? |
We start the controllers and that will populate the local caches (listers) |
Clarified on Slack with @aledbf:
My assumption was that the initial caches were always fully populated. |
What this PR does / why we need it:
Waiting 1 extra second before starting the cache Controller for Ingress objects seems unnecessary because the call to
n.store.Run()
is blocking until all caches are synced.Special notes for your reviewer:
The change was originally introduced as part of #1891, but doesn't seem relevant anymore (unless I missed some evil detail).