-
Notifications
You must be signed in to change notification settings - Fork 183
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
Support new 1.16 metrics with filter to exclude deprecated metrics #401
Conversation
@@ -10,6 +10,15 @@ | |||
@label @DATAPOINT | |||
</match> | |||
<label @DATAPOINT> | |||
{{- if and (eq .Capabilities.KubeVersion.Major "1") (eq .Capabilities.KubeVersion.Minor "16") }} |
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.
what about 1.17 and above? Or these metrics only apply for 1.16?
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 they are getting removed in 1.17
Beta release target 1.17
All previously marked deprecated metrics will be removed from the codebase.
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.
They are getting removed, but i believe both metrics co-exist in 1.14, 1.15 and 1.16
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.
https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG-1.14.md#deprecated-metrics - you can see that they deprecated the old and added the new in 1.14. So, i think this version check should be for 1.14 and above.
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 just realized that too, let me update the condition. Thanks!
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.
hmm, what should we do for the non-helm yaml file? it looks like CI added the filter to the generated file by helm template
@maimaisie I just noticed a binary file tgz file got added to the PR, that's not expected is it? |
oh oops, thanks for noticing that! removed |
Description
This PR brings back changes in #372 and adds a grep filter plugin to exclude the deprecated metrics by checking the k8s version (which is a built-in object in helm)
Testing performed