diff --git a/integration/images/image_list.go b/integration/images/image_list.go index 000ce73ef..87cef8e09 100644 --- a/integration/images/image_list.go +++ b/integration/images/image_list.go @@ -52,7 +52,7 @@ func initImages(imageListFile string) { BusyBox: "ghcr.io/containerd/busybox:1.36", Pause: "registry.k8s.io/pause:3.9", ResourceConsumer: "registry.k8s.io/e2e-test-images/resource-consumer:1.10", - VolumeCopyUp: "ghcr.io/containerd/volume-copy-up:2.1", + VolumeCopyUp: "ghcr.io/containerd/volume-copy-up:2.2", VolumeOwnership: "ghcr.io/containerd/volume-ownership:2.1", ArgsEscaped: "cplatpublic.azurecr.io/args-escaped-test-image-ns:1.0", } diff --git a/integration/main_test.go b/integration/main_test.go index 5e86ec934..a6d5d2d4f 100644 --- a/integration/main_test.go +++ b/integration/main_test.go @@ -66,7 +66,6 @@ var ( ) var criEndpoint = flag.String("cri-endpoint", "unix:///run/containerd/containerd.sock", "The endpoint of cri plugin.") -var criRoot = flag.String("cri-root", "/var/lib/containerd/io.containerd.grpc.v1.cri", "The root directory of cri plugin.") var runtimeHandler = flag.String("runtime-handler", "", "The runtime handler to use in the test.") var containerdBin = flag.String("containerd-bin", "containerd", "The containerd binary name. The name is used to restart containerd during test.") diff --git a/integration/volume_copy_up_test.go b/integration/volume_copy_up_test.go index 00a539050..32d24eabf 100644 --- a/integration/volume_copy_up_test.go +++ b/integration/volume_copy_up_test.go @@ -17,16 +17,20 @@ package integration import ( + "context" + "encoding/json" "fmt" "os" "path/filepath" - goruntime "runtime" + "runtime" "testing" "time" "github.com/containerd/containerd/integration/images" + specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + v1 "k8s.io/cri-api/pkg/apis/runtime/v1" ) const ( @@ -36,6 +40,16 @@ const ( containerUserSID = "S-1-5-93-2-2" ) +type volumeFile struct { + fileName string + contents string +} + +type containerVolume struct { + containerPath string + files []volumeFile +} + func TestVolumeCopyUp(t *testing.T) { var ( testImage = images.Get(images.VolumeCopyUp) @@ -59,23 +73,85 @@ func TestVolumeCopyUp(t *testing.T) { t.Logf("Start the container") require.NoError(t, runtimeService.StartContainer(cn)) - // ghcr.io/containerd/volume-copy-up:2.1 contains a test_dir - // volume, which contains a test_file with content "test_content". - t.Logf("Check whether volume contains the test file") - stdout, stderr, err := runtimeService.ExecSync(cn, []string{ - "cat", - "/test_dir/test_file", - }, execTimeout) + expectedVolumes := []containerVolume{ + { + containerPath: "/test_dir", + files: []volumeFile{ + { + fileName: "test_file", + contents: "test_content\n", + }, + }, + }, + { + containerPath: "/:colon_prefixed", + files: []volumeFile{ + { + fileName: "colon_prefixed_file", + contents: "test_content\n", + }, + }, + }, + { + containerPath: "C:/weird_test_dir", + files: []volumeFile{ + { + fileName: "weird_test_file", + contents: "test_content\n", + }, + }, + }, + } + + if runtime.GOOS == "windows" { + expectedVolumes = []containerVolume{ + { + containerPath: "C:\\test_dir", + files: []volumeFile{ + { + fileName: "test_file", + contents: "test_content\n", + }, + }, + }, + { + containerPath: "D:", + files: []volumeFile{}, + }, + } + } + + volumeMappings, err := getContainerBindVolumes(t, cn) require.NoError(t, err) - assert.Empty(t, stderr) - assert.Equal(t, "test_content\n", string(stdout)) t.Logf("Check host path of the volume") - volumePaths, err := getHostPathForVolumes(*criRoot, cn) - require.NoError(t, err) - assert.Equal(t, len(volumePaths), 1, "expected exactly 1 volume") + for _, vol := range expectedVolumes { + _, ok := volumeMappings[vol.containerPath] + assert.Equalf(t, true, ok, "expected to find volume %s", vol.containerPath) + } - testFilePath := filepath.Join(volumePaths[0], "test_file") + // ghcr.io/containerd/volume-copy-up:2.2 contains 3 volumes on Linux and 2 volumes on Windows. + // On linux, each of the volumes contains a single file, all with the same conrent. On Windows, + // non C volumes defined in the image start out as empty. + for _, vol := range expectedVolumes { + files, err := os.ReadDir(volumeMappings[vol.containerPath]) + require.NoError(t, err) + assert.Equal(t, len(vol.files), len(files)) + + for _, file := range vol.files { + t.Logf("Check whether volume %s contains the test file %s", vol.containerPath, file.fileName) + stdout, stderr, err := runtimeService.ExecSync(cn, []string{ + "cat", + filepath.ToSlash(filepath.Join(vol.containerPath, file.fileName)), + }, execTimeout) + require.NoError(t, err) + assert.Empty(t, stderr) + assert.Equal(t, file.contents, string(stdout)) + } + } + + testFilePath := filepath.Join(volumeMappings[expectedVolumes[0].containerPath], expectedVolumes[0].files[0].fileName) + inContainerPath := filepath.Join(expectedVolumes[0].containerPath, expectedVolumes[0].files[0].fileName) contents, err := os.ReadFile(testFilePath) require.NoError(t, err) assert.Equal(t, "test_content\n", string(contents)) @@ -84,7 +160,7 @@ func TestVolumeCopyUp(t *testing.T) { _, _, err = runtimeService.ExecSync(cn, []string{ "sh", "-c", - "echo new_content > /test_dir/test_file", + fmt.Sprintf("echo new_content > %s", filepath.ToSlash(inContainerPath)), }, execTimeout) require.NoError(t, err) @@ -124,15 +200,17 @@ func TestVolumeOwnership(t *testing.T) { // exist inside the container that returns the owner in the form of USERNAME:SID. t.Logf("Check ownership of test directory inside container") + volumePath := "/test_dir" cmd := []string{ - "stat", "-c", "%u:%g", "/test_dir", + "stat", "-c", "%u:%g", volumePath, } expectedContainerOutput := "65534:65534\n" expectedHostOutput := "65534:65534\n" - if goruntime.GOOS == "windows" { + if runtime.GOOS == "windows" { + volumePath = "C:\\volumes\\test_dir" cmd = []string{ "C:\\bin\\get_owner.exe", - "C:\\volumes\\test_dir", + volumePath, } expectedContainerOutput = fmt.Sprintf("%s:%s", containerUserName, containerUserSID) // The username is unknown on the host, but we can still get the SID. @@ -144,34 +222,36 @@ func TestVolumeOwnership(t *testing.T) { assert.Equal(t, expectedContainerOutput, string(stdout)) t.Logf("Check ownership of test directory on the host") - volumePaths, err := getHostPathForVolumes(*criRoot, cn) + volumePaths, err := getContainerBindVolumes(t, cn) require.NoError(t, err) - assert.Equal(t, len(volumePaths), 1, "expected exactly 1 volume") - output, err := getOwnership(volumePaths[0]) + output, err := getOwnership(volumePaths[volumePath]) require.NoError(t, err) assert.Equal(t, expectedHostOutput, output) } -func getHostPathForVolumes(criRoot, containerID string) ([]string, error) { - hostPath := filepath.Join(criRoot, "containers", containerID, "volumes") - if _, err := os.Stat(hostPath); err != nil { - return nil, err +func getContainerBindVolumes(t *testing.T, containerID string) (map[string]string, error) { + client, err := RawRuntimeClient() + require.NoError(t, err, "failed to get raw grpc runtime service client") + request := &v1.ContainerStatusRequest{ + ContainerId: containerID, + Verbose: true, } + response, err := client.ContainerStatus(context.TODO(), request) + require.NoError(t, err) + ret := make(map[string]string) - volumes, err := os.ReadDir(hostPath) - if err != nil { - return nil, err + mounts := struct { + RuntimeSpec struct { + Mounts []specs.Mount `json:"mounts"` + } `json:"runtimeSpec"` + }{} + + info := response.Info["info"] + err = json.Unmarshal([]byte(info), &mounts) + require.NoError(t, err) + for _, mount := range mounts.RuntimeSpec.Mounts { + ret[mount.Destination] = mount.Source } - - if len(volumes) == 0 { - return []string{}, nil - } - - volumePaths := make([]string, len(volumes)) - for idx, volume := range volumes { - volumePaths[idx] = filepath.Join(hostPath, volume.Name()) - } - - return volumePaths, nil + return ret, nil } diff --git a/pkg/cri/opts/container.go b/pkg/cri/opts/container.go index c7f226b13..1840d6cc5 100644 --- a/pkg/cri/opts/container.go +++ b/pkg/cri/opts/container.go @@ -24,6 +24,7 @@ import ( "strings" "github.com/containerd/continuity/fs" + imagespec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/containerd/containerd" "github.com/containerd/containerd/containers" @@ -55,7 +56,7 @@ func WithNewSnapshot(id string, i containerd.Image, opts ...snapshots.Opt) conta // WithVolumes copies ownership of volume in rootfs to its corresponding host path. // It doesn't update runtime spec. // The passed in map is a host path to container path map for all volumes. -func WithVolumes(volumeMounts map[string]string) containerd.NewContainerOpts { +func WithVolumes(volumeMounts map[string]string, platform imagespec.Platform) containerd.NewContainerOpts { return func(ctx context.Context, client *containerd.Client, c *containers.Container) (err error) { if c.Snapshotter == "" { return errors.New("no snapshotter set for container") @@ -97,8 +98,24 @@ func WithVolumes(volumeMounts map[string]string) containerd.NewContainerOpts { }() for host, volume := range volumeMounts { - // The volume may have been defined with a C: prefix, which we can't use here. - volume = strings.TrimPrefix(volume, "C:") + if platform.OS == "windows" { + // Windows allows volume mounts in subfolders under C: and as any other drive letter like D:, E:, etc. + // An image may contain files inside a folder defined as a VOLUME in a Dockerfile. On Windows, images + // can only contain pre-existing files for volumes situated on the root filesystem, which is C:. + // For any other volumes, we need to skip attempting to copy existing contents. + // + // C:\some\volume --> \some\volume + // D:\some\volume --> skip + if len(volume) >= 2 && string(volume[1]) == ":" { + // Perform a case insensitive comparison to "C", and skip non-C mounted volumes. + if !strings.EqualFold(string(volume[0]), "c") { + continue + } + // This is a volume mounted somewhere under C:\. We strip the drive letter and allow fs.RootPath() + // to append the remaining path to the rootfs path as seen by the host OS. + volume = volume[2:] + } + } src, err := fs.RootPath(root, volume) if err != nil { return fmt.Errorf("rootpath on mountPath %s, volume %s: %w", root, volume, err) diff --git a/pkg/cri/sbserver/container_create.go b/pkg/cri/sbserver/container_create.go index f274e671f..5d0ebc516 100644 --- a/pkg/cri/sbserver/container_create.go +++ b/pkg/cri/sbserver/container_create.go @@ -223,7 +223,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta for _, v := range volumeMounts { mountMap[filepath.Clean(v.HostPath)] = v.ContainerPath } - opts = append(opts, customopts.WithVolumes(mountMap)) + opts = append(opts, customopts.WithVolumes(mountMap, platform)) } meta.ImageRef = image.ID meta.StopSignal = image.ImageSpec.Config.StopSignal diff --git a/pkg/cri/server/container_create.go b/pkg/cri/server/container_create.go index 9483778d1..52d18aee8 100644 --- a/pkg/cri/server/container_create.go +++ b/pkg/cri/server/container_create.go @@ -203,7 +203,14 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta for _, v := range volumeMounts { mountMap[filepath.Clean(v.HostPath)] = v.ContainerPath } - opts = append(opts, customopts.WithVolumes(mountMap)) + platform := imagespec.Platform{ + OS: image.ImageSpec.OS, + Architecture: image.ImageSpec.Architecture, + OSVersion: image.ImageSpec.OSVersion, + OSFeatures: image.ImageSpec.OSFeatures, + Variant: image.ImageSpec.Variant, + } + opts = append(opts, customopts.WithVolumes(mountMap, platform)) } meta.ImageRef = image.ID meta.StopSignal = image.ImageSpec.Config.StopSignal diff --git a/script/test/cri-integration.sh b/script/test/cri-integration.sh index 8d1a2a29a..442054a4b 100755 --- a/script/test/cri-integration.sh +++ b/script/test/cri-integration.sh @@ -37,7 +37,6 @@ fi # RUNTIME is the runtime handler to use in the test. RUNTIME=${RUNTIME:-""} -CRI_ROOT="${CONTAINERD_ROOT}/io.containerd.grpc.v1.cri" mkdir -p "${REPORT_DIR}" test_setup "${REPORT_DIR}" @@ -54,7 +53,6 @@ CMD+="${PWD}/bin/cri-integration.test" ${CMD} --test.run="${FOCUS}" --test.v \ --cri-endpoint="${CONTAINERD_SOCK}" \ - --cri-root="${CRI_ROOT}" \ --runtime-handler="${RUNTIME}" \ --containerd-bin="${CONTAINERD_BIN}" \ --image-list="${TEST_IMAGE_LIST:-}" && test_exit_code=$? || test_exit_code=$?