-
Notifications
You must be signed in to change notification settings - Fork 2k
change: relax kube_.+_created to allow namespaces #2684
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
base: main
Are you sure you want to change the base?
Conversation
Do we maintain any stability guarantees (or backward-compatibilities) for the deny-list? I've assumed that the deny-list, with adequate reason, is subject to change based on observations seen over prolonged periods, but I'm happy to do this in a safer manner (say, in a separate |
If this still seems odd, I'll try to put together a more makeshift solution downstream and close this out. |
I suspect that cluster-monitoring-operator is the only downstream user for his addon. I'm not sure that it's worth keeping it in this repository given that it's not documented and it provides no configuration option. If we want to keep it here, I'd advocate for 1) adding arguments to customize the deny list and 2) minimal documentation. WDYT @slashpai @philipgough? |
@simonpasquier yep, agree that makes sense |
d10c3b3
to
3abfc7a
Compare
3abfc7a
to
b222bf7
Compare
containers, | ||
); | ||
|
||
local defaultDenylist = [ |
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.
can we use the "standard" values pattern?
{
values+:: {
ksmDenyList: [
...
],
},
kubeStateMetrics+: {
....
}
}
```
b222bf7
to
787bc20
Compare
185a97b
to
150d07a
Compare
Not sure why the test expects |
150d07a
to
64184d1
Compare
Tests seem to pass on my fork: https://github.com/rexagod/kube-prometheus/actions/runs/16932879487 for the same commit. Triggering a retest. |
64184d1
to
81ae339
Compare
Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
81ae339
to
7b63e15
Compare
cc @simonpasquier for a review 🙇 |
Ping @simonpasquier for a look here 🙂 |
Thanks! #2684 (comment) is still valid. |
So just to make sure I'm understanding this correctly, do you mean instead of |
Signed-off-by: Pranshu Srivastava rexagod@gmail.com
Description
Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request.
If it fixes a bug or resolves a feature request, be sure to link to that issue.
KSM v2.16 introduced look around support for regexes (kubernetes/kube-state-metrics#2616), and since namespaces are an extremely common native resource, it might make sense to have their creation metrics OOTB for downstream consumers.
Type of change
What type of changes does your code introduce to the kube-prometheus? Put an
x
in the box that apply.CHANGE
(fix or feature that would cause existing functionality to not work as expected)FEATURE
(non-breaking change which adds functionality)BUGFIX
(non-breaking change which fixes an issue)ENHANCEMENT
(non-breaking change which improves existing functionality)NONE
(if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)Changelog entry
Please put a one-line changelog entry below. Later this will be copied to the changelog file.
generate-versions.sh
seems to returnnull
for KSM, but I can updateversions.json
if needed.