From 654cee27510475e4ff11098cac1ad58c7b939812 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Mon, 11 Jun 2018 08:37:55 +0000 Subject: [PATCH] Fix empty volume ownership. Signed-off-by: Lantao Liu --- .../images/volume-ownership/Dockerfile | 18 +++++++ integration/images/volume-ownership/Makefile | 27 ++++++++++ integration/volume_copy_up_test.go | 50 ++++++++++++++++++- pkg/containerd/opts/container.go | 9 ---- 4 files changed, 94 insertions(+), 10 deletions(-) create mode 100644 integration/images/volume-ownership/Dockerfile create mode 100644 integration/images/volume-ownership/Makefile diff --git a/integration/images/volume-ownership/Dockerfile b/integration/images/volume-ownership/Dockerfile new file mode 100644 index 000000000..8253d8bc6 --- /dev/null +++ b/integration/images/volume-ownership/Dockerfile @@ -0,0 +1,18 @@ +# Copyright 2018 The Containerd Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +FROM busybox +RUN mkdir -p /test_dir && \ + chown -R nobody:nogroup /test_dir +VOLUME /test_dir diff --git a/integration/images/volume-ownership/Makefile b/integration/images/volume-ownership/Makefile new file mode 100644 index 000000000..85e4b7f88 --- /dev/null +++ b/integration/images/volume-ownership/Makefile @@ -0,0 +1,27 @@ +# Copyright 2018 The Containerd Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +all: build + +PROJ=gcr.io/k8s-cri-containerd +VERSION=1.0 +IMAGE=$(PROJ)/volume-ownership:$(VERSION) + +build: + docker build -t $(IMAGE) . + +push: + gcloud docker -- push $(IMAGE) + +.PHONY: build push diff --git a/integration/volume_copy_up_test.go b/integration/volume_copy_up_test.go index 2830bb18a..1fa4cbea5 100644 --- a/integration/volume_copy_up_test.go +++ b/integration/volume_copy_up_test.go @@ -70,7 +70,7 @@ func TestVolumeCopyUp(t *testing.T) { assert.Equal(t, "test_content\n", string(stdout)) t.Logf("Check host path of the volume") - hostCmd := fmt.Sprintf("ls %s/containers/%s/volumes/*/test_file | xargs cat", *criRoot, cn) + hostCmd := fmt.Sprintf("find %s/containers/%s/volumes/*/test_file | xargs cat", *criRoot, cn) output, err := exec.Command("sh", "-c", hostCmd).CombinedOutput() require.NoError(t, err) assert.Equal(t, "test_content\n", string(output)) @@ -88,3 +88,51 @@ func TestVolumeCopyUp(t *testing.T) { require.NoError(t, err) assert.Equal(t, "new_content\n", string(output)) } + +func TestVolumeOwnership(t *testing.T) { + const ( + testImage = "gcr.io/k8s-cri-containerd/volume-ownership:1.0" + execTimeout = time.Minute + ) + + t.Logf("Create a sandbox") + sbConfig := PodSandboxConfig("sandbox", "volume-ownership") + sb, err := runtimeService.RunPodSandbox(sbConfig) + require.NoError(t, err) + defer func() { + assert.NoError(t, runtimeService.StopPodSandbox(sb)) + assert.NoError(t, runtimeService.RemovePodSandbox(sb)) + }() + + t.Logf("Pull test image") + _, err = imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil) + require.NoError(t, err) + + t.Logf("Create a container with volume-ownership test image") + cnConfig := ContainerConfig( + "container", + testImage, + WithCommand("tail", "-f", "/dev/null"), + ) + cn, err := runtimeService.CreateContainer(sb, cnConfig, sbConfig) + require.NoError(t, err) + + t.Logf("Start the container") + require.NoError(t, runtimeService.StartContainer(cn)) + + // gcr.io/k8s-cri-containerd/volume-ownership:1.0 contains a test_dir + // volume, which is owned by nobody:nogroup. + t.Logf("Check ownership of test directory inside container") + stdout, stderr, err := runtimeService.ExecSync(cn, []string{ + "stat", "-c", "%U:%G", "/test_dir", + }, execTimeout) + require.NoError(t, err) + assert.Empty(t, stderr) + assert.Equal(t, "nobody:nogroup\n", string(stdout)) + + t.Logf("Check ownership of test directory on the host") + hostCmd := fmt.Sprintf("find %s/containers/%s/volumes/* | xargs stat -c %%U:%%G", *criRoot, cn) + output, err := exec.Command("sh", "-c", hostCmd).CombinedOutput() + require.NoError(t, err) + assert.Equal(t, "nobody:nogroup\n", string(output)) +} diff --git a/pkg/containerd/opts/container.go b/pkg/containerd/opts/container.go index 3d2c1cac9..7647c373c 100644 --- a/pkg/containerd/opts/container.go +++ b/pkg/containerd/opts/container.go @@ -53,7 +53,6 @@ func WithNewSnapshot(id string, i containerd.Image) containerd.NewContainerOpts // 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. -// TODO(random-liu): Figure out whether we need to copy volume content. func WithVolumes(volumeMounts map[string]string) containerd.NewContainerOpts { return func(ctx context.Context, client *containerd.Client, c *containers.Container) (err error) { if c.Snapshotter == "" { @@ -108,14 +107,6 @@ func WithVolumes(volumeMounts map[string]string) containerd.NewContainerOpts { // copyExistingContents copies from the source to the destination and // ensures the ownership is appropriately set. func copyExistingContents(source, destination string) error { - srcList, err := ioutil.ReadDir(source) - if err != nil { - return err - } - if len(srcList) == 0 { - // Skip copying if source directory is empty. - return nil - } dstList, err := ioutil.ReadDir(destination) if err != nil { return err