From 87cb532ab33f1f242a56e362e799f95549519d3b Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Wed, 24 Oct 2018 15:15:40 -0400 Subject: [PATCH] runSetupBuiltinVolumes(): break up volume setup Break setup for built-in volumes into independent steps where we create the volume's mount point, the directory that will hold its contents, and if there is content under the mount point, populate the volume with the mount point's contents. Signed-off-by: Nalin Dahyabhai Closes: #1126 Approved by: rhatdan --- run.go | 28 ++++++++++++++++++++++------ tests/run.bats | 30 +++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/run.go b/run.go index eaf16f96fca..c4382abb64b 100644 --- a/run.go +++ b/run.go @@ -451,7 +451,7 @@ func (b *Builder) setupMounts(mountPoint string, spec *specs.Spec, bundlePath st // Add temporary copies of the contents of volume locations at the // volume locations, unless we already have something there. copyWithTar := b.copyWithTar(nil, nil) - builtins, err := runSetupBuiltinVolumes(b.MountLabel, mountPoint, cdir, copyWithTar, builtinVolumes) + builtins, err := runSetupBuiltinVolumes(b.MountLabel, mountPoint, cdir, copyWithTar, builtinVolumes, int(rootUID), int(rootGID)) if err != nil { return err } @@ -493,15 +493,21 @@ func runSetupBoundFiles(bundlePath string, bindFiles map[string]string) (mounts return mounts, nil } -func runSetupBuiltinVolumes(mountLabel, mountPoint, containerDir string, copyWithTar func(srcPath, dstPath string) error, builtinVolumes []string) ([]specs.Mount, error) { +func runSetupBuiltinVolumes(mountLabel, mountPoint, containerDir string, copyWithTar func(srcPath, dstPath string) error, builtinVolumes []string, rootUID, rootGID int) ([]specs.Mount, error) { var mounts []specs.Mount + hostOwner := idtools.IDPair{UID: rootUID, GID: rootGID} // Add temporary copies of the contents of volume locations at the // volume locations, unless we already have something there. for _, volume := range builtinVolumes { subdir := digest.Canonical.FromString(volume).Hex() volumePath := filepath.Join(containerDir, "buildah-volumes", subdir) + srcPath := filepath.Join(mountPoint, volume) + initializeVolume := false // If we need to, initialize the volume path's initial contents. - if _, err := os.Stat(volumePath); err != nil && os.IsNotExist(err) { + if _, err := os.Stat(volumePath); err != nil { + if !os.IsNotExist(err) { + return nil, errors.Wrapf(err, "failed to stat %q for volume %q", volumePath, volume) + } logrus.Debugf("setting up built-in volume at %q", volumePath) if err = os.MkdirAll(volumePath, 0755); err != nil { return nil, errors.Wrapf(err, "error creating directory %q for volume %q", volumePath, volume) @@ -509,11 +515,21 @@ func runSetupBuiltinVolumes(mountLabel, mountPoint, containerDir string, copyWit if err = label.Relabel(volumePath, mountLabel, false); err != nil { return nil, errors.Wrapf(err, "error relabeling directory %q for volume %q", volumePath, volume) } - srcPath := filepath.Join(mountPoint, volume) - stat, err := os.Stat(srcPath) - if err != nil { + initializeVolume = true + } + stat, err := os.Stat(srcPath) + if err != nil { + if !os.IsNotExist(err) { return nil, errors.Wrapf(err, "failed to stat %q for volume %q", srcPath, volume) } + if err = idtools.MkdirAllAndChownNew(srcPath, 0755, hostOwner); err != nil { + return nil, errors.Wrapf(err, "error creating directory %q for volume %q", srcPath, volume) + } + if stat, err = os.Stat(srcPath); err != nil { + return nil, errors.Wrapf(err, "failed to stat %q for volume %q", srcPath, volume) + } + } + if initializeVolume { if err = os.Chmod(volumePath, stat.Mode().Perm()); err != nil { return nil, errors.Wrapf(err, "failed to chmod %q for volume %q", volumePath, volume) } diff --git a/tests/run.bats b/tests/run.bats index 1d3b6b9f77e..590ac5e25c8 100644 --- a/tests/run.bats +++ b/tests/run.bats @@ -404,20 +404,20 @@ load helpers fi cid=$(buildah from --pull --signature-policy ${TESTSDIR}/policy.json alpine) run buildah --debug=false run $cid awk '/open files/{print $4}' /proc/self/limits + echo $output [ "$status" -eq 0 ] [ "$output" = 1048576 ] - echo $output run buildah --debug=false run $cid awk '/processes/{print $3}' /proc/self/limits + echo $output [ "$status" -eq 0 ] [ "$output" = 1048576 ] - echo $output buildah rm $cid cid=$(buildah from --ulimit nofile=300:400 --pull --signature-policy ${TESTSDIR}/policy.json alpine) run buildah --debug=false run $cid awk '/open files/{print $4}' /proc/self/limits + echo $output [ "$status" -eq 0 ] [ "$output" = 300 ] - echo $output run buildah --debug=false run $cid awk '/processes/{print $3}' /proc/self/limits echo $output [ "$status" -eq 0 ] @@ -426,12 +426,32 @@ load helpers cid=$(buildah from --ulimit nproc=100:200 --ulimit nofile=300:400 --pull --signature-policy ${TESTSDIR}/policy.json alpine) run buildah --debug=false run $cid awk '/open files/{print $4}' /proc/self/limits + echo $output [ "$status" -eq 0 ] [ "$output" = 300 ] - echo $output run buildah --debug=false run $cid awk '/processes/{print $3}' /proc/self/limits + echo $output [ "$status" -eq 0 ] [ "$output" = 100 ] - echo $output buildah rm $cid } + +@test "run-builtin-volume-omitted" { + # This image is known to include a volume, but not include the mountpoint + # in the image. + cid=$(buildah from --pull --signature-policy ${TESTSDIR}/policy.json docker.io/library/registry@sha256:a25e4660ed5226bdb59a5e555083e08ded157b1218282840e55d25add0223390) + mnt=$(buildah mount $cid) + # By default, the mountpoint should not be there. + run test -d "$mnt"/var/lib/registry + echo "$output" + [ "$status" -ne 0 ] + # We'll create the mountpoint for "run". + run buildah --debug=false run $cid ls -1 /var/lib + echo "$output" + [[ "$output" =~ registry ]] + [ "$status" -eq 0 ] + # Double-check that the mountpoint is there. + run test -d "$mnt"/var/lib/registry + echo "$output" + [ "$status" -eq 0 ] +}