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

engine: storage/mounts freshness #21420

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dvdksn
Copy link
Collaborator

@dvdksn dvdksn commented Nov 13, 2024

Various freshness updates to improve the flow and structure of the documentation related to storage mounts.

  • Make the storage overview page shorter, deduplicating some content and moving other, mount-specific content over into the corresponding page
  • Improve the documentation for the various mount options, and make sure all the available mount options are documented
  • Fix various style/wording nits

Preview:

Copy link

netlify bot commented Nov 13, 2024

Deploy Preview for docsdocker ready!

Name Link
🔨 Latest commit 710cae8
🔍 Latest deploy log https://app.netlify.com/sites/docsdocker/deploys/673b098f46e35c0008cecee7
😎 Deploy Preview https://deploy-preview-21420--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added area/engine Issue affects Docker engine/daemon area/storage Relates to storage, volumes area/tests labels Nov 13, 2024
@dvdksn dvdksn requested review from thaJeztah, vvoland and a team November 13, 2024 14:51
@dvdksn dvdksn marked this pull request as ready for review November 13, 2024 14:51
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

just some first comments; will need to take a closer look later

content/manuals/engine/storage/_index.md Outdated Show resolved Hide resolved
- [Volume mounts](#volume-mounts)
- [Bind mounts](#bind-mounts)
- [tmpfs mounts](#tmpfs-mounts)
- [Named pipes](#named-pipes)
Copy link
Member

Choose a reason for hiding this comment

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

This one is still so odd to have in this section (as it's not about storage). Not sure what a good solution is though to distinguish it from "storing data").

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we should consider/frame named pipes as a type of bind mount?

content/manuals/engine/storage/_index.md Outdated Show resolved Hide resolved
Comment on lines 44 to 45
For performance-critical data processing, Volumes are the best way to persist
data in Docker, as they're generally speaking faster than bind mounts.
Copy link
Member

Choose a reason for hiding this comment

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

This also somewhat relates to that, because the storage location used is on the daemon host, so you'd get the same raw file-performance as you'd get when running directly on your host.

Comment on lines 54 to 56
Use bind mounts when you need to be able to access files from both the
container and the host.

Copy link
Member

Choose a reason for hiding this comment

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

Is this the place to mention the pitfalls?

  • bind-mounts happen from the host where the daemon runs (docker desktop mostly takes care of that, but it means that you cannot bind-mount a local directory if the daemon is remote)
  • bind-mounts grants your container access to your host; you're punching a hole in its security boundary, so never grant it access to paths it should not have access to.

There's also the difference between volumes "propagating" the content from the image, and for bind-mounts it's purely "grant access" (but don't copy files to the location).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think this information is for here, but we should include it on the bind mounts page.

content/manuals/engine/storage/_index.md Outdated Show resolved Hide resolved
content/manuals/engine/storage/_index.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/engine Issue affects Docker engine/daemon area/storage Relates to storage, volumes area/tests status/review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants