[RFC] Start using guestfish for U-Boot fs tests

[Tom Rini]

On Sat, Jul 03, 2021 at 05:27:44PM +0300, Alper Nebi Yasak wrote:
> 
> 
> On 02/07/2021 23:24, Tom Rini wrote:
> > On Fri, Jul 02, 2021 at 11:03:52PM +0300, Alper Nebi Yasak wrote:
> >> On 02/07/2021 22:01, Tom Rini wrote:
> >>> Hey all,
> >>>
> >>> I started taking a look at moving to guestfish to see if this resolves
> >>> the latest problem I've run in to:
> >>> https://source.denx.de/u-boot/u-boot/-/jobs/284763#L307
> >>> which I think is due to guestmount not being done in time for the test.
> >>
> >> That failing test's setup uses virt-make-fs, different from what you're
> >> changing below. I locally only see that failure for the clang build, and
> >> it still fails after adding time.sleep(300) after its virt-make-fs
> >> calls. I don't think it's an issue in the test setup.
> > 
> > Ah good, I need to go catch up on that thread again, thanks for looking.
> > 
> >>> So I started converting things to use guestfish directly:
> >>> diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
> >>> index 7325486cdb1a..e8899cfdd118 100644
> >>> --- a/test/py/tests/test_fs/conftest.py
> >>> +++ b/test/py/tests/test_fs/conftest.py
> >>> @@ -265,10 +265,10 @@ def fs_obj_basic(request, u_boot_config):
> >>>      fs_ubtype = fstype_to_ubname(fs_type)
> >>>      check_ubconfig(u_boot_config, fs_ubtype)
> >>>  
> >>> -    mount_dir = u_boot_config.persistent_data_dir + '/mnt'
> >>> +    data_dir = u_boot_config.persistent_data_dir + '/data'
> >>>  
> >>> -    small_file = mount_dir + '/' + SMALL_FILE
> >>> -    big_file = mount_dir + '/' + BIG_FILE
> >>> +    small_file = data_dir + '/' + SMALL_FILE
> >>> +    big_file = data_dir + '/' + BIG_FILE
> >>>  
> >>>      try:
> >>>  
> >>> @@ -279,26 +279,14 @@ def fs_obj_basic(request, u_boot_config):
> >>>          return
> >>>  
> >>>      try:
> >>> -        check_call('mkdir -p %s' % mount_dir, shell=True)
> >>> +        check_call('mkdir -p %s' % data_dir, shell=True)
> >>>      except CalledProcessError as err:
> >>>          pytest.skip('Preparing mount folder failed for filesystem: ' + fs_type + '. {}'.format(err))
> >>>          call('rm -f %s' % fs_img, shell=True)
> >>>          return
> >>>  
> >>>      try:
> >>> -        # Mount the image so we can populate it.
> >>> -        mount_fs(fs_type, fs_img, mount_dir)
> >>> -    except CalledProcessError as err:
> >>> -        pytest.skip('Mounting to folder failed for filesystem: ' + fs_type + '. {}'.format(err))
> >>> -        call('rmdir %s' % mount_dir, shell=True)
> >>> -        call('rm -f %s' % fs_img, shell=True)
> >>> -        return
> >>> -
> >>> -    try:
> >>> -        # Create a subdirectory.
> >>> -        check_call('mkdir %s/SUBDIR' % mount_dir, shell=True)
> >>> -
> >>> -        # Create big file in this image.
> >>> +        # Create big file to copy in to the image.
> >>>          # Note that we work only on the start 1MB, couple MBs in the 2GB range
> >>>          # and the last 1 MB of the huge 2.5GB file.
> >>>          # So, just put random values only in those areas.
> >>> @@ -309,10 +297,14 @@ def fs_obj_basic(request, u_boot_config):
> >>>          check_call('dd if=/dev/urandom of=%s bs=1M count=1 seek=2499'
> >>>              % big_file, shell=True)
> >>>  
> >>> -        # Create a small file in this image.
> >>> +        # Create a small file to copy in to the image.
> >>>          check_call('dd if=/dev/urandom of=%s bs=1M count=1'
> >>>  	    % small_file, shell=True)
> >>>  
> >>> +        # Copy the files in to the image and add a subdirectory.
> >>> +        # Create a subdirectory.
> >>> +        check_call('guestfish add %s : run : mount /dev/sda / : mkdir /SUBDIR : copy-in %s %s /'
> >>> +            % (fs_img, big_file, small_file), shell=True)
> >>
> >> It could be faster to do things within guestfish as much as possible,
> >> instead of preparing the files outside and copying them in.
> > 
> > This is, I believe, as much as possible.  You can't run arbitrary
> > commands via guestfish, or at least I didn't see how.  "dd" isn't really
> > dd, for example.
> > 
> >> Also it looks like python bindings are available as python3-guestfs on
> >> Debian and Ubuntu, just not on pypi.org.
> > 
> > I saw there's some reason why you can't pip install them, and wasn't
> > sure it would be any faster, based on doing these commands outside of
> > pytest first.
> > 
> >>>          # Delete the small file copies which possibly are written as part of a
> >>>          # previous test.
> >>>          # check_call('rm -f "%s.w"' % MB1, shell=True)
> >>> @@ -357,13 +349,11 @@ def fs_obj_basic(request, u_boot_config):
> >>>  
> >>>      except CalledProcessError as err:
> >>>          pytest.skip('Setup failed for filesystem: ' + fs_type + '. {}'.format(err))
> >>> -        umount_fs(mount_dir)
> >>>          return
> >>>      else:
> >>> -        umount_fs(mount_dir)
> >>>          yield [fs_ubtype, fs_img, md5val]
> >>>      finally:
> >>> -        call('rmdir %s' % mount_dir, shell=True)
> >>> +        call('rmdir %s' % data_dir, shell=True)
> >>>          call('rm -f %s' % fs_img, shell=True)
> >>>  
> >>>  #
> >>>
> >>> The problem here is that a test run went from taking about 5 minutes to
> >>> taking about 17 minutes.  I can reduce this to closer to 15 minutes with
> >>> LIBGUESTFS_BACKEND=direct and using libguestfs-make-fixed-appliance to
> >>> make an appliance we reuse.  But that's still too long to be usable.
> >>> I'm hoping someone has some ideas here on how to improve things.
> >>
> >> If libguestfs is falling back to slow emulation because /dev/kvm isn't
> >> available, maybe it's appropriate to check for that and skip the fs tests...
> > 
> > This is, I think, part of the problem.  We're jumping through a huge
> > number of hoops to avoid "sudo mount ..." in the tests directly. 
> 
> Well, whether 'sudo mount' works also depends on CI config.

Yes.  But it's also how the tests can be run outside of CI, and some
level of CI nodes are things we do control.  I do want to get this
working in the most friendly and non-privileged way possible, I just
lament a little that this was all working for the case of "just use
sudo", at one point, and was also relatively fast.

> > thought I was getting kvm used locally, but nope, it's not.  I'm going
> > to take another break and see if I can figure out what I'm doing wrong
> > since:
> > sudo docker run --privileged --cap-add SYS_ADMIN --security-opt
> > apparmor=unconfined --rm -v /dev/fuse:/dev/fuse -v /dev/kvm:/dev/kvm
> > isn't doing it.
> 
> It should be --device instead of -v (--volume) for /dev/{fuse,kvm}; but
> just --privileged should be enough (no --cap-add, --securtiy-opt, or
> --devices necessary) for Docker itself. Something like:
> 
>   sudo docker run --privileged -v $(pwd):/home/uboot/u-boot --rm \
>       trini/u-boot-gitlab-ci-runner:focal-20210609-01Jul2021 \
>       bash /home/uboot/u-boot/test.sh

Ah right.  I'll give that another whirl and see if things look
reasonably quick again.

> where test.sh should be the things in .gitlab-ci.yml or
> .azure-pipelines.yml.
> 
> I got gitlab-runner working instead and use e.g.:
> 
>   gitlab-runner exec docker --docker-privileged "sandbox test.py"

Right, and one of the things that's done for the gitlab runners we
control (rather than the free community ones) is to run like that.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210703/3103e081/attachment.sig>