✨ Enhancement/get node resources #2987

sanderegg · 2022-04-12T15:43:30Z

What do these changes do?

A resource is a number (int or float) or a string that represents a named resource (currently such as CPU, RAM, VRAM, AIRAM, MPI).
Each resource has 2 such values, one that represents a reservation (for schedule-time) to define if a service can be started (is there still machine with enough CPUs to start he service?) and a limit (for runtime) that is used to stop a service that would go over that limit.
A resource limit is always at least equal to a resource reservation.

Currently the resources used for running osparc services are statically defined on the service docker images.
These values may be retrieved using the following new API endpoints in the webserver:

GET /catalog/services/{service_key}/{service_version}/resources
GET /projects/{puuid}/nodes/{node_uuid}/resources

And in the catalog

GET /services/{service_key}/{service_version}/resources

In this PR, only the static values are ever retrieved. In the near future, it will be possible to modify the limits on a particular service.

This means the current implementation in the webserver ALWAYS returns the values from the catalog service

NOTE: this implementation returns "raw" resources such as VRAM. these are used as is for running dynamic services

Related issue/s

ITISFoundation/osparc-issues#618

How to test

Checklist

codecov · 2022-04-12T15:49:17Z

Codecov Report

Merging #2987 (9fa8567) into master (74f158c) will increase coverage by 0.0%.
The diff coverage is 83.6%.

@@           Coverage Diff           @@
##           master   #2987    +/-   ##
=======================================
  Coverage    79.5%   79.6%            
=======================================
  Files         686     688     +2     
  Lines       28571   28694   +123     
  Branches     3683    3701    +18     
=======================================
+ Hits        22731   22847   +116     
+ Misses       5030    5029     -1     
- Partials      810     818     +8

Flag	Coverage Δ
integrationtests	`65.5% <30.5%> (-0.3%)`	⬇️
unittests	`75.3% <83.6%> (+0.1%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...mcore_service_catalog/api/dependencies/director.py	`100.0% <ø> (+18.1%)`	⬆️
...er/src/simcore_service_webserver/security_roles.py	`100.0% <ø> (ø)`
...ages/models-library/src/models_library/generics.py	`95.1% <50.0%> (+3.2%)`	⬆️
...er/src/simcore_service_webserver/catalog_client.py	`57.6% <64.2%> (+1.1%)`	⬆️
...s-library/src/models_library/services_resources.py	`73.3% <73.3%> (ø)`
...vice_webserver/projects/projects_nodes_handlers.py	`71.7% <82.5%> (+4.7%)`	⬆️
...src/simcore_service_catalog/api/routes/services.py	`66.1% <83.7%> (+7.0%)`	⬆️
...rary/src/models_library/service_settings_labels.py	`100.0% <100.0%> (ø)`
...mcore_service_catalog/api/dependencies/services.py	`100.0% <100.0%> (ø)`
...talog/src/simcore_service_catalog/core/settings.py	`100.0% <100.0%> (ø)`
... and 16 more

pcrespov

Excellent. Some comments and thoughts

packages/models-library/src/models_library/service_settings_labels.py

packages/models-library/src/models_library/services_resources.py

services/catalog/src/simcore_service_catalog/api/routes/services.py

pcrespov · 2022-04-21T09:58:47Z

services/catalog/src/simcore_service_catalog/api/routes/services.py

+    logger.debug("received %s", f"{service_settings}")
+
+    def _from_service_settings(
+        settings: List[SimcoreServiceSettingLabelEntry],


THOUGHT: I wonder if at some point this function should be attached as utils close to SimcoreServiceSettingsLabelEntrty .

See for instance in integration-library
There a model can be exported to or parsed from to label annotations

NOTE1: export and parse are the fundamental operations in pydantic

NOTE2: OCI standard officially call to the docker labels as "label annotations".

yes unless there are dependencies that the model_library cannot cope with.

and I find a bit confusing to rename labels into annotations actually. but if the next docker version does it I'll do it too.

pcrespov · 2022-04-21T10:03:58Z

services/catalog/src/simcore_service_catalog/api/routes/services.py

+        get_default_service_resources
+    ),
+):
+    # TODO: --> PC: I'll need to go through that with you for function services,


Yes, we have to review this. Currently these servcies do not consume any significant resources. So either we set them to 0 or remove them from the listing. Let's discuss about it offline.

0 is out of the discussion. I disagree. or we will end up with things suddenly consuming resource somewhere totally wild.

services/catalog/src/simcore_service_catalog/models/schemas/services.py

services/web/server/src/simcore_service_webserver/projects/projects_nodes_handlers.py

pcrespov · 2022-04-21T10:22:57Z

services/web/server/tests/unit/with_dbs/01/test_catalog_api.py

-        aiohttp_client(app, server_kwargs={"port": app_cfg["main"]["port"]})
-    )
+# @pytest.fixture
+# def app_cfg(default_app_cfg: ConfigDict, unused_tcp_port_factory: Callable):


Forgot to delete? Why commented

ah yes, that is what I was trying to ask you this morning...

I would like to discuss app_cfg and client fixtures, cause I find them a bit confusing. and I wonder if I'm not missing something there.

GitHK

Nice. Just not sure about the 2Gb limit for all services which do not specify any limit. As said below I would go for something much lower and let the users adjust if their services fail.

GitHK · 2022-04-22T06:14:56Z

services/catalog/openapi.json

+          "bootOptions": {
+            "title": "Bootoptions",
+            "type": "object",
+            "description": "Some services provide alternative parameters to be injected at boot time. The user selection should be stored here, and it will overwrite the services's defaults."


how come I see bootOptions here? Uhm... I wonder what happened.

well this is generated with the make openapi-spec recipe.. not sure when it was run last time...

GitHK · 2022-04-22T06:24:14Z

services/catalog/src/simcore_service_catalog/core/settings.py

+            "limit": ByteSize(2 * 1024**3),
+            "reservation": ByteSize(2 * 1024**3),


Wait, so by default you get 2 GB limit and reservation? Are we sure about this? Should we not bring the values in the MB ranges?

For example, when defining services like the sim4life. You have an Nginx without any constraints. It is not worth it to assign so many resources for a service which requires almost 100 MB.

I'd suggest to bump it down to 100MB (or something in this range), and let people explicitly ask for more resources if required. I think this will help with reducing memory consumption by a lot.

ok, so that is the value that was here already before. Open to discussion but in a separate PR cause it might have other implications. That is what the current director-v0 also does (but not wired to this one for now), so it could have issues with some older services.

Now concerning your use case:

this call returns the total envelope, not the nginx service. so for computing the nginx service in your compose it should go with: get service limit/resources - what is defined in the compose for specifics = nginx (+ maybe other) services limits

services/catalog/tests/unit/with_dbs/test_entrypoint_services.py

sanderegg added the changelog:🎨enhancement label Apr 12, 2022

sanderegg added this to the Macarons milestone Apr 12, 2022

sanderegg self-assigned this Apr 12, 2022

sanderegg force-pushed the enhancement/get_node_resources branch 3 times, most recently from 8270e68 to 295b0e7 Compare April 20, 2022 20:47

sanderegg changed the title ~~WIP: ✨ Enhancement/get node resources~~ ✨ Enhancement/get node resources Apr 21, 2022

sanderegg marked this pull request as ready for review April 21, 2022 08:36

sanderegg requested review from pcrespov and GitHK as code owners April 21, 2022 08:36

sanderegg requested review from odeimaiz and mguidon April 21, 2022 08:36

pcrespov approved these changes Apr 21, 2022

View reviewed changes

sanderegg added 16 commits April 21, 2022 22:24

removed unused code

dcfa8ab

added entrypoint for resources

2260d21

added replace entrypoint

0799e9c

setup test for getting resources

b1a3d04

added test for replacing resources

27c0d48

added api for getting service resources

b611665

adding tests

9d1445a

fix typing

1ed4d2f

add update method to DictModel

3f0e95e

use ListModel

dbe5c6b

add service resources

499b012

cleanup

5c3d08a

add services dependencies

c55dafd

added entrypoint to get services resources and default

cf184d9

cleanup

91e7892

create a test for the catalog entrypoint

ea9b439

sanderegg added 23 commits April 21, 2022 22:24

test entrypoint for getting service resources

b67de3f

refactor

f6e1e53

added test for risen error

9294171

@Crespov: help needed here

6a5da8e

define resources directly with limit and reservation

8cd43a4

updated specs of catalog

85d85d9

updated specs of webserver

2ed9afa

add caching in getting service resources

1da5f69

getting project node resources returns defaults

aa9bf46

test for webserver in progress

7ca9263

typing

1036e1d

test passes

38852e9

missing product plugin

e85511d

handle client connection error

4d0c1b6

fixed unit test for getting node resources

c5335d6

tests running fine

e7be250

also check for not found

ed82db2

test not found works

74e6c73

linter

481b7c3

use capital letters for resources names

591fae5

@pcrespov review: use alias instead of explicit inheritance

d2caacf

@pcrespov review: use alias instead of explicit inheritance

7f2ea5f

@pcrespov review: remove comments

9fa8567

sanderegg force-pushed the enhancement/get_node_resources branch from 7a699ad to 9fa8567 Compare April 21, 2022 20:24

GitHK approved these changes Apr 22, 2022

View reviewed changes

sanderegg merged commit 98d6049 into ITISFoundation:master Apr 22, 2022

sanderegg deleted the enhancement/get_node_resources branch April 22, 2022 07:54

GitHK mentioned this pull request Apr 26, 2022

✨dynamic-sidecar limits all spawned containers (⚠️ devops) #2988

Merged

10 tasks

sanderegg mentioned this pull request Apr 29, 2022

personalizable resource limits ITISFoundation/osparc-issues#618

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Enhancement/get node resources #2987

✨ Enhancement/get node resources #2987

sanderegg commented Apr 12, 2022 •

edited by odeimaiz

Loading

codecov bot commented Apr 12, 2022 •

edited

Loading

pcrespov left a comment

pcrespov Apr 21, 2022

sanderegg Apr 21, 2022

sanderegg Apr 21, 2022

pcrespov Apr 21, 2022

sanderegg Apr 21, 2022

pcrespov Apr 21, 2022

sanderegg Apr 21, 2022

sanderegg Apr 21, 2022

GitHK left a comment

GitHK Apr 22, 2022

sanderegg Apr 22, 2022

GitHK Apr 22, 2022

sanderegg Apr 22, 2022

		"limit": ByteSize(2 * 1024**3),
		"reservation": ByteSize(2 * 1024**3),

✨ Enhancement/get node resources #2987

✨ Enhancement/get node resources #2987

Conversation

sanderegg commented Apr 12, 2022 • edited by odeimaiz Loading

What do these changes do?

Related issue/s

How to test

Checklist

codecov bot commented Apr 12, 2022 • edited Loading

Codecov Report

pcrespov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GitHK left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanderegg commented Apr 12, 2022 •

edited by odeimaiz

Loading

codecov bot commented Apr 12, 2022 •

edited

Loading