* [LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup @ 2024-09-18 10:03 Wei Gao via ltp 2024-09-18 11:46 ` Martin Doucha 2024-09-25 3:57 ` [LTP] [PATCH v2] ftp01.sh: Add support for test lftp Wei Gao via ltp 0 siblings, 2 replies; 15+ messages in thread From: Wei Gao via ltp @ 2024-09-18 10:03 UTC (permalink / raw) To: 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup 2024-09-18 10:03 [LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup Wei Gao via ltp @ 2024-09-18 11:46 ` Martin Doucha 2024-09-24 10:15 ` Wei Gao via ltp 2024-09-25 3:57 ` [LTP] [PATCH v2] ftp01.sh: Add support for test lftp Wei Gao via ltp 1 sibling, 1 reply; 15+ messages in thread From: Martin Doucha @ 2024-09-18 11:46 UTC (permalink / raw) To: Wei Gao, ltp 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup 2024-09-18 11:46 ` Martin Doucha @ 2024-09-24 10:15 ` Wei Gao via ltp 2024-09-24 12:08 ` Martin Doucha 0 siblings, 1 reply; 15+ messages in thread From: Wei Gao via ltp @ 2024-09-24 10:15 UTC (permalink / raw) To: Martin Doucha; +Cc: 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup 2024-09-24 10:15 ` Wei Gao via ltp @ 2024-09-24 12:08 ` Martin Doucha 0 siblings, 0 replies; 15+ messages in thread From: Martin Doucha @ 2024-09-24 12:08 UTC (permalink / raw) To: Wei Gao; +Cc: ltp 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2] ftp01.sh: Add support for test lftp 2024-09-18 10:03 [LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup Wei Gao via ltp 2024-09-18 11:46 ` Martin Doucha @ 2024-09-25 3:57 ` Wei Gao via ltp 2024-10-15 19:39 ` Petr Vorel 1 sibling, 1 reply; 15+ messages in thread From: Wei Gao via ltp @ 2024-09-25 3:57 UTC (permalink / raw) To: 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp 2024-09-25 3:57 ` [LTP] [PATCH v2] ftp01.sh: Add support for test lftp Wei Gao via ltp @ 2024-10-15 19:39 ` Petr Vorel 2024-10-16 3:13 ` Wei Gao via ltp 2024-10-16 12:47 ` Cyril Hrubis 0 siblings, 2 replies; 15+ messages in thread From: Petr Vorel @ 2024-10-15 19:39 UTC (permalink / raw) To: Wei Gao; +Cc: Martin Doucha, ltp 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp 2024-10-15 19:39 ` Petr Vorel @ 2024-10-16 3:13 ` Wei Gao via ltp 2024-10-16 13:41 ` Petr Vorel 2024-11-04 16:20 ` Petr Vorel 2024-10-16 12:47 ` Cyril Hrubis 1 sibling, 2 replies; 15+ messages in thread From: Wei Gao via ltp @ 2024-10-16 3:13 UTC (permalink / raw) To: Petr Vorel; +Cc: Martin Doucha, 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp 2024-10-16 3:13 ` Wei Gao via ltp @ 2024-10-16 13:41 ` Petr Vorel 2024-11-04 16:20 ` Petr Vorel 1 sibling, 0 replies; 15+ messages in thread From: Petr Vorel @ 2024-10-16 13:41 UTC (permalink / raw) To: Wei Gao; +Cc: Martin Doucha, ltp 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp 2024-10-16 3:13 ` Wei Gao via ltp 2024-10-16 13:41 ` Petr Vorel @ 2024-11-04 16:20 ` Petr Vorel 1 sibling, 0 replies; 15+ messages in thread From: Petr Vorel @ 2024-11-04 16:20 UTC (permalink / raw) To: Wei Gao; +Cc: ltp 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp 2024-10-15 19:39 ` Petr Vorel 2024-10-16 3:13 ` Wei Gao via ltp @ 2024-10-16 12:47 ` Cyril Hrubis 2024-10-16 13:48 ` Petr Vorel 2024-10-16 16:17 ` Martin Doucha 1 sibling, 2 replies; 15+ messages in thread From: Cyril Hrubis @ 2024-10-16 12:47 UTC (permalink / raw) To: Petr Vorel; +Cc: Martin Doucha, ltp 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp 2024-10-16 12:47 ` Cyril Hrubis @ 2024-10-16 13:48 ` Petr Vorel 2024-10-16 15:32 ` Cyril Hrubis 2024-10-16 16:17 ` Martin Doucha 1 sibling, 1 reply; 15+ messages in thread From: Petr Vorel @ 2024-10-16 13:48 UTC (permalink / raw) To: Cyril Hrubis; +Cc: Martin Doucha, ltp > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp 2024-10-16 13:48 ` Petr Vorel @ 2024-10-16 15:32 ` Cyril Hrubis 0 siblings, 0 replies; 15+ messages in thread From: Cyril Hrubis @ 2024-10-16 15:32 UTC (permalink / raw) To: Petr Vorel; +Cc: Martin Doucha, ltp 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp 2024-10-16 12:47 ` Cyril Hrubis 2024-10-16 13:48 ` Petr Vorel @ 2024-10-16 16:17 ` Martin Doucha 2024-10-16 21:15 ` Petr Vorel 1 sibling, 1 reply; 15+ messages in thread From: Martin Doucha @ 2024-10-16 16:17 UTC (permalink / raw) To: Cyril Hrubis, Petr Vorel; +Cc: ltp 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp 2024-10-16 16:17 ` Martin Doucha @ 2024-10-16 21:15 ` Petr Vorel 2024-11-01 12:11 ` Cyril Hrubis 0 siblings, 1 reply; 15+ messages in thread From: Petr Vorel @ 2024-10-16 21:15 UTC (permalink / raw) To: Martin Doucha; +Cc: ltp > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [LTP] [PATCH v2] ftp01.sh: Add support for test lftp 2024-10-16 21:15 ` Petr Vorel @ 2024-11-01 12:11 ` Cyril Hrubis 0 siblings, 0 replies; 15+ messages in thread From: Cyril Hrubis @ 2024-11-01 12:11 UTC (permalink / raw) To: Petr Vorel; +Cc: ltp 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-11-04 16:20 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-18 10:03 [LTP] [PATCH v1] tst_test.sh: Add support for localhost ssh key setup Wei Gao via ltp 2024-09-18 11:46 ` Martin Doucha 2024-09-24 10:15 ` Wei Gao via ltp 2024-09-24 12:08 ` Martin Doucha 2024-09-25 3:57 ` [LTP] [PATCH v2] ftp01.sh: Add support for test lftp Wei Gao via ltp 2024-10-15 19:39 ` Petr Vorel 2024-10-16 3:13 ` Wei Gao via ltp 2024-10-16 13:41 ` Petr Vorel 2024-11-04 16:20 ` Petr Vorel 2024-10-16 12:47 ` Cyril Hrubis 2024-10-16 13:48 ` Petr Vorel 2024-10-16 15:32 ` Cyril Hrubis 2024-10-16 16:17 ` Martin Doucha 2024-10-16 21:15 ` Petr Vorel 2024-11-01 12:11 ` Cyril Hrubis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox