Drop ECR for testing builds and Add 22.04 Support #201
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We no longer support Python 3.8 so we don't need to build or test with it.
Rather than relying on a different build workflow to have run on master, build the image(s) we need for testing within this workflow. To reduce test time, use the github actions cache to cache intermediate layers. See https://docs.docker.com/build/cache/backends/gha/ for mre details.
Since we're now building the image(s) we need as a part of CI, we have no need for these docker images to be published to the 2U Tools Elastic Container Registry.
We want to be able to test codejail on multiple different versions of ubuntu and since we use docker containers to do this testing, we want to make sure this will work with different versions of the built image as well as different base versions of ubuntu.
feanil
force-pushed
the
feanil/publish_ubuntu_images
branch
from
2398892 to
3a3bf93
Compare
In the 24.04 image, the ubuntu user and group already exists in the default image so we don't need to create it ourselves.
feanil
force-pushed
the
feanil/publish_ubuntu_images
branch
5 times, most recently
from
297f899 to
28d97c6
Compare
|
@MoisesGSalas this is about how far I've gotten, I think it makes sense to land this without 24.04 for now and then try to fix that as a separate PR in the future. It looks like something has changed either with docker or with apparmor in that version causing it to not enforce the network limits. Can you review this and let me know if you have any concerns. The commit messages should generally explain things but please ask any questions you might have. |
This drops a burch of config variables that pylint is no longer aware of.
The new warnings don't show anything that we're uncomfortable with so ignore them rather than fixing them in this case. It's riskier to change the method signatures or the types of exceptions thrown from this critical bit of code.
Importing all of numpy loads OpenBlas which spawns many worker threads behind the scenes. Increasing the number of threads didn't seem to work. I got up to 30 before deciding that it would be better for the test to just limit the number of OpenBLAS threads. The addition of the following code seems to resolve the issue: ``` import os os.environ['OPENBLAS_NUM_THREADS'] = '1' ``` Reference: https://stackoverflow.com/questions/52026652/openblas-blas-thread-init-pthread-create-resource-temporarily-unavailable
This was happening automatically in previous versions of ubuntu but seems to not be happening correctly with the most recent profiles so making it part of the profile. There is a known issue with this and docker containers.
Codejail now supports running with ubunt 22.04 as well as ubuntu 20.04
This is not necessary for fixing issues but is good maintenance to do.
feanil
force-pushed
the
feanil/publish_ubuntu_images
branch
from
55bcf27 to
9a36bbd
Compare
|
@MoisesGSalas that's something we can consider so that we can just test the profiles without the docker related confusion but since we use docker as a part of tutor, it's useful to also test inside containers so I'm of two minds on the issue. For now let's focus on landing this for 22.04 and we can come back to 24.04 after the release. |
feanil
force-pushed
the
feanil/publish_ubuntu_images
branch
from
9a36bbd to
1b99481
Compare
arbrandes
approved these changes
Oct 22, 2024
MoisesGSalas
approved these changes
Oct 22, 2024
There was a problem hiding this comment.
Thanks for coming to the rescue, @feanil. The errors in the newer ubuntu versions were really tricky to pinpoint when I tried tackling the upgrade
feanil
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR simplifies the build and CI for codejail to make it easire to understand the testing and make it easier to test on multiple versions of Ubuntu.
Once this lands, the codejail code will support working on both Ubuntu 20.04 and Ubuntu 22.04 as long as the instructions in the readme are followed for setup. Especially with respect the sudo PAM config: 47842d9