From 54ca6e4b2e058a991eeef8f7e0306ac6a97afcd2 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Fri, 29 Jul 2011 15:43:14 +0200
Subject: [PATCH] watcher.state: Use strings, not objects
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Until now the state class would receive instances as objects
(ganeti.watcher.Instance), but this is not necessary. By using strings
the interface is simplified.

This patch also simplifies some code accessing the internal structures,
e.g. setting a key of a dictionary. Some instances of β€œdel dict[key]”
are replaced with β€œdict.pop(key, None)” to suppress any exceptions if
the key doesn't exist.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
 lib/watcher/__init__.py | 16 ++++++++--------
 lib/watcher/state.py    | 36 ++++++++++++++----------------------
 2 files changed, 22 insertions(+), 30 deletions(-)

diff --git a/lib/watcher/__init__.py b/lib/watcher/__init__.py
index 7ffbfe679..23047df97 100644
--- a/lib/watcher/__init__.py
+++ b/lib/watcher/__init__.py
@@ -280,7 +280,7 @@ class Watcher(object):
 
     for instance in self.instances.values():
       if instance.status in BAD_STATES:
-        n = notepad.NumberOfRestartAttempts(instance)
+        n = notepad.NumberOfRestartAttempts(instance.name)
 
         if n > MAXTRIES:
           logging.warning("Not restarting instance %s, retries exhausted",
@@ -289,7 +289,7 @@ class Watcher(object):
         elif n < MAXTRIES:
           last = " (Attempt #%d)" % (n + 1)
         else:
-          notepad.RecordRestartAttempt(instance)
+          notepad.RecordRestartAttempt(instance.name)
           logging.error("Could not restart %s after %d attempts, giving up",
                         instance.name, MAXTRIES)
           continue
@@ -301,13 +301,13 @@ class Watcher(object):
           logging.exception("Error while restarting instance %s",
                             instance.name)
 
-        notepad.RecordRestartAttempt(instance)
+        notepad.RecordRestartAttempt(instance.name)
       elif instance.status in HELPLESS_STATES:
-        if notepad.NumberOfRestartAttempts(instance):
-          notepad.RemoveInstance(instance)
+        if notepad.NumberOfRestartAttempts(instance.name):
+          notepad.RemoveInstance(instance.name)
       else:
-        if notepad.NumberOfRestartAttempts(instance):
-          notepad.RemoveInstance(instance)
+        if notepad.NumberOfRestartAttempts(instance.name):
+          notepad.RemoveInstance(instance.name)
           logging.info("Restart of %s succeeded", instance.name)
 
   def _CheckForOfflineNodes(self, instance):
@@ -511,7 +511,7 @@ def Main():
 
     finally:
       if update_file:
-        notepad.Save()
+        notepad.Save(constants.WATCHER_STATEFILE)
       else:
         logging.debug("Not updating status file due to failure")
   except SystemExit:
diff --git a/lib/watcher/state.py b/lib/watcher/state.py
index ec659b825..776ea6830 100644
--- a/lib/watcher/state.py
+++ b/lib/watcher/state.py
@@ -28,7 +28,6 @@ import time
 import logging
 
 from ganeti import utils
-from ganeti import constants
 from ganeti import serializer
 from ganeti import errors
 
@@ -100,7 +99,7 @@ class WatcherState(object):
 
     self._orig_data = serializer.Dump(self._data)
 
-  def Save(self):
+  def Save(self, filename):
     """Save state to file, then unlock and close it.
 
     """
@@ -109,12 +108,12 @@ class WatcherState(object):
     serialized_form = serializer.Dump(self._data)
     if self._orig_data == serialized_form:
       logging.debug("Data didn't change, just touching status file")
-      os.utime(constants.WATCHER_STATEFILE, None)
+      os.utime(filename, None)
       return
 
     # We need to make sure the file is locked before renaming it, otherwise
     # starting ganeti-watcher again at the same time will create a conflict.
-    fd = utils.WriteFile(constants.WATCHER_STATEFILE,
+    fd = utils.WriteFile(filename,
                          data=serialized_form,
                          prewrite=utils.LockFile, close=False)
     self.statefile = os.fdopen(fd, 'w+')
@@ -147,12 +146,9 @@ class WatcherState(object):
 
     ndata = self._data["node"]
 
-    if name not in ndata:
-      ndata[name] = {}
+    ndata.setdefault(name, {})[KEY_BOOT_ID] = bootid
 
-    ndata[name][KEY_BOOT_ID] = bootid
-
-  def NumberOfRestartAttempts(self, instance):
+  def NumberOfRestartAttempts(self, instance_name):
     """Returns number of previous restart attempts.
 
     @type instance: L{Instance}
@@ -161,8 +157,8 @@ class WatcherState(object):
     """
     idata = self._data["instance"]
 
-    if instance.name in idata:
-      return idata[instance.name][KEY_RESTART_COUNT]
+    if instance_name in idata:
+      return idata[instance_name][KEY_RESTART_COUNT]
 
     return 0
 
@@ -174,11 +170,12 @@ class WatcherState(object):
 
     """
     idict = self._data["instance"]
+
     # First, delete obsolete instances
     obsolete_instances = set(idict).difference(instances)
     for inst in obsolete_instances:
       logging.debug("Forgetting obsolete instance %s", inst)
-      del idict[inst]
+      idict.pop(inst, None)
 
     # Second, delete expired records
     earliest = time.time() - RETRY_EXPIRATION
@@ -186,9 +183,9 @@ class WatcherState(object):
                          if idict[i][KEY_RESTART_WHEN] < earliest]
     for inst in expired_instances:
       logging.debug("Expiring record for instance %s", inst)
-      del idict[inst]
+      idict.pop(inst, None)
 
-  def RecordRestartAttempt(self, instance):
+  def RecordRestartAttempt(self, instance_name):
     """Record a restart attempt.
 
     @type instance: L{Instance}
@@ -197,15 +194,11 @@ class WatcherState(object):
     """
     idata = self._data["instance"]
 
-    if instance.name not in idata:
-      inst = idata[instance.name] = {}
-    else:
-      inst = idata[instance.name]
-
+    inst = idata.setdefault(instance_name, {})
     inst[KEY_RESTART_WHEN] = time.time()
     inst[KEY_RESTART_COUNT] = inst.get(KEY_RESTART_COUNT, 0) + 1
 
-  def RemoveInstance(self, instance):
+  def RemoveInstance(self, instance_name):
     """Update state to reflect that a machine is running.
 
     This method removes the record for a named instance (as we only
@@ -217,5 +210,4 @@ class WatcherState(object):
     """
     idata = self._data["instance"]
 
-    if instance.name in idata:
-      del idata[instance.name]
+    idata.pop(instance_name, None)
-- 
GitLab