Make sure plugin rootfs is unmounted on upgrade#32525
Conversation
vdemeester
left a comment
There was a problem hiding this comment.
LGTM 🦁
/cc @anusha-ragunathan @tiborvass @vieux
|
@cpuguy83 : That is no cute animal. |
Ok, let's close |
There was a problem hiding this comment.
log the error and continue trying to unmount other mounts?
There was a problem hiding this comment.
I didn't want to do that because we really can't proceed if there is anything mounted.
This is what we do (sort of) when disabling the plugin, but for upgrade we really need to error out.
There was a problem hiding this comment.
Looks like recursiveUnmount can be used. Also, not related to this PR, but https://github.com/cpuguy83/docker/blob/564839a8d8958ecc06600c8f730955c89cc32626/plugin/backend_linux.go#L651 error message should not include while performing recursive unmount
There was a problem hiding this comment.
typo: "If the anything" -> "If anything"
There was a problem hiding this comment.
🤦♂️ classic case of starting the setance one way and finishing it another :(
564839a to
59bffd2
Compare
In some cases, if a user specifies `-f` when disabling a plugin mounts can still exist on the plugin rootfs. This can cause problems during upgrade where the rootfs is removed and may cause data loss. To resolve this, ensure the rootfs is unmounted before performing an upgrade. Signed-off-by: Brian Goff <[email protected]>
59bffd2 to
83f44d2
Compare
|
Updated |
|
|
|
LGTM (CI failures are unrelated.) |
|
@cpuguy83 @anusha-ragunathan should this be cherry-picked into 17.05? |
|
@thaJeztah : If its not too late, that would be nice. |
|
ping @vieux FYI ^^ |
In some cases, if a user specifies
-fwhen disabling a plugin mountscan still exist on the plugin rootfs.
This can cause problems during upgrade where the rootfs is removed and
may cause data loss.
To resolve this, ensure the rootfs is unmounted
before performing an upgrade.
Fixes #32515