-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Replace multiListerWatcher with independent listWatchers per namespace #1499
Conversation
Welcome @fpetkovski! |
6d390eb
to
aa77e27
Compare
50b9477
to
cc47c3b
Compare
cc47c3b
to
edcd2a8
Compare
edcd2a8
to
3ececc1
Compare
e071c9f
to
f77a978
Compare
9461f61
to
cc1d509
Compare
The multiListerWatcher is a composite object encapsulating multiple ListerWatchers and implements the ListerWatcher interface. With the current implementation, when an individual lister fails, the entire List operation fails. This causes no metrics to be shown when KSM has no permissions to a single namespace. In addition to this, the multiListerWatcher takes advantage of internal implementation details if the client-go library by modifiying and relying on the ResourceVersion metadata field. This introduces a bug where reconnecting to the API server will break the multiListerWatcher completely. This commit replaces the multiListerWatcher with individual ListerWatchers per each configured namespace, resolving both issues. Signed-off-by: fpetkovski <filip.petkovsky@gmail.com>
Co-authored-by: Damien Grisonnet <damien.grisonnet@epita.fr>
10d6cf8
to
222f271
Compare
Co-authored-by: Manuel Rüger <manuel@rueg.eu>
/assign @tariq1890 we might want to backport this to release-2.1 as well. |
Nice work! /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brancz, dgrisonnet, fpetkovski, mrueg 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 |
I have a suspicion that this might also help with #694 |
* [CHANGE] Replace multiListWatcher with independent listWatchers per namespace kubernetes#1499 * [BUGFIX] Use BuilderInterface instead of internal/store.Builder in metricshandler kubernetes#1537 * [BUGFIX] Add WithAllowLabels to public BuilderInterface kubernetes#1514
* [CHANGE] Replace multiListWatcher with independent listWatchers per namespace kubernetes#1499 * [CHANGE] go.mod: Update and minimize dependencies kubernetes#1529 * [BUGFIX] Use BuilderInterface instead of internal/store.Builder in metricshandler kubernetes#1537 * [BUGFIX] Add WithAllowLabels to public BuilderInterface kubernetes#1514
* [CHANGE] Replace multiListWatcher with independent listWatchers per namespace kubernetes#1499 * [CHANGE] go.mod: Update and minimize dependencies kubernetes#1529 * [BUGFIX] Use BuilderInterface instead of internal/store.Builder in metricshandler kubernetes#1537 * [BUGFIX] Add WithAllowLabels to public BuilderInterface kubernetes#1514
* [CHANGE] Replace multiListWatcher with independent listWatchers per namespace kubernetes#1499 * [CHANGE] go.mod: Update and minimize dependencies kubernetes#1529 * [BUGFIX] Use BuilderInterface instead of internal/store.Builder in metricshandler kubernetes#1537 * [BUGFIX] Add WithAllowLabels to public BuilderInterface kubernetes#1514
* [CHANGE] Replace multiListWatcher with independent listWatchers per namespace kubernetes#1499 * [CHANGE] go.mod: Update and minimize dependencies kubernetes#1529 * [BUGFIX] Use BuilderInterface instead of internal/store.Builder in metricshandler kubernetes#1537 * [BUGFIX] Add WithAllowLabels to public BuilderInterface kubernetes#1514
* [CHANGE] Replace multiListWatcher with independent listWatchers per namespace kubernetes#1499 * [CHANGE] go.mod: Update and minimize dependencies kubernetes#1529 * [BUGFIX] Use BuilderInterface instead of internal/store.Builder in metricshandler kubernetes#1537 * [BUGFIX] Add WithAllowLabels to public BuilderInterface kubernetes#1514
* [CHANGE] Replace multiListWatcher with independent listWatchers per namespace kubernetes#1499 * [CHANGE] go.mod: Update and minimize dependencies kubernetes#1529 * [BUGFIX] Use BuilderInterface instead of internal/store.Builder in metricshandler kubernetes#1537 * [BUGFIX] Add WithAllowLabels to public BuilderInterface kubernetes#1514
The
multiListerWatcher
is a composite object encapsulating multipleListerWatchers
and implements theListerWatcher
interface.With the current implementation, when an individual lister fails, the
entire List operation fails. This causes no metrics to be shown when KSM
has no permissions to a single namespace.
In addition to this, the
multiListerWatcher
takes advantage of internalimplementation details if the client-go library by modifiying and
relying on the
ResourceVersion
metadata field. This introduces a bugwhere reconnecting to the API server will break the
multiListerWatcher
completely.
This commit replaces the
multiListerWatcher
with individualListerWatchers
per each configured namespace, resolving both issues.It introduces a
MetricsWriter
interface to writes out metrics from stores,and implements it in the
MetricsStore
struct. It also introduces aMultiNamespaceMetricsWriter
struct implementing
MetricsWriter
to allow writing out metrics from multiplerelated stores (stores holding data for objects of the same kind but different namespace).
What this PR does / why we need it:
When KSM has no permissions for resources in at least one namespace, it will not export any metrics on the
/metrics
endpoint.Which issue(s) this PR fixes:
Fixes #1413