Skip to content

Commit

Permalink
Keep linux mounts for linux sandboxes on Windows/Darwin
Browse files Browse the repository at this point in the history 
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
  • Loading branch information
@mxpv @egernst
mxpv authored and egernst committed Mar 30, 2023
1 parent 6ea9bc5 commit 17c52a2
Show file tree
Hide file tree
Showing 8 changed files with 275 additions and 297 deletions.
68 changes: 63 additions & 5 deletions pkg/cri/sbserver/container_create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,6 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
log.G(ctx).Debugf("Ignoring volumes defined in image %v because IgnoreImageDefinedVolumes is set", image.ID)
}

// Generate container mounts.
mounts := c.containerMounts(sandboxID, config)

ociRuntime, err := c.getSandboxRuntime(sandboxConfig, sandbox.Metadata.RuntimeHandler)
if err != nil {
return nil, fmt.Errorf("failed to get sandbox runtime: %w", err)
Expand All @@ -181,7 +178,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
config,
sandboxConfig,
&image.ImageSpec.Config,
append(mounts, volumeMounts...),
volumeMounts,
ociRuntime,
)
if err != nil {
Expand Down Expand Up @@ -445,6 +442,10 @@ func (c *criService) buildContainerSpec(

switch {
case isLinux:
// Generate container mounts.
// No mounts are passed for other platforms.
linuxMounts := c.linuxContainerMounts(sandboxID, config)

specOpts, err = c.buildLinuxSpec(
id,
sandboxID,
Expand All @@ -455,7 +456,7 @@ func (c *criService) buildContainerSpec(
config,
sandboxConfig,
imageConfig,
extraMounts,
append(linuxMounts, extraMounts...),
ociRuntime,
)
case isWindows:
Expand Down Expand Up @@ -874,3 +875,60 @@ func (c *criService) buildDarwinSpec(

return specOpts, nil
}

// containerMounts sets up necessary container system file mounts
// including /dev/shm, /etc/hosts and /etc/resolv.conf.
func (c *criService) linuxContainerMounts(sandboxID string, config *runtime.ContainerConfig) []*runtime.Mount {
var mounts []*runtime.Mount
securityContext := config.GetLinux().GetSecurityContext()
if !isInCRIMounts(etcHostname, config.GetMounts()) {
// /etc/hostname is added since 1.1.6, 1.2.4 and 1.3.
// For in-place upgrade, the old sandbox doesn't have the hostname file,
// do not mount this in that case.
// TODO(random-liu): Remove the check and always mount this when
// containerd 1.1 and 1.2 are deprecated.
hostpath := c.getSandboxHostname(sandboxID)
if _, err := c.os.Stat(hostpath); err == nil {
mounts = append(mounts, &runtime.Mount{
ContainerPath: etcHostname,
HostPath: hostpath,
Readonly: securityContext.GetReadonlyRootfs(),
SelinuxRelabel: true,
})
}
}

if !isInCRIMounts(etcHosts, config.GetMounts()) {
mounts = append(mounts, &runtime.Mount{
ContainerPath: etcHosts,
HostPath: c.getSandboxHosts(sandboxID),
Readonly: securityContext.GetReadonlyRootfs(),
SelinuxRelabel: true,
})
}

// Mount sandbox resolv.config.
// TODO: Need to figure out whether we should always mount it as read-only
if !isInCRIMounts(resolvConfPath, config.GetMounts()) {
mounts = append(mounts, &runtime.Mount{
ContainerPath: resolvConfPath,
HostPath: c.getResolvPath(sandboxID),
Readonly: securityContext.GetReadonlyRootfs(),
SelinuxRelabel: true,
})
}

if !isInCRIMounts(devShm, config.GetMounts()) {
sandboxDevShm := c.getSandboxDevShm(sandboxID)
if securityContext.GetNamespaceOptions().GetIpc() == runtime.NamespaceMode_NODE {
sandboxDevShm = devShm
}
mounts = append(mounts, &runtime.Mount{
ContainerPath: devShm,
HostPath: sandboxDevShm,
Readonly: false,
SelinuxRelabel: sandboxDevShm != devShm,
})
}
return mounts
}
57 changes: 0 additions & 57 deletions pkg/cri/sbserver/container_create_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,63 +51,6 @@ const (
seccompDefaultProfile = dockerDefault
)

// containerMounts sets up necessary container system file mounts
// including /dev/shm, /etc/hosts and /etc/resolv.conf.
func (c *criService) containerMounts(sandboxID string, config *runtime.ContainerConfig) []*runtime.Mount {
var mounts []*runtime.Mount
securityContext := config.GetLinux().GetSecurityContext()
if !isInCRIMounts(etcHostname, config.GetMounts()) {
// /etc/hostname is added since 1.1.6, 1.2.4 and 1.3.
// For in-place upgrade, the old sandbox doesn't have the hostname file,
// do not mount this in that case.
// TODO(random-liu): Remove the check and always mount this when
// containerd 1.1 and 1.2 are deprecated.
hostpath := c.getSandboxHostname(sandboxID)
if _, err := c.os.Stat(hostpath); err == nil {
mounts = append(mounts, &runtime.Mount{
ContainerPath: etcHostname,
HostPath: hostpath,
Readonly: securityContext.GetReadonlyRootfs(),
SelinuxRelabel: true,
})
}
}

if !isInCRIMounts(etcHosts, config.GetMounts()) {
mounts = append(mounts, &runtime.Mount{
ContainerPath: etcHosts,
HostPath: c.getSandboxHosts(sandboxID),
Readonly: securityContext.GetReadonlyRootfs(),
SelinuxRelabel: true,
})
}

// Mount sandbox resolv.config.
// TODO: Need to figure out whether we should always mount it as read-only
if !isInCRIMounts(resolvConfPath, config.GetMounts()) {
mounts = append(mounts, &runtime.Mount{
ContainerPath: resolvConfPath,
HostPath: c.getResolvPath(sandboxID),
Readonly: securityContext.GetReadonlyRootfs(),
SelinuxRelabel: true,
})
}

if !isInCRIMounts(devShm, config.GetMounts()) {
sandboxDevShm := c.getSandboxDevShm(sandboxID)
if securityContext.GetNamespaceOptions().GetIpc() == runtime.NamespaceMode_NODE {
sandboxDevShm = devShm
}
mounts = append(mounts, &runtime.Mount{
ContainerPath: devShm,
HostPath: sandboxDevShm,
Readonly: false,
SelinuxRelabel: sandboxDevShm != devShm,
})
}
return mounts
}

func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageConfig *imagespec.ImageConfig) ([]oci.SpecOpts, error) {
var specOpts []oci.SpecOpts
securityContext := config.GetLinux().GetSecurityContext()
Expand Down
182 changes: 1 addition & 181 deletions pkg/cri/sbserver/container_create_linux_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package sbserver

import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -360,23 +359,16 @@ func TestContainerSpecWithExtraMounts(t *testing.T) {
HostPath: "test-sys-extra",
Readonly: false,
},
{
ContainerPath: "/dev",
HostPath: "test-dev-extra",
Readonly: false,
},
}
spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, extraMounts, ociRuntime)
require.NoError(t, err)
specCheck(t, testID, testSandboxID, testPid, spec)
var mounts, sysMounts, devMounts []runtimespec.Mount
var mounts, sysMounts []runtimespec.Mount
for _, m := range spec.Mounts {
if strings.HasPrefix(m.Destination, "test-container-path") {
mounts = append(mounts, m)
} else if m.Destination == "/sys" {
sysMounts = append(sysMounts, m)
} else if strings.HasPrefix(m.Destination, "/dev") {
devMounts = append(devMounts, m)
}
}
t.Logf("CRI mount should override extra mount")
Expand All @@ -388,11 +380,6 @@ func TestContainerSpecWithExtraMounts(t *testing.T) {
require.Len(t, sysMounts, 1)
assert.Equal(t, "test-sys-extra", sysMounts[0].Source)
assert.Contains(t, sysMounts[0].Options, "rw")

t.Logf("Dev mount should override all default dev mounts")
require.Len(t, devMounts, 1)
assert.Equal(t, "test-dev-extra", devMounts[0].Source)
assert.Contains(t, devMounts[0].Options, "rw")
}

func TestContainerAndSandboxPrivileged(t *testing.T) {
Expand Down Expand Up @@ -444,173 +431,6 @@ func TestContainerAndSandboxPrivileged(t *testing.T) {
}
}

func TestContainerMounts(t *testing.T) {
const testSandboxID = "test-id"
for desc, test := range map[string]struct {
statFn func(string) (os.FileInfo, error)
criMounts []*runtime.Mount
securityContext *runtime.LinuxContainerSecurityContext
expectedMounts []*runtime.Mount
}{
"should setup ro mount when rootfs is read-only": {
securityContext: &runtime.LinuxContainerSecurityContext{
ReadonlyRootfs: true,
},
expectedMounts: []*runtime.Mount{
{
ContainerPath: "/etc/hostname",
HostPath: filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hostname"),
Readonly: true,
SelinuxRelabel: true,
},
{
ContainerPath: "/etc/hosts",
HostPath: filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hosts"),
Readonly: true,
SelinuxRelabel: true,
},
{
ContainerPath: resolvConfPath,
HostPath: filepath.Join(testRootDir, sandboxesDir, testSandboxID, "resolv.conf"),
Readonly: true,
SelinuxRelabel: true,
},
{
ContainerPath: "/dev/shm",
HostPath: filepath.Join(testStateDir, sandboxesDir, testSandboxID, "shm"),
Readonly: false,
SelinuxRelabel: true,
},
},
},
"should setup rw mount when rootfs is read-write": {
securityContext: &runtime.LinuxContainerSecurityContext{},
expectedMounts: []*runtime.Mount{
{
ContainerPath: "/etc/hostname",
HostPath: filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hostname"),
Readonly: false,
SelinuxRelabel: true,
},
{
ContainerPath: "/etc/hosts",
HostPath: filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hosts"),
Readonly: false,
SelinuxRelabel: true,
},
{
ContainerPath: resolvConfPath,
HostPath: filepath.Join(testRootDir, sandboxesDir, testSandboxID, "resolv.conf"),
Readonly: false,
SelinuxRelabel: true,
},
{
ContainerPath: "/dev/shm",
HostPath: filepath.Join(testStateDir, sandboxesDir, testSandboxID, "shm"),
Readonly: false,
SelinuxRelabel: true,
},
},
},
"should use host /dev/shm when host ipc is set": {
securityContext: &runtime.LinuxContainerSecurityContext{
NamespaceOptions: &runtime.NamespaceOption{Ipc: runtime.NamespaceMode_NODE},
},
expectedMounts: []*runtime.Mount{
{
ContainerPath: "/etc/hostname",
HostPath: filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hostname"),
Readonly: false,
SelinuxRelabel: true,
},
{
ContainerPath: "/etc/hosts",
HostPath: filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hosts"),
Readonly: false,
SelinuxRelabel: true,
},
{
ContainerPath: resolvConfPath,
HostPath: filepath.Join(testRootDir, sandboxesDir, testSandboxID, "resolv.conf"),
Readonly: false,
SelinuxRelabel: true,
},
{
ContainerPath: "/dev/shm",
HostPath: "/dev/shm",
Readonly: false,
},
},
},
"should skip container mounts if already mounted by CRI": {
criMounts: []*runtime.Mount{
{
ContainerPath: "/etc/hostname",
HostPath: "/test-etc-hostname",
},
{
ContainerPath: "/etc/hosts",
HostPath: "/test-etc-host",
},
{
ContainerPath: resolvConfPath,
HostPath: "test-resolv-conf",
},
{
ContainerPath: "/dev/shm",
HostPath: "test-dev-shm",
},
},
securityContext: &runtime.LinuxContainerSecurityContext{},
expectedMounts: nil,
},
"should skip hostname mount if the old sandbox doesn't have hostname file": {
statFn: func(path string) (os.FileInfo, error) {
assert.Equal(t, filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hostname"), path)
return nil, errors.New("random error")
},
securityContext: &runtime.LinuxContainerSecurityContext{},
expectedMounts: []*runtime.Mount{
{
ContainerPath: "/etc/hosts",
HostPath: filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hosts"),
Readonly: false,
SelinuxRelabel: true,
},
{
ContainerPath: resolvConfPath,
HostPath: filepath.Join(testRootDir, sandboxesDir, testSandboxID, "resolv.conf"),
Readonly: false,
SelinuxRelabel: true,
},
{
ContainerPath: "/dev/shm",
HostPath: filepath.Join(testStateDir, sandboxesDir, testSandboxID, "shm"),
Readonly: false,
SelinuxRelabel: true,
},
},
},
} {
t.Run(desc, func(t *testing.T) {
config := &runtime.ContainerConfig{
Metadata: &runtime.ContainerMetadata{
Name: "test-name",
Attempt: 1,
},
Mounts: test.criMounts,
Linux: &runtime.LinuxContainerConfig{
SecurityContext: test.securityContext,
},
}
c := newTestCRIService()
c.os.(*ostesting.FakeOS).StatFn = test.statFn
mounts := c.containerMounts(testSandboxID, config)
assert.Equal(t, test.expectedMounts, mounts, desc)
})
}
}

func TestPrivilegedBindMount(t *testing.T) {
testPid := uint32(1234)
c := newTestCRIService()
Expand Down
6 changes: 0 additions & 6 deletions pkg/cri/sbserver/container_create_other.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ import (
"github.com/containerd/containerd/snapshots"
)

// containerMounts sets up necessary container system file mounts
// including /dev/shm, /etc/hosts and /etc/resolv.conf.
func (c *criService) containerMounts(sandboxID string, config *runtime.ContainerConfig) []*runtime.Mount {
return []*runtime.Mount{}
}

func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageConfig *imagespec.ImageConfig) ([]oci.SpecOpts, error) {
return []oci.SpecOpts{}, nil
}
Expand Down

0 comments on commit 17c52a2

Please sign in to comment.