Fix vitastor-cli ls-pgs printing an empty table instead of an error

When the pool was not found or the etcd request failed, load_pg_stats()
stored the error, but loop() still ran format_pgs() which replaced the
message with a table built from the pool config. ls-pgs then printed
only a table header, or a table without stats, instead of the error.

src/cmd/cli_ls.cpp

@@ -250,17 +250,11 @@ resume_1:
 
     json11::Json::array to_tree(const json11::Json::array & array)
     {
-        std::set<uint64_t> listed;
-        for (const auto & item: array)
-        {
-            listed.insert(item["inode_id"].uint64_value());
-        }
         std::map<uint64_t, json11::Json::array> children;
         for (const auto & item: array)
         {
             uint64_t parent_id = item["parent_inode_id"].uint64_value();
-            // Layers with a filtered out parent are shown as roots
-            children[listed.find(parent_id) != listed.end() ? parent_id : 0].push_back(item);
+            children[parent_id].push_back(item);
         }
         json11::Json::array tree;
         std::function<void(uint64_t, const std::string &)> add_children = [&](uint64_t parent_id, const std::string & prefix)

src/cmd/cli_pg_ls.cpp

@@ -251,7 +251,7 @@ resume_1:
             { "title", "MISPLACED" },
         });
         cols.push_back(json11::Json::object{
-            { "key", "fmt_misplaced" },
+            { "key", "fmt_degraded" },
             { "title", "DEGRADED" },
         });
         cols.push_back(json11::Json::object{
@@ -273,6 +273,8 @@ resume_1:
         load_pg_stats();
         if (parent->waiting > 0)
             return;
+        if (state == 100)
+            return;
         format_pgs();
         state = 100;
     }

tests/run_tests.sh

@@ -61,6 +61,7 @@ OLD=1 SCHEME=ec ./test_snapshot.sh
 OLD=1 ./test_move_reappear.sh
 
 ./test_degraded.sh
+./test_pg_ls.sh
 
 ./test_rm.sh
 
@@ -122,7 +123,6 @@ OLD=1 ./test_corrupt_all.sh
 
 ./test_reweight_half.sh
 ./test_snapshot_pool2.sh
-./test_ls_tree.sh
 ./test_snapshot_read_bitmap.sh
 
 TEST_NAME=csum_32k_dmj OSD_ARGS="--data_csum_type crc32c --csum_block_size 32k --inmemory_metadata false --inmemory_journal false" OFFSET_ARGS=$OSD_ARGS ./test_heal.sh

tests/test_ls_tree.sh

@@ -1,40 +0,0 @@
-#!/bin/bash -ex
-
-# Test vitastor-cli ls --tree with filters which leave out parent layers
-
-. `dirname $0`/run_3osds.sh
-
-$VITASTOR_CLI create -s 128M testimg
-$VITASTOR_CLI snap-create testimg@snap1
-$VITASTOR_CLI create --parent testimg@snap1 testclone
-
-$VITASTOR_CLI create-pool testpool2 -s 3 -n 4 --failure_domain osd
-wait_pool_up 30 2 3 4
-$VITASTOR_CLI create --parent testimg@snap1 -p testpool2 testclone2
-
-# First column of each row without the header
-check_tree()
-{
-    local expected=$1
-    shift
-    local rows=$($VITASTOR_CLI ls --tree "$@" | tail -n +2 | sed 's/  .*//' | tr '\n' ';')
-    if [[ "$rows" != "$expected" ]]; then
-        format_error "ls --tree $* shows '$rows', expected '$expected'"
-    fi
-}
-
-$VITASTOR_CLI ls --tree
-
-check_tree 'testimg@snap1;├─ testclone;├─ testclone2;└─ testimg;'
-check_tree 'testimg@snap1;└─ testimg;' 'testimg*'
-# Layers whose parent is filtered out are shown as roots
-check_tree 'testimg;' testimg
-check_tree 'testclone;testimg;' testclone testimg
-check_tree 'testclone;' -n 1
-check_tree 'testclone2;' -p testpool2
-
-if ! $VITASTOR_CLI ls --tree testimg | grep -q ' testimg@snap1 *$'; then
-    format_error "ls --tree testimg should show testimg@snap1 as the parent"
-fi
-
-format_green OK

tests/test_pg_ls.sh

@@ -0,0 +1,47 @@
+#!/bin/bash -ex
+
+# Test vitastor-cli ls-pgs on degraded PGs
+
+PG_COUNT=4
+PG_SIZE=3
+PG_MINSIZE=2
+# Keep degraded objects until the checks are done
+GLOBAL_CONFIG=',"no_recovery":true'
+. `dirname $0`/run_3osds.sh
+
+$VITASTOR_CLI create -s 32M testimg
+
+# Data written while OSD 3 is down is degraded when it comes back
+kill -9 $OSD3_PID
+$ETCDCTL del /vitastor/osd/state/3
+wait_condition 60 "$ETCDCTL get --prefix /vitastor/pg/state/ --print-value-only | jq -s -e '[ .[] | select(.state | index(\"degraded\")) ] | length == $PG_COUNT'" "PG peering without OSD 3"
+$VITASTOR_CLI dd if=/dev/urandom oimg=testimg bs=1M count=32
+start_osd 3
+wait_condition 60 "$ETCDCTL get --prefix /vitastor/pgstats/ --print-value-only | jq -s -e '[ .[] | select(.degraded_count > 0) ] | length == $PG_COUNT'" "PG degradation"
+
+$VITASTOR_CLI ls-pgs
+
+# NUM and DEGRADED of every PG from the table, and degraded_count from JSON
+$VITASTOR_CLI ls-pgs | awk 'NR == 1 { n = index($0, "NUM"); d = index($0, "DEGRADED"); e = index($0, "INCOMPLETE") }
+    NR > 1 { num = substr($0, n); sub(/ .*/, "", num); deg = substr($0, d, e-d); sub(/ +$/, "", deg); print num " " deg }' > ./testdata/pg_ls.txt
+$VITASTOR_CLI ls-pgs --json | jq -r '.[] | "\(.pg_num) \(.degraded_count // 0)"' > ./testdata/pg_ls_json.txt
+
+if [[ $(wc -l < ./testdata/pg_ls.txt) != $PG_COUNT ]]; then
+    format_error "ls-pgs should list $PG_COUNT PGs"
+fi
+while read num count; do
+    shown=$(awk '$1 == '$num' { $1 = ""; sub(/^ /, ""); print }' ./testdata/pg_ls.txt)
+    if [[ -z "$shown" || $count -gt 0 && "$shown" = "0 B" || $count -eq 0 && "$shown" != "0 B" ]]; then
+        format_error "ls-pgs shows DEGRADED=$shown for PG $num with $count degraded objects"
+    fi
+done < ./testdata/pg_ls_json.txt
+
+# An unknown pool is an error, not an empty table
+if out=$($VITASTOR_CLI ls-pgs --pool nosuchpool 2>&1); then
+    format_error "ls-pgs should fail for an unknown pool"
+fi
+if [[ "$out" != "Pool nosuchpool not found" ]]; then
+    format_error "ls-pgs prints '$out' instead of the unknown pool error"
+fi
+
+format_green OK