-
Notifications
You must be signed in to change notification settings - Fork 3.6k
*: image volume feature's follow-up #11605
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
Conversation
Unmounting a writable overlayfs can trigger [syncfs][1], causing I/O pressure and impacting running containers. If possible, we should apply volatile option to image volume mount. REF: [1]: containerd#6478 Signed-off-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
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.
looks nice just a couple notes and a question
@@ -102,7 +102,11 @@ func (c *criService) mutateImageMount( | |||
target := c.getImageVolumeHostPath(sandboxID, imageID) | |||
|
|||
// Already mounted in another container on the same pod | |||
if stat, err := os.Stat(target); err == nil && stat.IsDir() { | |||
mounted, err := ensureImageVolumeMounted(target) |
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.
note to self requires .. "careful" rebase against..: https://github.com/containerd/containerd/pull/11578/files#diff-08bc007bc2a695705409801af29731a387ce26d53e8161dc01e69c7e7e3fa758R107
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.
Sure! Thanks!
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.
I will carefully rebase 😄 since this one is probably merged before mine.
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
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
@@ -102,7 +102,11 @@ func (c *criService) mutateImageMount( | |||
target := c.getImageVolumeHostPath(sandboxID, imageID) | |||
|
|||
// Already mounted in another container on the same pod | |||
if stat, err := os.Stat(target); err == nil && stat.IsDir() { | |||
mounted, err := ensureImageVolumeMounted(target) |
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.
I will carefully rebase 😄 since this one is probably merged before mine.
Unmounting a writable overlayfs can trigger syncfs - #6478, causing I/O pressure
and impacting running containers. If possible, we should apply volatile
option to image volume mount.
REF: #10579