common: ping existing admin socket before unlink

When a daemon initializes it tries to create an admin socket and unlinks
any pre-existing file, regardless. If such a file is in use, it causes
the existing daemon to loose its admin socket.

The AdminSocketClient::ping is implemented to probe an existing socket,
using the "0" message. The AdminSocket::bind_and_listen function is
modified to call ping() on when it finds existing file. It unlinks the
file only if the ping fails.

http://tracker.ceph.com/issues/7188 fixes: #7188

Backport: emperor, dumpling
Reviewed-by: Sage Weil <sage@inktank.com>
Signed-off-by: Loic Dachary <loic@dachary.org>
(cherry picked from commit 45600789f1ca399dddc5870254e5db883fb29b38)

diff --git a/src/common/admin_socket.cc b/src/common/admin_socket.cc
index dc208c30f643d33b997335675a350909a4c62da5..1190b6d7ff8bfdb97a7f631da9c249d01004fdc9 100644 (file)
--- a/src/common/admin_socket.cc
+++ b/src/common/admin_socket.cc
@@ -16,6 +16,7 @@
 
 #include "common/Thread.h"
 #include "common/admin_socket.h"
+#include "common/admin_socket_client.h"
 #include "common/config.h"
 #include "common/cmdparse.h"
 #include "common/dout.h"
@@ -184,15 +185,21 @@ std::string AdminSocket::bind_and_listen(const std::string &sock_path, int *fd)
           sizeof(struct sockaddr_un)) != 0) {
     int err = errno;
     if (err == EADDRINUSE) {
-      // The old UNIX domain socket must still be there.
-      // Let's unlink it and try again.
-      TEMP_FAILURE_RETRY(unlink(sock_path.c_str()));
-      if (bind(sock_fd, (struct sockaddr*)&address,
-              sizeof(struct sockaddr_un)) == 0) {
-       err = 0;
-      }
-      else {
-       err = errno;
+      AdminSocketClient client(sock_path);
+      bool ok;
+      client.ping(&ok);
+      if (ok) {
+       ldout(m_cct, 20) << "socket " << sock_path << " is in use" << dendl;
+       err = EEXIST;
+      } else {
+       ldout(m_cct, 20) << "unlink stale file " << sock_path << dendl;
+       TEMP_FAILURE_RETRY(unlink(sock_path.c_str()));
+       if (bind(sock_fd, (struct sockaddr*)&address,
+                sizeof(struct sockaddr_un)) == 0) {
+         err = 0;
+       } else {
+         err = errno;
+       }
       }
     }
     if (err != 0) {

diff --git a/src/common/admin_socket_client.cc b/src/common/admin_socket_client.cc
index 335695f9b4ba80f504bb82840b430ea13c933377..8750560289594650c0c74479d81ead00e064602d 100644 (file)
--- a/src/common/admin_socket_client.cc
+++ b/src/common/admin_socket_client.cc
@@ -127,6 +127,14 @@ AdminSocketClient(const std::string &path)
 {
 }
 
+std::string AdminSocketClient::ping(bool *ok)
+{
+  std::string version;
+  std::string result = do_request("{\"prefix\":\"0\"}", &version);
+  *ok = result == "" && version.length() == 1;
+  return result;
+}
+
 std::string AdminSocketClient::do_request(std::string request, std::string *result)
 {
   int socket_fd = 0, res;

diff --git a/src/common/admin_socket_client.h b/src/common/admin_socket_client.h
index 45654bbf5f7bf6de274231cb0bb078250309f105..b46ed24fff13da82a8f84736663beeb96b3e3c2e 100644 (file)
--- a/src/common/admin_socket_client.h
+++ b/src/common/admin_socket_client.h
@@ -26,6 +26,7 @@ class AdminSocketClient
 public:
   AdminSocketClient(const std::string &path);
   std::string do_request(std::string request, std::string *result);
+  std::string ping(bool *ok);
 private:
   std::string m_path;
 };

diff --git a/src/test/admin_socket.cc b/src/test/admin_socket.cc
index 6484740674f6b555785ef2cbde5ccc76ee1e81ee..248f24ecc5c379c630382ae1e6f1217894fc895d 100644 (file)
--- a/src/test/admin_socket.cc
+++ b/src/test/admin_socket.cc
@@ -12,11 +12,15 @@
  *
  */
 
+#include <gtest/gtest.h>
+
 #include "common/Mutex.h"
+#include "common/Cond.h"
 #include "common/admin_socket.h"
 #include "common/admin_socket_client.h"
-#include "common/ceph_context.h"
-#include "test/unit.h"
+#include "common/ceph_argparse.h"
+#include "global/global_init.h"
+#include "global/global_context.h"
 
 #include <stdint.h>
 #include <string.h>
@@ -33,6 +37,9 @@ public:
   bool init(const std::string &uri) {
     return m_asokc->init(uri);
   }
+  string bind_and_listen(const std::string &sock_path, int *fd) {
+    return m_asokc->bind_and_listen(sock_path, fd);
+  }
   bool shutdown() {
     m_asokc->shutdown();
     return true;
@@ -179,13 +186,125 @@ TEST(AdminSocket, RegisterCommandPrefixes) {
   ASSERT_EQ(true, asoct.shutdown());
 }
 
+class BlockingHook : public AdminSocketHook {
+public:
+  Mutex _lock;
+  Cond _cond;
+
+  BlockingHook() : _lock("BlockingHook::_lock") {}
+
+  bool call(std::string command, cmdmap_t& cmdmap, std::string format, bufferlist& result) {
+    Mutex::Locker l(_lock);
+    _cond.Wait(_lock);
+    return true;
+  }
+};
+
+TEST(AdminSocketClient, Ping) {
+  string path = get_rand_socket_path();
+  std::auto_ptr<AdminSocket>
+      asokc(new AdminSocket(g_ceph_context));
+  AdminSocketClient client(path);
+  // no socket
+  {
+    bool ok;
+    std::string result = client.ping(&ok);
+    EXPECT_NE(std::string::npos, result.find("No such file or directory"));
+    ASSERT_FALSE(ok);
+  }
+  // file exists but does not allow connections (no process, wrong type...)
+  ASSERT_TRUE(::creat(path.c_str(), 0777));
+  {
+    bool ok;
+    std::string result = client.ping(&ok);
+    EXPECT_NE(std::string::npos, result.find("Connection refused"));
+    ASSERT_FALSE(ok);
+  }
+  // a daemon is connected to the socket
+  {
+    AdminSocketTest asoct(asokc.get());
+    ASSERT_TRUE(asoct.init(path));
+    bool ok;
+    std::string result = client.ping(&ok);
+    EXPECT_EQ("", result);
+    ASSERT_TRUE(ok);
+    ASSERT_TRUE(asoct.shutdown());
+  }
+  // hardcoded five seconds timeout prevents infinite blockage
+  {
+    AdminSocketTest asoct(asokc.get());
+    BlockingHook *blocking = new BlockingHook();
+    ASSERT_EQ(0, asoct.m_asokc->register_command("0", "0", blocking, ""));
+    ASSERT_TRUE(asoct.init(path));
+    bool ok;
+    std::string result = client.ping(&ok);
+    EXPECT_NE(std::string::npos, result.find("Resource temporarily unavailable"));
+    ASSERT_FALSE(ok);
+    {
+      Mutex::Locker l(blocking->_lock);
+      blocking->_cond.Signal();
+    }
+    ASSERT_TRUE(asoct.shutdown());
+    delete blocking;
+  }
+}
+
+TEST(AdminSocket, bind_and_listen) {
+  string path = get_rand_socket_path();
+  std::auto_ptr<AdminSocket>
+      asokc(new AdminSocket(g_ceph_context));
+
+  AdminSocketTest asoct(asokc.get());
+  // successfull bind
+  {
+    int fd = 0;
+    string message;
+    message = asoct.bind_and_listen(path, &fd);
+    ASSERT_NE(0, fd);
+    ASSERT_EQ("", message);
+    ASSERT_EQ(0, ::close(fd));
+    ASSERT_EQ(0, ::unlink(path.c_str()));
+  }
+  // silently discard an existing file
+  {
+    int fd = 0;
+    string message;
+    ASSERT_TRUE(::creat(path.c_str(), 0777));
+    message = asoct.bind_and_listen(path, &fd);
+    ASSERT_NE(0, fd);
+    ASSERT_EQ("", message);
+    ASSERT_EQ(0, ::close(fd));
+    ASSERT_EQ(0, ::unlink(path.c_str()));
+  }
+  // do not take over a live socket
+  {
+    ASSERT_TRUE(asoct.init(path));
+    int fd = 0;
+    string message;
+    message = asoct.bind_and_listen(path, &fd);
+    EXPECT_NE(std::string::npos, message.find("File exists"));
+    ASSERT_TRUE(asoct.shutdown());
+  }
+}
+
+int main(int argc, char **argv) {
+  vector<const char*> args;
+  argv_to_vec(argc, (const char **)argv, args);
+
+  vector<const char*> def_args;
+  global_init(&def_args, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
+  common_init_finish(g_ceph_context);
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}
+
 /*
  * Local Variables:
  * compile-command: "cd .. ;
  *   make unittest_admin_socket &&
  *    valgrind \
  *    --max-stackframe=20000000 --tool=memcheck \
- *   ./unittest_admin_socket # --gtest_filter=AdminSocket.*
+ *   ./unittest_admin_socket --debug-asok 20 # --gtest_filter=AdminSocket*.*
  * "
  * End:
  */