Kubernetes step parsing and command line options #298
Conversation
hylje
force-pushed
the
leo-kubernetes-options
branch
2 times, most recently
from
558a8a2 to
5f89570
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #298 +/- ##
==========================================
+ Coverage 86.67% 86.74% +0.06%
==========================================
Files 126 126
Lines 4308 4359 +51
Branches 894 907 +13
==========================================
+ Hits 3734 3781 +47
- Misses 393 395 +2
- Partials 181 183 +2 ☔ View full report in Codecov by Sentry. |
hylje
force-pushed
the
leo-kubernetes-options
branch
2 times, most recently
from
0134aa9 to
f6e7b1a
Compare
hylje
force-pushed
the
leo-kubernetes-options
branch
from
f6e7b1a to
37e691d
Compare
hylje
force-pushed
the
leo-kubernetes-options
branch
from
37e691d to
2bd0153
Compare
| "kubernetes": { | ||
| 'containers': { |
There was a problem hiding this comment.
mixing of the " and ' hurts my souls a bit but doesn't really matter 👻
| assert run_test_setup.run_api_mock.last_create_execution_payload["runtime_config"] == { | ||
| "kubernetes": { | ||
| 'containers': { | ||
| 'workload': { | ||
| 'resources': { | ||
| 'requests': { | ||
| 'cpu': 1.0, | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
the rest are not sent in the payload as we except that the API fills them from the YAML 👍
| assert run_test_setup_kube.run_api_mock.last_create_execution_payload["runtime_config"] == { | ||
| "kubernetes": { | ||
| 'containers': { | ||
| 'workload': { | ||
| 'resources': { | ||
| 'requests': { | ||
| 'cpu': 1.0, | ||
| 'memory': 3, | ||
| }, | ||
| 'limits': { | ||
| 'cpu': 2.0, | ||
| 'memory': 4, | ||
| 'devices': {} | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
not sure I understand this, why are the non-gpu resources sent in this case?
There was a problem hiding this comment.
Resources from YAML are parsed and sent if available (see the run_test_setup_kube fixture), and command line arguments can override them further. --k8s-device-none overrides just the devices dict into an explicit empty dict, while other resources are not overridden.
In the above partial case, the YAML doesn't define any resources so we won't send them either
There was a problem hiding this comment.
but if we send YAML defined resources, wouldn't that mix up e.g. future copying? (we don't know know which of the passed parameters are API overrides vs. YAML overrides)
assuming that API doesn't compare passed resources to the YAML it knows 🤔
There was a problem hiding this comment.
but I guess it's fine if it work like the web UI, could double check it
There was a problem hiding this comment.
I don't think executions keep track of where the resulting configuration comes from, it's all put together and saved for later use. Copying doesn't refresh changes from yaml Steps, whether or not they were also supplied through the API or not.
I think the principle that front-end resolves the YAML itself and submits from it is alright, we already do it in CLI for parameters, inputs and environment variables.
I'm not against just leaving it for the API backend to work out, but I think that's its own project.
There was a problem hiding this comment.
I think the principle that front-end resolves the YAML itself and submits from it is alright
yeah, I also feel that it's all-OK 👍
I'm not against just leaving it for the API backend to work out, but I think that's its own project
yeah, definitely a separate thing if we feel that it's needed
Included in this PR:
resourcestructure from YAML when usingvh execution run, if provided.vh execution runcommand line argumentsvh pipeline rundoes not have execution runtime option override support for now.