Re: [Fuego] fixes for the linaro and rt tests

[<Tim.Bird@sony.com>]

> -----Original Message-----
> From: tho1.nguyendat@toshiba.co.jp <tho1.nguyendat@toshiba.co.jp>
> 
> Dear Tim,
> 
> I updated "patch [1/4] linaro: localhost does not require ssh"
> Could you please help me check the attachment?

OK.  I took a look at this.

I'll copy the code from the attachment here, so I can comment.

> From 14dc2fc73db6bc99c7ef2830bc8230481e64f427 Mon Sep 17 00:00:00 2001
> From: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> Date: Fri, 20 Aug 2021 10:51:37 +0900
> Subject: [PATCH] linaro: localhost does not require ssh
> 
> When running the test on localhost ssh is not required
> 
> Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
> tests/Functional.linaro/fuego_test.sh | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/Functional.linaro/fuego_test.sh b/tests/Functional.linaro/fuego_test.sh
> index 86d3c43..71c77e7 100755
> --- a/tests/Functional.linaro/fuego_test.sh
> +++ b/tests/Functional.linaro/fuego_test.sh
> @@ -19,7 +19,10 @@ function test_pre_check {
>      # vi ~/.ssh/config
>      #  Host 192.167.1.99 <- replace with your boards ip address ($IPADDR)
>      #    IdentityFile ~/.ssh/bbb_id_rsa
> -    assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
> +    # [Note] when running on localhost SSH_KEY is not required
> +    if [ "$TRANSPORT" != "local" ]; then
I would prefer to restrict the SSH_KEY check to only TRANSPORT=="ssh".

We have several other transports (including some very special custom ones), that
are password-less from Fuego's perspective.  For example 'ttc', 'lc', and 'ebf'.

So the only TRANSPORT that really requires that SSH_KEY be defined, in order
to be password-less is the 'ssh' TRANSPORT.

I converted this to:
if [ "$TRANSPORT" = "ssh" ]; then

Even this is a bit of a 'proxy' check, that might yield a false positive for the real attribute
you're trying to check for.  The real attribute is password-less ssh operations by the user
account which is running Fuego (ftc).

One way to achieve password-less operation is to the use SSH_KEY, the other is through
very insecure settings on the target board's sshd.config.  Yet another way is through
using 'sshpass'.  But Linaro's test_runner does not support an option for doing this.

In any event, testing TRANSPORT is a close enough proxy for the attribute you're trying
to discover, so the next line is fine.  If people have workarounds for password-less access
to a board, they can always just disable this assert_define.
> +        assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
> +    fi
>  }

Since we know that test_runner won't work with other Fuego Transports (serial, ebf, ttc, lc, ssh2serial, etc.)
I decided to add more checking here, and do an abort_job for all anything besides 'local' and 'ssh'.

> 
>  function test_build {
> @@ -57,7 +60,12 @@ function test_run {
>         SKIPFLAG=""
>      fi
>  
> -    test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
> +    # SSH is not required when running on localhost
> +    if [ "$TRANSPORT" != "local" ]; then
IMHO, This should also be: TRANSPORT == "ssh"
> +        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
> +    else
> +        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS $SKIPFLAG -e
> +    fi
>  }
>  
>  # FIXTHIS: the log directory is populated with a copy of the whole repository, clean unnecessary files
> -- 
> 2.20.1

I went ahead and made the changes, and committed the patch.  Please give it a try
and let me know if it works to your satisfaction.
 -- Tim

Here's the eventual patch I committed:

commit f86d3d0235d476d4580b3e829af7795f758172e5
Author: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
Date:   Fri Aug 20 10:51:37 2021 +0900

    linaro: localhost does not require ssh
    
    When running the test on ssh password-less ssh access to the
    board is required.  test-runner only supports local and ssh
    as the Fuego TRANSPORT.
    
    Signed-off-by: Nguyen Dat Tho <tho1.nguyendat@toshiba.co.jp>
    Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>

diff --git a/tests/Functional.linaro/fuego_test.sh b/tests/Functional.linaro/fuego_test.sh
index 86d3c43..eb6e933 100755
--- a/tests/Functional.linaro/fuego_test.sh
+++ b/tests/Functional.linaro/fuego_test.sh
@@ -19,7 +19,13 @@ function test_pre_check {
     # vi ~/.ssh/config
     #  Host 192.167.1.99 <- replace with your boards ip address ($IPADDR)
     #    IdentityFile ~/.ssh/bbb_id_rsa
-    assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
+    # [Note] when running on ssh, password-less ssh access is required
+    # check the TRANSPORT, and make sure SSH_KEY is provided if needed
+    if [ "$TRANSPORT" = "ssh" ]; then
+        assert_define SSH_KEY "Please setup SSH_KEY on your board file (fuego-ro/boards/$NODE_NAME.board)"
+    elif [ "$TRANSPORT" != "local" ]; then
+        abort_job "Linaro test_runner only works with TRANSPORT of 'ssh' or 'local', not '$TRANSPORT'"
+    fi
 }
 
 function test_build {
@@ -57,7 +63,12 @@ function test_run {
         SKIPFLAG=""
     fi
 
-    test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
+    # use -g option to test-runner, if using 'ssh' transport
+    if [ "$TRANSPORT" = "ssh" ]; then
+        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS -g $LOGIN@$IPADDR $SKIPFLAG -e
+    else
+        test-runner -o ${LOGDIR} $test_or_plan_flag ${REPO_PATH}/$yaml_file $PARAMS $SKIPFLAG -e
+    fi
 }
 
 # FIXTHIS: the log directory is populated with a copy of the whole repository, clean unnecessary files

> -----Original Message-----
> From: sangorrin daniel(サンゴリン ダニエル □ＳＷＣ◯ＡＣＴ) <daniel.sangorrin@toshiba.co.jp>
> Sent: Thursday, August 26, 2021 9:46 AM
> To: Tim.Bird@sony.com
> Cc: fuego@lists.linuxfoundation.org; nguyen dat tho(ＴＳＤＶ Ｅｎｇ　１) <tho1.nguyendat@toshiba.co.jp>
> Subject: RE: fixes for the linaro and rt tests
> 
> Hi Tim
> 
> Thanks for reviewing the patches
> 
> > -----Original Message-----
> > From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> > > [PATCH 1/4] linaro: localhost does not require ssh
> > >
> > > This ones is self explanatory (no SSH if you use the local board)
> > This looks OK.  I'm wondering if there's another way to detect a local
> > operation.  Can we check the TRANSPORT (e.g. if [ $TRANSPORT = "local" ] ...)?
> > Checking the network address seems a bit iffy.
> 
> You are right, we will try with TRANSPORT or another variable and re-send the patch.
> 
> Thanks,
> Daniel