Ensure that the rootfs dir is created in the bundle#3302
Ensure that the rootfs dir is created in the bundle#3302fuweid merged 1 commit intocontainerd:masterfrom
Conversation
|
@estesp this fixes the issue that you ran into on slack the other day |
|
Build succeeded.
|
Codecov Report
@@ Coverage Diff @@
## master #3302 +/- ##
=======================================
Coverage 44.66% 44.66%
=======================================
Files 112 112
Lines 12192 12192
=======================================
Hits 5445 5445
Misses 5913 5913
Partials 834 834
Continue to review full report at Codecov.
|
|
We removed this logic in c0f0b21. If we add it back now, will it break them? |
|
@georgethebeatle can you take a look at this? |
|
@crosbymichael the idea of the linked commit was to make it possible to run containerd rootlessly. The main trick is that we will skip unmounting the rootfs when the rootfs dir does not exist: containerd/mount/mount_linux.go Lines 122 to 124 in 0e7a3c9 I don't think that this change will break our use case, because we are not yet using any snapshotter, so we do not have anything for containerd to unmount (we have worked arount the unmount in v1 by setting the rootfs to "" and most of the times it just log errors anyway). So I think we will work, but will have annoying EPERMS in the log. One more thing - the newBundle method seems to have an analogue in v1: |
This fixes issues running gvisor on top of containerd without docker. Signed-off-by: Michael Crosby <[email protected]>
|
Build succeeded.
|
|
Updated to include v1 |
|
Setting the cherry pick labels; please remove if not applicable |
|
cherry-picks sound good |
|
Hi @crosbymichael, If you are bringing back bundle dir creation you can remove the code from the patch below: diff --git a/mount/mount_linux.go b/mount/mount_linux.go
index 6bbc50bb..2cecff63 100644
--- a/mount/mount_linux.go
+++ b/mount/mount_linux.go
@@ -114,14 +114,10 @@ func unmount(target string, flags int) error {
// UnmountAll all is noop when the first argument is an empty string.
// This is done when the containerd client did not specify any rootfs
// mounts (e.g. because the rootfs is managed outside containerd)
-// UnmountAll is noop when the mount path does not exist.
func UnmountAll(mount string, flags int) error {
if mount == "" {
return nil
}
- if _, err := os.Stat(mount); os.IsNotExist(err) {
- return nil
- }
for {
if err := unmount(mount, flags); err != nil {
diff --git a/runtime/v1/shim/service.go b/runtime/v1/shim/service.go
index 6e87f052..af83ee81 100644
--- a/runtime/v1/shim/service.go
+++ b/runtime/v1/shim/service.go
@@ -127,9 +127,6 @@ func (s *Service) Create(ctx context.Context, r *shimapi.CreateTaskRequest) (_ *
rootfs := ""
if len(mounts) > 0 {
rootfs = filepath.Join(r.Bundle, "rootfs")
- if err := os.Mkdir(rootfs, 0711); err != nil && !os.IsExist(err) {
- return nil, err
- }
}
config := &proc.CreateConfig{
diff --git a/runtime/v2/runc/container.go b/runtime/v2/runc/container.go
index ae8fb004..6c417dbf 100644
--- a/runtime/v2/runc/container.go
+++ b/runtime/v2/runc/container.go
@@ -21,7 +21,6 @@ package runc
import (
"context"
"io/ioutil"
- "os"
"path/filepath"
"sync"
@@ -68,9 +67,6 @@ func NewContainer(ctx context.Context, platform rproc.Platform, r *task.CreateTa
rootfs := ""
if len(mounts) > 0 {
rootfs = filepath.Join(r.Bundle, "rootfs")
- if err := os.Mkdir(rootfs, 0711); err != nil && !os.IsExist(err) {
- return nil, err
- }
}
config := &proc.CreateConfig{
The Mkdir calls in shim service and container.go are redundant and so is the mount point existence check. I tested that the empty string check is enough for our use case to work, the rest was done so that we do not get false positive logs. |
7 participants
This fixes issues running gvisor on top of containerd without docker.
Signed-off-by: Michael Crosby [email protected]