Skip to content
This repository was archived by the owner on Mar 9, 2022. It is now read-only.

Commit 063f815

Browse files
Random-LiuRandom-Liu
committed
Sort volume mount.
Signed-off-by: Lantao Liu <[email protected]>
1 parent 3da8bed commit 063f815

4 files changed

Lines changed: 140 additions & 18 deletions

File tree

‎pkg/server/container_create.go‎

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package server
1919
import (
2020
"os"
2121
"path/filepath"
22+
"sort"
2223
"strconv"
2324
"strings"
2425
"time"
@@ -349,8 +350,8 @@ func (c *criService) generateContainerSpec(id string, sandboxID string, sandboxP
349350
return nil, errors.Wrapf(err, "failed to init selinux options %+v", securityContext.GetSelinuxOptions())
350351
}
351352

352-
// Add extra mounts first so that CRI specified mounts can override.
353-
mounts := append(extraMounts, config.GetMounts()...)
353+
// Merge extra mounts and CRI mounts.
354+
mounts := mergeMounts(config.GetMounts(), extraMounts)
354355
if err := c.addOCIBindMounts(&g, mounts, mountLabel); err != nil {
355356
return nil, errors.Wrapf(err, "failed to set OCI bind mounts %+v", mounts)
356357
}
@@ -596,13 +597,40 @@ func setOCIDevicesPrivileged(g *generate.Generator) error {
596597

597598
// addOCIBindMounts adds bind mounts.
598599
func (c *criService) addOCIBindMounts(g *generate.Generator, mounts []*runtime.Mount, mountLabel string) error {
600+
// Sort mounts in number of parts. This ensures that high level mounts don't
601+
// shadow other mounts.
602+
sort.Sort(orderedMounts(mounts))
603+
599604
// Mount cgroup into the container as readonly, which inherits docker's behavior.
600605
g.AddMount(runtimespec.Mount{
601606
Source: "cgroup",
602607
Destination: "/sys/fs/cgroup",
603608
Type: "cgroup",
604609
Options: []string{"nosuid", "noexec", "nodev", "relatime", "ro"},
605610
})
611+
612+
// Copy all mounts from default mounts, except for
613+
// - mounts overriden by supplied mount;
614+
// - all mounts under /dev if a supplied /dev is present.
615+
mountSet := make(map[string]struct{})
616+
for _, m := range mounts {
617+
mountSet[filepath.Clean(m.ContainerPath)] = struct{}{}
618+
}
619+
defaultMounts := g.Mounts()
620+
g.ClearMounts()
621+
for _, m := range defaultMounts {
622+
dst := filepath.Clean(m.Destination)
623+
if _, ok := mountSet[dst]; ok {
624+
// filter out mount overridden by a supplied mount
625+
continue
626+
}
627+
if _, mountDev := mountSet["/dev"]; mountDev && strings.HasPrefix(dst, "/dev/") {
628+
// filter out everything under /dev if /dev is a supplied mount
629+
continue
630+
}
631+
g.AddMount(m)
632+
}
633+
606634
for _, mount := range mounts {
607635
dst := mount.GetContainerPath()
608636
src := mount.GetHostPath()
@@ -821,10 +849,6 @@ func defaultRuntimeSpec(id string) (*runtimespec.Spec, error) {
821849
if mount.Destination == "/run" {
822850
continue
823851
}
824-
// CRI plugin handles `/dev/shm` itself.
825-
if mount.Destination == "/dev/shm" {
826-
continue
827-
}
828852
mounts = append(mounts, mount)
829853
}
830854
spec.Mounts = mounts
@@ -968,3 +992,25 @@ func generateUserString(username string, uid, gid *runtime.Int64Value) (string,
968992
}
969993
return userstr, nil
970994
}
995+
996+
// mergeMounts merge CRI mounts with extra mounts. If a mount destination
997+
// is mounted by both a CRI mount and an extra mount, the CRI mount will
998+
// be kept.
999+
func mergeMounts(criMounts, extraMounts []*runtime.Mount) []*runtime.Mount {
1000+
var mounts []*runtime.Mount
1001+
mounts = append(mounts, criMounts...)
1002+
// Copy all mounts from extra mounts, except for mounts overriden by CRI.
1003+
for _, e := range extraMounts {
1004+
found := false
1005+
for _, c := range criMounts {
1006+
if filepath.Clean(e.ContainerPath) == filepath.Clean(c.ContainerPath) {
1007+
found = true
1008+
break
1009+
}
1010+
}
1011+
if !found {
1012+
mounts = append(mounts, e)
1013+
}
1014+
}
1015+
return mounts
1016+
}

‎pkg/server/container_create_test.go‎

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package server
1919
import (
2020
"path/filepath"
2121
"reflect"
22+
"strings"
2223
"testing"
2324

2425
"github.com/containerd/containerd/contrib/apparmor"
@@ -312,26 +313,50 @@ func TestContainerSpecWithExtraMounts(t *testing.T) {
312313
Readonly: false,
313314
}
314315
config.Mounts = append(config.Mounts, mountInConfig)
315-
extraMount := &runtime.Mount{
316-
ContainerPath: "test-container-path",
317-
HostPath: "test-host-path-extra",
318-
Readonly: true,
316+
extraMounts := []*runtime.Mount{
317+
{
318+
ContainerPath: "test-container-path",
319+
HostPath: "test-host-path-extra",
320+
Readonly: true,
321+
},
322+
{
323+
ContainerPath: "/sys",
324+
HostPath: "test-sys-extra",
325+
Readonly: false,
326+
},
327+
{
328+
ContainerPath: "/dev",
329+
HostPath: "test-dev-extra",
330+
Readonly: false,
331+
},
319332
}
320-
spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, []*runtime.Mount{extraMount})
333+
spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, config, sandboxConfig, imageConfig, extraMounts)
321334
require.NoError(t, err)
322335
specCheck(t, testID, testSandboxID, testPid, spec)
323-
var mounts []runtimespec.Mount
336+
var mounts, sysMounts, devMounts []runtimespec.Mount
324337
for _, m := range spec.Mounts {
325338
if m.Destination == "test-container-path" {
326339
mounts = append(mounts, m)
340+
} else if m.Destination == "/sys" {
341+
sysMounts = append(sysMounts, m)
342+
} else if strings.HasPrefix(m.Destination, "/dev") {
343+
devMounts = append(devMounts, m)
327344
}
328345
}
329-
t.Logf("Extra mounts should come first")
330-
require.Len(t, mounts, 2)
331-
assert.Equal(t, "test-host-path-extra", mounts[0].Source)
332-
assert.Contains(t, mounts[0].Options, "ro")
333-
assert.Equal(t, "test-host-path", mounts[1].Source)
334-
assert.Contains(t, mounts[1].Options, "rw")
346+
t.Logf("CRI mount should override extra mount")
347+
require.Len(t, mounts, 1)
348+
assert.Equal(t, "test-host-path", mounts[0].Source)
349+
assert.Contains(t, mounts[0].Options, "rw")
350+
351+
t.Logf("Extra mount should override default mount")
352+
require.Len(t, sysMounts, 1)
353+
assert.Equal(t, "test-sys-extra", sysMounts[0].Source)
354+
assert.Contains(t, sysMounts[0].Options, "rw")
355+
356+
t.Logf("Dev mount should override all default dev mounts")
357+
require.Len(t, devMounts, 1)
358+
assert.Equal(t, "test-dev-extra", devMounts[0].Source)
359+
assert.Contains(t, devMounts[0].Options, "rw")
335360
}
336361

337362
func TestContainerAndSandboxPrivileged(t *testing.T) {

‎pkg/server/helpers.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package server
1919
import (
2020
"encoding/json"
2121
"fmt"
22+
"os"
2223
"path"
2324
"path/filepath"
2425
"regexp"
@@ -472,3 +473,30 @@ func toRuntimeAuthConfig(a criconfig.AuthConfig) *runtime.AuthConfig {
472473
IdentityToken: a.IdentityToken,
473474
}
474475
}
476+
477+
// mounts defines how to sort runtime.Mount.
478+
// This is the same with the Docker implementation:
479+
// https://github.com/moby/moby/blob/17.05.x/daemon/volumes.go#L26
480+
type orderedMounts []*runtime.Mount
481+
482+
// Len returns the number of mounts. Used in sorting.
483+
func (m orderedMounts) Len() int {
484+
return len(m)
485+
}
486+
487+
// Less returns true if the number of parts (a/b/c would be 3 parts) in the
488+
// mount indexed by parameter 1 is less than that of the mount indexed by
489+
// parameter 2. Used in sorting.
490+
func (m orderedMounts) Less(i, j int) bool {
491+
return m.parts(i) < m.parts(j)
492+
}
493+
494+
// Swap swaps two items in an array of mounts. Used in sorting
495+
func (m orderedMounts) Swap(i, j int) {
496+
m[i], m[j] = m[j], m[i]
497+
}
498+
499+
// parts returns the number of parts in the destination of a mount. Used in sorting.
500+
func (m orderedMounts) parts(i int) int {
501+
return strings.Count(filepath.Clean(m[i].ContainerPath), string(os.PathSeparator))
502+
}

‎pkg/server/helpers_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package server
1818

1919
import (
20+
"sort"
2021
"testing"
2122

2223
"github.com/containerd/containerd"
@@ -25,6 +26,7 @@ import (
2526
imagedigest "github.com/opencontainers/go-digest"
2627
"github.com/stretchr/testify/assert"
2728
"golang.org/x/net/context"
29+
runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
2830

2931
criconfig "github.com/containerd/cri/pkg/config"
3032
"github.com/containerd/cri/pkg/util"
@@ -190,3 +192,24 @@ func TestGetRuntimeConfigFromContainerInfo(t *testing.T) {
190192
})
191193
}
192194
}
195+
196+
func TestOrderedMounts(t *testing.T) {
197+
mounts := []*runtime.Mount{
198+
{ContainerPath: "/a/b/c"},
199+
{ContainerPath: "/a/b"},
200+
{ContainerPath: "/a/b/c/d"},
201+
{ContainerPath: "/a"},
202+
{ContainerPath: "/b"},
203+
{ContainerPath: "/b/c"},
204+
}
205+
expected := []*runtime.Mount{
206+
{ContainerPath: "/a"},
207+
{ContainerPath: "/b"},
208+
{ContainerPath: "/a/b"},
209+
{ContainerPath: "/b/c"},
210+
{ContainerPath: "/a/b/c"},
211+
{ContainerPath: "/a/b/c/d"},
212+
}
213+
sort.Stable(orderedMounts(mounts))
214+
assert.Equal(t, expected, mounts)
215+
}

0 commit comments

Comments
 (0)