cri: Don't use rel path for image volumes

Runc 1.1 throws a warning when using rel destination paths, and runc 1.2
is planning to thow an error (i.e. won't start the container).

Let's just make this an abs path in the only place it might not be: the
mounts created due to `VOLUME` directives in the Dockerfile.

Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
This commit is contained in:
Rodrigo Campos 2023-07-26 18:20:22 +02:00
parent 81895d22c9
commit 2d64ab8d79
4 changed files with 43 additions and 9 deletions

View File

@ -93,7 +93,7 @@ func TestVolumeCopyUp(t *testing.T) {
},
},
{
containerPath: "C:/weird_test_dir",
containerPath: "/C:/weird_test_dir",
files: []volumeFile{
{
fileName: "weird_test_file",

View File

@ -22,6 +22,7 @@ import (
"fmt"
"path/filepath"
"strconv"
"strings"
"time"
"github.com/containerd/typeurl/v2"
@ -148,10 +149,15 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
}
}()
platform, err := controller.Platform(ctx, sandboxID)
if err != nil {
return nil, fmt.Errorf("failed to query sandbox platform: %w", err)
}
var volumeMounts []*runtime.Mount
if !c.config.IgnoreImageDefinedVolumes {
// Create container image volumes mounts.
volumeMounts = c.volumeMounts(containerRootDir, config.GetMounts(), &image.ImageSpec.Config)
volumeMounts = c.volumeMounts(platform, containerRootDir, config.GetMounts(), &image.ImageSpec.Config)
} else if len(image.ImageSpec.Config.Volumes) != 0 {
log.G(ctx).Debugf("Ignoring volumes defined in image %v because IgnoreImageDefinedVolumes is set", image.ID)
}
@ -162,11 +168,6 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
}
log.G(ctx).Debugf("Use OCI runtime %+v for sandbox %q and container %q", ociRuntime, sandboxID, id)
platform, err := controller.Platform(ctx, sandboxID)
if err != nil {
return nil, fmt.Errorf("failed to query sandbox platform: %w", err)
}
spec, err := c.buildContainerSpec(
platform,
id,
@ -340,7 +341,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
// volumeMounts sets up image volumes for container. Rely on the removal of container
// root directory to do cleanup. Note that image volume will be skipped, if there is criMounts
// specified with the same destination.
func (c *criService) volumeMounts(containerRootDir string, criMounts []*runtime.Mount, config *imagespec.ImageConfig) []*runtime.Mount {
func (c *criService) volumeMounts(platform platforms.Platform, containerRootDir string, criMounts []*runtime.Mount, config *imagespec.ImageConfig) []*runtime.Mount {
if len(config.Volumes) == 0 {
return nil
}
@ -355,6 +356,16 @@ func (c *criService) volumeMounts(containerRootDir string, criMounts []*runtime.
}
volumeID := util.GenerateID()
src := filepath.Join(containerRootDir, "volumes", volumeID)
// When the platform OS is Linux, ensure dst is a _Linux_ abs path.
// We can't use filepath.IsAbs() because, when executing on Windows, it checks for
// Windows abs paths.
if platform.OS == "linux" && !strings.HasPrefix(dst, "/") {
// On Windows, ToSlash() is needed to ensure the path is a valid Linux path.
// On Linux, ToSlash() is a no-op.
oldDst := dst
dst = filepath.ToSlash(filepath.Join("/", dst))
log.L.Debugf("Volume destination %q is not absolute, converted to %q", oldDst, dst)
}
// addOCIBindMounts will create these volumes.
mounts = append(mounts, &runtime.Mount{
ContainerPath: dst,

View File

@ -233,6 +233,7 @@ func TestVolumeMounts(t *testing.T) {
testContainerRootDir := "test-container-root"
for _, test := range []struct {
desc string
platform platforms.Platform
criMounts []*runtime.Mount
imageVolumes map[string]struct{}
expectedMountDest []string
@ -280,6 +281,22 @@ func TestVolumeMounts(t *testing.T) {
"/test-volume-2/",
},
},
{
desc: "should make relative paths absolute on Linux",
platform: platforms.Platform{OS: "linux"},
imageVolumes: map[string]struct{}{
"./test-volume-1": {},
"C:/test-volume-2": {},
"../../test-volume-3": {},
"/abs/test-volume-4": {},
},
expectedMountDest: []string{
"/test-volume-1",
"/C:/test-volume-2",
"/test-volume-3",
"/abs/test-volume-4",
},
},
} {
test := test
t.Run(test.desc, func(t *testing.T) {
@ -287,7 +304,7 @@ func TestVolumeMounts(t *testing.T) {
Volumes: test.imageVolumes,
}
c := newTestCRIService()
got := c.volumeMounts(testContainerRootDir, test.criMounts, config)
got := c.volumeMounts(test.platform, testContainerRootDir, test.criMounts, config)
assert.Len(t, got, len(test.expectedMountDest))
for _, dest := range test.expectedMountDest {
found := false

View File

@ -21,6 +21,7 @@ import (
"errors"
"fmt"
"path/filepath"
goruntime "runtime"
"time"
"github.com/containerd/typeurl/v2"
@ -332,6 +333,11 @@ func (c *criService) volumeMounts(containerRootDir string, criMounts []*runtime.
}
volumeID := util.GenerateID()
src := filepath.Join(containerRootDir, "volumes", volumeID)
if !filepath.IsAbs(dst) && goruntime.GOOS != "windows" {
oldDst := dst
dst = filepath.Join("/", dst)
log.L.Debugf("Volume destination %q is not absolute, converted to %q", oldDst, dst)
}
// addOCIBindMounts will create these volumes.
mounts = append(mounts, &runtime.Mount{
ContainerPath: dst,