fix: Tear down underlying atoms if atomfs mount fails (#565)

The atomfs bad verity test case ends up leaving verity and loop
devices which then break other test cases.  The root cause is that
when the test case attempts to mount the invalid atomfs the underlying
atom was already mounted.  Once the verity fails, stacker exits but
does not remove any previously mounted at atoms in the molecule.  Fix
this by creating a cleanup function which runs if we fail to mount an
atom.

- Add a check to the atomfs.bats which checks for leftover loop mounts

Fixes: #541

Signed-off-by: Ryan Harper <[email protected]>

pkg/atomfs/molecule.go

@@ -24,6 +24,19 @@ type Molecule struct {
 // mountUnderlyingAtoms mounts all the underlying atoms at
 // config.MountedAtomsPath().
 func (m Molecule) mountUnderlyingAtoms() error {
+	// in the case that we have a verity or other mount error we need to
+	// tear down the other underlying atoms so we don't leave verity and loop
+	// devices around unused.
+	atomsMounted := []string{}
+	cleanupAtoms := func(err error) error {
+		for _, target := range atomsMounted {
+			if umountErr := squashfs.Umount(target); umountErr != nil {
+				return errors.Wrapf(umountErr, "failed to unmount atom @ target %q while handling error: %s", target, err)
+			}
+		}
+		return err
+	}
+
 	for _, a := range m.Atoms {
 		target := m.config.MountedAtomsPath(a.Digest.Encoded())
 
@@ -58,8 +71,10 @@ func (m Molecule) mountUnderlyingAtoms() error {
 
 		err = squashfs.Mount(m.config.AtomsPath(a.Digest.Encoded()), target, rootHash)
 		if err != nil {
-			return errors.Wrapf(err, "couldn't mount")
+			return cleanupAtoms(err)
 		}
+
+		atomsMounted = append(atomsMounted, target)
 	}
 
 	return nil

test/atomfs.bats

@@ -8,6 +8,23 @@ function teardown() {
     cleanup
 }
 
+function verity_checkusedloops() {
+    # search for loopdevices which have backing files with the current
+    # BATS_TEST_DIRNAME value and complain if they're present.
+    local usedloops="" found="" x=""
+    for ((x=0; x<5; x++)); do
+        usedloops=$(losetup -a | grep $BATS_TEST_DIRNAME || echo)
+        if [ -n "$usedloops" ]; then
+            found=1
+            udevadm settle
+        else
+            return 0
+        fi
+    done
+    echo "found used loops in testdir=$BATS_TEST_DIRNAME :$usedloops" >&3
+    [ $found = 1 ]
+}
+
 function basic_test() {
     require_privilege priv
     local verity_arg=$1
@@ -30,6 +47,7 @@ EOF
 
 @test "--no-squashfs-verity works" {
     basic_test --no-squashfs-verity
+    verity_checkusedloops
 }
 
 @test "mount + umount works" {
@@ -40,6 +58,7 @@ EOF
     last_layer_num=$(($(cat oci/blobs/sha256/$manifest | jq -r '.layers | length')-1))
     last_layer_hash=$(cat oci/blobs/sha256/$manifest | jq -r .layers[$last_layer].digest | cut -f2 -d:)
     [ ! -b "/dev/mapper/$last_layer_hash-verity" ]
+    verity_checkusedloops
 }
 
 @test "mount + umount + mount a tree of images works" {
@@ -113,6 +132,7 @@ EOF
     last_layer_num=$(($(cat oci/blobs/sha256/$manifest | jq -r '.layers | length')-1))
     last_layer_hash=$(cat oci/blobs/sha256/$manifest | jq -r .layers[$last_layer].digest | cut -f2 -d:)
     [ ! -b "/dev/mapper/$last_layer_hash-verity" ]
+    verity_checkusedloops
 }
 
 @test "bad existing verity device is rejected" {
@@ -140,4 +160,5 @@ EOF
     mkdir mountpoint
     bad_stacker internal-go atomfs mount test-squashfs mountpoint | grep "invalid root hash"
     veritysetup close "$devname"
+    verity_checkusedloops
 }