LU-12597 tests: return comma-separated osts_nodes()

Start to return comma-separated OSTs list from osts_nodes(),
to avoid the redundant calling of comma_list() for each user.

Add the tgts_nodes() helper to print combined MDT and OST nodes to
avoid duplicate nodes vs. separate mdts_nodes() + osts_nodes().

Fix the few places that do not call comma_list() on osts_nodes()
output afterward, and code style in sanity-sec test_31.

Fix a minor bug in the error handling of sanity check_stats() where
it was calling "osts-nodes" instead of "osts_nodes".

Later patches will clean up all of the callers of osts_nodes,
but there are too many places it is used to do in a single patch.

Fixes: f0324c5c2f ("LU-14992 tests: sanity/replay-vbr mkdir on MDT0")
Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
Change-Id: I7ae3278321e7552dc2afd5fbb9f48033af3ebbe5
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/56635
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
Reviewed-by: Timothy Day <timday@amazon.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/contrib/scripts/spelling.txt b/contrib/scripts/spelling.txt
index 55a8314..886a721 100644 (file)
--- a/contrib/scripts/spelling.txt
+++ b/contrib/scripts/spelling.txt
@@ -127,6 +127,7 @@ OS_STATE_ENOINO||OS_STATFS_ENOINO
 OS_STATE_SUM||OS_STATFS_SUM
 OS_STATE_NONROT||OS_STATFS_NONROT
 OS_STATFS_NOPRECREATE||OS_STATFS_NOCREATE
+comma_list.*osts_nodes||osts_nodes
 page_cache_get||get_page
 PAGE_CACHE_MASK||PAGE_MASK
 page_cache_release||put_page

diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh
index 05f19d1..81661a2 100755 (executable)
--- a/lustre/tests/sanity-sec.sh
+++ b/lustre/tests/sanity-sec.sh
@@ -2592,6 +2592,7 @@ test_31() {
        local addr1=${mdsnid%@*}
        local nid2=${addr}@$net2
        local addr2 failover_mds1
+       local all=$(comma_list $(all_nodes))
 
        export LNETCTL=$(which lnetctl 2> /dev/null)
 
@@ -2615,7 +2616,7 @@ test_31() {
        fi
 
        # build list of interface on nodes
-       for node in $(all_nodes); do
+       for node in ${all//,/ }; do
                infname=inf_$(echo $node | cut -d'.' -f1 | sed s+-+_+g)
                itf=$(do_node $node $LNETCTL net show --net $net |
                      awk 'BEGIN{inf=0} \
@@ -2627,6 +2628,7 @@ test_31() {
        local mgsnid_orig=$MGSNID
        # compute new MGSNID
        local mgsnid_new=${MGSNID%@*}@$net2
+       local tgts=$(tgts_nodes)
 
        # save mds failover nids for restore at cleanup
        failover_mds1=$(do_facet mds1 $TUNEFS --dryrun $(mdsdevname 1))
@@ -2647,24 +2649,22 @@ test_31() {
        fi
 
        do_facet mgs "$LCTL set_param mgs.MGS.exports.clear=clear"
-       do_nodes $(comma_list $(mdts_nodes) $(osts_nodes)) \
-               "$LCTL set_param *.${FSNAME}*.exports.clear=clear"
+       do_nodes $tgts "$LCTL set_param *.${FSNAME}*.exports.clear=clear"
 
        # check exports on servers are empty for client
        wait_update_facet_cond mgs \
                "$LCTL get_param -N mgs.MGS.exports.* | grep $nid |
                cut -d'.' -f4-" '!=' $nid
-       for node in $(mdts_nodes) $(osts_nodes); do
+       for node in ${tgts//,/ }; do
                wait_update_cond $node \
                        "$LCTL get_param -N *.${FSNAME}*.exports | grep $nid |
                        cut -d'.' -f4-" '!=' $nid
        done
-       do_facet mgs "lctl get_param *.MGS*.exports.*.export"
-       do_facet mgs "lctl get_param -n *.MGS*.exports.'$nid'.uuid 2>/dev/null |
+       do_facet mgs "$LCTL get_param *.MGS*.exports.*.export"
+       do_facet mgs "$LCTL get_param -n *.MGS*.exports.'$nid'.uuid 2>/dev/null|
                      grep -q -" && error "export on MGS should be empty"
-       do_nodes $(comma_list $(mdts_nodes) $(osts_nodes)) \
-                "lctl get_param -n *.${FSNAME}*.exports.'$nid'.uuid \
-                 2>/dev/null | grep -q -" &&
+       do_nodes $tgts "$LCTL get_param -n *.${FSNAME}*.exports.'$nid'.uuid \
+                        2>/dev/null | grep -q -" &&
                error "export on servers should be empty"
 
        KZPOOL=$KEEP_ZPOOL
@@ -2674,9 +2674,8 @@ test_31() {
                error "Failed to unload modules"
 
        # add network $net2 on all nodes
-       do_rpc_nodes $(comma_list $(all_nodes)) load_modules ||
-               error "unable to load modules on $(all_nodes)"
-       for node in $(all_nodes); do
+       do_rpc_nodes $all load_modules || error "unable to load modules on $all"
+       for node in ${all//,/ }; do
                do_node $node "$LNETCTL lnet configure" ||
                        error "unable to configure lnet on node $node"
                infname=inf_$(echo $node | cut -d'.' -f1 | sed s+-+_+g)
@@ -2717,34 +2716,29 @@ test_31() {
                error "unable to remount client"
 
        # check export on MGS
-       do_facet mgs "lctl get_param *.MGS*.exports.*.export"
-       do_facet mgs "lctl get_param -n *.MGS*.exports.'$nid'.uuid 2>/dev/null |
-                     grep -"
-       [ $? -ne 0 ] || error "export for $nid on MGS should not exist"
-
-       do_facet mgs \
-               "lctl get_param -n *.MGS*.exports.'$nid2'.uuid \
-                2>/dev/null | grep -"
-       [ $? -eq 0 ] ||
+       do_facet mgs "$LCTL get_param *.MGS*.exports.*.export"
+       do_facet mgs "$LCTL get_param -n *.MGS*.exports.'$nid'.uuid 2>/dev/null|
+                     grep -" &&
+               error "export for $nid on MGS should not exist"
+
+       do_facet mgs "$LCTL get_param -n *.MGS*.exports.'$nid2'.uuid"|grep - ||
                error "export for $nid2 on MGS should exist"
 
        # check {mdc,osc} imports
-       lctl get_param mdc.${FSNAME}-*.import | grep current_connection |
-           grep $net2
-       [ $? -eq 0 ] ||
+       $LCTL get_param mdc.${FSNAME}-*.import | grep current_connection |
+               grep $net2 ||
                error "import for mdc should use ${addr1}@$net2"
-       lctl get_param osc.${FSNAME}-*.import | grep current_connection |
-           grep $net2
-       [ $? -eq 0 ] ||
+       $LCTL get_param osc.${FSNAME}-*.import | grep current_connection |
+               grep $net2 ||
                error "import for osc should use ${addr1}@$net2"
 
        # no NIDs on other networks should be listed
-       lctl get_param mdc.${FSNAME}-*.import | grep failover_nids |
+       $LCTL get_param mdc.${FSNAME}-*.import | grep failover_nids |
            grep -w ".*@$net" &&
                error "MDC import shouldn't have failnids at @$net"
 
        # failover NIDs on net999 should be listed
-       lctl get_param mdc.${FSNAME}-*.import | grep failover_nids |
+       $LCTL get_param mdc.${FSNAME}-*.import | grep failover_nids |
            grep ${addr2}@$net2 ||
                error "MDC import should have failnid ${addr2}@$net2"
 
@@ -2752,24 +2746,22 @@ test_31() {
        zconf_umount $HOSTNAME $MOUNT || error "unable to umount client"
 
        do_facet mgs "$LCTL set_param mgs.MGS.exports.clear=clear"
-       do_nodes $(comma_list $(mdts_nodes) $(osts_nodes)) \
-               "$LCTL set_param *.${FSNAME}*.exports.clear=clear"
+       do_nodes $tgts "$LCTL set_param *.${FSNAME}*.exports.clear=clear"
 
        wait_update_facet_cond mgs \
                "$LCTL get_param -N mgs.MGS.exports.* | grep $nid2 |
                cut -d'.' -f4-" '!=' $nid2
-       for node in $(mdts_nodes) $(osts_nodes); do
+       for node in ${tgts//,/ }; do
                wait_update_cond $node \
                        "$LCTL get_param -N *.${FSNAME}*.exports | grep $nid2 |
                        cut -d'.' -f4-" '!=' $nid2
        done
-       do_facet mgs "lctl get_param *.MGS*.exports.*.export"
+       do_facet mgs "$LCTL get_param *.MGS*.exports.*.export"
 
        # on client, configure LNet and turn LNet Dynamic Discovery on (default)
        $LUSTRE_RMMOD || error "$LUSTRE_RMMOD failed (2)"
        load_modules || error "Failed to load modules"
-       $LNETCTL lnet configure ||
-               error "unable to configure lnet on client"
+       $LNETCTL lnet configure || error "unable to configure lnet on client"
        infname=inf_$(echo $(hostname -s) | sed s+-+_+g)
        $LNETCTL net add --if ${!infname} --net $net2 ||
                error "unable to configure NID on $net2 on client (2)"
@@ -2779,7 +2771,7 @@ test_31() {
        mount_client $MOUNT ${MOUNT_OPTS},network=$net2 &&
                error "client mount with '-o network' option should be refused"
 
-       echo
+       return 0
 }
 run_test 31 "client mount option '-o network'"
 

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index 4f24c6a..731d0e7 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -17002,7 +17002,7 @@ check_stats() {
                                do_nodes $(comma_list $(mdts_nodes)) \
                                        $LCTL get_param mdt.*.md_stats
                        else
-                               do_nodes $(comma_list $(osts-nodes)) \
+                               do_nodes $(osts_nodes) \
                                        $LCTL get_param obdfilter.*.stats
                        fi
                        error "The $op counter on $facet is $count, not $want"

diff --git a/lustre/tests/test-framework.sh b/lustre/tests/test-framework.sh
index 8a09d86..e918b3c 100755 (executable)
--- a/lustre/tests/test-framework.sh
+++ b/lustre/tests/test-framework.sh
@@ -8027,9 +8027,14 @@ mdts_nodes () {
        echo -n $(facets_nodes $(get_facets MDS))
 }
 
-# Get all of the active OSS nodes.
-osts_nodes () {
-       echo -n $(facets_nodes $(get_facets OST))
+# Get all of the active OSS nodes in a comma-separated list.
+osts_nodes() {
+       comma_list $(facets_nodes $(get_facets OST))
+}
+
+# Get all of the active server nodes in a comma-separated list.
+tgts_nodes() {
+       comma_list $(facets_nodes $(get_facets MDS OST))
 }
 
 # Get all of the client nodes and active server nodes.
@@ -8074,22 +8079,19 @@ all_mdts_nodes () {
 }
 
 # Get all of the OSS nodes, including active and passive nodes.
-all_osts_nodes () {
+all_osts_nodes() {
        local host
        local failover_host
-       local nodes=
-       local nodes_sort
+       local nodes=""
        local i
 
-       for i in $(seq $OSTCOUNT); do
+       for ((i = 1; i <= $OSTCOUNT; i++)); do
                host=ost${i}_HOST
                failover_host=ost${i}failover_HOST
                nodes="$nodes ${!host} ${!failover_host}"
        done
 
-       [ -n "$nodes" ] || nodes="${ost_HOST} ${ostfailover_HOST}"
-       nodes_sort=$(for i in $nodes; do echo $i; done | sort -u)
-       echo -n $nodes_sort
+       comma_list $nodes
 }
 
 # Get all of the server nodes, including active and passive nodes.