ganeti-rapi: Watch directory, not file for user file changes

We noticed several issues when just watching the file, among them race
conditions upon replacing the file using rename(2) (the new watcher
would be created too soon). By just watching the directory for events on
the rapi_users file, this can be avoided.

A nice side-effect is that now the users file is also reloaded if it
didn't exist upon ganeti-rapi's start (see the documentation update).

Since ganeti-rapi now becomes active for virtually every change in the
configuration directory (…/lib/ganeti), moving the rapi_users file to a
separate directory will be considered. It doesn't have to happen in or
before this patch, though.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

diff --git a/daemons/ganeti-rapi b/daemons/ganeti-rapi
index 643fcf9..29469fe 100755 (executable)
--- a/daemons/ganeti-rapi
+++ b/daemons/ganeti-rapi
@@ -31,6 +31,7 @@ import optparse
 import sys
 import os
 import os.path
+import errno
 
 try:
   from pyinotify import pyinotify # pylint: disable-msg=E0611
@@ -103,20 +104,27 @@ class RemoteApiHttpServer(http.auth.HttpServerRequestAuthentication,
     @param filename: Path to file
 
     """
+    logging.info("Reading users file at %s", filename)
     try:
-      contents = utils.ReadFile(filename)
-    except EnvironmentError, err:
-      logging.warning("Error while reading %s: %s", filename, err)
-      return False
+      try:
+        contents = utils.ReadFile(filename)
+      except EnvironmentError, err:
+        self._users = None
+        if err.errno == errno.ENOENT:
+          logging.warning("No users file at %s", filename)
+        else:
+          logging.warning("Error while reading %s: %s", filename, err)
+        return False
 
-    try:
       users = http.auth.ParsePasswordFile(contents)
+
     except Exception, err: # pylint: disable-msg=W0703
       # We don't care about the type of exception
       logging.error("Error while parsing %s: %s", filename, err)
       return False
 
     self._users = users
+
     return True
 
   def _GetRequestContext(self, req):
@@ -229,39 +237,51 @@ class RemoteApiHttpServer(http.auth.HttpServerRequestAuthentication,
     return serializer.DumpJson(result)
 
 
-class FileWatcher:
-  def __init__(self, filename, cb):
+class FileEventHandler(asyncnotifier.FileEventHandlerBase):
+  def __init__(self, wm, path, cb):
     """Initializes this class.
 
-    @type filename: string
-    @param filename: File to watch
+    @param wm: Inotify watch manager
+    @type path: string
+    @param path: File path
     @type cb: callable
     @param cb: Function called on file change
 
     """
-    self._filename = filename
+    asyncnotifier.FileEventHandlerBase.__init__(self, wm)
+
     self._cb = cb
+    self._filename = os.path.basename(path)
 
-    wm = pyinotify.WatchManager()
-    self._handler = asyncnotifier.SingleFileEventHandler(wm, self._OnInotify,
-                                                         filename)
-    asyncnotifier.AsyncNotifier(wm, default_proc_fun=self._handler)
-    self._handler.enable()
+    # Class '...' has no 'IN_...' member, pylint: disable-msg=E1103
+    mask = (pyinotify.EventsCodes.IN_CLOSE_WRITE |
+            pyinotify.EventsCodes.IN_DELETE |
+            pyinotify.EventsCodes.IN_MOVED_FROM |
+            pyinotify.EventsCodes.IN_MOVED_TO)
 
-  def _OnInotify(self, notifier_enabled):
-    """Called upon update of the RAPI users file by pyinotify.
+    self._handle = self.AddWatch(os.path.dirname(path), mask)
 
-    @type notifier_enabled: boolean
-    @param notifier_enabled: whether the notifier is still enabled
+  def process_default(self, event):
+    """Called upon inotify event.
 
     """
-    logging.info("Reloading modified %s", self._filename)
+    if event.name == self._filename:
+      logging.debug("Received inotify event %s", event)
+      self._cb()
+
+
+def SetupFileWatcher(filename, cb):
+  """Configures an inotify watcher for a file.
 
-    self._cb()
+  @type filename: string
+  @param filename: File to watch
+  @type cb: callable
+  @param cb: Function called on file change
 
-    # Renable the watch again if we'd an atomic update of the file (e.g. mv)
-    if not notifier_enabled:
-      self._handler.enable()
+  """
+  wm = pyinotify.WatchManager()
+  handler = FileEventHandler(wm, filename, cb)
+  asyncnotifier.AsyncNotifier(wm, default_proc_fun=handler)
 
 
 def CheckRapi(options, args):
@@ -294,18 +314,19 @@ def PrepRapi(options, _):
                                ssl_verify_peer=False,
                                request_executor_class=JsonErrorRequestExecutor)
 
-  if os.path.exists(constants.RAPI_USERS_FILE):
-    # Setup file watcher (it'll be driven by asyncore)
-    FileWatcher(constants.RAPI_USERS_FILE,
-                compat.partial(server.LoadUsers, constants.RAPI_USERS_FILE))
+  # Setup file watcher (it'll be driven by asyncore)
+  SetupFileWatcher(constants.RAPI_USERS_FILE,
+                   compat.partial(server.LoadUsers, constants.RAPI_USERS_FILE))
 
   server.LoadUsers(constants.RAPI_USERS_FILE)
 
   # pylint: disable-msg=E1101
   # it seems pylint doesn't see the second parent class there
   server.Start()
+
   return (mainloop, server)
 
+
 def ExecRapi(options, args, prep_data): # pylint: disable-msg=W0613
   """Main remote API function, executed with the PID file held.
 

diff --git a/doc/rapi.rst b/doc/rapi.rst
index d3c0dfa..c25ba6e 100644 (file)
--- a/doc/rapi.rst
+++ b/doc/rapi.rst
@@ -21,10 +21,8 @@ Users and passwords
 -------------------
 
 ``ganeti-rapi`` reads users and passwords from a file (usually
-``/var/lib/ganeti/rapi_users``) on startup. If the file existed when
-``ganeti-rapi`` was started, it'll automatically reload the file upon
-changes. If the users file is newly created, ``ganeti-rapi`` must be
-restarted.
+``/var/lib/ganeti/rapi_users``) on startup. Changes to the file will be
+read automatically.
 
 Each line consists of two or three fields separated by whitespace. The
 first two fields are for username and password. The third field is