-
Notifications
You must be signed in to change notification settings - Fork 240
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
Change to new Aventer Mesos URL #4290
Conversation
I'm also glomming a fix for #4282 onto here since I am messing about with deb dependencies anyway. |
OK, I had to adjust https://github.com/vgteam/dind to do some cgroup surgery on cgroups v2 machines, to get all the processes in the container out of the container's root cgroup, and into another cgroup, to allow new containers under the container's root cgroup to be created, because processes and cgroups with limits on them can't exist in the same cgroup under cgroups v2. Then I rebuilt the Toil and vg CI prebake containers. I also had to upgrade Singularity to one that works with cgroups v2, which meant I had to upgrade the base Ubuntu image to ine with a new enough libc for the newer Singularity builds. |
ed6504b
to
6b5a591
Compare
Now I have Singularity working if the Kubernetes pod is privileged , or if it has an
This is related to apptainer/singularity#5857 and to https://gitlab.nrp-nautilus.io/prp/nautilus-cluster/-/issues/570 The default Ubuntu AppArmor profiles on the new host nodes break Singularity (at least on cgroups v2). I think I have to turn apparmor off on the nodes because we can't just annotate all the pods. I don't think the old RMP-based base AMI has AppArmor on? |
I've modified the ASG templates for the cluster, but the nodes need to scale away and come back again before Singularity will work again, I think. |
Now I have set up AppArmor so it can allow Singularity to start in Docker containers: adamnovak/gi-kubernetes-autoscaling-config@b81b482 It might still not work under Toil at which point I can try blanket allowing "mount". |
# CPU affinity may limit the size | ||
affinity_size: Union[float, int] = float('inf') | ||
if hasattr(os, 'sched_getaffinity'): | ||
try: | ||
logger.debug('CPU affinity available') | ||
affinity_size = len(os.sched_getaffinity(0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using len(psutil.Process().cpu_affinity()
which is supported for Linux, Windows, FreeBSD (and we already use psutil
)
https://psutil.readthedocs.io/en/latest/index.html#psutil.Process.cpu_affinity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, nevermind, the implementation is quite similar between the two
https://github.com/python/cpython/blob/0076ca48e9cd116bb9d283d014fe69a72c9f1f6f/Modules/posixmodule.c#L7229
https://github.com/giampaolo/psutil/blob/5b3016ba3d7429c827c992699f50c2b7ff29b023/psutil/_psutil_linux.c#L239
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for this!
@@ -83,7 +84,7 @@ def heredoc(s): | |||
motd = ''.join(l + '\\n\\\n' for l in motd.splitlines()) | |||
|
|||
print(heredoc(''' | |||
FROM ubuntu:20.04 | |||
FROM ubuntu:22.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice...
OK, using this and a general Hopefully the Cactus and other integration tests are similarly happy with the move. |
This ought to fix #4289.
Changelog Entry
To be copied to the draft changelog by merger:
docker
binary in the container has been updated to that included in the Ubuntu repos (fixes Docker in the Toil appliance container does not know about --gpus #4282)Reviewer Checklist
issues/XXXX-fix-the-thing
in the Toil repo, or from an external repo.camelCase
that want to be insnake_case
.docs/running/{cliOptions,cwl,wdl}.rst
Merger Checklist