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.

Sign up for GitHub

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

Jump to bottom

Add CPU information (model, vendor, frequency) and memory details (clock, size, type) to API #544

Open

aliel wants to merge 18 commits into main from aliel-feature-add-cpu-mem-capability

Member

aliel commented Feb 16, 2024

Update CPU and memory details by switching to lshw method instead of cpuinfo
Add CPU information (model, vendor, frequency) and memory details (clock, size, type) to API

github-actions bot added the BLUE label

aliel force-pushed the aliel-feature-add-cpu-mem-capability branch 3 times, most recently from 54ff260 to 7185d02 Compare

February 20, 2024 14:09

hoh reviewed

View reviewed changes

Member

hoh left a comment

Can we have automated tests that ensure that this is working fine ?

Maybe in the .github/workflows/test-on-droplets-matrix.yml file ?

src/aleph/vm/orchestrator/machine.py

+                  if "x86_64" in cpu_info["capabilities"] or "x86-64" in cpu_info["capabilities"]:
+                      architecture = "x86_64"
+                  elif "arm64" in cpu_info["capabilities"] or "arm-64" in cpu_info["capabilities"]:
+                      architecture = "arm64"

Member

hoh Feb 20, 2024

Is this really the best approach to get the architecture ? Seems weird to look for it in the CPU capabilities.
What if a 32 bit system is running on a x86-64 capable system ?

Member Author

aliel Apr 5, 2024

here we want to know the architecture supported by the processor, not by the system
Obviously, 64-bit processors support 32-bit systems, but not vice versa.

Member

hoh commented Feb 20, 2024

Can you also provide an example of the JSON format returned by the API ?

aliel force-pushed the aliel-feature-add-cpu-mem-capability branch from 7185d02 to 9df2486 Compare

March 5, 2024 14:42

hoh reviewed

View reviewed changes

src/aleph/vm/orchestrator/machine.py Outdated Show resolved Hide resolved

src/aleph/vm/orchestrator/machine.py Outdated

+              @lru_cache
+              def get_hardware_info():
+                  lshw = subprocess.Popen(["lshw", "-sanitize", "-json"], stdout=subprocess.PIPE, shell=False)

Member

hoh Mar 7, 2024

A check in the settings that verifies that lshw is available on the machine would be a good addition.

src/aleph/vm/orchestrator/machine.py Outdated Show resolved Hide resolved

hoh force-pushed the aliel-feature-add-cpu-mem-capability branch from 9df2486 to 55fae43 Compare

March 7, 2024 16:12

Member

hoh commented Mar 7, 2024

Rebased on main.

hoh reviewed

View reviewed changes

src/aleph/vm/orchestrator/machine.py

+                      "type": memory_type,
+                      "clock": memory_clock,
+                      "clock_units": "Hz",
+                  }

Member

hoh Mar 7, 2024

Why not return an instance of MemoryProperties ?

src/aleph/vm/orchestrator/machine.py

+                      "model": cpu_info["product"],
+                      "frequency": cpu_info["capacity"],
+                      "count": psutil.cpu_count(),
+                  }

Member

hoh Mar 7, 2024

Why return a dict and not instances of the Properties / Capabilities classes defined blow in the diff ?

hoh reviewed

View reviewed changes

src/aleph/vm/orchestrator/machine.py Outdated Show resolved Hide resolved

hoh assigned aliel

aliel force-pushed the aliel-feature-add-cpu-mem-capability branch from 55fae43 to 1ae4da2 Compare

April 5, 2024 09:38


          Update CPU and memory details by switching to lshw method instead of …

d82570d

…cpuinfo

Add CPU information (model, vendor, frequency) and memory details (clock, size, type) to API

aliel force-pushed the aliel-feature-add-cpu-mem-capability branch 2 times, most recently from 3bcda91 to a483efd Compare

April 5, 2024 10:09


          Refactor get_hardware_info() to use async subprocess.run for improved…

a4d2a5c

… concurrency

avoid Try-Catch using assertions

aliel force-pushed the aliel-feature-add-cpu-mem-capability branch from a483efd to a4d2a5c Compare

April 5, 2024 10:26


          Fix code style

1a492ba

hoh requested a review from olethanh

April 9, 2024 12:41

aleph-im deleted a comment from github-actions bot

olethanh added 2 commits

April 30, 2024 15:25


          Merge remote-tracking branch 'origin/main' into aliel-feature-add-cpu…

76c4982

…-mem-capability


          Remove duplicate cors headers

7e54078

olethanh reviewed

View reviewed changes

src/aleph/vm/orchestrator/machine.py Outdated

+                  memory_clock = ""
+                  for bank in mem_info["children"]:
+                      memory_clock = bank["clock"]

Collaborator

olethanh Apr 30, 2024

On CRN-TEST-2204 I have this in mem_info. No clock key. Should I remove the information conditionally?

 {'capabilities': {'ecc': 'Multi-bit error-correcting code (ECC)'},
            'children': [{'claimed': True,
                          'class': 'memory',
                          'description': 'DIMM RAM',
                          'handle': 'DMI:1100',
                          'id': 'bank',
                          'physid': '0',
                          'size': 17179869184,
                          'slot': 'DIMM 0',
                          'units': 'bytes',
                          'vendor': 'QEMU'}],
            'claimed': True,
            'class': 'memory',
            'configuration': {'errordetection': 'multi-bit-ecc'},
            'description': 'System Memory',
            'handle': 'DMI:1000',
            'id': 'memory',
            'physid': '1000',
            'size': 17179869184,
            'units': 'bytes'}}

olethanh added 3 commits

April 30, 2024 16:03


          Problem: async funcs cannott be lru_cached

b7aaf47

Solution: Introduce our own decorator


          fix lshw parsing for memory info

6a3aff9


          Move async_cache decorator to utils module

c4c2d0d

olethanh mentioned this pull request

Problem /about/usage/system was not tested #609

Merged


          Change router order

0e079a0

it's not really important but it was somehow anoying me


          fix imports

4795a85

codecov bot commented Apr 30, 2024 •

edited

Loading

Codecov Report

Attention: Patch coverage is 42.68293% with 47 lines in your changes are missing coverage. Please review.

Project coverage is 43.73%. Comparing base (7b89523) to head (4795a85).

❗ Current head 4795a85 differs from pull request most recent head befc521

Please upload reports for the commit befc521 to get more accurate results.

Files	Patch %	Lines
src/aleph/vm/orchestrator/machine.py	17.07%	34 Missing ⚠️
src/aleph/vm/orchestrator/resources.py	70.00%	9 Missing ⚠️
src/aleph/vm/utils.py	60.00%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #544       +/-   ##
===========================================
- Coverage   53.87%   43.73%   -10.15%     
===========================================
  Files          58       56        -2     
  Lines        5310     5053      -257     
  Branches      594      598        +4     
===========================================
- Hits         2861     2210      -651     
- Misses       2311     2726      +415     
+ Partials      138      117       -21

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

olethanh added 8 commits

May 3, 2024 16:23


          Merge remote-tracking branch 'origin/main' into aliel-feature-add-cpu…

b926497

…-mem-capability


          Merge remote-tracking branch 'origin/main' into aliel-feature-add-cpu…

e5c5a13

…-mem-capability


          fix test

eac96f2


          add test for /about/capability

783e5ea


          Remove unused deps

da65e94


          black

c42e480


          Merge remote-tracking branch 'origin/main' into aliel-feature-add-cpu…

87122e8

…-mem-capability


          Fix depencies in ci

befc521

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BLUE