feat(openapi): add K8s resource requests and limits in reana.yaml #486
Conversation
…nahub#486) Allow users to set CPU and memory requests/limits via `reana.yaml`. If not specified, default values configured by cluster admins will be applied. Closes reanahub/reana#883
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #486 +/- ##
==========================================
- Coverage 41.79% 41.62% -0.17%
==========================================
Files 28 28
Lines 2010 2042 +32
==========================================
+ Hits 840 850 +10
- Misses 1170 1192 +22
🚀 New features to boost your workflow:
|
…nahub#486) Allow users to set CPU and memory requests/limits via `reana.yaml`. If not specified, default values configured by cluster admins will be applied. Closes reanahub/reana#883
…nahub#486) Allow users to set CPU and memory requests/limits via `reana.yaml`. If not specified, default values configured by cluster admins will be applied. Closes reanahub/reana#883
…nahub#486) Allow users to set CPU and memory requests/limits via `reana.yaml`. If not specified, default values configured by cluster admins will be applied. Closes reanahub/reana#883
…nahub#486) Allow users to set CPU and memory requests/limits via `reana.yaml`. If not specified, default values configured by cluster admins will be applied. Closes reanahub/reana#883
| "value": "2" | ||
| }, | ||
| "kubernetes_cpu_request": { | ||
| "title": "Default cpu request for Kubernetes jobs", |
There was a problem hiding this comment.
In the title, please use CPU is uppercase, here and elsewhere.
| "kubernetes_max_memory_limit": { | ||
| "title": "Maximum allowed memory limit for Kubernetes jobs", | ||
| "value": "10Gi" | ||
| "value": "10Gi\"" |
There was a problem hiding this comment.
Please remove extra backquote.
| "kubernetes_cpu_request": { | ||
| "type": "string", | ||
| "title": "Kubernetes cpu request", | ||
| "description": "Kubernetes cpu request (e.g. 1 - read more about the expected memory values on the official Kubernetes documentation: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#meaning-of-cpu)" |
There was a problem hiding this comment.
Please put CPU in uppercase, here and in lower lines.
tests/test_job_utils.py
Outdated
| ], | ||
| ) | ||
| def test_kubernetes_cpu_to_millicores(k8s_cpu, millicores): | ||
| """Test conversion of k8s CPU format to millicores.""" |
There was a problem hiding this comment.
Cosmetics; In textual documentation, the usual abbreviation is "K8s" i.e. uppercase K.
reana_commons/config.py
Outdated
| @@ -420,6 +420,9 @@ def default_workspace(): | |||
| ) | |||
| """Kubernetes valid memory format regular expression e.g. Ki, M, Gi, G, etc.""" | |||
|
|
|||
| KUBERNETES_CPU_FORMAT = r"^(?P<value_millicpu>[1-9]\d*)m$|^(?P<value_cpu>(0*[1-9]\d*(\.\d+)?|0*\.\d*[1-9]\d*))$" | |||
| """Kubernetes valid CPU format regex. Supports formats like 500m, 1, 0.1. Values must be greater than 0.""" | |||
There was a problem hiding this comment.
Cosmetics: please put the example values in ascending order, so... "0.1" (or "100m"), "0.9" (or "900m"). This would also gently hint that 0.1=100m and that the two value systems are just two ways how to express the same thing.
Example: Valid K8s CPU core limit format, such as "0.1" (or "100m"), "0.9" (or "900m").
| @@ -99,6 +99,9 @@ def submit( # noqa: C901 | |||
| compute_backend=None, | |||
| kerberos=False, | |||
| kubernetes_uid=None, | |||
| kubernetes_cpu_request=None, | |||
There was a problem hiding this comment.
Since this is a minor patchlevel release, and since somebody may be using Python API to submit workflows, perhaps we should add the new optional arguments at the end? Ideally, they would be close together, but let's check how RECAST uses the API to make sure that we don't break their submission workflow by rearranging argument order.
(The safest option would be just to add them at the end, even though it's not pretty...)
There was a problem hiding this comment.
OK, just checked recast-atlas sources: https://github.com/recast-hep/recast-atlas/blob/main/src/recastatlas/backends/reana.py
We should be good WRT parameter order, since the RECAST client does not seem to be using submit() from the REANA client API.
…nahub#486) Allow users to set CPU and memory requests/limits via `reana.yaml`. If not specified, default values configured by cluster admins will be applied. Closes reanahub/reana#883
6834cb2 to
a9a0031
Compare
…anahub#486) Allow users to set CPU and memory requests/limits via `reana.yaml`. If not specified, default values configured by cluster admins will be applied. Closes reanahub/reana#883
…anahub#486) Allow users to set CPU and memory requests/limits via `reana.yaml`. If not specified, default values configured by cluster admins will be applied. Closes reanahub/reana#883
Allow users to set CPU and memory requests/limits via
reana.yaml. If not specified, default values configured by cluster admins will be applied.Closes reanahub/reana#883