[Fuego] [PATCH 01/11] rebuild: add if_source_changed flag

Daniel Sangorrin daniel.sangorrin at toshiba.co.jp
Fri Sep 28 01:43:32 UTC 2018


Hi Tim,

> Does the ftc run-test code depend on this, or can I take them out of order?
> I'd like to apply that patch, but consider this one a bit longer.

No, it doesn't depend on this.

> I'd like to keep these issues separate, if possible, because while this
> series includes things that are very valuable, I think there are issues
> with a confusion between the software under test and test software,
> for the Functional.kernel_build test.
>
> I think I supported the idea to move the actual build steps from
> Functional.kernel_build's test_run() function to the test_build()
> function.  But now I am not sure that was the correct decision.

I understand what you say, but the if_source_changed flag is useful for any test, not just for kernel_build.
In addition, even if we move the "build test" back to test_run, the source code will still be fetched during the build phase.

The user can choose between cloning the entire kernel repository each time (Rebuild=True) or only pulling the latest changes (if_source_changed). Where we run the build test (make uImage etc) , either on test_build or on test_run, is a separate issue from how we are fetching the source code, which is what this patch is mainly about.

Yest another separate issue is whether we want to build from pristine state (do make mrproper) or reuse the cache of objects as much as possible.

> Logically: the kernel build test should consist of:
> build the kernel, get compiler report of errors as the testlog, parse those errors
> there should not be a deploy step at all, as it is purely
> a host-side operation.
> 
> Well - the deploy should consist of storing the build artifacts somewhere that
> can be used for other tests that utilize this software.

The results (the kernel and modules) are copied to a local directory. The user can choose to copy them to the log directory, or to /var/lib/tftpboot (after installing tftpd on Fuego's docker) for example.

The next step would be to create a Functional.kernel_deploy test to deploy the kernel to the target directly (using put), or indirectly through a server (ftp, html, tftp, LAVA..).

Finally, we could create Functional.reboot to reboot the DUT and check if the boot was succesful. This "check" can be many things: check it booted in less than 10 seconds, check we get the expected prompt, check we can connect to the board, check Functional.hello_world works, etc..

> I guess that's what the current  deploy step does, but it's not
> quite as explicit about it as I'd like.
Could you elaborate a bit on that?

> A separate boot test might consist of:
> build the kernel, deploy the kernel, boot the DUT, get console logs from DUT as the
> testlog, parse for any errors
> (also, detect if the DUT did not boot at all)

That could be done like this:
ftc run-test -b myboard -t Funtional.kernel_build -s myboard-tftpboot
[Opt] ftc run-test -b myboard -t Functional.kernel_deploy
ftc run-test -b myboard -t Functional.reboot -s myboard-console-prompt

The first part would build the kernel and copy it into /var/lib/tftpboot. Optionally, we can leave the deploy step to Functional.kernel_deploy.
Then, Functional.reboot would power cycle the board (or ssh reboot it) and check that it boots succesfully.
 
> In this case the kernel is the test program (it executes on the board to perform the
> test, and
> produces the log used to check for results).
> 
> The part to build the kernel might be: 'ftc test-run -b $BOARD -t
> Functional.kernel_build'.
> I can imagine other tests (eg. a static kernel size checker) that would want a built
> kernel, but
> would do different operations on it, rather than executing it.

That could be easily done:
ftc test-run -b $BOARD -t Functional.kernel_build -s logdir
ftc test-run -b $BOARD -t Functional.kernel_sizecheck -s latestbuildnumber

> The bottom line is this:  I know I recommended it, but I think this solves a
> different problem for the kernel_build test than it solves for other tests.
> And this difference is making me want to take a step back before I apply this
> change.
> 
> I want to consider this more, before creating a new rebuild flag.

I understand what you say, but the rebuild flag is more about what to do with the source code.
Do we want to clone the kernel again or just do a git pull?
The default is if_source_changed, but the user can set --rebuid true
If we keep a hardcoded "export Rebuild=true" then the user cannot decide.

Thanks,
Daniel


>  -- Tim
> 
> > -----Original Message-----
> > From: Daniel Sangorrin
> >
> > if_source_changed allows rebuilding only when the test source
> > has changed which is the main use case. For that reason,
> > we set it as the default option.
> >
> > FIXTHIS: consider conflicts when updating via git pull
> >
> > Signed-off-by: Daniel Sangorrin <daniel.sangorrin at toshiba.co.jp>
> > ---
> >  engine/scripts/ftc                                 | 10 +++---
> >  engine/scripts/functions.sh                        | 40
> +++++++++++++++++++---
> >  engine/tests/Functional.kernel_build/fuego_test.sh | 12 -------
> >  3 files changed, 42 insertions(+), 20 deletions(-)
> >
> > diff --git a/engine/scripts/ftc b/engine/scripts/ftc
> > index 7de2b70..b675dea 100755
> > --- a/engine/scripts/ftc
> > +++ b/engine/scripts/ftc
> > @@ -96,13 +96,14 @@ where_help = \
> >  command_help = {
> >  "add-jobs": ("Adds jobs to Jenkins.",
> >      """Usage: ftc add-jobs -b <board>[,board2...] [-p <testplan> | -t
> > <testcase> -s <testspec>]
> > -       [-k <kill timeout>] [--rebuild <true|false>] [--reboot <true|false>]
> > +       [-k <kill timeout>] [--rebuild <true|false|if_source_changed>]
> [--reboot
> > <true|false>]
> >         [--precleanup <true|false>] [--postcleanup <true|false>]
> >    Example: ftc add-jobs -b docker -p testplan_docker
> >    Example: ftc add-jobs -b docker -t Benchmark.Dhrystone -k 5m --rebuild
> > false
> >    board,testplan,testpec: use list-board/plans/specs to see the available
> > ones.
> >    timeout: integer with a suffix from 'smhd' (seconds, minutes, hours, days).
> >    rebuild: if true rebuild the test source even if it was already built.
> > +  if_source_changed can be used to rebuild the test only when its source
> > changes.
> >    reboot: if true reboot the board before the test.
> >    precleanup: if true clean the board's test folder before the test.
> >    postcleanup: if true clean the board's test folder after the test.
> > @@ -241,7 +242,7 @@ You can obtain a list of run_ids for runs on the local
> > system with
> >
> >  "run-test": ("Run a test on a board.",
> >      """Usage: ftc run-test -b <board> -t <test> [-s <spec>] [-p <phases>]
> > -    [-k <kill timeout>] [--rebuild <true|false>] [--reboot <true|false>]
> > +    [-k <kill timeout>] [--rebuild <true|false|if_source_changed>] [--reboot
> > <true|false>]
> >      [--precleanup <true|false>] [--postcleanup <true|false>]
> >      [--dynamic-vars python_dict]
> >  Run the indicated test on the specified board.  Use the
> > @@ -250,6 +251,7 @@ indicated spec, if one is provided.
> >  board,spec: use list-board/specs to see the available ones.
> >  timeout: integer with a suffix from 'smhd' (seconds, minutes, hours, days).
> >  rebuild: if true rebuild the test source even if it was already built.
> > +if_source_changed can be used to rebuild the test only when its source
> > changes.
> >  reboot: if true reboot the board before the test.
> >  precleanup: if true clean the board's test folder before the test.
> >  postcleanup: if true clean the board's test folder after the test.
> > @@ -652,7 +654,7 @@ class test_class:
> >          'timeout' : '30m',
> >          'spec'    : 'default',
> >          'reboot'  : 'false',
> > -        'rebuild' : 'false',
> > +        'rebuild' : 'if_source_changed',
> >          'precleanup'  : 'true',
> >          'postcleanup' : 'true'
> >      }
> > @@ -1489,7 +1491,7 @@ def do_add_jobs(conf, options):
> >              rebuild = options[options.index('--rebuild') + 1]
> >          except IndexError:
> >              error_out('Rebuild option not provided after --rebuild')
> > -        if rebuild not in ['true', 'false']:
> > +        if rebuild not in ['true', 'false', 'if_source_changed']:
> >              error_out("Invalid rebuild option '%s'" % rebuild)
> >          options.remove(rebuild)
> >          options.remove('--rebuild')
> > diff --git a/engine/scripts/functions.sh b/engine/scripts/functions.sh
> > index 952e9a4..05bdbc1 100755
> > --- a/engine/scripts/functions.sh
> > +++ b/engine/scripts/functions.sh
> > @@ -94,6 +94,9 @@ function untar {
> >          *) echo "Unknown $tarball file format. Not unpacking."; return 1;;
> >      esac
> >      tar ${key}xf $TEST_HOME/$tarball --strip-components=1
> > +
> > +    # record md5sum for possible source code updates
> > +    md5sum $TEST_HOME/$tarball > fuego_tarball_src_md5sum
> >  }
> >
> >  # Unpacks/clones the test source code into the current directory.
> > @@ -321,6 +324,11 @@ function build_error {
> >      abort_job "Build failed: $@"
> >  }
> >
> > +function prepare_build_dir {
> > +    find . -maxdepth 1 -mindepth 1 ! -name "fuego.build.lock" -print0 | xargs
> > -0 rm -rf
> > +    unpack || abort_job "Error while unpacking"
> > +}
> > +
> >  # process Rebuild flag, and unpack test sources if necessary.
> >  function pre_build {
> >      cd ${WORKSPACE}
> > @@ -341,11 +349,35 @@ function pre_build {
> >
> >      lock_build_dir
> >
> > -    if [ "$Rebuild" = "false" ] && [ -e fuego_test_successfully_built ]; then
> > -        echo "The test is already built"
> > +    if [ "$Rebuild" = "true" ]; then
> > +        prepare_build_dir
> > +    elif [ "$Rebuild" = "false" ]; then
> > +        if [ -e fuego_test_successfully_built ]; then
> > +            echo "The test was already successfully built"
> > +        else
> > +            prepare_build_dir
> > +        fi
> > +    elif [ "$Rebuild" = "if_source_changed" ]; then
> > +        if [ -e fuego_tarball_src_md5sum ]; then
> > +            if md5sum -c fuego_tarball_src_md5sum; then
> > +                echo "No updates to the source code tarball"
> > +            else
> > +                echo "The source code tarball got updated, rebuilding"
> > +                prepare_build_dir
> > +            fi
> > +        elif [ -d ".git" ]; then
> > +            echo "Trying to update the source code with git pull"
> > +            if git pull | grep "Already up-to-date"; then
> > +                echo "No source code updates"
> > +            else
> > +                echo "The source code was updated, rebuilding"
> > +                rm -f fuego_test_successfully_built
> > +            fi
> > +        else
> > +            prepare_build_dir
> > +        fi
> >      else
> > -        find . -maxdepth 1 -mindepth 1 ! -name "fuego.build.lock" -print0 |
> > xargs -0 rm -rf
> > -        unpack || abort_job "Error while unpacking"
> > +        abort_job "Rebuild flag $Rebuild is not recognized."
> >      fi
> >  }
> >
> > diff --git a/engine/tests/Functional.kernel_build/fuego_test.sh
> > b/engine/tests/Functional.kernel_build/fuego_test.sh
> > index 0b4ac3c..2d36055 100755
> > --- a/engine/tests/Functional.kernel_build/fuego_test.sh
> > +++ b/engine/tests/Functional.kernel_build/fuego_test.sh
> > @@ -18,21 +18,9 @@ function test_pre_check {
> >      if [[ "$FUNCTIONAL_KERNEL_BUILD_TARGET" == *"uImage"* ]]; then
> >          is_on_sdk mkimage
> >      fi
> > -
> > -    # Force the kernel to be rebuilt
> > -    # That's the whole point of the test - don't let Fuego skip the build.
> > -    export Rebuild=True
> >  }
> >
> >  function test_build {
> > -    # make sure we have the latest source code.
> > -    cd "$WORKSPACE/$JOB_BUILD_DIR"
> > -    if [ -d ".git" ]; then
> > -        git pull || echo "WARNING: git pull failed"
> > -    else
> > -        echo "WARNING: no git repository, assuming you used a tarball"
> > -    fi
> > -
> >      # Config
> >      if [ -z "$FUNCTIONAL_KERNEL_BUILD_CONFIG" ]; then
> >          FUNCTIONAL_KERNEL_BUILD_CONFIG="defconfig"
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Fuego mailing list
> > Fuego at lists.linuxfoundation.org
> > https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__lists.linuxfoundation.org_mailman_listinfo_fuego&d=DwICAg&c=fP4tf-
> > -1dS0biCFlB0saz0I0kjO5v7-
> > GLPtvShAo4cc&r=jjTc71ylyJg68rRxrFQuDFMMybIqPCnrHF85A-
> > GzCRg&m=8ywuWmFMqK6gIy82Qwk2GhR4SAydFRy_Hm1rE1T6C0o&s=zVM
> > w_UdedEmHCijaZOExVLun0-MxGge7Ey5eo0gkIeY&e=



More information about the Fuego mailing list