Skip to content

[release-1.0] container_create: sort mounts before adding them to the spec #1089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 30, 2017
Merged

[release-1.0] container_create: sort mounts before adding them to the spec #1089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 77 additions & 29 deletions server/container_create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"os"
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -46,29 +47,54 @@ const (
defaultSystemdParent = "system.slice"
)

func addOCIBindMounts(mountLabel string, containerConfig *pb.ContainerConfig, specgen *generate.Generator) ([]oci.ContainerVolume, error) {
type orderedMounts []rspec.Mount

// Len returns the number of mounts. Used in sorting.
func (m orderedMounts) Len() int {
return len(m)
}

// Less returns true if the number of parts (a/b/c would be 3 parts) in the
// mount indexed by parameter 1 is less than that of the mount indexed by
// parameter 2. Used in sorting.
func (m orderedMounts) Less(i, j int) bool {
return m.parts(i) < m.parts(j)
}

// Swap swaps two items in an array of mounts. Used in sorting
func (m orderedMounts) Swap(i, j int) {
m[i], m[j] = m[j], m[i]
}

// parts returns the number of parts in the destination of a mount. Used in sorting.
func (m orderedMounts) parts(i int) int {
return strings.Count(filepath.Clean(m[i].Destination), string(os.PathSeparator))
}

func addOCIBindMounts(mountLabel string, containerConfig *pb.ContainerConfig, specgen *generate.Generator) ([]oci.ContainerVolume, []rspec.Mount, error) {
volumes := []oci.ContainerVolume{}
ociMounts := []rspec.Mount{}
mounts := containerConfig.GetMounts()
for _, mount := range mounts {
dest := mount.ContainerPath
if dest == "" {
return nil, fmt.Errorf("Mount.ContainerPath is empty")
return nil, nil, fmt.Errorf("Mount.ContainerPath is empty")
}

src := mount.HostPath
if src == "" {
return nil, fmt.Errorf("Mount.HostPath is empty")
return nil, nil, fmt.Errorf("Mount.HostPath is empty")
}

if _, err := os.Stat(src); err != nil && os.IsNotExist(err) {
if err1 := os.MkdirAll(src, 0644); err1 != nil {
return nil, fmt.Errorf("Failed to mkdir %s: %s", src, err)
return nil, nil, fmt.Errorf("Failed to mkdir %s: %s", src, err)
}
}

src, err := resolveSymbolicLink(src)
if err != nil {
return nil, fmt.Errorf("failed to resolve symlink %q: %v", src, err)
return nil, nil, fmt.Errorf("failed to resolve symlink %q: %v", src, err)
}

options := []string{"rw"}
Expand All @@ -80,7 +106,7 @@ func addOCIBindMounts(mountLabel string, containerConfig *pb.ContainerConfig, sp
if mount.SelinuxRelabel {
// Need a way in kubernetes to determine if the volume is shared or private
if err := label.Relabel(src, mountLabel, true); err != nil && err != unix.ENOTSUP {
return nil, fmt.Errorf("relabel failed %s: %v", src, err)
return nil, nil, fmt.Errorf("relabel failed %s: %v", src, err)
}
}

Expand All @@ -90,45 +116,55 @@ func addOCIBindMounts(mountLabel string, containerConfig *pb.ContainerConfig, sp
Readonly: mount.Readonly,
})

specgen.AddBindMount(src, dest, options)
ociMounts = append(ociMounts, rspec.Mount{
Source: src,
Destination: dest,
Options: options,
})
}

return volumes, nil
return volumes, ociMounts, nil
}

func addImageVolumes(rootfs string, s *Server, containerInfo *storage.ContainerInfo, specgen *generate.Generator, mountLabel string) error {
func addImageVolumes(rootfs string, s *Server, containerInfo *storage.ContainerInfo, specgen *generate.Generator, mountLabel string) ([]rspec.Mount, error) {
mounts := []rspec.Mount{}
for dest := range containerInfo.Config.Config.Volumes {
fp, err := symlink.FollowSymlinkInScope(filepath.Join(rootfs, dest), rootfs)
if err != nil {
return err
return nil, err
}
switch s.config.ImageVolumes {
case libkpod.ImageVolumesMkdir:
if err1 := os.MkdirAll(fp, 0644); err1 != nil {
return err1
return nil, err1
}
case libkpod.ImageVolumesBind:
volumeDirName := stringid.GenerateNonCryptoID()
src := filepath.Join(containerInfo.RunDir, "mounts", volumeDirName)
if err1 := os.MkdirAll(src, 0644); err1 != nil {
return err1
return nil, err1
}
// Label the source with the sandbox selinux mount label
if mountLabel != "" {
if err1 := label.Relabel(src, mountLabel, true); err1 != nil && err1 != unix.ENOTSUP {
return fmt.Errorf("relabel failed %s: %v", src, err1)
return nil, fmt.Errorf("relabel failed %s: %v", src, err1)
}
}

logrus.Debugf("Adding bind mounted volume: %s to %s", src, dest)
specgen.AddBindMount(src, dest, []string{"rw"})
mounts = append(mounts, rspec.Mount{
Source: src,
Destination: dest,
Options: []string{"rw"},
})

case libkpod.ImageVolumesIgnore:
logrus.Debugf("Ignoring volume %v", dest)
default:
logrus.Fatalf("Unrecognized image volumes setting")
}
}
return nil
return mounts, nil
}

// resolveSymbolicLink resolves a possbile symlink path. If the path is a symlink, returns resolved
Expand Down Expand Up @@ -385,16 +421,13 @@ func ensureSaneLogPath(logPath string) error {
}

// addSecretsBindMounts mounts user defined secrets to the container
func addSecretsBindMounts(mountLabel, ctrRunDir string, defaultMounts []string, specgen generate.Generator) error {
func addSecretsBindMounts(mountLabel, ctrRunDir string, defaultMounts []string, specgen generate.Generator) ([]rspec.Mount, error) {
containerMounts := specgen.Spec().Mounts
mounts, err := secretMounts(defaultMounts, mountLabel, ctrRunDir, containerMounts)
if err != nil {
return err
}
for _, m := range mounts {
specgen.AddBindMount(m.Source, m.Destination, nil)
return nil, err
}
return nil
return mounts, nil
}

// CreateContainer creates a new container in specified PodSandbox
Expand Down Expand Up @@ -562,7 +595,7 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
}
}

containerVolumes, err := addOCIBindMounts(mountLabel, containerConfig, &specgen)
containerVolumes, ociMounts, err := addOCIBindMounts(mountLabel, containerConfig, &specgen)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -934,12 +967,6 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
return nil, err
}

if len(s.config.DefaultMounts) > 0 {
if err = addSecretsBindMounts(mountLabel, containerInfo.RunDir, s.config.DefaultMounts, specgen); err != nil {
return nil, fmt.Errorf("failed to mount secrets: %v", err)
}
}

mountPoint, err := s.StorageRuntimeServer().StartContainer(containerID)
if err != nil {
return nil, fmt.Errorf("failed to mount container %s(%s): %v", containerName, containerID, err)
Expand All @@ -957,7 +984,8 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
}

// Add image volumes
if err := addImageVolumes(mountPoint, s, &containerInfo, &specgen, mountLabel); err != nil {
volumeMounts, err := addImageVolumes(mountPoint, s, &containerInfo, &specgen, mountLabel)
if err != nil {
return nil, err
}

Expand Down Expand Up @@ -1008,6 +1036,26 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
}
specgen.SetProcessCwd(containerCwd)

var secretMounts []rspec.Mount
if len(s.config.DefaultMounts) > 0 {
var err error
secretMounts, err = addSecretsBindMounts(mountLabel, containerInfo.RunDir, s.config.DefaultMounts, specgen)
if err != nil {
return nil, fmt.Errorf("failed to mount secrets: %v", err)
}
}

mounts := []rspec.Mount{}
mounts = append(mounts, ociMounts...)
mounts = append(mounts, volumeMounts...)
mounts = append(mounts, secretMounts...)

sort.Sort(orderedMounts(mounts))

for _, m := range mounts {
specgen.AddBindMount(m.Source, m.Destination, m.Options)
}

if err := s.setupOCIHooks(&specgen, sb, containerConfig, processArgs[0]); err != nil {
return nil, err
}
Expand Down
34 changes: 0 additions & 34 deletions test/crio_secrets.bats
View file
Open in desktop

This file was deleted.

69 changes: 69 additions & 0 deletions test/default_mounts.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
#!/usr/bin/env bats

load helpers

IMAGE="redis:alpine"

function teardown() {
cleanup_test
}

@test "bind secrets mounts to container" {
start_crio
run crioctl pod run --config "$TESTDATA"/sandbox_config.json
echo "$output"
[ "$status" -eq 0 ]
pod_id="$output"
run crioctl image pull "$IMAGE"
[ "$status" -eq 0 ]
run crioctl ctr create --config "$TESTDATA"/container_redis.json --pod "$pod_id"
echo "$output"
[ "$status" -eq 0 ]
ctr_id="$output"
run crioctl ctr execsync --id "$ctr_id" cat /proc/mounts
echo "$output"
[ "$status" -eq 0 ]
mount_info="$output"
run grep /container/path1 <<< "$mount_info"
echo "$output"
[ "$status" -eq 0 ]
cleanup_ctrs
cleanup_pods
stop_crio
}

@test "default mounts correctly sorted with other mounts" {
start_crio
run crioctl pod run --config "$TESTDATA"/sandbox_config.json
echo "$output"
[ "$status" -eq 0 ]
pod_id="$output"
run crioctl image pull "$IMAGE"
[ "$status" -eq 0 ]
host_path="$TESTDIR"/clash
mkdir "$host_path"
echo "clashing..." > "$host_path"/clashing.txt
sed -e "s,%HPATH%,$host_path,g" "$TESTDATA"/container_redis_default_mounts.json > "$TESTDIR"/defmounts_pre.json
sed -e 's,%CPATH%,\/container\/path1\/clash,g' "$TESTDIR"/defmounts_pre.json > "$TESTDIR"/defmounts.json
run crioctl ctr create --config "$TESTDIR"/defmounts.json --pod "$pod_id"
echo "$output"
[ "$status" -eq 0 ]
ctr_id="$output"
run crioctl ctr execsync --id "$ctr_id" ls -la /container/path1/clash
echo "$output"
[ "$status" -eq 0 ]
run crioctl ctr execsync --id "$ctr_id" cat /container/path1/clash/clashing.txt
echo "$output"
[ "$status" -eq 0 ]
[[ "$output" =~ "clashing..." ]]
run crioctl ctr execsync --id "$ctr_id" ls -la /container/path1
echo "$output"
[ "$status" -eq 0 ]
run crioctl ctr execsync --id "$ctr_id" cat /container/path1/test.txt
echo "$output"
[ "$status" -eq 0 ]
[[ "$output" =~ "Testing secrets mounts!" ]]
cleanup_ctrs
cleanup_pods
stop_crio
}
67 changes: 67 additions & 0 deletions test/testdata/container_redis_default_mounts.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
{
"metadata": {
"name": "podsandbox1-redis"
},
"image": {
"image": "redis:alpine"
},
"args": [
"docker-entrypoint.sh",
"redis-server"
],
"mounts": [
{
"container_path": "%CPATH%",
"host_path": "%HPATH%"
}
],
"working_dir": "/data",
"envs": [
{
"key": "PATH",
"value": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
},
{
"key": "TERM",
"value": "xterm"
},
{
"key": "REDIS_VERSION",
"value": "3.2.3"
},
{
"key": "REDIS_DOWNLOAD_URL",
"value": "http://download.redis.io/releases/redis-3.2.3.tar.gz"
},
{
"key": "REDIS_DOWNLOAD_SHA1",
"value": "92d6d93ef2efc91e595c8bf578bf72baff397507"
}
],
"labels": {
"tier": "backend"
},
"annotations": {
"pod": "podsandbox1"
},
"readonly_rootfs": false,
"log_path": "",
"stdin": false,
"stdin_once": false,
"tty": false,
"linux": {
"resources": {
"cpu_period": 10000,
"cpu_quota": 20000,
"cpu_shares": 512,
"oom_score_adj": 30
},
"security_context": {
"capabilities": {
"add_capabilities": [
"sys_admin"
]
}
}
}
}