ganga-devs / ganga

Ganga is an easy-to-use frontend for job definition and management
GNU General Public License v3.0
100 stars 159 forks source link

#2293: Change name of the Singularity plugin to Apptainer #2303

Closed dg1223 closed 8 months ago

dg1223 commented 8 months ago

Related issue: #2293

Changelog

- created a copy of Singularity.py called Apptainer.py and refactored it
- created a copy of temp_test.py called temp_test_apptainer.py and refactored it
- added function checkApptainer to Virtualization.py
- added checkApptainer to import statement in variable extra
- added one-to-one mapping of singularity <> apptainer in container_controllers.py
- added apptainer_handler to migrate.py
- added one-to-one mapping of singularity <> apptainer in TestContainerHandler.py
- added Apptainer import statement to ganga/GangaCore/Lib/Virtualization/__init__.py
- created a copy of TestUtilitiesVirtualizationCheckSingularity.py called TestUtilitiesVirtualizationCheckApptainer.py and refactored it
- added one-to-one mapping of singularity <> apptainer in GangaRepositoryDatabase.py
- added apptainer as an option to database config in ganga/GangaCore/__init__.py
- added documentation for Apptainer to Virtualization.rst
- added apptainer to documentation in WhatIsGanga.rst

Testing

Ensure Singularity is not broken

j = Job(name='sing1')
j.virtualization = Singularity("docker://gitlab-registry.cern.ch/lhcb-core/lbdocker/fedora:latest")
j.submit()

jobs # expected to fail because of fake docker image

Result: Test ran successfully after making Apptainer related updates

Run unit tests

python -m pytest ganga/GangaCore/test/Unit

Result: Passed all unit tests including the two newly added tests in TestUtilitiesVirtualizationCheckApptainer.py

Install and test Apptainer

Install Apptainer from source

Run test

j = Job(name='apptainer2')
j.virtualization = Apptainer("docker://gitlab-registry.cern.ch/linuxsupport/alma9-base")
j.submit()

Result: Job finished with completed status.

dg1223 commented 8 months ago

Looks like the linter is unhappy.

I checked the CI output. Most, if not all, are pre-existing lint violations such as:

ganga/GangaCore/Lib/Virtualization/__init__.py:1:1: F401 '.Docker.Docker' imported but unused
ganga/GangaCore/Core/GangaRepository/GangaRepositoryDatabase.py:178:128: E501 line too long (142 > 127 characters)

I'm gonna go ahead and fix them all. Adding a flake8 unit test to the Ganga repo should prevent non-lint-checked codes from being submitted in future.

Another nice little update to the CI pipeline could be running an autopep8 step before the lint checking step and then perform the create PR task separately. That may resolve formatting issues and possibly prevent minor lint failures. Currently, the Create autopep8 PR step is doing two things. It can be divided as above.

dg1223 commented 8 months ago

Fixed the lint errors and pushed the updates.

Resolution

Results

No more lint errors

(gangaenv) []@[]: ganga:$flake8 ganga/GangaCore/Lib/Virtualization/Apptainer.py
0
(gangaenv) []@[]: ganga:$flake8 ganga/GangaCore/test/Unit/Utility/TestUtilitiesVirtualizationCheckApptainer.py
0
(gangaenv) []@[]: ganga:$flake8 ganga/GangaCore/test/gangaDB/db_controllers/temp_test_apptainer.py
0
(gangaenv) []@[]: ganga:$flake8 ganga/GangaCore/Core/GangaRepository/GangaRepositoryDatabase.py
0
(gangaenv) []@[]: ganga:$flake8 ganga/GangaCore/Core/GangaRepository/container_controllers.py
0
(gangaenv) []@[]: ganga:$flake8 ganga/GangaCore/Core/GangaRepository/migrate.py
0
(gangaenv) []@[]: ganga:$flake8 ganga/GangaCore/Lib/Virtualization/__init__.py
0
(gangaenv) []@[]: ganga:$flake8 ganga/GangaCore/Utility/Virtualization.py
0
(gangaenv) []@[]: ganga:$flake8 ganga/GangaCore/test/gangaDB/db_controllers/TestContainerHandler.py
0

Regular tests passed

Unit tests as well as tests on Singularity and Apptainer finished successfully.

egede commented 8 months ago

Looks like the linter is unhappy.

I checked the CI output. Most, if not all, are pre-existing lint violations such as:

ganga/GangaCore/Lib/Virtualization/__init__.py:1:1: F401 '.Docker.Docker' imported but unused
ganga/GangaCore/Core/GangaRepository/GangaRepositoryDatabase.py:178:128: E501 line too long (142 > 127 characters)

I'm gonna go ahead and fix them all. Adding a flake8 unit test to the Ganga repo should prevent non-lint-checked codes from being submitted in future.

Another nice little update to the CI pipeline could be running an autopep8 step before the lint checking step and then perform the create PR task separately. That may resolve formatting issues and possibly prevent minor lint failures. Currently, the Create autopep8 PR step is doing two things. It can be divided as above.

I am not quite sure that I follow the suggestion here. The linting check is mainly catching old non-compliant code. So the changes to make it pass are in general not related to the issue getting fixed. This is the reason that we prefer to get the linting done in a separate PR - it makes it easier to review. Now there is an extra small problem in that PRs from forks can't create the new PR. That is an issue that we have not solved in a satisfactory way.

dg1223 commented 8 months ago

Understood. I'll open seperate PRs for linting from next time.

The issue with PRs from forks sounds like an interesting thing to investigate. Thanks.

dg1223 commented 8 months ago

Happy to contribute :)

dg1223 commented 8 months ago

Resovled a couple of merge conflicts in documentation files WhatIsGanga.rst and Virtualization.rst.

mesmith75 commented 8 months ago

@egede What is the plan with this? I would like to release a new version and it could be nice to include it

dg1223 commented 8 months ago

Should I be updating the branch again and trigger another CI run? It is out-of-date right now with the develop branch.

egede commented 8 months ago

Should I be updating the branch again and trigger another CI run? It is out-of-date right now with the develop branch.

I just did. This is all done from your side. Many thanks.

dg1223 commented 8 months ago

Should I be updating the branch again and trigger another CI run? It is out-of-date right now with the develop branch.

I just did. This is all done from your side. Many thanks.

You are welcome :)