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

♻️ Refactor DiskUsage functionality to support efs-guardian #6536

Merged

Conversation

GitHK
Copy link
Contributor

@GitHK GitHK commented Oct 15, 2024

What do these changes do?

It is now possible to overwrite DiskUsage for some paths mounted by the dynamic-sidecar. This is done via an RPC endpoint and should be used by the efs-guardian service.

Example data to send:

from servicelib.rabbitmq.rpc_interfaces.dynamic_sidecar.disk_usage import update_disk_usage

usage = {
    ".data_assets": DiskUsage.from_efs_guardian(used=10, total=100),
    "home_user_workspace": DiskUsage.from_efs_guardian(used=10, total=100)
}
await disk_usage.update_disk_usage(rpc_client, usage=usage)

Before this change disk usage was propagated to the frontend via dictionary containing paths and quotas. Paths are no longer supported and have been replaced with labels.

The disk_usage object supports the following keys HOST and STATES_VOLUMES which identify:

  • the quotas for the disks mounted to the HOST machine where the service was started
  • the quotas for the STATES_VOLUMES which can be provided via EFS and are optional.If this entry is not present all quotas are aggregated in the HOST

Currently the frontend only uses the HOST label, which should always be present

Example 1: all volumes exist on same machine

{
  "HOST": {
    "used": 0,
    "free": 1294390525952,
    "total": 1294390525952,
    "used_percent": 0.0
  }
}

Example 2: state volumes are mounted via EFS

{
  "HOST": {
    "used": 0,
    "free": 1294390525952,
    "total": 1294390525952,
    "used_percent": 0.0
  },
  "STATES_VOLUMES": {
    "used": 0,
    "free": 1000000000,
    "total": 1000000000,
    "used_percent": 0.0
  }
}

Related issue/s

How to test

Dev-ops checklist

@GitHK GitHK self-assigned this Oct 15, 2024
@GitHK GitHK added the a:dynamic-sidecar dynamic-sidecar service label Oct 15, 2024
@GitHK GitHK added this to the MartinKippenberger milestone Oct 15, 2024
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 92.48826% with 16 lines in your changes missing coverage. Please review.

Project coverage is 85.9%. Comparing base (cafbf96) to head (6c52bae).
Report is 647 commits behind head on master.

Files with missing lines Patch % Lines
...bitmq/rpc_interfaces/dynamic_sidecar/disk_usage.py 0.0% 12 Missing ⚠️
...amic_sidecar/modules/system_monitor/_disk_usage.py 95.3% 3 Missing ⚠️
...s_library/api_schemas_dynamic_sidecar/telemetry.py 97.8% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6536      +/-   ##
=========================================
+ Coverage    84.5%   85.9%    +1.3%     
=========================================
  Files          10    1395    +1385     
  Lines         214   59940   +59726     
  Branches       25    1571    +1546     
=========================================
+ Hits          181   51501   +51320     
- Misses         23    8202    +8179     
- Partials       10     237     +227     
Flag Coverage Δ
unittests 85.9% <92.4%> (+1.3%) ⬆️

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

Files with missing lines Coverage Δ
...ls_library/api_schemas_dynamic_sidecar/__init__.py 100.0% <100.0%> (ø)
...mcore_service_dynamic_sidecar/api/rest/__init__.py 100.0% <100.0%> (ø)
..._service_dynamic_sidecar/api/rest/_dependencies.py 100.0% <100.0%> (ø)
...mcore_service_dynamic_sidecar/api/rest/_routing.py 100.0% <100.0%> (ø)
...ore_service_dynamic_sidecar/api/rest/containers.py 98.3% <100.0%> (ø)
...e_dynamic_sidecar/api/rest/containers_extension.py 94.5% <100.0%> (ø)
..._sidecar/api/rest/containers_long_running_tasks.py 100.0% <100.0%> (ø)
...c/simcore_service_dynamic_sidecar/api/rest/disk.py 100.0% <100.0%> (ø)
...simcore_service_dynamic_sidecar/api/rest/health.py 100.0% <100.0%> (ø)
...ice_dynamic_sidecar/api/rest/prometheus_metrics.py 100.0% <100.0%> (ø)
... and 11 more

... and 1359 files with indirect coverage changes

@GitHK GitHK marked this pull request as ready for review October 16, 2024 08:32
@GitHK GitHK requested a review from giancarloromeo October 16, 2024 08:33
Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

I do not quite follow the need to remove the path from the point of view of the sidecar. these are still mounted paths for it so why change that?
maybe we can discuss tomorrow..

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.

Thx for the PR. I left some suggestions but I am not sure i fully grasp the idea here. Perhaps you can show me offline? thx

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.

Thanks 👍

Copy link
Contributor

@jsaq007 jsaq007 left a comment

Choose a reason for hiding this comment

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

👍🏼

@GitHK GitHK requested review from pcrespov and sanderegg October 18, 2024 07:59
@GitHK GitHK changed the title ♻️ Refactor DiskUsage functionality to support EFS ♻️ Refactor DiskUsage functionality to support efs-guardian Oct 18, 2024
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

thanks

@GitHK GitHK enabled auto-merge (squash) October 18, 2024 11:36
Copy link

@GitHK GitHK merged commit d200c57 into ITISFoundation:master Oct 18, 2024
56 of 57 checks passed
@GitHK GitHK deleted the pr-osparc-reword-disk-usage-notification branch October 18, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:dynamic-sidecar dynamic-sidecar service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants