github/containerd
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

Merge pull request #6651 from ambarve/mount_path_fix

Browse Source 
Bug fix for mount path handling
This commit is contained in:
Phil Estes 2022-03-23 10:56:21 -04:00 committed by GitHub
commit b06938ce10
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 90 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

103
pkg/cri/opts/spec_windows.go
View File

@ -61,6 +61,61 @@ func cleanMount(p string) string {
return filepath.Clean(p)
}
func parseMount(osi osinterface.OS, mount *runtime.Mount) (*runtimespec.Mount, error) {
var (
dst = mount.GetContainerPath()
src = mount.GetHostPath()
)
// In the case of a named pipe mount on Windows, don't stat the file
// or do other operations that open it, as that could interfere with
// the listening process. filepath.Clean also breaks named pipe
// paths, so don't use it.
if !namedPipePath(src) {
if _, err := osi.Stat(src); err != nil {
// Create the host path if it doesn't exist. This will align
// the behavior with the Linux implementation, but it doesn't
// align with Docker's behavior on Windows.
if !os.IsNotExist(err) {
return nil, fmt.Errorf("failed to stat %q: %w", src, err)
}
if err := osi.MkdirAll(src, 0755); err != nil {
return nil, fmt.Errorf("failed to mkdir %q: %w", src, err)
}
}
var err error
src, err = osi.ResolveSymbolicLink(src)
if err != nil {
return nil, fmt.Errorf("failed to resolve symlink %q: %w", src, err)
}
// hcsshim requires clean path, especially '/' -> '\'. Additionally,
// for the destination, absolute paths should have the C: prefix.
src = filepath.Clean(src)
// filepath.Clean adds a '.' at the end if the path is a
// drive (like Z:, E: etc.). Keeping this '.' in the path
// causes incorrect parameter error when starting the
// container on windows. Remove it here.
if !(len(dst) == 2 && dst[1] == ':') {
dst = filepath.Clean(dst)
if dst[0] == '\\' {
dst = "C:" + dst
}
} else if dst[0] == 'c' || dst[0] == 'C' {
return nil, fmt.Errorf("destination path can not be C drive")
}
}
var options []string
// NOTE(random-liu): we don't change all mounts to `ro` when root filesystem
// is readonly. This is different from docker's behavior, but make more sense.
if mount.GetReadonly() {
options = append(options, "ro")
} else {
options = append(options, "rw")
}
return &runtimespec.Mount{Source: src, Destination: dst, Options: options}, nil
}
// WithWindowsMounts sorts and adds runtime and CRI mounts to the spec for
// windows container.
func WithWindowsMounts(osi osinterface.OS, config *runtime.ContainerConfig, extra []*runtime.Mount) oci.SpecOpts {
@ -110,53 +165,11 @@ func WithWindowsMounts(osi osinterface.OS, config *runtime.ContainerConfig, extr
}
for _, mount := range mounts {
var (
dst = mount.GetContainerPath()
src = mount.GetHostPath()
)
// In the case of a named pipe mount on Windows, don't stat the file
// or do other operations that open it, as that could interfere with
// the listening process. filepath.Clean also breaks named pipe
// paths, so don't use it.
if !namedPipePath(src) {
if _, err := osi.Stat(src); err != nil {
// Create the host path if it doesn't exist. This will align
// the behavior with the Linux implementation, but it doesn't
// align with Docker's behavior on Windows.
if !os.IsNotExist(err) {
return fmt.Errorf("failed to stat %q: %w", src, err)
}
if err := osi.MkdirAll(src, 0755); err != nil {
return fmt.Errorf("failed to mkdir %q: %w", src, err)
}
}
var err error
src, err = osi.ResolveSymbolicLink(src)
parsedMount, err := parseMount(osi, mount)
if err != nil {
return fmt.Errorf("failed to resolve symlink %q: %w", src, err)
return err
}
// hcsshim requires clean path, especially '/' -> '\'. Additionally,
// for the destination, absolute paths should have the C: prefix.
src = filepath.Clean(src)
dst = filepath.Clean(dst)
if dst[0] == '\\' {
dst = "C:" + dst
}
}
var options []string
// NOTE(random-liu): we don't change all mounts to `ro` when root filesystem
// is readonly. This is different from docker's behavior, but make more sense.
if mount.GetReadonly() {
options = append(options, "ro")
} else {
options = append(options, "rw")
}
s.Mounts = append(s.Mounts, runtimespec.Mount{
Source: src,
Destination: dst,
Options: options,
})
s.Mounts = append(s.Mounts, *parsedMount)
}
return nil
}

31
pkg/cri/opts/spec_windows_test.go
View File

@ -18,11 +18,14 @@ package opts
import (
"context"
"fmt"
"strings"
"testing"
"github.com/containerd/containerd/containers"
"github.com/containerd/containerd/namespaces"
"github.com/containerd/containerd/oci"
osinterface "github.com/containerd/containerd/pkg/os"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@ -215,3 +218,31 @@ func TestWithDevices(t *testing.T) {
})
}
}
func TestDriveMounts(t *testing.T) {
tests := []struct {
mnt *runtime.Mount
expectedContainerPath string
expectedError error
}{
{&runtime.Mount{HostPath: `C:\`, ContainerPath: `D:\foo`}, `D:\foo`, nil},
{&runtime.Mount{HostPath: `C:\`, ContainerPath: `D:\`}, `D:\`, nil},
{&runtime.Mount{HostPath: `C:\`, ContainerPath: `D:`}, `D:`, nil},
{&runtime.Mount{HostPath: `\\.\pipe\a_fake_pipe_name_that_shouldnt_exist`, ContainerPath: `\\.\pipe\foo`}, `\\.\pipe\foo`, nil},
// If `C:\` is passed as container path it should continue and forward that to HCS and fail
// to align with docker's behavior.
{&runtime.Mount{HostPath: `C:\`, ContainerPath: `C:\`}, `C:\`, nil},
// If `C:` is passed we can detect and fail immediately.
{&runtime.Mount{HostPath: `C:\`, ContainerPath: `C:`}, ``, fmt.Errorf("destination path can not be C drive")},
}
var realOS osinterface.RealOS
for _, test := range tests {
parsedMount, err := parseMount(realOS, test.mnt)
if err != nil && !strings.EqualFold(err.Error(), test.expectedError.Error()) {
t.Fatalf("expected err: %s, got %s instead", test.expectedError, err)
} else if err == nil && test.expectedContainerPath != parsedMount.Destination {
t.Fatalf("expected container path: %s, got %s instead", test.expectedContainerPath, parsedMount.Destination)
}
}
}