rook-ceph: Allow user to provide sub-component resources #1483
Conversation
surajssd
force-pushed
the
surajssd/rook-add-resources
branch
from
ea5ab94 to
bbb162a
Compare
assets/terraform-modules/tinkerbell-sandbox/assets/deploy/tls/Dockerfile
Outdated
Show resolved
Hide resolved
surajssd
force-pushed
the
surajssd/rook-add-resources
branch
from
05e2196 to
a1cd8dc
Compare
There was a problem hiding this comment.
Are there reasonable examples out on in the world that provide resource limits/requests for the Rook osd, monitor, prepare etc jobs ? I am merely asking whats the need for this feature ? Has there been instances where the resources in question are consuming lots of cpu and memory ?
If so, might we also suggest sane values to use in the configuration reference document or the the existing how to guide for configuring rook/rook-ceph (
| @@ -103,6 +104,29 @@ func MatchJSONPathInt64Value(t *testing.T, yamlConfig string, jsonPath string, e | |||
| } | |||
| } | |||
|
|
|||
| // MatchJSONPathJSONValue is a helper function for component unit tests. It compares the JSON values | |||
There was a problem hiding this comment.
Any new util functions that need not be visible should be part of the internal package
example this file : internal/util_test.go
There was a problem hiding this comment.
But this pkg/components/internal/testutil/jsonpath.go has all the relevant code that deals with finding values at JSON path, converting into appropriate concrete data types and comparing values.
There was a problem hiding this comment.
And it will be needed in various _test.go files, I don't fully understand what are you suggesting?
There was a problem hiding this comment.
I'm suggesting to use the internal package for utility functions such as the one below
https://github.com/kinvolk/lokomotive/blob/master/internal/util_test.go
There was a problem hiding this comment.
But the file that you are linking is a test code of the internal package. And internal package is consumed by anyone and everyone meanwhile the testutil is only consumed by tests. I still don't understand what is the rationale behind putting the test helper code away from the similar code?
Yes, the linked issue describes the problem: #1476.
Yes,this is a good idea to point folks to https://rook.io/docs/rook/v1.6/ceph-cluster-crd.html#cluster-wide-resources-configuration-settings. But instead of doing it via how-to-guide, I think pointing folks from the config ref makes it more accessible. How-to-guide by the nature of it could hide the details. |
Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
This commit adds a new function which compares JSON value at a JSON
path. For e.g.
```
spec:
resources:
osd: {"requests":{"cpu":"5","memory":"5Gi"},"limits":{"cpu":"5","memory":"5Gi"}}
```
Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
This commit adds a new parameter `resources` for sub-components' cpu and memory requests and limits. Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
surajssd
force-pushed
the
surajssd/rook-add-resources
branch
from
a1cd8dc to
aafdcca
Compare
|
@ipochi I have addressed your concerns regarding documentation, PTAL. |
invidian
added
area/components
This PR adds a new parameter
resourcesfor sub-components' cpu andmemory requests and limits.
Fixes #1476