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 <[email protected]>

Closes: #1126
Approved by: rhatdan

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,27 +493,43 @@ 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)
 			}
 			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)
 			}

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 ]
+}