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

add size to tmpfs mounts #2411

Merged
merged 2 commits into from
Oct 18, 2021
Merged

add size to tmpfs mounts #2411

merged 2 commits into from
Oct 18, 2021

Conversation

crazy-max
Copy link
Member

Follow-up docker/buildx#790
Related to #2384

Change the shm-size implementation to instead add size to tmpfs mounts.

Signed-off-by: CrazyMax [email protected]

@tonistiigi
Copy link
Member

tonistiigi commented Oct 13, 2021

Don't forget to add a capability for this.

We may also need to make sure we deduplicate the mounts in executor so it is not inefficient by mounting twice if you overwrite default mountpoint. Unless we confirm that containerd/runc already does it but I think not.

solver/pb/ops.proto Outdated Show resolved Hide resolved
@crazy-max crazy-max force-pushed the tpmfs-size branch 4 times, most recently from 82746f8 to c6cae16 Compare October 15, 2021 09:11
@crazy-max crazy-max marked this pull request as ready for review October 15, 2021 09:20
@crazy-max crazy-max requested a review from tonistiigi October 15, 2021 09:21
Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

Did you validate that in the runtime spec or in runc the mounts are deduplicated? If not then we should check it in the executor. Eg. we don't want /dev/shm to be mounted twice per container.

client/llb/exec.go Outdated Show resolved Hide resolved
@crazy-max
Copy link
Member Author

Did you validate that in the runtime spec or in runc the mounts are deduplicated?

They are not indeed. Will add a dedup impl in spec.

executor/oci/mounts.go Outdated Show resolved Hide resolved
@crazy-max crazy-max force-pushed the tpmfs-size branch 3 times, most recently from 317702b to f5f9a77 Compare October 17, 2021 02:54
@crazy-max crazy-max force-pushed the tpmfs-size branch 2 times, most recently from c628459 to 459449d Compare October 17, 2021 15:38
@crazy-max crazy-max requested a review from tonistiigi October 17, 2021 15:39
solver/pb/ops.proto Outdated Show resolved Hide resolved
@tonistiigi tonistiigi merged commit 41c0a18 into moby:master Oct 18, 2021
@crazy-max crazy-max deleted the tpmfs-size branch October 18, 2021 20:58
@crazy-max crazy-max added this to the v0.10.0 milestone Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants