Re: [PULL 15/42] tests/functional: enable pre-emptive caching of assets

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Nov 26, 2024 at 04:45:58PM +0000, Peter Maydell wrote:
> On Tue, 26 Nov 2024 at 16:44, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >
> > Hi,
> >
> > On 4/9/24 12:38, Thomas Huth wrote:
> > > From: Daniel P. Berrangé <berrange@redhat.com>
> > >
> > > Many tests need to access assets stored on remote sites. We don't want
> > > to download these during test execution when run by meson, since this
> > > risks hitting test timeouts when data transfers are slow.
> > >
> > > Add support for pre-emptive caching of assets by setting the env var
> > > QEMU_TEST_PRECACHE to point to a timestamp file. When this is set,
> > > instead of running the test, the assets will be downloaded and saved
> > > to the cache, then the timestamp file created.
> > >
> > > A meson custom target is created as a dependency of each test suite
> > > to trigger the pre-emptive caching logic before the test runs.
> > >
> > > When run in caching mode, it will locate assets by looking for class
> > > level variables with a name prefix "ASSET_", and type "Asset".
> > >
> > > At the ninja level
> > >
> > >     ninja test --suite functional
> > >
> > > will speculatively download any assets that are not already cached,
> > > so it is advisable to set a timeout multiplier.
> > >
> > >     QEMU_TEST_NO_DOWNLOAD=1 ninja test --suite functional
> > >
> > > will fail the test if a required asset is not already cached
> > >
> > >     ninja precache-functional
> > >
> > > will download and cache all assets required by the functional
> > > tests
> > >
> > > At the make level, precaching is always done by
> > >
> > >     make check-functional
> > >
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > Tested-by: Richard Henderson <richard.henderson@linaro.org>
> > > [thuth: Remove the duplicated "path = os.path.basename(...)" line]
> > > Message-ID: <20240830133841.142644-16-thuth@redhat.com>
> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > ---
> > >   tests/Makefile.include                 |  3 ++-
> > >   tests/functional/meson.build           | 33 +++++++++++++++++++++++--
> > >   tests/functional/qemu_test/asset.py    | 34 ++++++++++++++++++++++++++
> > >   tests/functional/qemu_test/testcase.py |  7 ++++++
> > >   4 files changed, 74 insertions(+), 3 deletions(-)
> >
> >
> > > diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
> > > index c0e675d847..b329ab7dbe 100644
> > > --- a/tests/functional/qemu_test/asset.py
> > > +++ b/tests/functional/qemu_test/asset.py
> > > @@ -9,6 +9,8 @@
> > >   import logging
> > >   import os
> > >   import subprocess
> > > +import sys
> > > +import unittest
> > >   import urllib.request
> > >   from pathlib import Path
> > >   from shutil import copyfileobj
> > > @@ -62,6 +64,9 @@ def fetch(self):
> > >                              self.cache_file, self.url)
> > >               return str(self.cache_file)
> > >
> > > +        if os.environ.get("QEMU_TEST_NO_DOWNLOAD", False):
> > > +            raise Exception("Asset cache is invalid and downloads disabled")
> > > +
> > >           self.log.info("Downloading %s to %s...", self.url, self.cache_file)
> > >           tmp_cache_file = self.cache_file.with_suffix(".download")
> > >
> > > @@ -95,3 +100,32 @@ def fetch(self):
> > >
> > >           self.log.info("Cached %s at %s" % (self.url, self.cache_file))
> > >           return str(self.cache_file)
> > > +
> > > +    def precache_test(test):
> > > +        log = logging.getLogger('qemu-test')
> > > +        log.setLevel(logging.DEBUG)
> > > +        handler = logging.StreamHandler(sys.stdout)
> > > +        handler.setLevel(logging.DEBUG)
> > > +        formatter = logging.Formatter(
> > > +            '%(asctime)s - %(name)s - %(levelname)s - %(message)s')
> > > +        handler.setFormatter(formatter)
> > > +        log.addHandler(handler)
> > > +        for name, asset in vars(test.__class__).items():
> > > +            if name.startswith("ASSET_") and type(asset) == Asset:
> > > +                log.info("Attempting to cache '%s'" % asset)
> > > +                asset.fetch()
> >
> > fetch() can fail [*] (see previous patch, various Exceptions returned).
> >
> > What should we do in this case? If we ignore a missing artifact,
> > the tests will eventually fail. Better bail out early and save
> > credit minutes?
> 
> And more generally, can we arrange to cache these images
> in a way that lets us share them across k8s CI runners?
> Store to local disk doesn't help much there...

THe gitlab shared runners have a global cache that is keyed off the
job name, so it will be reused on each pipeline.

Our private k8s runners were configured without any cache, but IIRC
there was work on enabling a cache, and I would expect that to be
globally shared again, keyed off job name.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|