svn commit: r343543 - head/sbin/bectl/tests

[Enji Cooper]

> On Jan 29, 2019, at 09:06, Rodney W. Grimes <freebsd at pdx.rh.cn85.dnsmgr.net> wrote:
> 
> [ Charset UTF-8 unsupported, converting... ]
>> On Jan 28, 2019, at 20:31, Rodney W. Grimes <freebsd at pdx.rh.cn85.dnsmgr.net> wrote:
>> 
>>>>> On Mon, Jan 28, 2019 at 10:09 PM Kyle Evans <kevans at freebsd.org> wrote:
>>>>> 
>>>>> Author: kevans
>>>>> Date: Tue Jan 29 04:08:49 2019
>>>>> New Revision: 343543
>>>>> URL: https://svnweb.freebsd.org/changeset/base/343543
>>>>> 
>>>>> Log:
>>>>> bectl(8) test: Force destroy the zpool in cleanup
>>>>> 
>>>>> This is a wild guess as to why bectl tests failed once upon a time in CI,
>>>>> given no apparent way to see a transcript of cleanup routines with Kyua. The
>>>>> bectl tests construct a new, clean zpool for every test. The failure
>>>>> indicated was because of a mount that was leftover from a previous test, but
>>>>> the previous test had succeeded so it's not clear how the mount remained
>>>>> leftover unless the `zpool get health ${pool}` had somehow failed.
>>>>> 
>>>> 
>>>> I left out: the tests are supposed to be constructed to clean up any
>>>> mounts that were left over in the course of the test, hence the
>>>> assumption that the failure lies in the cleanup.
>> 
>> Hi Rod,
>> 
>>> From my experience as a hardware test engineer the test
>>> setup was required to make sure any of those assumptions
>>> are valid.  Meaning that the test would have to validate
>>> that no left over cruft was going to interfere with the
>>> test about to be run.
>>> 
>>> Ie, you should probably do a force destroy of the pool
>>> *before* the test too.
>> 
>> While this approach makes sense and is valid, it would leave open/orphaned resources after each test run (in this case a single zpool). It?s best to fix the underlying issue with how the test formulates, sets up, and tears down the zpool.
> 
> I did not advocate in any way that the post run cleanup
> should be removed.  Infact you should have both a pre-test
> assurance that the environment is correct and a post-test
> cleanup trying to removall all artifacts.
> 
> You can fix the current issue, but eventually if you get
> enough testing going on your going to start to find that
> things fail in ways that do not clean up (machine crash
> is one instance) and not doing a pretest cleanup is eventually
> going to run aground of this pitfall.

I understand what you mean, but we don’t have to worry about this. Our CI runs don’t persist today as the bhyve instance is setup at start and torn down at exit (it was a problem in the first few iterations). Furthermore, resources like zpools that the bectl tests construct are should transient in nature at import (don’t affect the zpool.cache file), meaning that it should not be reconnected on the next boot.

Assuming $TMPDIR is set to /tmp (or unset for that matter) and a standard rc.conf configuration (or better yet, a tmpfs/md-backed filesystem), all leftover remnants on disk will be cleaned up at next boot on test hosts that are more persistent.

Cheers,
-Enji