[LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup

[LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup

[Wei Gao via ltp]

When RHOST=localhost, ssh@localhost will encounter error since
no correct setup for authorized_keys and known_hosts etc.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/lib/tst_test.sh               | 32 +++++++++++++++++++++++++
 testcases/network/tcp_cmds/ftp/ftp01.sh | 12 ++++++++--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index c19c30b76..6df16bd17 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -784,6 +784,34 @@ tst_run()
 	_tst_do_exit
 }
 
+_tst_setup_localhost_ssh()
+{
+	if [ -z "$TST_SSHD_CONFIG" ]; then
+		TST_SSHD_CONFIG="/etc/ssh/sshd_config"
+	fi
+
+	if [ -z "$TST_SSH_DIR" ]; then
+		TST_SSH_DIR="/root/.ssh/"
+	fi
+
+	if [ ! -e "$TST_SSHD_CONFIG" ]; then
+		echo 'PermitRootLogin yes' >$TST_SSHD_CONFIG
+	elif [ ! `grep "^PermitRootLogin *yes" $TST_SSHD_CONFIG | wc -l` -gt 0 ]; then
+		echo 'PermitRootLogin yes' >>$TST_SSHD_CONFIG
+	fi
+
+	if [ ! -e "$TST_SSH_DIR/id_rsa" ]; then
+		ssh-keygen -q -N "" -t rsa -b 4096 -f $TST_SSH_DIR/id_rsa
+	fi
+
+	if [ -e "$TST_SSH_DIR/id_rsa.pub" ]; then
+		cat $TST_SSH_DIR/id_rsa.pub >> $TST_SSH_DIR/authorized_keys
+		ssh-keyscan -H localhost >> $TST_SSH_DIR/known_hosts
+	fi
+
+	systemctl restart sshd
+}
+
 _tst_run_iterations()
 {
 	local _tst_i=$TST_ITERATIONS
@@ -910,6 +938,10 @@ if [ -z "$TST_NO_DEFAULT_RUN" ]; then
 	tst_res TINFO "Running: $(basename $0) $TST_ARGS"
 	tst_res TINFO "Tested kernel: $(uname -a)"
 
+	if [ "$TST_NEEDS_LOCALHOST_SSH" = 1 ]; then
+		_tst_setup_localhost_ssh
+	fi
+
 	OPTIND=1
 
 	while getopts ":hi:$TST_OPTS" _tst_name $TST_ARGS; do
diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
index 53d1eec53..8ec7f4fca 100755
--- a/testcases/network/tcp_cmds/ftp/ftp01.sh
+++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
@@ -4,13 +4,21 @@
 # Copyright (c) 2003 Manoj Iyer <manjo@mail.utexas.edu>
 # Copyright (c) 2001 Robbie Williamson <robbiew@us.ibm.com>
 
+TST_SETUP=setup
 TST_TESTFUNC=do_test
 TST_CNT=4
 TST_NEEDS_CMDS='awk ftp'
 TST_NEEDS_TMPDIR=1
+TST_NEEDS_LOCALHOST_SSH=1
 
 RUSER="${RUSER:-root}"
 RHOST="${RHOST:-localhost}"
+FTP_CLIENT_CMD="ftp -nv"
+
+setup()
+{
+    grep -q 'sles' /etc/os-release && FTP_CLIENT_CMD='lftp'
+}
 
 do_test()
 {
@@ -41,7 +49,7 @@ test_get()
             echo cd $TST_NET_DATAROOT
             echo get $file
             echo quit
-        } | ftp -nv $RHOST
+        } | $FTP_CLIENT_CMD $RHOST
 
         sum1="$(ls -l $file | awk '{print $5}')"
         sum2="$(ls -l $TST_NET_DATAROOT/$file | awk '{print $5}')"
@@ -62,7 +70,7 @@ test_put()
             echo cd $TST_TMPDIR
             echo put $file
             echo quit
-        } | ftp -nv $RHOST
+        } | $FTP_CLIENT_CMD $RHOST
 
         sum1="$(tst_rhost_run -c "sum $TST_TMPDIR/$file" -s | awk '{print $1}')"
         sum2="$(sum $TST_NET_DATAROOT/$file | awk '{print $1}')"
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup

[Martin Doucha]

Hi,
this kind of configuration should be done by the test runner. LTP should 
not modify global system configuration unless doing so is a key part of 
the test scenario itself.

The test runner can also create an alias for the lftp command so that 
the test can call it using the old name.

On 18. 09. 24 12:03, Wei Gao via ltp wrote:
> When RHOST=localhost, ssh@localhost will encounter error since
> no correct setup for authorized_keys and known_hosts etc.
> 
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>   testcases/lib/tst_test.sh               | 32 +++++++++++++++++++++++++
>   testcases/network/tcp_cmds/ftp/ftp01.sh | 12 ++++++++--
>   2 files changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index c19c30b76..6df16bd17 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -784,6 +784,34 @@ tst_run()
>   	_tst_do_exit
>   }
>   
> +_tst_setup_localhost_ssh()
> +{
> +	if [ -z "$TST_SSHD_CONFIG" ]; then
> +		TST_SSHD_CONFIG="/etc/ssh/sshd_config"
> +	fi
> +
> +	if [ -z "$TST_SSH_DIR" ]; then
> +		TST_SSH_DIR="/root/.ssh/"
> +	fi
> +
> +	if [ ! -e "$TST_SSHD_CONFIG" ]; then
> +		echo 'PermitRootLogin yes' >$TST_SSHD_CONFIG
> +	elif [ ! `grep "^PermitRootLogin *yes" $TST_SSHD_CONFIG | wc -l` -gt 0 ]; then
> +		echo 'PermitRootLogin yes' >>$TST_SSHD_CONFIG
> +	fi
> +
> +	if [ ! -e "$TST_SSH_DIR/id_rsa" ]; then
> +		ssh-keygen -q -N "" -t rsa -b 4096 -f $TST_SSH_DIR/id_rsa
> +	fi
> +
> +	if [ -e "$TST_SSH_DIR/id_rsa.pub" ]; then
> +		cat $TST_SSH_DIR/id_rsa.pub >> $TST_SSH_DIR/authorized_keys
> +		ssh-keyscan -H localhost >> $TST_SSH_DIR/known_hosts
> +	fi
> +
> +	systemctl restart sshd
> +}
> +
>   _tst_run_iterations()
>   {
>   	local _tst_i=$TST_ITERATIONS
> @@ -910,6 +938,10 @@ if [ -z "$TST_NO_DEFAULT_RUN" ]; then
>   	tst_res TINFO "Running: $(basename $0) $TST_ARGS"
>   	tst_res TINFO "Tested kernel: $(uname -a)"
>   
> +	if [ "$TST_NEEDS_LOCALHOST_SSH" = 1 ]; then
> +		_tst_setup_localhost_ssh
> +	fi
> +
>   	OPTIND=1
>   
>   	while getopts ":hi:$TST_OPTS" _tst_name $TST_ARGS; do
> diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
> index 53d1eec53..8ec7f4fca 100755
> --- a/testcases/network/tcp_cmds/ftp/ftp01.sh
> +++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
> @@ -4,13 +4,21 @@
>   # Copyright (c) 2003 Manoj Iyer <manjo@mail.utexas.edu>
>   # Copyright (c) 2001 Robbie Williamson <robbiew@us.ibm.com>
>   
> +TST_SETUP=setup
>   TST_TESTFUNC=do_test
>   TST_CNT=4
>   TST_NEEDS_CMDS='awk ftp'
>   TST_NEEDS_TMPDIR=1
> +TST_NEEDS_LOCALHOST_SSH=1
>   
>   RUSER="${RUSER:-root}"
>   RHOST="${RHOST:-localhost}"
> +FTP_CLIENT_CMD="ftp -nv"
> +
> +setup()
> +{
> +    grep -q 'sles' /etc/os-release && FTP_CLIENT_CMD='lftp'
> +}
>   
>   do_test()
>   {
> @@ -41,7 +49,7 @@ test_get()
>               echo cd $TST_NET_DATAROOT
>               echo get $file
>               echo quit
> -        } | ftp -nv $RHOST
> +        } | $FTP_CLIENT_CMD $RHOST
>   
>           sum1="$(ls -l $file | awk '{print $5}')"
>           sum2="$(ls -l $TST_NET_DATAROOT/$file | awk '{print $5}')"
> @@ -62,7 +70,7 @@ test_put()
>               echo cd $TST_TMPDIR
>               echo put $file
>               echo quit
> -        } | ftp -nv $RHOST
> +        } | $FTP_CLIENT_CMD $RHOST
>   
>           sum1="$(tst_rhost_run -c "sum $TST_TMPDIR/$file" -s | awk '{print $1}')"
>           sum2="$(sum $TST_NET_DATAROOT/$file | awk '{print $1}')"

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup

[Wei Gao via ltp]

On Wed, Sep 18, 2024 at 01:46:30PM +0200, Martin Doucha wrote:
> Hi,
> this kind of configuration should be done by the test runner. LTP should not
> modify global system configuration unless doing so is a key part of the test
> scenario itself.
Will remove ssh configuration part to test runner.
> 
> The test runner can also create an alias for the lftp command so that the
> test can call it using the old name.
Create an alias for lftp not enough since current code use parameter "-nv" which
not supported by lftp so following error will popup:

lftp: invalid option -- 'n'
Try `lftp --help' for more information

> 
> On 18. 09. 24 12:03, Wei Gao via ltp wrote:
> > When RHOST=localhost, ssh@localhost will encounter error since
> > no correct setup for authorized_keys and known_hosts etc.
> > 
> > Signed-off-by: Wei Gao <wegao@suse.com>
> > ---
> >   testcases/lib/tst_test.sh               | 32 +++++++++++++++++++++++++
> >   testcases/network/tcp_cmds/ftp/ftp01.sh | 12 ++++++++--
> >   2 files changed, 42 insertions(+), 2 deletions(-)
> > 
> > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > index c19c30b76..6df16bd17 100644
> > --- a/testcases/lib/tst_test.sh
> > +++ b/testcases/lib/tst_test.sh
> > @@ -784,6 +784,34 @@ tst_run()
> >   	_tst_do_exit
> >   }
> > +_tst_setup_localhost_ssh()
> > +{
> > +	if [ -z "$TST_SSHD_CONFIG" ]; then
> > +		TST_SSHD_CONFIG="/etc/ssh/sshd_config"
> > +	fi
> > +
> > +	if [ -z "$TST_SSH_DIR" ]; then
> > +		TST_SSH_DIR="/root/.ssh/"
> > +	fi
> > +
> > +	if [ ! -e "$TST_SSHD_CONFIG" ]; then
> > +		echo 'PermitRootLogin yes' >$TST_SSHD_CONFIG
> > +	elif [ ! `grep "^PermitRootLogin *yes" $TST_SSHD_CONFIG | wc -l` -gt 0 ]; then
> > +		echo 'PermitRootLogin yes' >>$TST_SSHD_CONFIG
> > +	fi
> > +
> > +	if [ ! -e "$TST_SSH_DIR/id_rsa" ]; then
> > +		ssh-keygen -q -N "" -t rsa -b 4096 -f $TST_SSH_DIR/id_rsa
> > +	fi
> > +
> > +	if [ -e "$TST_SSH_DIR/id_rsa.pub" ]; then
> > +		cat $TST_SSH_DIR/id_rsa.pub >> $TST_SSH_DIR/authorized_keys
> > +		ssh-keyscan -H localhost >> $TST_SSH_DIR/known_hosts
> > +	fi
> > +
> > +	systemctl restart sshd
> > +}
> > +
> >   _tst_run_iterations()
> >   {
> >   	local _tst_i=$TST_ITERATIONS
> > @@ -910,6 +938,10 @@ if [ -z "$TST_NO_DEFAULT_RUN" ]; then
> >   	tst_res TINFO "Running: $(basename $0) $TST_ARGS"
> >   	tst_res TINFO "Tested kernel: $(uname -a)"
> > +	if [ "$TST_NEEDS_LOCALHOST_SSH" = 1 ]; then
> > +		_tst_setup_localhost_ssh
> > +	fi
> > +
> >   	OPTIND=1
> >   	while getopts ":hi:$TST_OPTS" _tst_name $TST_ARGS; do
> > diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
> > index 53d1eec53..8ec7f4fca 100755
> > --- a/testcases/network/tcp_cmds/ftp/ftp01.sh
> > +++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
> > @@ -4,13 +4,21 @@
> >   # Copyright (c) 2003 Manoj Iyer <manjo@mail.utexas.edu>
> >   # Copyright (c) 2001 Robbie Williamson <robbiew@us.ibm.com>
> > +TST_SETUP=setup
> >   TST_TESTFUNC=do_test
> >   TST_CNT=4
> >   TST_NEEDS_CMDS='awk ftp'
> >   TST_NEEDS_TMPDIR=1
> > +TST_NEEDS_LOCALHOST_SSH=1
> >   RUSER="${RUSER:-root}"
> >   RHOST="${RHOST:-localhost}"
> > +FTP_CLIENT_CMD="ftp -nv"
> > +
> > +setup()
> > +{
> > +    grep -q 'sles' /etc/os-release && FTP_CLIENT_CMD='lftp'
> > +}
> >   do_test()
> >   {
> > @@ -41,7 +49,7 @@ test_get()
> >               echo cd $TST_NET_DATAROOT
> >               echo get $file
> >               echo quit
> > -        } | ftp -nv $RHOST
> > +        } | $FTP_CLIENT_CMD $RHOST
> >           sum1="$(ls -l $file | awk '{print $5}')"
> >           sum2="$(ls -l $TST_NET_DATAROOT/$file | awk '{print $5}')"
> > @@ -62,7 +70,7 @@ test_put()
> >               echo cd $TST_TMPDIR
> >               echo put $file
> >               echo quit
> > -        } | ftp -nv $RHOST
> > +        } | $FTP_CLIENT_CMD $RHOST
> >           sum1="$(tst_rhost_run -c "sum $TST_TMPDIR/$file" -s | awk '{print $1}')"
> >           sum2="$(sum $TST_NET_DATAROOT/$file | awk '{print $1}')"
> 
> -- 
> Martin Doucha   mdoucha@suse.cz
> SW Quality Engineer
> SUSE LINUX, s.r.o.
> CORSO IIa
> Krizikova 148/34
> 186 00 Prague 8
> Czech Republic
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup

[Martin Doucha]

On 24. 09. 24 12:15, Wei Gao wrote:
> On Wed, Sep 18, 2024 at 01:46:30PM +0200, Martin Doucha wrote:
>> The test runner can also create an alias for the lftp command so that the
>> test can call it using the old name.
> Create an alias for lftp not enough since current code use parameter "-nv" which
> not supported by lftp so following error will popup:
> 
> lftp: invalid option -- 'n'
> Try `lftp --help' for more information

In that case we can either leave the test as is or detect the available 
ftp client in setup():

if tst_cmd_available ftp; then
	FTP_CMD="ftp -nv"
elif tst_cmd_available lftp; then
	FTP_CMD="lftp -v --norc"
else
	tst_brkm TCONF "No FTP client found"
fi

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/network/tcp_cmds/ftp/ftp01.sh | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
index 53d1eec53..12d32a9a9 100755
--- a/testcases/network/tcp_cmds/ftp/ftp01.sh
+++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
@@ -4,6 +4,7 @@
 # Copyright (c) 2003 Manoj Iyer <manjo@mail.utexas.edu>
 # Copyright (c) 2001 Robbie Williamson <robbiew@us.ibm.com>
 
+TST_SETUP=setup
 TST_TESTFUNC=do_test
 TST_CNT=4
 TST_NEEDS_CMDS='awk ftp'
@@ -11,6 +12,16 @@ TST_NEEDS_TMPDIR=1
 
 RUSER="${RUSER:-root}"
 RHOST="${RHOST:-localhost}"
+FTP_CMD="ftp -nv"
+
+setup()
+{
+    if tst_cmd_available lftp; then
+        FTP_CMD="lftp --norc"
+    else
+        tst_brkm TCONF "No FTP client found"
+    fi
+}
 
 do_test()
 {
@@ -41,7 +52,7 @@ test_get()
             echo cd $TST_NET_DATAROOT
             echo get $file
             echo quit
-        } | ftp -nv $RHOST
+        } | $FTP_CMD $RHOST
 
         sum1="$(ls -l $file | awk '{print $5}')"
         sum2="$(ls -l $TST_NET_DATAROOT/$file | awk '{print $5}')"
@@ -62,7 +73,7 @@ test_put()
             echo cd $TST_TMPDIR
             echo put $file
             echo quit
-        } | ftp -nv $RHOST
+        } | $FTP_CMD $RHOST
 
         sum1="$(tst_rhost_run -c "sum $TST_TMPDIR/$file" -s | awk '{print $1}')"
         sum2="$(sum $TST_NET_DATAROOT/$file | awk '{print $1}')"
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Petr Vorel]

Hi Wei,

I suppose the v1 is
https://patchwork.ozlabs.org/project/ltp/patch/20240918100344.21316-1-wegao@suse.com/

To be honest, I would rather to remove this FTP test because FTP protocol is
legacy. I know it is supposed to be a smoke test, but maybe using modern tools
would be better than keeping test working among various old FTP implementations.
(Also nontrivial setup is required just for few FTP tests:
https://github.com/linux-test-project/ltp/tree/master/testcases/network )
But probably Cyril would be against. @Cyril @Martin WDYT?

@Wei I suppose the reason for adding lftp (you did not explain it in the commit
message) is that is the only supported client in SLE Micro? Or something else?

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  testcases/network/tcp_cmds/ftp/ftp01.sh | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)

> diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
> index 53d1eec53..12d32a9a9 100755
> --- a/testcases/network/tcp_cmds/ftp/ftp01.sh
> +++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
> @@ -4,6 +4,7 @@
>  # Copyright (c) 2003 Manoj Iyer <manjo@mail.utexas.edu>
>  # Copyright (c) 2001 Robbie Williamson <robbiew@us.ibm.com>

> +TST_SETUP=setup
>  TST_TESTFUNC=do_test
>  TST_CNT=4
>  TST_NEEDS_CMDS='awk ftp'
> @@ -11,6 +12,16 @@ TST_NEEDS_TMPDIR=1

>  RUSER="${RUSER:-root}"
>  RHOST="${RHOST:-localhost}"
> +FTP_CMD="ftp -nv"
> +
> +setup()
> +{
> +    if tst_cmd_available lftp; then
> +        FTP_CMD="lftp --norc"
> +    else
> +        tst_brkm TCONF "No FTP client found"
Test was converted to the new API, it must be tst_brk.
Also, this code basically means that testing is done only for lftp,
otherwise TCONF.
+ You keep requiring ftp in TST_NEEDS_CMDS, but at least on Tumbleweed and
current Debian testing lftp does not provide symlink to ftp, only tnftp does
this. Therefore you require both lftp and tnftp for testing.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Wei Gao via ltp]

On Tue, Oct 15, 2024 at 09:39:58PM +0200, Petr Vorel wrote:
> Hi Wei,
> 
> I suppose the v1 is
> https://patchwork.ozlabs.org/project/ltp/patch/20240918100344.21316-1-wegao@suse.com/
Correct
> 
> To be honest, I would rather to remove this FTP test because FTP protocol is
> legacy. I know it is supposed to be a smoke test, but maybe using modern tools
> would be better than keeping test working among various old FTP implementations.
> (Also nontrivial setup is required just for few FTP tests:
> https://github.com/linux-test-project/ltp/tree/master/testcases/network )
> But probably Cyril would be against. @Cyril @Martin WDYT?
> 
> @Wei I suppose the reason for adding lftp (you did not explain it in the commit
> message) is that is the only supported client in SLE Micro? Or something else?
Adding lftp is used for fix SLE test such as SLE-15SP6, on SLE Micro will result TCONF
since no any ftp command support currently.

BTW: there is another PR on openqa side to setup local ssh network env.
https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/20278 

All detail discussion can be also found in:
https://progress.opensuse.org/issues/165848

> 
> > Signed-off-by: Wei Gao <wegao@suse.com>
> > ---
> >  testcases/network/tcp_cmds/ftp/ftp01.sh | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> > diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
> > index 53d1eec53..12d32a9a9 100755
> > --- a/testcases/network/tcp_cmds/ftp/ftp01.sh
> > +++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
> > @@ -4,6 +4,7 @@
> >  # Copyright (c) 2003 Manoj Iyer <manjo@mail.utexas.edu>
> >  # Copyright (c) 2001 Robbie Williamson <robbiew@us.ibm.com>
> 
> > +TST_SETUP=setup
> >  TST_TESTFUNC=do_test
> >  TST_CNT=4
> >  TST_NEEDS_CMDS='awk ftp'
> > @@ -11,6 +12,16 @@ TST_NEEDS_TMPDIR=1
> 
> >  RUSER="${RUSER:-root}"
> >  RHOST="${RHOST:-localhost}"
> > +FTP_CMD="ftp -nv"
> > +
> > +setup()
> > +{
> > +    if tst_cmd_available lftp; then
> > +        FTP_CMD="lftp --norc"
> > +    else
> > +        tst_brkm TCONF "No FTP client found"
> Test was converted to the new API, it must be tst_brk.
> Also, this code basically means that testing is done only for lftp,
> otherwise TCONF.
Thanks for point out this. it should replace to following code as Martin suggested:

if tst_cmd_available ftp; then
	FTP_CMD="ftp -nv"
elif tst_cmd_available lftp; then
	FTP_CMD="lftp --norc"
else
	tst_brk TCONF "No FTP client found"
fi

> + You keep requiring ftp in TST_NEEDS_CMDS, but at least on Tumbleweed and
> current Debian testing lftp does not provide symlink to ftp, only tnftp does
> this. Therefore you require both lftp and tnftp for testing.
> 
I suppose ONLY requiring ftp in TST_NEEDS_CMDS is enough, lftp is specific case for sle check.
For Tumbleweed and other OS will directly use ftp(TW only contain ftp by default).

> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Cyril Hrubis]

Hi!
> To be honest, I would rather to remove this FTP test because FTP protocol is
> legacy. I know it is supposed to be a smoke test, but maybe using modern tools
> would be better than keeping test working among various old FTP implementations.
> (Also nontrivial setup is required just for few FTP tests:
> https://github.com/linux-test-project/ltp/tree/master/testcases/network )
> But probably Cyril would be against. @Cyril @Martin WDYT?

So where is the ftp server setup code? Or do we expect ftp server to be
installed and configured prior to the test execution?

The actuall test does not look that complex to me.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Petr Vorel]

Hi Wei, all,

first, please wait little bit before sending next version.
We might decide to remove these tests.
Below are notes in case we keep them.

> On Tue, Oct 15, 2024 at 09:39:58PM +0200, Petr Vorel wrote:
> > Hi Wei,

> > I suppose the v1 is
> > https://patchwork.ozlabs.org/project/ltp/patch/20240918100344.21316-1-wegao@suse.com/
> Correct

> > To be honest, I would rather to remove this FTP test because FTP protocol is
> > legacy. I know it is supposed to be a smoke test, but maybe using modern tools
> > would be better than keeping test working among various old FTP implementations.
> > (Also nontrivial setup is required just for few FTP tests:
> > https://github.com/linux-test-project/ltp/tree/master/testcases/network )
> > But probably Cyril would be against. @Cyril @Martin WDYT?

> > @Wei I suppose the reason for adding lftp (you did not explain it in the commit
> > message) is that is the only supported client in SLE Micro? Or something else?
> Adding lftp is used for fix SLE test such as SLE-15SP6, on SLE Micro will result TCONF
> since no any ftp command support currently.

> BTW: there is another PR on openqa side to setup local ssh network env.
> https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/20278 

> All detail discussion can be also found in:
> https://progress.opensuse.org/issues/165848


> > > Signed-off-by: Wei Gao <wegao@suse.com>
> > > ---
> > >  testcases/network/tcp_cmds/ftp/ftp01.sh | 15 +++++++++++++--
> > >  1 file changed, 13 insertions(+), 2 deletions(-)

> > > diff --git a/testcases/network/tcp_cmds/ftp/ftp01.sh b/testcases/network/tcp_cmds/ftp/ftp01.sh
> > > index 53d1eec53..12d32a9a9 100755
> > > --- a/testcases/network/tcp_cmds/ftp/ftp01.sh
> > > +++ b/testcases/network/tcp_cmds/ftp/ftp01.sh
> > > @@ -4,6 +4,7 @@
> > >  # Copyright (c) 2003 Manoj Iyer <manjo@mail.utexas.edu>
> > >  # Copyright (c) 2001 Robbie Williamson <robbiew@us.ibm.com>

> > > +TST_SETUP=setup
> > >  TST_TESTFUNC=do_test
> > >  TST_CNT=4
> > >  TST_NEEDS_CMDS='awk ftp'
> > > @@ -11,6 +12,16 @@ TST_NEEDS_TMPDIR=1

> > >  RUSER="${RUSER:-root}"
> > >  RHOST="${RHOST:-localhost}"
> > > +FTP_CMD="ftp -nv"
> > > +
> > > +setup()
> > > +{
> > > +    if tst_cmd_available lftp; then
> > > +        FTP_CMD="lftp --norc"
> > > +    else
> > > +        tst_brkm TCONF "No FTP client found"
> > Test was converted to the new API, it must be tst_brk.
> > Also, this code basically means that testing is done only for lftp,
> > otherwise TCONF.
> Thanks for point out this. it should replace to following code as Martin suggested:

> if tst_cmd_available ftp; then
> 	FTP_CMD="ftp -nv"
> elif tst_cmd_available lftp; then
> 	FTP_CMD="lftp --norc"
> else
> 	tst_brk TCONF "No FTP client found"
> fi

Yes, this would work.

> > + You keep requiring ftp in TST_NEEDS_CMDS, but at least on Tumbleweed and
> > current Debian testing lftp does not provide symlink to ftp, only tnftp does
> > this. Therefore you require both lftp and tnftp for testing.

> I suppose ONLY requiring ftp in TST_NEEDS_CMDS is enough, lftp is specific case for sle check.
> For Tumbleweed and other OS will directly use ftp(TW only contain ftp by default).

IMHO no. TST_NEEDS_CMDS should not contain ftp. On Tumbleweed with lftp
installed there is no ftp symlink:

$ lftp --version
LFTP | Version 4.9.2 | Copyright (c) 1996-2020 Alexander V. Lukyanov
...

$ ftp

The program 'ftp' can be found in following packages:
  * ftp [ path: /usr/bin/ftp, repository: OSS ]
  * tnftp [ path: /usr/bin/ftp, repository: OSS ]

Try installing with:
    sudo zypper install <selected_package>

Kind regards,
Petr

> > Kind regards,
> > Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Petr Vorel]

> Hi!
> > To be honest, I would rather to remove this FTP test because FTP protocol is
> > legacy. I know it is supposed to be a smoke test, but maybe using modern tools
> > would be better than keeping test working among various old FTP implementations.
> > (Also nontrivial setup is required just for few FTP tests:
> > https://github.com/linux-test-project/ltp/tree/master/testcases/network )
> > But probably Cyril would be against. @Cyril @Martin WDYT?

First, there are 7 FTP related tests:

testcases/network/tcp_cmds/ftp/ftp01.sh
testcases/network/stress/ftp/ftp-upload-stress02-rmt.sh
testcases/network/stress/ftp/ftp-upload-stress01-rmt.sh
testcases/network/stress/ftp/ftp-upload-stress.sh
testcases/network/stress/ftp/ftp-download-stress02-rmt.sh
testcases/network/stress/ftp/ftp-download-stress01-rmt.sh
testcases/network/stress/ftp/ftp-download-stress.sh

rmt files are using in ftp-upload-stress.sh and ftp-download-stress.sh. They are
part of runtest/net_stress.appl. Others tools/protocols for testing are SSH and DNS,
I wonder if we should get rid of whole tests in runtest/net_stress.appl.

Only ftp01.sh is converted to the new API, the rest is using the legacy API.
While it wouldn't be difficult to convert them, I wonder if it's really useful
for modern kernel testing. Is it really worth to have these tests as a smoke
tests?

> So where is the ftp server setup code? Or do we expect ftp server to be
> installed and configured prior to the test execution?

ftp-download-stress.sh and ftp-upload-stress.sh have some vsftpd server setup.

Here is a description of some of the tests:

	ftp-download-stress01
		Verify the ftp server or the kernel is not down after a ftp client
		requests large data via IPv4/IPv6

	ftp-download-stress02
		Verify the ftp server or the kernel is not down after many ftp
		clients request data over IPv4/IPv6 asynchronously for a long time

	ftp-upload-stress01
		Verify the ftp server or the kernel is not down after a ftp client
		uploads a large data via IPv4/IPv6

	ftp-upload-stress02
		Verify the ftp server or the kernel is not down after many ftp clients
		uploads data over IPv4/IPv6 asynchronously for a long time

I doubt that FTP server would crash the kernel, IMHO it's more about testing the
FTP server itself.

Kind regards,
Petr

> The actuall test does not look that complex to me.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Cyril Hrubis]

Hi!
> I doubt that FTP server would crash the kernel, IMHO it's more about testing the
> FTP server itself.

I doubt that these tests would crash kernel, but they are sending a
large amount of data over TCP and are checking that the data arrives the
same at the other end. Which is something we should probably do.

It would be probably cleaner to write a C server that would fork a child
for each connection and just echo data it reads back and a client that
connects to the server, writes some patternts and makes sure that it's
read back exactly the same.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Martin Doucha]

On 16. 10. 24 14:47, Cyril Hrubis wrote:
> Hi!
>> To be honest, I would rather to remove this FTP test because FTP protocol is
>> legacy. I know it is supposed to be a smoke test, but maybe using modern tools
>> would be better than keeping test working among various old FTP implementations.
>> (Also nontrivial setup is required just for few FTP tests:
>> https://github.com/linux-test-project/ltp/tree/master/testcases/network )
>> But probably Cyril would be against. @Cyril @Martin WDYT?
> 
> So where is the ftp server setup code? Or do we expect ftp server to be
> installed and configured prior to the test execution?
> 
> The actuall test does not look that complex to me.

Hi,
I've replied to patchset v1[1] that the server setup is better left to 
the user, like we do with NFS. In our case, it should be done in OpenQA 
during VM environment setup.

I agree that the test could be replaced by a socket test, either using 
improved netstress tool or some new syscall tests for send()/recv()/...

[1] 
https://patchwork.ozlabs.org/project/ltp/patch/20240918100344.21316-1-wegao@suse.com/#3381493

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Petr Vorel]

> On 16. 10. 24 14:47, Cyril Hrubis wrote:
> > Hi!
> > > To be honest, I would rather to remove this FTP test because FTP protocol is
> > > legacy. I know it is supposed to be a smoke test, but maybe using modern tools
> > > would be better than keeping test working among various old FTP implementations.
> > > (Also nontrivial setup is required just for few FTP tests:
> > > https://github.com/linux-test-project/ltp/tree/master/testcases/network )
> > > But probably Cyril would be against. @Cyril @Martin WDYT?

> > So where is the ftp server setup code? Or do we expect ftp server to be
> > installed and configured prior to the test execution?

> > The actuall test does not look that complex to me.

> Hi,
> I've replied to patchset v1[1] that the server setup is better left to the
> user, like we do with NFS. In our case, it should be done in OpenQA during
> VM environment setup.

NFS case is kind of different from FTP. 1) There is the only nfsd implementation,
in kernel. But there are more FTP servers (userspace).

Also it's not just about starting the service (I agree with leaving services
enabled in tooling outside LTP, e.g. *not* using testcases/lib/daemonlib.sh),
but there is also service configuration which is required to be done in the test
(part of the testing). NFS tests just run exportfs. All FTP scripts but ftp01.sh
use vsftpd configuration. ftp01.sh could reuse this code (if it's working).

Besides this FTP server it also involves some additional setup [2] (adjust
/etc/ftpusers or /etc/vsftpd.ftpusers), but sure, this could be left for the
test runner.

> I agree that the test could be replaced by a socket test, either using
> improved netstress tool or some new syscall tests for send()/recv()/...

I was also thinking about using netstress.c (which is already used for
network many tests) for FTP download and upload tests.

There are also other tests in runtest/net_stress.appl:

* dns-stress.sh bind9 resolving test - IMHO not relevant for LTP (bind has nice
  CI testing done by ISC).
* ssh-stress.sh sshd stress test - IMHO not relevant for LTP even as a smoke
  test. Also touching ssh configuration can break some LTP users as LTP is often
  used as a connection to SUT.
* http-stress.sh http downloading stress test - IMHO redundant to FTP download
  (if we replace FTP tests with netstress / other C code, we can delete this as well).

Back to ftp01.sh (file modified in this patch). It tests downloading and
uploading with FTP protocol. It looks to me more as FTP client/server test than
anything else (it's not a stress test). IMHO not relevant for LTP.

Kind regards,
Petr

> [1] https://patchwork.ozlabs.org/project/ltp/patch/20240918100344.21316-1-wegao@suse.com/#3381493

[2] https://github.com/linux-test-project/ltp/tree/master/testcases/network#ftp-and-telnet-setup

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Cyril Hrubis]

Hi!
> Back to ftp01.sh (file modified in this patch). It tests downloading and
> uploading with FTP protocol. It looks to me more as FTP client/server test than
> anything else (it's not a stress test). IMHO not relevant for LTP.

Fair enough, ftp01.sh really looks like a ftp server unit test, that is
something that is out of scope for LTP. So I do agree with ftp01.sh
removal.

And the ftp stress tests these should be replaced with a generic TCP
stress test later on.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp

[Petr Vorel]

Hi Wei, all,

we agreed that ftp01.sh can be removed and this happen [1], thus setting this
in patchwork "Not applicable". Also there is an issue for other FTP tests.

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/commit/b8531debea4f08c7c97927eb16c0152549d2ab0a
[2] https://github.com/linux-test-project/ltp/issues/1207

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp