runtime: Don' call bindUnmountContainerRootfs if container's rootfs u… #2915
Conversation
|
/test-ubuntu |
|
@bergwolf, do we have the same condition on the rust agent? If so, this PR must be forward-ported to the kata-containers/kata-containers branch. For sure it also deserves a backport to the stable-1.11 branch as well. |
Codecov Report
@@ Coverage Diff @@
## master #2915 +/- ##
==========================================
+ Coverage 51.44% 51.68% +0.23%
==========================================
Files 118 118
Lines 17428 17440 +12
==========================================
+ Hits 8966 9013 +47
+ Misses 7379 7342 -37
- Partials 1083 1085 +2 |
|
Thanks for the change. I’m wondering if we should be augmenting our unit tests to exercise this. Any interest in helping with this @keloyang ? |
keloyang
force-pushed
the
bindmount
branch
2 times, most recently
from
3ce7b97
to
522f3e0
Compare
buildContainerRootfs don't call bindMountContainerRootfs if container's rootfs use devicemapper device in https://github.com/kata-containers/runtime/blob/master/virtcontainers/kata_agent.go#L1300, so bindUnmountContainerRootfs should not be called if container's rootfs use devicemapper device in https://github.com/kata-containers/runtime/blob/master/virtcontainers/container.go#L1123 Fixes: #2914 Signed-off-by: Shukui Yang <[email protected]>
|
/test |
There was a problem hiding this comment.
Thanks @keloyang.
Would you be able to forward-port this to Kata 2.0 too (https://github.com/kata-containers/kata-containers/tree/2.0-dev/src/runtime) ?
lgtm
jodh-intel
added
backport
|
ping @egernst @jodh-intel @fidencio |
|
/retest |
|
@keloyang this is failing on 2 required checks ("jenkins-ci-ubuntu-18-04" and "jenkins-metrics-ubuntu-18-04") I'm seeing a 404 error instead of the results, so I'm retesting. /test |
| @@ -351,7 +355,7 @@ func bindUnmountAllRootfs(ctx context.Context, sharedDir string, sandbox *Sandbo | |||
| if c.state.Fstype == "" { | |||
There was a problem hiding this comment.
This check here should prevent bindUnmountContainerRootfs from being called. If it were being called, the bindUnmountContainerRootfs call would have returned an error which would have been propogated up the stack.
Are you seeing otherwise?
There was a problem hiding this comment.
@amshinde thanks for your review. I think we still need this change. bindUnmountContainerRootfs is called in several places ，there is no check in https://github.com/kata-containers/runtime/blob/master/virtcontainers/container.go#L1123.
in addition，call bindUnmountContainerRootfs don't always return error, see https://github.com/kata-containers/runtime/blob/master/virtcontainers/mount.go#L329.
|
forward port PR: kata-containers/kata-containers#940 |
buildContainerRootfs don't call bindMountContainerRootfs if container's rootfs use devicemapper
device in https://github.com/kata-containers/runtime/blob/master/virtcontainers/kata_agent.go#L1300,
so bindUnmountContainerRootfs should not be called if container's rootfs use devicemapper device
in https://github.com/kata-containers/runtime/blob/master/virtcontainers/container.go#L1123
Fixes: #2914
Signed-off-by: Shukui Yang [email protected]