Centralize metadata, move mount logic to atomfs pkg, add tests

README.md

@@ -42,18 +42,19 @@ ab
 
 ## Implementation details
 
-We create $mountpoint/meta and pass that to `atomfs` as the
-Metadatapath.  We do the readonly `atomfs` molecule mount
-onto $metadir/ro.  Then if a readonly mount is requested
-$metadir/ro is bind mounted onto $metadir.  Otherwise, we create
-$metadir/work and $metadir/upper, and use these to do a rw
-overlay mount of $metadir/ro onto $mountpoint.
+The `atomfs` binary uses the `atomfs` package's Molecule API to mount oci
+images.
+
+Each squashfs layer is mounted separately at a subdir under
+`/run/atomfs/meta/$mountnsid/$mountpoint/`, and then an overlay mount is
+constructed for the specified mountpath. If specified in the config, a writeable
+upperdir is added to the overlay mount.
 
 Note that if you simply call `umount` on the mountpoint, then
 you will be left with all the individual squashfs mounts under
-`dest/mounts/*/`.
+`/run/atomfs/meta/$mountnsid/$mountpoint/`. Use `atomfs umount` instead.
 
 Note that you do need to be root in your namespace in order to
-do the final bind or overlay mount.  (We could get around this
+do the final overlay mount.  (We could get around this
 by using fuse-overlay, but creating a namespace seems overall
 tidy).

cmd/atomfs/mount.go

@@ -1,16 +1,15 @@
 package main
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
-	"syscall"
 
+	"github.com/pkg/errors"
 	"github.com/urfave/cli"
-	"golang.org/x/sys/unix"
+
 	"machinerun.io/atomfs"
 	"machinerun.io/atomfs/squashfs"
 )
@@ -44,7 +43,7 @@ func findImage(ctx *cli.Context) (string, string, error) {
 	}
 	ocidir := r[0]
 	tag := r[1]
-	if !PathExists(ocidir) {
+	if !atomfs.PathExists(ocidir) {
 		return "", "", fmt.Errorf("oci directory %s does not exist: %w", ocidir, mountUsage(ctx.App.Name))
 	}
 	return ocidir, tag, nil
@@ -70,92 +69,42 @@ func doMount(ctx *cli.Context) error {
 		os.Exit(1)
 	}
 	target := ctx.Args()[1]
-	metadir := filepath.Join(target, "meta")
-
-	complete := false
-
-	defer func() {
-		if !complete {
-			cleanupDest(metadir)
-		}
-	}()
-
-	if PathExists(metadir) {
-		return fmt.Errorf("%q exists: cowardly refusing to mess with it", metadir)
-	}
-
-	if err := EnsureDir(metadir); err != nil {
-		return err
-	}
-
-	rodest := filepath.Join(metadir, "ro")
-	if err = EnsureDir(rodest); err != nil {
-		return err
-	}
-
-	opts := atomfs.MountOCIOpts{
-		OCIDir:       ocidir,
-		MetadataPath: metadir,
-		Tag:          tag,
-		Target:       rodest,
-	}
-
-	mol, err := atomfs.BuildMoleculeFromOCI(opts)
+	absTarget, err := filepath.Abs(target)
 	if err != nil {
 		return err
 	}
 
-	err = mol.Mount(rodest)
+	absOCIDir, err := filepath.Abs(ocidir)
 	if err != nil {
 		return err
 	}
 
-	if ctx.Bool("writeable") || ctx.IsSet("persist") {
-		err = overlay(target, rodest, metadir, ctx)
-	} else {
-		err = bind(target, rodest)
-	}
-
-	complete = err == nil
-	return err
-}
-
-func cleanupDest(metadir string) {
-	fmt.Printf("Failure detected: cleaning up %q", metadir)
-	rodest := filepath.Join(metadir, "ro")
-	if PathExists(rodest) {
-		if err := unix.Unmount(rodest, 0); err != nil {
-			fmt.Printf("Failed unmounting %q: %v", rodest, err)
+	persistPath := ""
+	if ctx.IsSet("persist") {
+		persistPath = ctx.String("persist")
+		if persistPath == "" {
+			return fmt.Errorf("--persist requires an argument")
 		}
 	}
+	opts := atomfs.MountOCIOpts{
+		OCIDir:               absOCIDir,
+		Tag:                  tag,
+		Target:               absTarget,
+		AddWriteableOverlay:  ctx.Bool("writeable") || ctx.IsSet("persist"),
+		WriteableOverlayPath: persistPath,
+	}
 
-	mountsdir := filepath.Join(metadir, "mounts")
-	entries, err := os.ReadDir(mountsdir)
+	mol, err := atomfs.BuildMoleculeFromOCI(opts)
 	if err != nil {
-		fmt.Printf("Failed reading contents of %q: %v", mountsdir, err)
-		os.RemoveAll(metadir)
-		return
+		return errors.Wrapf(err, "couldn't build molecule with opts %+v", opts)
 	}
 
-	wd, err := os.Getwd()
+	err = mol.Mount(target)
 	if err != nil {
-		fmt.Printf("Failed getting working directory")
-		os.RemoveAll(metadir)
-	}
-	for _, e := range entries {
-		n := filepath.Base(e.Name())
-		if n == "workaround" {
-			continue
-		}
-		if strings.HasSuffix(n, ".log") {
-			continue
-		}
-		p := filepath.Join(wd, mountsdir, e.Name())
-		if err := squashUmount(p); err != nil {
-			fmt.Printf("Failed unmounting %q: %v\n", p, err)
-		}
+		return errors.Wrapf(err, "couldn't mount molecule at mntpt %q ", target)
 	}
-	os.RemoveAll(metadir)
+
+	return nil
 }
 
 func RunCommand(args ...string) error {
@@ -177,23 +126,3 @@ func squashUmount(p string) error {
 	}
 	return RunCommand("fusermount", "-u", p)
 }
-
-func overlay(target, rodest, metadir string, ctx *cli.Context) error {
-	workdir := filepath.Join(metadir, "work")
-	if err := EnsureDir(workdir); err != nil {
-		return err
-	}
-	upperdir := filepath.Join(metadir, "persist")
-	if ctx.IsSet("persist") {
-		upperdir = ctx.String("persist")
-	}
-	if err := EnsureDir(upperdir); err != nil {
-		return err
-	}
-	overlayArgs := fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s,index=off,userxattr", rodest, upperdir, workdir)
-	return unix.Mount("overlayfs", target, "overlay", 0, overlayArgs)
-}
-
-func bind(target, source string) error {
-	return syscall.Mount(source, target, "", syscall.MS_BIND, "")
-}

cmd/atomfs/umount.go

@@ -7,6 +7,7 @@ import (
 	"syscall"
 
 	"github.com/urfave/cli"
+	"machinerun.io/atomfs"
 	"machinerun.io/atomfs/mount"
 )
 
@@ -43,36 +44,37 @@ func doUmount(ctx *cli.Context) error {
 		}
 	}
 
-	// We expect the argument to be the mountpoint - either a readonly
-	// bind mount, or a writeable overlay.
+	// We expect the argument to be the mountpoint of the overlay
 	err = syscall.Unmount(mountpoint, 0)
 	if err != nil {
 		errs = append(errs, fmt.Errorf("Failed unmounting %s: %v", mountpoint, err))
 	}
 
-	// Now that we've unmounted the mountpoint, we expect the following
-	// under there:
-	// $mountpoint/meta/ro - the original readonly overlay mountpoint
-	// $mountpoint/meta/mounts/* - the original squashfs mounts
-	metadir := filepath.Join(mountpoint, "meta")
-	p := filepath.Join(metadir, "ro")
-	err = syscall.Unmount(p, 0)
+	// We expect the following in the metadir
+	//
+	// $metadir/mounts/* - the original squashfs mounts
+	// $metadir/meta/config.json
+
+	// TODO: want to know mountnsname for a target mountpoint... not for our current proc???
+	mountNSName, err := atomfs.GetMountNSName()
 	if err != nil {
-		errs = append(errs, fmt.Errorf("Failed unmounting RO mountpoint %s: %v", p, err))
+		errs = append(errs, fmt.Errorf("Failed to get mount namespace name"))
 	}
+	metadir := filepath.Join(atomfs.RuntimeDir(), "meta", mountNSName, atomfs.ReplacePathSeparators(mountpoint))
 
 	mountsdir := filepath.Join(metadir, "mounts")
 	mounts, err := os.ReadDir(mountsdir)
 	if err != nil {
 		errs = append(errs, fmt.Errorf("Failed reading list of mounts: %v", err))
-		return fmt.Errorf("Encountered errors: %#v", errs)
+		return fmt.Errorf("Encountered errors: %v", errs)
 	}
 
 	for _, m := range mounts {
-		p = filepath.Join(mountsdir, m.Name())
+		p := filepath.Join(mountsdir, m.Name())
 		if !m.IsDir() || !isMountpoint(p) {
 			continue
 		}
+
 		err = syscall.Unmount(p, 0)
 		if err != nil {
 			errs = append(errs, fmt.Errorf("Failed unmounting squashfs dir %s: %v", p, err))

cmd/atomfs/verify.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/urfave/cli"
+	"machinerun.io/atomfs"
 	"machinerun.io/atomfs/mount"
 	"machinerun.io/atomfs/squashfs"
 )
@@ -41,9 +42,12 @@ func doVerify(ctx *cli.Context) error {
 		return fmt.Errorf("%s is not a mountpoint", mountpoint)
 	}
 
-	// hidden by the final overlay mount, but visible in the mountinfo:
-	// $mountpoint/meta/mounts/* - the original squashfs mounts
-	mountsdir := filepath.Join(mountpoint, "meta", "mounts")
+	mountNSName, err := atomfs.GetMountNSName()
+	if err != nil {
+		return err
+	}
+
+	mountsdir := filepath.Join(atomfs.RuntimeDir(), "meta", mountNSName, atomfs.ReplacePathSeparators(mountpoint), "mounts")
 
 	mounts, err := mount.ParseMounts("/proc/self/mountinfo")
 	if err != nil {