From 1f9430d6b46b87eea37681f5979fd1824bdeb9e0 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Thu, 10 Apr 2008 16:38:58 +0000
Subject: [PATCH] Rework the results of OpDiagnoseOS opcode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently, the opcode DiagnoseOS is the only opcode that return a
structure of objects.OS (which is a custom class, and not a simple
python object) and furthermore all the processing of OS validity across
nodes is left to the clients of this opcode.

It would be more logical to have this opcode be similar to list
instances/nodes, in the sense that:
  - it should return a table of results
  - the fields in the table should be selectable

This patch does the above. The possible fields are:
  - name (os name)
  - valid (bool representing validity across all nodes)
  - node_status, which is a complicated structure required for β€˜gnt-os
    diagnose’

With this patch, gnt-os list becomes a very simple iteration over the
list of results, filtering out non-valid ones. gnt-os diagnose is still
complicated, but no more than before.

The burnin tool has also been modified to work with the modified
results, and is simpler because of this (it only needs to know if an OS
is valid or not, not the per-node details).

Reviewed-by: imsnah
---
 lib/cmdlib.py  | 61 ++++++++++++++++++++++++++++++++++++++--
 lib/opcodes.py |  2 +-
 scripts/gnt-os | 75 ++++++++++++--------------------------------------
 tools/burnin   | 13 ++-------
 4 files changed, 80 insertions(+), 71 deletions(-)

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index f8654d0e5..dbeef5d05 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -1163,7 +1163,7 @@ class LUDiagnoseOS(NoHooksLU):
   """Logical unit for OS diagnose/query.
 
   """
-  _OP_REQP = []
+  _OP_REQP = ["output_fields", "names"]
 
   def CheckPrereq(self):
     """Check prerequisites.
@@ -1171,7 +1171,44 @@ class LUDiagnoseOS(NoHooksLU):
     This always succeeds, since this is a pure query LU.
 
     """
-    return
+    if self.op.names:
+      raise errors.OpPrereqError("Selective OS query not supported")
+
+    self.dynamic_fields = frozenset(["name", "valid", "node_status"])
+    _CheckOutputFields(static=[],
+                       dynamic=self.dynamic_fields,
+                       selected=self.op.output_fields)
+
+  @staticmethod
+  def _DiagnoseByOS(node_list, rlist):
+    """Remaps a per-node return list into an a per-os per-node dictionary
+
+      Args:
+        node_list: a list with the names of all nodes
+        rlist: a map with node names as keys and OS objects as values
+
+      Returns:
+        map: a map with osnames as keys and as value another map, with
+             nodes as
+             keys and list of OS objects as values
+             e.g. {"debian-etch": {"node1": [<object>,...],
+                                   "node2": [<object>,]}
+                  }
+
+    """
+    all_os = {}
+    for node_name, nr in rlist.iteritems():
+      if not nr:
+        continue
+      for os in nr:
+        if os.name not in all_os:
+          # build a list of nodes for this os containing empty lists
+          # for each node in node_list
+          all_os[os.name] = {}
+          for nname in node_list:
+            all_os[os.name][nname] = []
+        all_os[os.name][node_name].append(os)
+    return all_os
 
   def Exec(self, feedback_fn):
     """Compute the list of OSes.
@@ -1181,7 +1218,25 @@ class LUDiagnoseOS(NoHooksLU):
     node_data = rpc.call_os_diagnose(node_list)
     if node_data == False:
       raise errors.OpExecError("Can't gather the list of OSes")
-    return node_data
+    pol = self._DiagnoseByOS(node_list, node_data)
+    output = []
+    for os_name, os_data in pol.iteritems():
+      row = []
+      for field in self.op.output_fields:
+        if field == "name":
+          val = os_name
+        elif field == "valid":
+          val = utils.all([osl and osl[0] for osl in os_data.values()])
+        elif field == "node_status":
+          val = {}
+          for node_name, nos_list in os_data.iteritems():
+            val[node_name] = [(v.status, v.path) for v in nos_list]
+        else:
+          raise errors.ParameterError(field)
+        row.append(val)
+      output.append(row)
+
+    return output
 
 
 class LURemoveNode(LogicalUnit):
diff --git a/lib/opcodes.py b/lib/opcodes.py
index a5d8dedfe..3d7a3b01e 100644
--- a/lib/opcodes.py
+++ b/lib/opcodes.py
@@ -379,7 +379,7 @@ class OpSetInstanceParams(OpCode):
 class OpDiagnoseOS(OpCode):
   """Compute the list of guest operating systems."""
   OP_ID = "OP_OS_DIAGNOSE"
-  __slots__ = []
+  __slots__ = ["output_fields", "names"]
 
 
 # Exports opcodes
diff --git a/scripts/gnt-os b/scripts/gnt-os
index 5918769eb..e4041c728 100755
--- a/scripts/gnt-os
+++ b/scripts/gnt-os
@@ -28,64 +28,27 @@ from ganeti import logger
 from ganeti import objects
 from ganeti import utils
 from ganeti import errors
-
-
-def _DiagnoseByOS(rlist):
-  """Remap an OpDiagnoseOS() return list into an a per-os per-node dictionary
-
-    Args:
-      rlist: a map with nodes as keys and diagnoseobjects as values
-
-    Returns:
-      map: a map with osnames as keys and as value another map, with nodes as
-           keys and diagnoseobjects as values
-           e.g. {"debian-etch": {"node1": <object>, "node2": <object>}}
-
-  """
-  all_os = {}
-  for node_name, nr in rlist.iteritems():
-    if not nr:
-      continue
-    for os in nr:
-      if os.name not in all_os:
-        all_os[os.name] = {}
-      if node_name not in all_os[os.name]:
-        all_os[os.name][node_name] = []
-      all_os[os.name][node_name].append(os)
-
-  return all_os
+from ganeti import constants
 
 
 def ListOS(opts, args):
   """List the OSes existing on this node.
 
   """
-  op = opcodes.OpDiagnoseOS()
+  op = opcodes.OpDiagnoseOS(output_fields=["name", "valid"], names=[])
   result = SubmitOpCode(op)
 
   if not result:
     logger.ToStdout("Can't get the OS list")
     return 1
 
-  node_data = result
-  num_nodes = len(node_data)
-  all_os = _DiagnoseByOS(node_data)
-
-  valid_os = []
-  for os_name, os_node_data in all_os.iteritems():
-    if len(os_node_data) != num_nodes:
-      continue
-
-    if utils.all(os_node_data.values(), lambda l: l[0]):
-      valid_os.append(os_name)
-
   if not opts.no_headers:
     headers = {"name": "Name"}
   else:
     headers = None
 
   data = GenerateTable(separator=None, headers=headers, fields=["name"],
-                       data=[[os] for os in valid_os])
+                       data=[[row[0]] for row in result if row[1]])
 
   for line in data:
     logger.ToStdout(line)
@@ -97,30 +60,33 @@ def DiagnoseOS(opts, args):
   """Analyse all OSes on this cluster.
 
   """
-  op = opcodes.OpDiagnoseOS()
+  op = opcodes.OpDiagnoseOS(output_fields=["name", "valid", "node_status"],
+                            names=[])
   result = SubmitOpCode(op)
 
   if not result:
     logger.ToStdout("Can't get the OS list")
     return 1
 
-  node_data = result
-  all_os = _DiagnoseByOS(node_data)
-
   has_bad = False
 
-  for os_name in all_os:
+  for os_name, os_valid, node_data in result:
     nodes_valid = {}
     nodes_bad = {}
-    for node_name in node_data:
-      if node_name in all_os[os_name]:
-        first_os = all_os[os_name][node_name].pop(0)
+    nodes_hidden = {}
+    for node_name, node_info in node_data.iteritems():
+      nodes_hidden[node_name] = []
+      if node_info: # at least one entry in the per-node list
+        first_os_status, first_os_path = node_info.pop(0)
         first_os_msg = ("%s (path: %s)" %
-                        (first_os.status, first_os.path))
-        if first_os:
+                        (first_os_status, first_os_path))
+        if first_os_status == constants.OS_VALID_STATUS:
           nodes_valid[node_name] = first_os_msg
         else:
           nodes_bad[node_name] = first_os_msg
+        for hstatus, hpath in node_info:
+          nodes_hidden[node_name].append("    [hidden] path: %s, status: %s" %
+                                         (hpath, hstatus))
       else:
         nodes_bad[node_name] = "OS not found"
 
@@ -133,18 +99,13 @@ def DiagnoseOS(opts, args):
       status = "partial valid"
       has_bad = True
 
-    def _OutputNodeHiddenOSStatus(dobj_list):
-      for dobj in dobj_list:
-        logger.ToStdout("    [hidden] path: %s, status: %s" %
-                        (dobj.path, dobj.status))
-
     def _OutputPerNodeOSStatus(msg_map):
       map_k = utils.NiceSort(msg_map.keys())
       for node_name in map_k:
         logger.ToStdout("  Node: %s, status: %s" %
                         (node_name, msg_map[node_name]))
-        if node_name in all_os[os_name]:
-          _OutputNodeHiddenOSStatus(all_os[os_name][node_name])
+        for msg in nodes_hidden[node_name]:
+          logger.ToStdout(msg)
 
     logger.ToStdout("OS: %s [global status: %s]" % (os_name, status))
     _OutputPerNodeOSStatus(nodes_valid)
diff --git a/tools/burnin b/tools/burnin
index 590cd15e1..0ecf11fd2 100755
--- a/tools/burnin
+++ b/tools/burnin
@@ -167,22 +167,15 @@ class Burner(object):
       sys.exit(err_code)
     self.nodes = [data[0] for data in result]
 
-    result = self.ExecOp(opcodes.OpDiagnoseOS())
+    result = self.ExecOp(opcodes.OpDiagnoseOS(output_fields=["name", "valid"],
+                                              names=[]))
 
     if not result:
       Log("Can't get the OS list")
       sys.exit(1)
 
     # filter non-valid OS-es
-    oses = {}
-    for node_name in result:
-      oses[node_name] = [obj for obj in result[node_name] if obj]
-
-    fnode = oses.keys()[0]
-    os_set = set([os_inst.name for os_inst in oses[fnode]])
-    del oses[fnode]
-    for node in oses:
-      os_set &= set([os_inst.name for os_inst in oses[node]])
+    os_set = [val[0] for val in result if val[1]]
 
     if self.opts.os not in os_set:
       Log("OS '%s' not found" % self.opts.os)
-- 
GitLab