osd: Improve dump_pgstate_history json output

Fixes: http://tracker.ceph.com/issues/38846
Signed-off-by: Brad Hubbard <bhubbard@redhat.com>

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index d1a8e7b754e38ddf1ffd573a860deee5d7c37a2a..5e3cf3c31e9b0bdf78c62e24f1dbb8b85fdc22d5 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -2652,13 +2652,18 @@ will start to track new ops received afterwards.";
     store->flush_cache(&ss);
   } else if (admin_command == "dump_pgstate_history") {
     f->open_object_section("pgstate_history");
+    f->open_array_section("pgs");
     vector<PGRef> pgs;
     _get_pgs(&pgs);
     for (auto& pg : pgs) {
+      f->open_object_section("pg");
       f->dump_stream("pg") << pg->pg_id;
+      f->dump_string("currently", pg->get_current_state());
       pg->dump_pgstate_history(f);
+      f->close_section();
     }
     f->close_section();
+    f->close_section();
   } else if (admin_command == "compact") {
     dout(1) << "triggering manual compaction" << dendl;
     auto start = ceph::coarse_mono_clock::now();

diff --git a/src/osd/PG.h b/src/osd/PG.h
index 95fbae2b58df5794572d796e1c5c08d1cc35cbfe..44bfe82033c17d1f050538130bab1fa64bf8daa1 100644 (file)
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -193,6 +193,10 @@ public:
     return ceph_subsys_osd;
   }
 
+  const char* const get_current_state() const {
+    return recovery_state.get_current_state();
+  }
+
   const OSDMapRef& get_osdmap() const {
     ceph_assert(is_locked());
     return recovery_state.get_osdmap();

diff --git a/src/osd/PGStateUtils.cc b/src/osd/PGStateUtils.cc
index 5ffc1f0f16b08e0f654bfd89dd3a61ecb002f440..98becffea297ce510fc0067ddcabfda4f0c71f36 100644 (file)
--- a/src/osd/PGStateUtils.cc
+++ b/src/osd/PGStateUtils.cc
@@ -38,14 +38,18 @@ void PGStateHistory::exit(const char* state) {
 void PGStateHistory::dump(Formatter* f) const {
   f->open_array_section("history");
   for (auto pi = buffer.begin(); pi != buffer.end(); ++pi) {
-    f->open_object_section("states");
+    f->open_object_section("epochs");
     f->dump_stream("epoch") << (*pi)->this_epoch;
+    f->open_array_section("states");
     for (auto she : (*pi)->state_history) {
+      f->open_object_section("state");
       f->dump_string("state", std::get<2>(she));
       f->dump_stream("enter") << std::get<0>(she);
       f->dump_stream("exit") << std::get<1>(she);
+      f->close_section();
     }
     f->close_section();
+    f->close_section();
   }
   f->close_section();
 }

diff --git a/src/test/admin_socket_output.cc b/src/test/admin_socket_output.cc
index 1302391c9cb3a2b46079bcb26fe2147d308056d5..76e6e567726fd6a15531ca9a27c205cbb6efd85b 100644 (file)
--- a/src/test/admin_socket_output.cc
+++ b/src/test/admin_socket_output.cc
@@ -74,16 +74,20 @@ void AdminSocketOutput::postpone(const std::string &target,
 
 bool AdminSocketOutput::init_sockets() {
   std::cout << "Initialising sockets" << std::endl;
-  for (const auto &x : fs::directory_iterator(socketdir)) {
+  std::string socket_regex = R"(\..*\.asok)";
+  for (const auto &x : fs::recursive_directory_iterator(socketdir)) {
     std::cout << x.path() << std::endl;
     if (x.path().extension() == ".asok") {
-      for (auto &target : targets) {
-        if (std::regex_search(x.path().filename().string(),
-            std::regex(prefix + target + R"(\..*\.asok)"))) {
-          std::cout << "Found " << target << " socket " << x.path()
+      for (auto target = targets.cbegin(); target != targets.cend();) {
+        std::regex reg(prefix + *target + socket_regex);
+        if (std::regex_search(x.path().filename().string(), reg)) {
+          std::cout << "Found " << *target << " socket " << x.path()
                     << std::endl;
-          sockets.insert(std::make_pair(target, x.path().string()));
-          targets.erase(target);
+          sockets.insert(std::make_pair(*target, x.path().string()));
+          target = targets.erase(target);
+        }
+        else {
+          ++target;
         }
       }
       if (targets.empty()) {

diff --git a/src/test/admin_socket_output_tests.cc b/src/test/admin_socket_output_tests.cc
index 58bc036540805a8a79d6335979cc88d451334279..f91a186b834c4289073e8dbe42fb5145119827df 100644 (file)
--- a/src/test/admin_socket_output_tests.cc
+++ b/src/test/admin_socket_output_tests.cc
@@ -35,37 +35,73 @@ bool test_dump_pgstate_history(std::string &output) {
     return false;
   }
 
-  JSONObjIter iter = parser.find_first();
-  if (iter.end()) { //Empty
+  JSONObjIter iterone = parser.find_first();
+  if (iterone.end()) { //Empty
     std::cerr << "test_dump_pgstate_history: command output empty, failing"
               << std::endl;
     return false;
   }
-  for (; !iter.end(); ++iter) {
-    if ((*iter)->get_name() == "pg") {
-      ret = !(*iter)->get_data().empty();
-      if (ret == false) {
-        std::cerr << "test_dump_pgstate_history: pg value empty, failing"
-                  << std::endl;
+
+  uint total = 0;
+  if ((*iterone)->get_name() == "pgs") {
+    JSONObjIter iter = (*(*iterone)->find_first())->find_first();
+    for (; !iter.end(); ++iter) {
+      if ((*iter)->get_name() == "pg") {
+        ret = !(*iter)->get_data().empty();
+        if (ret == false) {
+          std::cerr << "test_dump_pgstate_history: pg value empty, failing"
+                    << std::endl;
+          std::cerr << "Dumping full output: " << std::endl;
+          std::cerr << output << std::endl;
+          break;
+        }
+        total++;
+      } else if ((*iter)->get_name() == "history") {
+        ret = std::string::npos != (*iter)->get_data().find("epoch") &&
+              std::string::npos != (*iter)->get_data().find("state") &&
+              std::string::npos != (*iter)->get_data().find("enter") &&
+              std::string::npos != (*iter)->get_data().find("exit");
+        if (ret == false) {
+          std::cerr << "test_dump_pgstate_history: Can't find expected values in "
+                       "history object, failing"
+                    << std::endl;
+          std::cerr << "Problem output was:" << std::endl;
+          std::cerr << (*iter)->get_data() << std::endl;
+          break;
+        }
+        total++;
+      } else if ((*iter)->get_name() == "currently") {
+        ret = !(*iter)->get_data().empty();
+        if (ret == false) {
+          std::cerr << "test_dump_pgstate_history: currently value empty, failing"
+                    << std::endl;
+          std::cerr << "Dumping full output: " << std::endl;
+          std::cerr << output << std::endl;
+          break;
+        }
+        total++;
+      } else {
+        std::cerr << "test_dump_pgstate_history: unrecognised field " << (*iter)->get_name()
+                  << ", failing" << std::endl;
         std::cerr << "Dumping full output: " << std::endl;
         std::cerr << output << std::endl;
         break;
       }
-    } else if ((*iter)->get_name() == "history") {
-      ret = std::string::npos != (*iter)->get_data().find("epoch") &&
-            std::string::npos != (*iter)->get_data().find("state") &&
-            std::string::npos != (*iter)->get_data().find("Initial") &&
-            std::string::npos != (*iter)->get_data().find("enter") &&
-            std::string::npos != (*iter)->get_data().find("exit");
-      if (ret == false) {
-        std::cerr << "test_dump_pgstate_history: Can't find expected values in "
-                     "history object, failing"
-                  << std::endl;
-        std::cerr << "Problem output was:" << std::endl;
-        std::cerr << (*iter)->get_data() << std::endl;
-        break;
-      }
     }
+  } else {
+    std::cerr << "test_dump_pgstate_history: unrecognised format, failing"
+              << std::endl;
+    std::cerr << "Dumping full output: " << std::endl;
+    std::cerr << output << std::endl;
+    return false;
   }
+
+  if (total != 3) {
+    std::cerr << "Could not find required elements, failing" << std::endl;
+    std::cerr << "Dumping full output: " << std::endl;
+    std::cerr << output << std::endl;
+    return false;
+  }
+
   return ret;
 }