fix: Add a verity_cleanup function to test helpers.bash

[raharper]

Add a cleanup function to unmount underlying atoms if one of the mount fails.

What type of PR is this?

bug

Which issue does this PR fix:

#541

What does this PR do / Why do we need it:

Add a cleanup function which tracks the successfully mounted atoms, and if one
fails, then it will unmount the successful atoms before returning the error.

If an issue # is not available please add repro steps and logs showing the issue:

Testing done on this change:

make check locally.

Automation added to e2e:

Will this break upgrades or downgrades?

Does this PR introduce any user-facing change?:

no

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[smoser]

Do you have evidence or experience that this actually fixes the issue?

Also, you should 'Fixes #541' in your commit message.

[raharper]

Do you have evidence or experience that this actually fixes the issue?

Not yet, I didn't have a setup to run the CI locally (going to set that up now);

Also, you should 'Fixes #541' in your commit message.

Sure, will add.

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (3897848) 53.01% compared to head (489cfec) 57.14%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
pkg/atomfs/molecule.go	75.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #565      +/-   ##
==========================================
+ Coverage   53.01%   57.14%   +4.12%     
==========================================
  Files          64       64              
  Lines        7505     7516      +11     
==========================================
+ Hits         3979     4295     +316     
+ Misses       2815     2479     -336     
- Partials      711      742      +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[raharper]

ok 6 --no-squashfs-verity works in 2sec
ok 7 mount + umount works in 2sec
ok 8 mount + umount + mount a tree of images works in 3sec
ok 9 bad existing verity device is rejected in 12sec

\o/

[smoser]

thank you for digging on this. i just ran 'losetup -a' on my laptop and found:

$ losetup -a | grep stackertest |wc -l
33

But only 9 of those 33 had 'verity' .
there are several others.

$ losetup -a | sed -e '/stackertest/!d' -e 's,.*/stackertest-,,' -e 's,[.].*,,' | sort -u
test_bad_existing_verity_device_is_rejected
test_mount_-2b_umount_-2b_mount_a_tree_of_images_works

Do you know if this will also fix the 'mount a tree of images' ?

[raharper]

thank you for digging on this. i just ran 'losetup -a' on my laptop and found:

$ losetup -a | grep stackertest |wc -l
33

But only 9 of those 33 had 'verity' . there are several others.

$ losetup -a | sed -e '/stackertest/!d' -e 's,.*/stackertest-,,' -e 's,[.].*,,' | sort -u
test_bad_existing_verity_device_is_rejected
test_mount_-2b_umount_-2b_mount_a_tree_of_images_works

Do you know if this will also fix the 'mount a tree of images' ?

Here's what I know...

In the bad image test, we use the busybox image, create a new top layer
then we fake the verity data and try to mount that up. The mount brings up
all layers of the image (top layer with a touch, and the real busybox layer)

Normally, when we call stacker atomfs unmount, it will walk the oci layer
list and tear down each verity device and backing loopdev. However, since the
mount failed, we have nothing to invoke stacker atomfs umount at.

Thinking about it now, it looks like we need to call umount internally if
the mount fails.

I'll look at doing that now.

To why we see the failure, I've not yet found out precisely the order, but I
believe bats runs tests within a .bats file in parallel (and we enable that).
In the atomfs.bats test, each uses the same base busybox image, and some
make modifications creating top layers above which are unique. Neccesarily,
when we mount any of these busybox images, then there is a common base busybox
layer that gets setup as a verity device. What I think is happening is that
when one of the tests goes to tear down the verity or loop device that the
base layer is using, it might get a "device in use" error.

I can't seem to reproduce that easily; I have previously, but not today it
seems

[smoser]

This does look great, but i have one more comment inline. I'm hope that I'm not wrong again and go 0-for-2 today on reading this PR.

[smoser]

this looks good. thanks.