From 297b0cd3faa9e114c40079e3490eb95cf0ec9701 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Mon, 30 May 2011 12:52:22 +0200
Subject: [PATCH] iallocator: add ht-checking for the request

Currently, we only ht-check the result value from the iallocator, and
we send whatever we happen to check manually in the LUs that call the
iallocator.

This is not good, as we have to duplicate checks in many places, and
still we might miss checks. So we add add ht information to the
per-request variables. As the cluster data is built in one place, the
iallocator code itself (and is more consistent), I didn't add checks
to that too.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>
---
 doc/iallocator.rst | 14 +++++++-------
 lib/cmdlib.py      | 46 ++++++++++++++++++++++++++++++++++++----------
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/doc/iallocator.rst b/doc/iallocator.rst
index 582f00a2b..6ecd62696 100644
--- a/doc/iallocator.rst
+++ b/doc/iallocator.rst
@@ -205,19 +205,19 @@ in the ``request`` dictionary:
   name
     the name of the instance; if the request is a realocation, then this
     name will be found in the list of instances (see below), otherwise
-    is the FQDN of the new instance
+    is the FQDN of the new instance; type *string*
 
   required_nodes
     how many nodes should the algorithm return; while this information
     can be deduced from the instace's disk template, it's better if
     this computation is left to Ganeti as then allocator scripts are
-    less sensitive to changes to the disk templates
+    less sensitive to changes to the disk templates; type *integer*
 
   disk_space_total
     the total disk space that will be used by this instance on the
     (new) nodes; again, this information can be computed from the list
     of instance disks and its template type, but Ganeti is better
-    suited to compute it
+    suited to compute it; type *integer*
 
 .. pyassert::
 
@@ -274,13 +274,13 @@ Relocation:
   relocate_from
      a list of nodes to move the instance away from (note that with
      Ganeti 2.0, this list will always contain a single node, the
-     current secondary of the instance)
+     current secondary of the instance); type *list of strings*
 
 As for ``multi-relocate``, it needs the three following request
 arguments:
 
   instances
-    a list of instance names to relocate
+    a list of instance names to relocate; type *list of strings*
 
   reloc_mode
     a string indicating the relocation mode; there are three possible
@@ -292,13 +292,13 @@ arguments:
     this argument is only accepted when ``reloc_mode``, as explained
     above, is *change_group*; if present, it must either be the empty
     list, or contain a list of group UUIDs that should be considered for
-    relocating instances to
+    relocating instances to; type *list of strings*
 
 Finally, in the case of multi-evacuate, there's one single request
 argument (in addition to ``type``):
 
   evac_nodes
-    the names of the nodes to be evacuated
+    the names of the nodes to be evacuated; type *list of strings*
 
 Response message
 ~~~~~~~~~~~~~~~~
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index dab6f26bc..16dde30a9 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -12020,11 +12020,13 @@ class IAllocator(object):
     self.success = self.info = self.result = None
 
     try:
-      (fn, keyset, self._result_check) = self._MODE_DATA[self.mode]
+      (fn, keydata, self._result_check) = self._MODE_DATA[self.mode]
     except KeyError:
       raise errors.ProgrammerError("Unknown mode '%s' passed to the"
                                    " IAllocator" % self.mode)
 
+    keyset = [n for (n, _) in keydata]
+
     for key in kwargs:
       if key not in keyset:
         raise errors.ProgrammerError("Invalid input parameter '%s' to"
@@ -12035,7 +12037,7 @@ class IAllocator(object):
       if key not in kwargs:
         raise errors.ProgrammerError("Missing input parameter '%s' to"
                                      " IAllocator" % key)
-    self._BuildInputData(compat.partial(fn, self))
+    self._BuildInputData(compat.partial(fn, self), keydata)
 
   def _ComputeClusterData(self):
     """Compute the generic allocator input data.
@@ -12310,7 +12312,7 @@ class IAllocator(object):
       "target_groups": self.target_groups,
       }
 
-  def _BuildInputData(self, fn):
+  def _BuildInputData(self, fn, keydata):
     """Build input data structures.
 
     """
@@ -12318,23 +12320,47 @@ class IAllocator(object):
 
     request = fn()
     request["type"] = self.mode
+    for keyname, keytype in keydata:
+      if keyname not in request:
+        raise errors.ProgrammerError("Request parameter %s is missing" %
+                                     keyname)
+      val = request[keyname]
+      if not keytype(val):
+        raise errors.ProgrammerError("Request parameter %s doesn't pass"
+                                     " validation, value %s, expected"
+                                     " type %s" % (keyname, val, keytype))
     self.in_data["request"] = request
 
     self.in_text = serializer.Dump(self.in_data)
 
+  _STRING_LIST = ht.TListOf(ht.TString)
   _MODE_DATA = {
     constants.IALLOCATOR_MODE_ALLOC:
       (_AddNewInstance,
-       ["name", "memory", "disks", "disk_template", "os", "tags", "nics",
-        "vcpus", "hypervisor"], ht.TList),
+       [
+        ("name", ht.TString),
+        ("memory", ht.TInt),
+        ("disks", ht.TListOf(ht.TDict)),
+        ("disk_template", ht.TString),
+        ("os", ht.TString),
+        ("tags", _STRING_LIST),
+        ("nics", ht.TListOf(ht.TDict)),
+        ("vcpus", ht.TInt),
+        ("hypervisor", ht.TString),
+        ], ht.TList),
     constants.IALLOCATOR_MODE_RELOC:
-      (_AddRelocateInstance, ["name", "relocate_from"], ht.TList),
+      (_AddRelocateInstance,
+       [("name", ht.TString), ("relocate_from", _STRING_LIST)],
+       ht.TList),
     constants.IALLOCATOR_MODE_MEVAC:
-      (_AddEvacuateNodes, ["evac_nodes"],
-       ht.TListOf(ht.TAnd(ht.TIsLength(2),
-                          ht.TListOf(ht.TString)))),
+      (_AddEvacuateNodes, [("evac_nodes", _STRING_LIST)],
+       ht.TListOf(ht.TAnd(ht.TIsLength(2), _STRING_LIST))),
     constants.IALLOCATOR_MODE_MRELOC:
-      (_AddMultiRelocate, ["instances", "reloc_mode", "target_groups"],
+      (_AddMultiRelocate, [
+        ("instances", _STRING_LIST),
+        ("reloc_mode", ht.TElemOf(constants.IALLOCATOR_MRELOC_MODES)),
+        ("target_groups", _STRING_LIST),
+        ],
        ht.TListOf(ht.TListOf(ht.TStrictDict(True, False, {
          # pylint: disable-msg=E1101
          # Class '...' has no 'OP_ID' member
-- 
GitLab