-
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
feat(CustomResourceState): Support quantities and percentages #1989
Conversation
10f8a9c
to
093cfb6
Compare
/triage accepted |
/triage accepted |
@mrueg Can we incorporate support for percentages as well? IIRC that's the only other quantity format that's causing issues. |
@rexagod good idea, I've added support for it as well. |
ee77e6e
to
3c40f03
Compare
I see this incorporates support for # Works.
type: Gauge
gauge:
path: [foo, bar, baz, que] # Needs to be addressed.
type: Gauge
gauge:
path: [foo]
valueFrom: [bar, baz, que] |
5449eb0
to
10b4ad8
Compare
@rexagod I've added a valueFrom testcase, this seems to work for both. Can you clarify if I am missing anything? |
10b4ad8
to
8f35094
Compare
Ah, yep, that was my bad. The PR is good for # Same as in the example above.
type: Gauge
gauge:
path: [foo]
valueFrom: [bar, baz, que] when I should've tested it on, # Works.
type: Gauge
gauge:
path: [foo, bar, baz]
valueFrom: [que] # Only single-paths are supported as of now. We just need to resolve the failing test and this should be good! 🚀 |
8f35094
to
d799f40
Compare
@rexagod I rebased on the latest main branch and fixed the test. :) |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mrueg, rexagod 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 |
Any chance of getting this included in a release soon? We need it :) |
What this PR does / why we need it:
In order to be able to deprecate the VPA completely, CRS needs to be aware of quantities. See #1790 (comment)
How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality)
None
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):See also #1790