Patchwork [V2,7/7] scripts/test-remote-image: Add script for running runtime tests on remotely built images

login
register
mail settings
Submitter Corneliu Stoicescu
Date June 6, 2014, 7:14 p.m.
Message ID <1402082078-10826-8-git-send-email-corneliux.stoicescu@intel.com>
Download mbox | patch
Permalink /patch/73447/
State New
Headers show

Comments

Corneliu Stoicescu - June 6, 2014, 7:14 p.m.
YB: #6254

Adding a new script that will fetch image files from a remote images repository.
These images will then be used for local runtime testing.

Use the '-h' option for more details on usage.

Signed-off-by: Corneliu Stoicescu <corneliux.stoicescu@intel.com>
---
 scripts/test-remote-image | 340 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 340 insertions(+)
 create mode 100755 scripts/test-remote-image
Stefan Stanacar - June 7, 2014, 2:08 p.m.
Hi Corneliu,

I haven't carefully reviewed this series so if I didn't quite got the
point of some things, sorry...
At a first look, here are my thoughts (this is actually a reply to the
entire series, I seem to have lost the cover letter):

- It seems wierd that you instantiate the target controllers classes
here... You do that so you can get the extra files, but that has
nothing to do with testimage anyway. Let bitbake/testimage do it's
part and it's configuration. Similar for image fs types (see my other
reply)
- I wouldn't mix the download stuff with the test/deploy stuff
(bitbake's job). You already have command line configuration knobs,
I'm sure you could add one for extra files... Surely the user know
it's testing BBB and core-image-minimal and it needs to download the
dtbs (because you need the dtbs only for minimal).
- the support for other fs types should be separate from the "test
external built image". E.g bbb uses tar.bz2, so even at this point you
can still test external images only with tar.gz (adding IMAGE_FSTYPES
+= "tar.gz"  to the Autobuilder config is a easy task). What I'm
trying to say is that this is a user/build config option and this
script shouldn't deal with every little thing
- What's with the postconfig stuff? QA_MACHINE = MACHINE? Same for
distro.. I really don't get that part...
- I'm missing the bigger picture here with all the profiles stuff
(plus some classes are unnecessarily abstract ).

So the whole point of this was to replace (assume default local.conf
and MACHINE="beaglebone" here)
this:
---
$ vi conf/local.conf
INHERIT += "testimage"
TEST_TARGET_IP = "10.11.12.2"
TEST_TARGET = "BeagleBoneTarget"
TEST_SERIALCONTROL_CMD = "picocom /dev/ttyUSB0 -b 115200"
$ bitbake core-image-minimal # (or  sato / sato-sdk)
$ bitbake <image> -c testimage
---
with this (except the local.conf part which should already be there,
it is not the job of this script to add that):
----
$ bitbake rpm psplash # rpm/smart tests require these, else skip
$ bitbake package-index
$ wget -r -l0 -nd -np -P "tmp/deploy/images"
http://autobuilder.yoctoproject.org/pub/nightly/20140606-3/machines/beaglebone/
$ bitbake <image> -c testimage
---
#yes, I cheated there, getting all the files, but you get the point..
alternatively:
wget -r -l0 -nd -np -P "tmp/deploy/images" -A "*boneblack*.dtb"
http://autobuilder.yoctoproject.org/pub/nightly/20140606-3/machines/beaglebone/
wget -P "tmp/deploy/images"
http://autobuilder.yoctoproject.org/pub/nightly/20140606-3/machines/beaglebone/core-image-minimal-beaglebone.manifest
wget -P "tmp/deploy/images"
http://autobuilder.yoctoproject.org/pub/nightly/20140606-3/machines/beaglebone/core-image-minimal-beaglebone.tar.gz
wget -P "tmp/deploy/images"
http://autobuilder.yoctoproject.org/pub/nightly/20140606-3/machines/beaglebone/uImage

It seems to me that you are doing a lot more with this series and
adding unnecessary complications. I guess I'm missing some pieces
here...

Cheers,
Stefan


On Fri, Jun 6, 2014 at 10:14 PM, Corneliu Stoicescu
<corneliux.stoicescu@intel.com> wrote:
> YB: #6254
>
> Adding a new script that will fetch image files from a remote images repository.
> These images will then be used for local runtime testing.
>
> Use the '-h' option for more details on usage.
>
> Signed-off-by: Corneliu Stoicescu <corneliux.stoicescu@intel.com>
> ---
>  scripts/test-remote-image | 340 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 340 insertions(+)
>  create mode 100755 scripts/test-remote-image
>
> diff --git a/scripts/test-remote-image b/scripts/test-remote-image
> new file mode 100755
> index 0000000..6da6672
> --- /dev/null
> +++ b/scripts/test-remote-image
> @@ -0,0 +1,340 @@
> +#!/usr/bin/env python
> +
> +# Copyright (c) 2014 Intel Corporation
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License version 2 as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License along
> +# with this program; if not, write to the Free Software Foundation, Inc.,
> +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> +
> +# DESCRIPTION
> +# This script is used to test public autobuilder images on remote hardware.
> +# The script is called from a machine that is able download the images from the remote images repository and to connect to the test hardware.
> +#
> +# test-remote-image --image-type core-image-sato --repo-link http://192.168.10.2/images --required-packages rpm psplash
> +#
> +# Translation: Build the 'rpm' and 'pslash' packages and test a remote core-image-sato image using the http://192.168.10.2/images repository.
> +#
> +# You can also use the '-h' option to see some help information.
> +
> +import os
> +import sys
> +import argparse
> +import logging
> +import shutil
> +from abc import ABCMeta, abstractmethod
> +
> +# Add meta/lib to sys.path
> +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..', 'meta/lib')))
> +
> +import oeqa.utils.ftools as ftools
> +from oeqa.utils.commands import runCmd, bitbake, get_bb_var
> +
> +# Add all lib paths relative to BBPATH to sys.path; this is used to find and import the target controllers.
> +for path in get_bb_var('BBPATH').split(":"):
> +    sys.path.insert(0, os.path.abspath(os.path.join(path, 'lib')))
> +
> +# In order to import modules that contain target controllers, we need the bitbake libraries in sys.path .
> +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..', 'bitbake/lib')))
> +
> +# create a logger
> +def logger_create():
> +    log = logging.getLogger('hwauto')
> +    log.setLevel(logging.DEBUG)
> +
> +    fh = logging.FileHandler(filename='hwauto.log', mode='w')
> +    fh.setLevel(logging.DEBUG)
> +
> +    ch = logging.StreamHandler(sys.stdout)
> +    ch.setLevel(logging.INFO)
> +
> +    formatter = logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s')
> +    fh.setFormatter(formatter)
> +    ch.setFormatter(formatter)
> +
> +    log.addHandler(fh)
> +    log.addHandler(ch)
> +
> +    return log
> +
> +# instantiate the logger
> +log = logger_create()
> +
> +
> +# Define and return the arguments parser for the script
> +def get_args_parser():
> +    description = "This script is used to run automated runtime tests using remotely published image files. You should prepare the build environment just like building local images and running the tests."
> +    parser = argparse.ArgumentParser(description=description)
> +    parser.add_argument('--image-types', required=True, action="store", nargs='*', dest="image_types", default=None, help='The image types to test(ex: core-image-minimal).')
> +    parser.add_argument('--repo-link', required=True, action="store", type=str, dest="repo_link", default=None, help='The link to the remote images repository.')
> +    parser.add_argument('--required-packages', required=False, action="store", nargs='*', dest="required_packages", default=None, help='Required packages for the tests. They will be built before the testing begins.')
> +    parser.add_argument('--targetprofile', required=False, action="store", nargs=1, dest="targetprofile", default='AutoTargetProfile', help='The target profile to be used.')
> +    parser.add_argument('--repoprofile', required=False, action="store", nargs=1, dest="repoprofile", default='PublicAB', help='The repo profile to be used.')
> +    return parser
> +
> +class BaseTargetProfile(object):
> +    """
> +    This class defines the meta profile for a specific target (MACHINE type + image type).
> +    """
> +
> +    __metaclass__ = ABCMeta
> +
> +    def __init__(self, image_type):
> +        self.image_type = image_type
> +
> +        self.kernel_file = None
> +        self.rootfs_file = None
> +        self.manifest_file = None
> +        self.extra_download_files = []          # Extra files (full name) to be downloaded. They should be situated in repo_link
> +
> +    # This method is used as the standard interface with the target profile classes.
> +    # It returns a dictionary containing a list of files and their meaning/description.
> +    def get_files_dict(self):
> +        files_dict = {}
> +
> +        if self.kernel_file:
> +            files_dict['kernel_file'] = self.kernel_file
> +        else:
> +            log.error('The target profile did not set a kernel file.')
> +            sys.exit(1)
> +
> +        if self.rootfs_file:
> +            files_dict['rootfs_file'] = self.rootfs_file
> +        else:
> +            log.error('The target profile did not set a rootfs file.')
> +            sys.exit(1)
> +
> +        if self.manifest_file:
> +            files_dict['manifest_file'] = self.manifest_file
> +        else:
> +            log.error('The target profile did not set a manifest file.')
> +            sys.exit(1)
> +
> +        for idx, f in enumerate(self.extra_download_files):
> +            files_dict['extra_download_file' + str(idx)] = f
> +
> +        return files_dict
> +
> +class AutoTargetProfile(BaseTargetProfile):
> +
> +    def __init__(self, image_type):
> +        super(AutoTargetProfile, self).__init__(image_type)
> +        self.image_name = get_bb_var('IMAGE_LINK_NAME', target=image_type)
> +        self.kernel_type = get_bb_var('KERNEL_IMAGETYPE', target=image_type)
> +        self.controller = self.get_controller()
> +
> +        self.set_kernel_file()
> +        self.set_rootfs_file()
> +        self.set_manifest_file()
> +        self.set_extra_download_files()
> +
> +
> +    def get_controller(self):
> +        from oeqa.controllers.testtargetloader import TestTargetLoader
> +
> +        target_controller = get_bb_var('TEST_TARGET')
> +        bbpath = get_bb_var('BBPATH').split(':')
> +
> +        if target_controller == "qemu":
> +            from oeqa.targetcontrol import QemuTarget
> +            controller = QemuTarget
> +        else:
> +            testtargetloader = TestTargetLoader()
> +            controller = testtargetloader.get_controller_module(target_controller, bbpath)
> +        return controller
> +
> +    def set_kernel_file(self):
> +        postconfig = "QA_GET_MACHINE = \"${MACHINE}\""
> +        machine = get_bb_var('QA_GET_MACHINE', postconfig=postconfig)
> +        self.kernel_file = self.kernel_type + '-' + machine + '.bin'
> +
> +    def set_rootfs_file(self):
> +        image_fstypes = get_bb_var('IMAGE_FSTYPES').split(' ')
> +        fstype = self.controller.get_image_fstype(d=None, image_fstypes=image_fstypes)
> +        self.rootfs_file = self.image_name + '.' + fstype
> +
> +    def set_manifest_file(self):
> +        self.manifest_file = self.image_name + ".manifest"
> +
> +    def set_extra_download_files(self):
> +        self.extra_download_files = self.get_controller_extra_files()
> +        if not self.extra_download_files:
> +            self.extra_download_files = []
> +
> +    def get_controller_extra_files(self):
> +        controller = self.get_controller()
> +        return controller.get_extra_files()
> +
> +
> +class BaseRepoProfile(object):
> +    """
> +    This class defines the meta profile for an images repository.
> +    """
> +
> +    __metaclass__ = ABCMeta
> +
> +    def __init__(self, repolink, localdir):
> +        self.localdir = localdir
> +        self.repolink = repolink
> +
> +    # The following abstract methods are the interfaces to the repository profile classes derived from this abstract class.
> +
> +    # This method should check the file named 'file_name' if it is different than the upstream one.
> +    # Should return False if the image is the same as the upstream and True if it differs.
> +    @abstractmethod
> +    def check_old_file(self, file_name):
> +        pass
> +
> +    # This method should fetch file_name and create a symlink to localname if set.
> +    @abstractmethod
> +    def fetch(self, file_name, localname=None):
> +        pass
> +
> +class PublicAB(BaseRepoProfile):
> +
> +    def __init__(self, repolink, localdir=None):
> +        super(PublicAB, self).__init__(repolink, localdir)
> +        if localdir is None:
> +            self.localdir = os.path.join(os.environ['BUILDDIR'], 'PublicABMirror')
> +
> +    # Not yet implemented. Always returning True.
> +    def check_old_file(self, file_name):
> +        return True
> +
> +    def get_repo_path(self):
> +        path = '/machines/'
> +
> +        postconfig = "QA_GET_MACHINE = \"${MACHINE}\""
> +        machine = get_bb_var('QA_GET_MACHINE', postconfig=postconfig)
> +        if 'qemu' in machine:
> +            path += 'qemu/'
> +
> +        postconfig = "QA_GET_DISTRO = \"${DISTRO}\""
> +        distro = get_bb_var('QA_GET_DISTRO', postconfig=postconfig)
> +        path += distro.replace('poky', machine) + '/'
> +        return path
> +
> +
> +    def fetch(self, file_name, localname=None):
> +        repo_path = self.get_repo_path()
> +        link = self.repolink + repo_path + file_name
> +
> +        self.wget(link, self.localdir, localname)
> +
> +    def wget(self, link, localdir, localname=None, extraargs=None):
> +        wget_cmd = '/usr/bin/env wget -t 2 -T 30 -nv --passive-ftp --no-check-certificate '
> +
> +        if localname:
> +            wget_cmd += ' -O ' + localname + ' '
> +
> +        if extraargs:
> +            wget_cmd += ' ' + extraargs + ' '
> +
> +        wget_cmd += " -P %s '%s'" % (localdir, link)
> +        runCmd(wget_cmd)
> +
> +class HwAuto():
> +
> +    def __init__(self, image_types, repolink, required_packages, targetprofile, repoprofile):
> +        log.info('Initializing..')
> +        self.image_types = image_types
> +        self.repolink = repolink
> +        self.required_packages = required_packages
> +        self.targetprofile = targetprofile
> +        self.repoprofile = repoprofile
> +        self.repo = self.get_repo_profile(self.repolink)
> +
> +    # Get the repository profile; for now we only look inside this module.
> +    def get_repo_profile(self, *args, **kwargs):
> +        repo = getattr(sys.modules[__name__], self.repoprofile)(*args, **kwargs)
> +        log.info("Using repo profile: %s" % repo.__class__.__name__)
> +        return repo
> +
> +    # Get the target profile; for now we only look inside this module.
> +    def get_target_profile(self, *args, **kwargs):
> +        target = getattr(sys.modules[__name__], self.targetprofile)(*args, **kwargs)
> +        log.info("Using target profile: %s" % target.__class__.__name__)
> +        return target
> +
> +    # Run the testimage task on a build while redirecting DEPLOY_DIR_IMAGE to repo.localdir, where the images are downloaded.
> +    def runTestimageBuild(self, image_type):
> +        log.info("Running the runtime tests for %s.." % image_type)
> +        postconfig = "DEPLOY_DIR_IMAGE = \"%s\"" % self.repo.localdir
> +        result = bitbake("%s -c testimage" % image_type, ignore_status=True, postconfig=postconfig)
> +        testimage_results = ftools.read_file(os.path.join(get_bb_var("T", image_type), "log.do_testimage"))
> +        log.info('Runtime tests results for %s:' % image_type)
> +        print testimage_results
> +        return result
> +
> +    # Start the procedure!
> +    def run(self):
> +        if self.required_packages:
> +            # Build the required packages for the tests
> +            log.info("Building the required packages: %s ." % ', '.join(map(str, self.required_packages)))
> +            result = bitbake(self.required_packages, ignore_status=True)
> +            if result.status != 0:
> +                log.error("Could not build required packages: %s. Output: %s" % (self.required_packages, result.output))
> +                sys.exit(1)
> +
> +            # Build the package repository meta data.
> +            log.info("Building the package index.")
> +            result = bitbake("package-index", ignore_status=True)
> +            if result.status != 0:
> +                log.error("Could not build 'package-index'. Output: %s" % result.output)
> +                sys.exit(1)
> +
> +        # Create the directory structure for the images to be downloaded
> +        log.info("Creating directory structure %s" % self.repo.localdir)
> +        if not os.path.exists(self.repo.localdir):
> +            os.makedirs(self.repo.localdir)
> +
> +        # For each image type, download the needed files and run the tests.
> +        noissuesfound = True
> +        for image_type in self.image_types:
> +            target = self.get_target_profile(image_type)
> +            files_dict = target.get_files_dict()
> +            log.info("Downloading files for %s" % image_type)
> +            for f in files_dict:
> +                if self.repo.check_old_file(files_dict[f]):
> +                    filepath = os.path.join(self.repo.localdir, files_dict[f])
> +                    if os.path.exists(filepath):
> +                        os.remove(filepath)
> +                    self.repo.fetch(files_dict[f])
> +
> +            result = self.runTestimageBuild(image_type)
> +            if result.status != 0:
> +                noissuesfound = False
> +
> +        if noissuesfound:
> +            log.info('Finished. No issues found.')
> +        else:
> +            log.error('Finished. Some runtime tests have failed. Returning non-0 status code.')
> +            sys.exit(1)
> +
> +
> +
> +def main():
> +
> +    parser = get_args_parser()
> +    args = parser.parse_args()
> +
> +    hwauto = HwAuto(image_types=args.image_types, repolink=args.repo_link, required_packages=args.required_packages, targetprofile=args.targetprofile, repoprofile=args.repoprofile)
> +
> +    hwauto.run()
> +
> +if __name__ == "__main__":
> +    try:
> +        ret = main()
> +    except Exception:
> +        ret = 1
> +        import traceback
> +        traceback.print_exc(5)
> +    sys.exit(ret)
> --
> 1.8.3.2
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
Corneliu Stoicescu - June 9, 2014, 12:06 p.m.
> -----Original Message-----
> From: Stefan Stanacar [mailto:sstncr@gmail.com]
> Sent: Saturday, June 07, 2014 5:09 PM
> To: Stoicescu, CorneliuX
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH V2 7/7] scripts/test-remote-image: Add script
> for running runtime tests on remotely built images
> 
> Hi Corneliu,
> 
> I haven't carefully reviewed this series so if I didn't quite got the point of
> some things, sorry...
> At a first look, here are my thoughts (this is actually a reply to the entire
> series, I seem to have lost the cover letter):
> 
> - It seems wierd that you instantiate the target controllers classes here... You
> do that so you can get the extra files, but that has nothing to do with
> testimage anyway. Let bitbake/testimage do it's part and it's configuration.
> Similar for image fs types (see my other
> reply)

- I believe it's a good idea to use the target controllers to get information from them using class methods. This way we use the same source to get the information we need both internally and externally. 

> - I wouldn't mix the download stuff with the test/deploy stuff (bitbake's job).
> You already have command line configuration knobs, I'm sure you could add
> one for extra files... Surely the user know it's testing BBB and core-image-
> minimal and it needs to download the dtbs (because you need the dtbs only
> for minimal).

- This feature required keeping the user configuration to a minimum(we did not want to introduce any more TEST_ variables and have as few required command line options for the script as possible). The way it is, all target controllers use predictable image files and the missing piece was the so called "extra files" that some special target controllers need. This method, along with the other new class methods, can be used by any kind of external script needing information on what testimage needs/uses. 

> - the support for other fs types should be separate from the "test external
> built image". E.g bbb uses tar.bz2, so even at this point you can still test
> external images only with tar.gz (adding IMAGE_FSTYPES
> += "tar.gz"  to the Autobuilder config is a easy task). What I'm
> trying to say is that this is a user/build config option and this script shouldn't
> deal with every little thing

- We wanted to use the current default IMAGE_FSTYPES as much as possible(this is why the bug was opened). For this we had to enable the master image to decompress tar.bz2 and make the target controllers use them. Having IMAGE_FTYPES += "tar.gz" for all test images was not desired. 
- We wanted to make the way the rootfs image fstype was chosen need as little maintenance as possible. For this we just define what image_fstypes a target controller can handle and the choose one from the available IMAGE_FSTYPES. Also having a classmethod decide this makes it easy to get this information from external sources, with higher accuracy. 

> - What's with the postconfig stuff? QA_MACHINE = MACHINE? Same for
> distro.. I really don't get that part...

Bitbake does not export the MACHINE variable so in order to get this information we had to use this trick (the runqemu script also uses it, if I remember right). We do QA_MACHINE = MACHINE and then the QA_MACHINE variable is exported by bitbake and we can get it from 'bitbake -e'. :D

> - I'm missing the bigger picture here with all the profiles stuff (plus some
> classes are unnecessarily abstract ).

The same way we have a BaseTarget class for the target controllers, we also have base target/repo profiles. These classes are abstract and define the bits and pieces that the script will actually use. We don't care how they are implemented. You can see them as interfaces to the target and images repo.

Also we wanted classes so they can be extended/modified easily by new ones. For example we have an AutoTargetProfile class that auto determines the images needed, but say a target controller cannot be predicted at all with the information we can access. We just create a new target profile extending the base target profile and add new functionality and information sources but still use the 'interfaces' defined. 

> 
> So the whole point of this was to replace (assume default local.conf and
> MACHINE="beaglebone" here)
> this:
> ---
> $ vi conf/local.conf
> INHERIT += "testimage"
> TEST_TARGET_IP = "10.11.12.2"
> TEST_TARGET = "BeagleBoneTarget"
> TEST_SERIALCONTROL_CMD = "picocom /dev/ttyUSB0 -b 115200"
> $ bitbake core-image-minimal # (or  sato / sato-sdk) $ bitbake <image> -c
> testimage
> ---
> with this (except the local.conf part which should already be there, it is not
> the job of this script to add that):
> ----
> $ bitbake rpm psplash # rpm/smart tests require these, else skip $ bitbake
> package-index $ wget -r -l0 -nd -np -P "tmp/deploy/images"
> http://autobuilder.yoctoproject.org/pub/nightly/20140606-
> 3/machines/beaglebone/
> $ bitbake <image> -c testimage
> ---
> #yes, I cheated there, getting all the files, but you get the point..
> alternatively:

> wget -r -l0 -nd -np -P "tmp/deploy/images" -A "*boneblack*.dtb"
> http://autobuilder.yoctoproject.org/pub/nightly/20140606-
> 3/machines/beaglebone/
> wget -P "tmp/deploy/images"
> http://autobuilder.yoctoproject.org/pub/nightly/20140606-
> 3/machines/beaglebone/core-image-minimal-beaglebone.manifest
> wget -P "tmp/deploy/images"
> http://autobuilder.yoctoproject.org/pub/nightly/20140606-
> 3/machines/beaglebone/core-image-minimal-beaglebone.tar.gz
> wget -P "tmp/deploy/images"
> http://autobuilder.yoctoproject.org/pub/nightly/20140606-
> 3/machines/beaglebone/uImage
> 
> It seems to me that you are doing a lot more with this series and adding
> unnecessary complications. I guess I'm missing some pieces here...
> 

THB the first version of this script did mainly what you pointed out it should do. But after a few talks among the team (here in RO QA and Paul), we decided we need to automate some configuration (like deciding what images we need, downloading them, checking if they are different than upstream, etc.) and also we need to make this easily extendable and thus the abstract classes and interfaces.
  
This series, apart from adding this script, addresses other issues too. Not everything is for the script, you can see them as issues found along the way (like hard coded image fstypes, multiple kernel files naming). These should have been fixed independently but I added them here because of lack of time(see below) and they are related somehow(especially to the script). The only change introduced that only helps the script (and other outside script that will want this information) is the get_extra_files method. If the only use case was to build the images locally and run the tests locally, this would have been easily avoided but unfortunately we need to use the AB images on remote hardware testing racks. 

Finally, why this series of patches was needed instead of standalone small patches. These bugs have been marked for 1.7 M1 and even though we tried to finish earlier some changes affected the others and there was no point to send something and then repair/change it. I will be OOO this week and the cutoff for M1 is on the 13-th. I am sorry for the large set and I hope, through the cover letter and these answers, I managed to point out what we intend to add and change.

I will not be able to answer any more mails until the 17-th but I hope Paul can help with any more questions/issues that appear. If we did extend too much with this and some changes proposed are not indeed needed, I will revise the script and make it work without them when I get back, though the features should be moved to M2.

Regards,
Corneliu
Paul Eggleton - June 13, 2014, 10:29 a.m.
On Monday 09 June 2014 12:06:44 Stoicescu, CorneliuX wrote:
> On Saturday, June 07, 2014 5:09 PM Stefan Stanacar wrote:
> > - It seems wierd that you instantiate the target controllers classes
> > here... You do that so you can get the extra files, but that has nothing
> > to do with testimage anyway. Let bitbake/testimage do it's part and it's
> > configuration. Similar for image fs types (see my other
> > reply)
> 
> - I believe it's a good idea to use the target controllers to get
> information from them using class methods. This way we use the same source
> to get the information we need both internally and externally.

Right, the intention was to avoid duplicating the code for handling machine-
specific behaviours. We aren't instantiating the target controllers, just using 
them as classes and call class functions on them.

> > - the support for other fs types should be separate from the "test
> > external built image". E.g bbb uses tar.bz2, so even at this point you can
> > still test external images only with tar.gz (adding IMAGE_FSTYPES
> > += "tar.gz"  to the Autobuilder config is a easy task). What I'm
> > trying to say is that this is a user/build config option and this script
> > shouldn't deal with every little thing
> 
> - We wanted to use the current default IMAGE_FSTYPES as much as
> possible(this is why the bug was opened). For this we had to enable the
> master image to decompress tar.bz2 and make the target controllers use
> them. Having IMAGE_FTYPES += "tar.gz" for all test images was not desired.

At the moment Beth is trying to reduce the number of artifacts produced by the 
autobuilder; it just seemed a bit wasteful to add another tarball format when 
we could simply handle whatever format is already produced (assuming it is a 
tarball, of course).

> > - What's with the postconfig stuff? QA_MACHINE = MACHINE? Same for
> > distro.. I really don't get that part...
> 
> Bitbake does not export the MACHINE variable so in order to get this
> information we had to use this trick (the runqemu script also uses it, if I
> remember right). We do QA_MACHINE = MACHINE and then the QA_MACHINE
> variable is exported by bitbake and we can get it from 'bitbake -e'. :D

This is unfortunately part of the pain of how oe-selftest and other external 
scripts like it query variables using bitbake -e. What bitbake -e does is to 
report variables broadly as they would be exported to the environment; if a 
variable is marked "unexport" (as MACHINE is) then you won't see its final 
value in bitbake -e output other than in comments. There are other ways we 
could deal with this, and maybe bitbake -e shouldn't pay attention to 
unexport, however the method this new script uses, whilst a little hacky, is 
the least disruptive.

> > - I'm missing the bigger picture here with all the profiles stuff (plus
> > some classes are unnecessarily abstract ).
> 
> The same way we have a BaseTarget class for the target controllers, we also
> have base target/repo profiles. These classes are abstract and define the
> bits and pieces that the script will actually use. We don't care how they
> are implemented. You can see them as interfaces to the target and images
> repo.
> 
> Also we wanted classes so they can be extended/modified easily by new ones.
> For example we have an AutoTargetProfile class that auto determines the
> images needed, but say a target controller cannot be predicted at all with
> the information we can access. We just create a new target profile
> extending the base target profile and add new functionality and information
> sources but still use the 'interfaces' defined.

We'll have to wait and see how useful this ends up being in practice, but the 
intention is to allow for some flexibility in future.

Cheers,
Paul

Patch

diff --git a/scripts/test-remote-image b/scripts/test-remote-image
new file mode 100755
index 0000000..6da6672
--- /dev/null
+++ b/scripts/test-remote-image
@@ -0,0 +1,340 @@ 
+#!/usr/bin/env python
+
+# Copyright (c) 2014 Intel Corporation
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# DESCRIPTION
+# This script is used to test public autobuilder images on remote hardware.
+# The script is called from a machine that is able download the images from the remote images repository and to connect to the test hardware.
+#
+# test-remote-image --image-type core-image-sato --repo-link http://192.168.10.2/images --required-packages rpm psplash
+#
+# Translation: Build the 'rpm' and 'pslash' packages and test a remote core-image-sato image using the http://192.168.10.2/images repository.
+#
+# You can also use the '-h' option to see some help information.
+
+import os
+import sys
+import argparse
+import logging
+import shutil
+from abc import ABCMeta, abstractmethod
+
+# Add meta/lib to sys.path
+sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..', 'meta/lib')))
+
+import oeqa.utils.ftools as ftools
+from oeqa.utils.commands import runCmd, bitbake, get_bb_var
+
+# Add all lib paths relative to BBPATH to sys.path; this is used to find and import the target controllers.
+for path in get_bb_var('BBPATH').split(":"):
+    sys.path.insert(0, os.path.abspath(os.path.join(path, 'lib')))
+
+# In order to import modules that contain target controllers, we need the bitbake libraries in sys.path .
+sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..', 'bitbake/lib')))
+
+# create a logger
+def logger_create():
+    log = logging.getLogger('hwauto')
+    log.setLevel(logging.DEBUG)
+
+    fh = logging.FileHandler(filename='hwauto.log', mode='w')
+    fh.setLevel(logging.DEBUG)
+
+    ch = logging.StreamHandler(sys.stdout)
+    ch.setLevel(logging.INFO)
+
+    formatter = logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s')
+    fh.setFormatter(formatter)
+    ch.setFormatter(formatter)
+
+    log.addHandler(fh)
+    log.addHandler(ch)
+
+    return log
+
+# instantiate the logger
+log = logger_create()
+
+
+# Define and return the arguments parser for the script
+def get_args_parser():
+    description = "This script is used to run automated runtime tests using remotely published image files. You should prepare the build environment just like building local images and running the tests."
+    parser = argparse.ArgumentParser(description=description)
+    parser.add_argument('--image-types', required=True, action="store", nargs='*', dest="image_types", default=None, help='The image types to test(ex: core-image-minimal).')
+    parser.add_argument('--repo-link', required=True, action="store", type=str, dest="repo_link", default=None, help='The link to the remote images repository.')
+    parser.add_argument('--required-packages', required=False, action="store", nargs='*', dest="required_packages", default=None, help='Required packages for the tests. They will be built before the testing begins.')
+    parser.add_argument('--targetprofile', required=False, action="store", nargs=1, dest="targetprofile", default='AutoTargetProfile', help='The target profile to be used.')
+    parser.add_argument('--repoprofile', required=False, action="store", nargs=1, dest="repoprofile", default='PublicAB', help='The repo profile to be used.')
+    return parser
+
+class BaseTargetProfile(object):
+    """
+    This class defines the meta profile for a specific target (MACHINE type + image type).
+    """
+
+    __metaclass__ = ABCMeta
+
+    def __init__(self, image_type):
+        self.image_type = image_type
+
+        self.kernel_file = None
+        self.rootfs_file = None
+        self.manifest_file = None
+        self.extra_download_files = []          # Extra files (full name) to be downloaded. They should be situated in repo_link
+
+    # This method is used as the standard interface with the target profile classes.
+    # It returns a dictionary containing a list of files and their meaning/description.
+    def get_files_dict(self):
+        files_dict = {}
+
+        if self.kernel_file:
+            files_dict['kernel_file'] = self.kernel_file
+        else:
+            log.error('The target profile did not set a kernel file.')
+            sys.exit(1)
+
+        if self.rootfs_file:
+            files_dict['rootfs_file'] = self.rootfs_file
+        else:
+            log.error('The target profile did not set a rootfs file.')
+            sys.exit(1)
+
+        if self.manifest_file:
+            files_dict['manifest_file'] = self.manifest_file
+        else:
+            log.error('The target profile did not set a manifest file.')
+            sys.exit(1)
+
+        for idx, f in enumerate(self.extra_download_files):
+            files_dict['extra_download_file' + str(idx)] = f
+
+        return files_dict
+
+class AutoTargetProfile(BaseTargetProfile):
+
+    def __init__(self, image_type):
+        super(AutoTargetProfile, self).__init__(image_type)
+        self.image_name = get_bb_var('IMAGE_LINK_NAME', target=image_type)
+        self.kernel_type = get_bb_var('KERNEL_IMAGETYPE', target=image_type)
+        self.controller = self.get_controller()
+
+        self.set_kernel_file()
+        self.set_rootfs_file()
+        self.set_manifest_file()
+        self.set_extra_download_files()
+
+
+    def get_controller(self):
+        from oeqa.controllers.testtargetloader import TestTargetLoader
+
+        target_controller = get_bb_var('TEST_TARGET')
+        bbpath = get_bb_var('BBPATH').split(':')
+
+        if target_controller == "qemu":
+            from oeqa.targetcontrol import QemuTarget
+            controller = QemuTarget
+        else:
+            testtargetloader = TestTargetLoader()
+            controller = testtargetloader.get_controller_module(target_controller, bbpath)
+        return controller
+
+    def set_kernel_file(self):
+        postconfig = "QA_GET_MACHINE = \"${MACHINE}\""
+        machine = get_bb_var('QA_GET_MACHINE', postconfig=postconfig)
+        self.kernel_file = self.kernel_type + '-' + machine + '.bin'
+
+    def set_rootfs_file(self):
+        image_fstypes = get_bb_var('IMAGE_FSTYPES').split(' ')
+        fstype = self.controller.get_image_fstype(d=None, image_fstypes=image_fstypes)
+        self.rootfs_file = self.image_name + '.' + fstype
+
+    def set_manifest_file(self):
+        self.manifest_file = self.image_name + ".manifest"
+
+    def set_extra_download_files(self):
+        self.extra_download_files = self.get_controller_extra_files()
+        if not self.extra_download_files:
+            self.extra_download_files = []
+
+    def get_controller_extra_files(self):
+        controller = self.get_controller()
+        return controller.get_extra_files()
+
+
+class BaseRepoProfile(object):
+    """
+    This class defines the meta profile for an images repository.
+    """
+
+    __metaclass__ = ABCMeta
+
+    def __init__(self, repolink, localdir):
+        self.localdir = localdir
+        self.repolink = repolink
+
+    # The following abstract methods are the interfaces to the repository profile classes derived from this abstract class.
+
+    # This method should check the file named 'file_name' if it is different than the upstream one.
+    # Should return False if the image is the same as the upstream and True if it differs.
+    @abstractmethod
+    def check_old_file(self, file_name):
+        pass
+
+    # This method should fetch file_name and create a symlink to localname if set.
+    @abstractmethod
+    def fetch(self, file_name, localname=None):
+        pass
+
+class PublicAB(BaseRepoProfile):
+
+    def __init__(self, repolink, localdir=None):
+        super(PublicAB, self).__init__(repolink, localdir)
+        if localdir is None:
+            self.localdir = os.path.join(os.environ['BUILDDIR'], 'PublicABMirror')
+
+    # Not yet implemented. Always returning True.
+    def check_old_file(self, file_name):
+        return True
+
+    def get_repo_path(self):
+        path = '/machines/'
+
+        postconfig = "QA_GET_MACHINE = \"${MACHINE}\""
+        machine = get_bb_var('QA_GET_MACHINE', postconfig=postconfig)
+        if 'qemu' in machine:
+            path += 'qemu/'
+
+        postconfig = "QA_GET_DISTRO = \"${DISTRO}\""
+        distro = get_bb_var('QA_GET_DISTRO', postconfig=postconfig)
+        path += distro.replace('poky', machine) + '/'
+        return path
+
+
+    def fetch(self, file_name, localname=None):
+        repo_path = self.get_repo_path()
+        link = self.repolink + repo_path + file_name
+
+        self.wget(link, self.localdir, localname)
+
+    def wget(self, link, localdir, localname=None, extraargs=None):
+        wget_cmd = '/usr/bin/env wget -t 2 -T 30 -nv --passive-ftp --no-check-certificate '
+
+        if localname:
+            wget_cmd += ' -O ' + localname + ' '
+
+        if extraargs:
+            wget_cmd += ' ' + extraargs + ' '
+
+        wget_cmd += " -P %s '%s'" % (localdir, link)
+        runCmd(wget_cmd)
+
+class HwAuto():
+
+    def __init__(self, image_types, repolink, required_packages, targetprofile, repoprofile):
+        log.info('Initializing..')
+        self.image_types = image_types
+        self.repolink = repolink
+        self.required_packages = required_packages
+        self.targetprofile = targetprofile
+        self.repoprofile = repoprofile
+        self.repo = self.get_repo_profile(self.repolink)
+
+    # Get the repository profile; for now we only look inside this module.
+    def get_repo_profile(self, *args, **kwargs):
+        repo = getattr(sys.modules[__name__], self.repoprofile)(*args, **kwargs)
+        log.info("Using repo profile: %s" % repo.__class__.__name__)
+        return repo
+
+    # Get the target profile; for now we only look inside this module.
+    def get_target_profile(self, *args, **kwargs):
+        target = getattr(sys.modules[__name__], self.targetprofile)(*args, **kwargs)
+        log.info("Using target profile: %s" % target.__class__.__name__)
+        return target
+
+    # Run the testimage task on a build while redirecting DEPLOY_DIR_IMAGE to repo.localdir, where the images are downloaded.
+    def runTestimageBuild(self, image_type):
+        log.info("Running the runtime tests for %s.." % image_type)
+        postconfig = "DEPLOY_DIR_IMAGE = \"%s\"" % self.repo.localdir
+        result = bitbake("%s -c testimage" % image_type, ignore_status=True, postconfig=postconfig)
+        testimage_results = ftools.read_file(os.path.join(get_bb_var("T", image_type), "log.do_testimage"))
+        log.info('Runtime tests results for %s:' % image_type)
+        print testimage_results
+        return result
+
+    # Start the procedure!
+    def run(self):
+        if self.required_packages:
+            # Build the required packages for the tests
+            log.info("Building the required packages: %s ." % ', '.join(map(str, self.required_packages)))
+            result = bitbake(self.required_packages, ignore_status=True)
+            if result.status != 0:
+                log.error("Could not build required packages: %s. Output: %s" % (self.required_packages, result.output))
+                sys.exit(1)
+
+            # Build the package repository meta data.
+            log.info("Building the package index.")
+            result = bitbake("package-index", ignore_status=True)
+            if result.status != 0:
+                log.error("Could not build 'package-index'. Output: %s" % result.output)
+                sys.exit(1)
+
+        # Create the directory structure for the images to be downloaded
+        log.info("Creating directory structure %s" % self.repo.localdir)
+        if not os.path.exists(self.repo.localdir):
+            os.makedirs(self.repo.localdir)
+
+        # For each image type, download the needed files and run the tests.
+        noissuesfound = True
+        for image_type in self.image_types:
+            target = self.get_target_profile(image_type)
+            files_dict = target.get_files_dict()
+            log.info("Downloading files for %s" % image_type)
+            for f in files_dict:
+                if self.repo.check_old_file(files_dict[f]):
+                    filepath = os.path.join(self.repo.localdir, files_dict[f])
+                    if os.path.exists(filepath):
+                        os.remove(filepath)
+                    self.repo.fetch(files_dict[f])
+
+            result = self.runTestimageBuild(image_type)
+            if result.status != 0:
+                noissuesfound = False
+
+        if noissuesfound:
+            log.info('Finished. No issues found.')
+        else:
+            log.error('Finished. Some runtime tests have failed. Returning non-0 status code.')
+            sys.exit(1)
+
+
+
+def main():
+
+    parser = get_args_parser()
+    args = parser.parse_args()
+
+    hwauto = HwAuto(image_types=args.image_types, repolink=args.repo_link, required_packages=args.required_packages, targetprofile=args.targetprofile, repoprofile=args.repoprofile)
+
+    hwauto.run()
+
+if __name__ == "__main__":
+    try:
+        ret = main()
+    except Exception:
+        ret = 1
+        import traceback
+        traceback.print_exc(5)
+    sys.exit(ret)