Skip to content
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

✨Personalized resource limits: only allow specific groups to override resources #4417

Conversation

sanderegg
Copy link
Member

@sanderegg sanderegg commented Jun 23, 2023

What do these changes do?

General idea:

  • we can define which group can or cannot override services resources by changing the values in the table groups_extra_properties manually.
  • create new override_services_specifications column in groups_extra_properties
  • PUT /projects/{project_uuid}/nodes/{node_id}/resources only usable by users within these groups
  • new entrypoint on webserver GET /me/permissions that returns a list of users permissions:
    • currently only returns the permission named override_services_specifications
    • should allow to synchronise _access_roles with the pendant in the frontend
      image
  • new GroupsExtraProperties repository in postgres-database library:
    • aggregates these by following priority: 1. primary group, 2. any Truthy value in standard groups, 3. everyone group
  • fixes issue where a service resources from a converted project where no specific required_resources were defined originally could not be changed
  • frontend only sets the limits, this ensures both limits and reservations are set to the same value

Bonus:

  • update adminer to 4.8.1
  • improves error message when there is not enough resources to run a computational service in the default cluster by making the message more human readable:
    image

Related issue/s

How to test

groups extra properties

make devenv
cd package/postgres-database
make install-dev
clear && pytest --asyncio-mode=auto tests/test_test_utils_groups_extra_properties.py

DevOps Checklist

@sanderegg sanderegg added a:webserver issue related to the webserver service a:database associated to postgres service and postgres-database package labels Jun 23, 2023
@sanderegg sanderegg added this to the Watermelon milestone Jun 23, 2023
@sanderegg sanderegg self-assigned this Jun 23, 2023
@codecov
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

Merging #4417 (0f722e7) into master (fd95cd3) will increase coverage by 2.1%.
The diff coverage is 92.9%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #4417     +/-   ##
========================================
+ Coverage    83.1%   85.3%   +2.1%     
========================================
  Files         963    1000     +37     
  Lines       40927   42817   +1890     
  Branches     1012    1026     +14     
========================================
+ Hits        34018   36526   +2508     
+ Misses       6675    6056    -619     
- Partials      234     235      +1     
Flag Coverage Δ
integrationtests 61.0% <54.9%> (-6.8%) ⬇️
unittests 83.8% <92.9%> (+3.5%) ⬆️

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

Impacted Files Coverage Δ
...ostgres_database/models/groups_extra_properties.py 100.0% <ø> (ø)
...imcore_service_webserver/security/_access_roles.py 100.0% <ø> (ø)
.../src/simcore_service_webserver/statics/settings.py 83.3% <ø> (ø)
...s-library/src/models_library/services_resources.py 86.4% <40.0%> (-4.4%) ⬇️
...simcore_service_webserver/projects/projects_api.py 83.0% <85.7%> (-8.2%) ⬇️
...r-v2/src/simcore_service_director_v2/utils/dask.py 88.8% <86.8%> (-1.0%) ⬇️
...postgres_database/utils_groups_extra_properties.py 95.6% <95.6%> (ø)
...ages/models-library/src/models_library/clusters.py 100.0% <100.0%> (ø)
...base/src/simcore_postgres_database/utils_models.py 100.0% <100.0%> (ø)
...core_service_webserver/projects/_nodes_handlers.py 90.2% <100.0%> (+0.1%) ⬆️
... and 7 more

... and 114 files with indirect coverage changes

@sanderegg sanderegg force-pushed the personalized-resource-limits/only-allow-specific-groups branch from e09fc37 to 5d2bd9e Compare June 23, 2023 17:08
@sanderegg sanderegg force-pushed the personalized-resource-limits/only-allow-specific-groups branch 2 times, most recently from abe4460 to 7b8ad90 Compare June 26, 2023 16:26
@sanderegg sanderegg force-pushed the personalized-resource-limits/only-allow-specific-groups branch 2 times, most recently from 8ac212a to d94bd41 Compare June 27, 2023 14:21
@sanderegg sanderegg marked this pull request as ready for review June 27, 2023 14:22
@mguidon
Copy link
Member

mguidon commented Jun 27, 2023

✔️

Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice job!

Copy link
Contributor

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice! 👍

@sanderegg sanderegg requested a review from GitHK June 28, 2023 08:41
@codeclimate
Copy link

codeclimate bot commented Jun 28, 2023

Code Climate has analyzed commit 0f722e7 and detected 0 issues on this pull request.

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Jun 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, I read over it and it looks very promissing :)

If you want feel free to add a DevOps Warning/Tasks to remove the previous service overwrites we had in the DB (for Taylor etc.) once this propagates, if you want

@sanderegg
Copy link
Member Author

Thanks a lot, I read over it and it looks very promissing :)

If you want feel free to add a DevOps Warning/Tasks to remove the previous service overwrites we had in the DB (for Taylor etc.) once this propagates, if you want

done in ITISFoundation/osparc-ops-environments#256

@sanderegg sanderegg merged commit 52fb373 into ITISFoundation:master Jun 29, 2023
@sanderegg sanderegg deleted the personalized-resource-limits/only-allow-specific-groups branch June 29, 2023 06:02
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jul 12, 2023
36 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:database associated to postgres service and postgres-database package a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants