From 255dcebd55db98a986318758fde7646d2ed15074 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Thu, 11 Jun 2009 11:46:39 +0200
Subject: [PATCH] Big rewrite of the OS-related functions
Currently the OSes have a special, customized error handling: the OS
object can represent either a valid OS, or an invalid OS. The associated
function, instead of raising other exception or failing, create custom
OS objects representing failed OSes.
While this was good when no other RPC had failure handling, it's
extremely different from how other function in backend.py expect
failures to be signalled.
This patch reworks this completely:
- the OS object always represents valid OSes (the next patch will
remove the valid/invalid field and associated constants)
- the call_os_diagnose returns instead of a list of OS objects, a list
of (name, path, status, diagnose_msg); the status is then used in
cmdlib to determine validity and the status and diagnose_msg values
are used in gnt-os for display
- call_os_get returns either a valid OS or a RPC remote failure (with
the error message)
- the other functions in backend.py now just call backend.OSFromDisk()
which will return either a valid OS object or raise an exception
- the bulk of the OSFromDisk was moved to _TryOSFromDisk which returns
status, value for the functions which don't want an exception raised
The gnt-os list and diagnose commands still work after this patch.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>
---
daemons/ganeti-noded | 9 ++--
lib/backend.py | 122 ++++++++++++++++++++++++++-----------------
lib/cmdlib.py | 39 +++++++-------
scripts/gnt-os | 29 +++++++---
4 files changed, 122 insertions(+), 77 deletions(-)
diff --git a/daemons/ganeti-noded b/daemons/ganeti-noded
index 15af1014c..b9b51df80 100755
--- a/daemons/ganeti-noded
+++ b/daemons/ganeti-noded
@@ -608,7 +608,7 @@ class NodeHttpServer(http.server.HttpServer):
"""Query detailed information about existing OSes.
"""
- return True, [os_obj.ToDict() for os_obj in backend.DiagnoseOS()]
+ return backend.DiagnoseOS()
@staticmethod
def perspective_os_get(params):
@@ -616,11 +616,8 @@ class NodeHttpServer(http.server.HttpServer):
"""
name = params[0]
- try:
- os_obj = backend.OSFromDisk(name)
- except errors.InvalidOS, err:
- os_obj = objects.OS.FromInvalidOS(err)
- return os_obj.ToDict()
+ os_obj = backend.OSFromDisk(name)
+ return True, os_obj.ToDict()
# hooks -----------------------
diff --git a/lib/backend.py b/lib/backend.py
index e5077c171..bf6316865 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -718,15 +718,8 @@ def InstanceOsAdd(instance, reinstall):
@return: the success of the operation
"""
- try:
- inst_os = OSFromDisk(instance.os)
- except errors.InvalidOS, err:
- os_name, os_dir, os_err = err.args
- if os_dir is None:
- return (False, "Can't find OS '%s': %s" % (os_name, os_err))
- else:
- return (False, "Error parsing OS '%s' in directory %s: %s" %
- (os_name, os_dir, os_err))
+ inst_os = OSFromDisk(instance.os)
+
create_env = OSEnvironment(instance)
if reinstall:
@@ -1530,12 +1523,12 @@ def _OSOndiskVersion(name, os_dir):
try:
st = os.stat(api_file)
except EnvironmentError, err:
- raise errors.InvalidOS(name, os_dir, "'ganeti_api_version' file not"
- " found (%s)" % _ErrnoOrStr(err))
+ return False, ("Required file 'ganeti_api_version' file not"
+ " found under path %s: %s" % (os_dir, _ErrnoOrStr(err)))
if not stat.S_ISREG(stat.S_IFMT(st.st_mode)):
- raise errors.InvalidOS(name, os_dir, "'ganeti_api_version' file is not"
- " a regular file")
+ return False, ("File 'ganeti_api_version' file at %s is not"
+ " a regular file" % os_dir)
try:
f = open(api_file)
@@ -1544,17 +1537,17 @@ def _OSOndiskVersion(name, os_dir):
finally:
f.close()
except EnvironmentError, err:
- raise errors.InvalidOS(name, os_dir, "error while reading the"
- " API version (%s)" % _ErrnoOrStr(err))
+ return False, ("Error while reading the API version file at %s: %s" %
+ (api_file, _ErrnoOrStr(err)))
api_versions = [version.strip() for version in api_versions]
try:
api_versions = [int(version) for version in api_versions]
except (TypeError, ValueError), err:
- raise errors.InvalidOS(name, os_dir,
- "API version is not integer (%s)" % str(err))
+ return False, ("API version(s) can't be converted to integer: %s" %
+ str(err))
- return api_versions
+ return True, api_versions
def DiagnoseOS(top_dirs=None):
@@ -1565,8 +1558,12 @@ def DiagnoseOS(top_dirs=None):
search (if not given defaults to
L{constants.OS_SEARCH_PATH})
@rtype: list of L{objects.OS}
- @return: an OS object for each name in all the given
- directories
+ @return: a list of tuples (name, path, status, diagnose)
+ for all (potential) OSes under all search paths, where:
+ - name is the (potential) OS name
+ - path is the full path to the OS
+ - status True/False is the validity of the OS
+ - diagnose is the error message for an invalid OS, otherwise empty
"""
if top_dirs is None:
@@ -1581,16 +1578,18 @@ def DiagnoseOS(top_dirs=None):
logging.exception("Can't list the OS directory %s", dir_name)
break
for name in f_names:
- try:
- os_inst = OSFromDisk(name, base_dir=dir_name)
- result.append(os_inst)
- except errors.InvalidOS, err:
- result.append(objects.OS.FromInvalidOS(err))
+ os_path = os.path.sep.join([dir_name, name])
+ status, os_inst = _TryOSFromDisk(name, base_dir=dir_name)
+ if status:
+ diagnose = ""
+ else:
+ diagnose = os_inst
+ result.append((name, os_path, status, diagnose))
- return result
+ return True, result
-def OSFromDisk(name, base_dir=None):
+def _TryOSFromDisk(name, base_dir=None):
"""Create an OS instance from disk.
This function will return an OS instance if the given name is a
@@ -1600,24 +1599,26 @@ def OSFromDisk(name, base_dir=None):
@type base_dir: string
@keyword base_dir: Base directory containing OS installations.
Defaults to a search in all the OS_SEARCH_PATH dirs.
- @rtype: L{objects.OS}
- @return: the OS instance if we find a valid one
- @raise errors.InvalidOS: if we don't find a valid OS
+ @rtype: tuple
+ @return: success and either the OS instance if we find a valid one,
+ or error message
"""
if base_dir is None:
os_dir = utils.FindFile(name, constants.OS_SEARCH_PATH, os.path.isdir)
if os_dir is None:
- raise errors.InvalidOS(name, None, "OS dir not found in search path")
+ return False, "Directory for OS %s not found in search path" % name
else:
os_dir = os.path.sep.join([base_dir, name])
- api_versions = _OSOndiskVersion(name, os_dir)
+ status, api_versions = _OSOndiskVersion(name, os_dir)
+ if not status:
+ # push the error up
+ return status, api_versions
if constants.OS_API_VERSION not in api_versions:
- raise errors.InvalidOS(name, os_dir, "API version mismatch"
- " (found %s want %s)"
- % (api_versions, constants.OS_API_VERSION))
+ return False, ("API version mismatch for path '%s': found %s, want %s." %
+ (os_dir, api_versions, constants.OS_API_VERSION))
# OS Scripts dictionary, we will populate it with the actual script names
os_scripts = dict.fromkeys(constants.OS_SCRIPTS)
@@ -1628,24 +1629,51 @@ def OSFromDisk(name, base_dir=None):
try:
st = os.stat(os_scripts[script])
except EnvironmentError, err:
- raise errors.InvalidOS(name, os_dir, "'%s' script missing (%s)" %
- (script, _ErrnoOrStr(err)))
+ return False, ("Script '%s' under path '%s' is missing (%s)" %
+ (script, os_dir, _ErrnoOrStr(err)))
if stat.S_IMODE(st.st_mode) & stat.S_IXUSR != stat.S_IXUSR:
- raise errors.InvalidOS(name, os_dir, "'%s' script not executable" %
- script)
+ return False, ("Script '%s' under path '%s' is not executable" %
+ (script, os_dir))
if not stat.S_ISREG(stat.S_IFMT(st.st_mode)):
- raise errors.InvalidOS(name, os_dir, "'%s' is not a regular file" %
- script)
+ return False, ("Script '%s' under path '%s' is not a regular file" %
+ (script, os_dir))
+
+ os_obj = objects.OS(name=name, path=os_dir, status=constants.OS_VALID_STATUS,
+ create_script=os_scripts[constants.OS_SCRIPT_CREATE],
+ export_script=os_scripts[constants.OS_SCRIPT_EXPORT],
+ import_script=os_scripts[constants.OS_SCRIPT_IMPORT],
+ rename_script=os_scripts[constants.OS_SCRIPT_RENAME],
+ api_versions=api_versions)
+ return True, os_obj
+
+
+def OSFromDisk(name, base_dir=None):
+ """Create an OS instance from disk.
+
+ This function will return an OS instance if the given name is a
+ valid OS name. Otherwise, it will raise an appropriate
+ L{RPCFail} exception, detailing why this is not a valid OS.
+
+ This is just a wrapper over L{_TryOSFromDisk}, which doesn't raise
+ an exception but returns true/false status data.
+
+ @type base_dir: string
+ @keyword base_dir: Base directory containing OS installations.
+ Defaults to a search in all the OS_SEARCH_PATH dirs.
+ @rtype: L{objects.OS}
+ @return: the OS instance if we find a valid one
+ @raise RPCFail: if we don't find a valid OS
+
+ """
+ status, payload = _TryOSFromDisk(name, base_dir)
+
+ if not status:
+ _Fail(payload)
+ return payload
- return objects.OS(name=name, path=os_dir, status=constants.OS_VALID_STATUS,
- create_script=os_scripts[constants.OS_SCRIPT_CREATE],
- export_script=os_scripts[constants.OS_SCRIPT_EXPORT],
- import_script=os_scripts[constants.OS_SCRIPT_IMPORT],
- rename_script=os_scripts[constants.OS_SCRIPT_RENAME],
- api_versions=api_versions)
def OSEnvironment(instance, debug=0):
"""Calculate the environment for an os script.
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 0676c2383..d983365b9 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -1801,10 +1801,11 @@ class LUDiagnoseOS(NoHooksLU):
@rtype: dict
@return: a dictionary with osnames as keys and as value another map, with
- nodes as keys and list of OS objects as values, eg::
+ nodes as keys and tuples of (path, status, diagnose) as values, eg::
- {"debian-etch": {"node1": [<object>,...],
- "node2": [<object>,]}
+ {"debian-etch": {"node1": [(/usr/lib/..., True, ""),
+ (/srv/..., False, "invalid api")],
+ "node2": [(/srv/..., True, "")]}
}
"""
@@ -1817,15 +1818,14 @@ class LUDiagnoseOS(NoHooksLU):
for node_name, nr in rlist.items():
if nr.RemoteFailMsg() or not nr.payload:
continue
- for os_serialized in nr.payload:
- os_obj = objects.OS.FromDict(os_serialized)
- if os_obj.name not in all_os:
+ for name, path, status, diagnose in nr.payload:
+ if 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_obj.name] = {}
+ all_os[name] = {}
for nname in good_nodes:
- all_os[os_obj.name][nname] = []
- all_os[os_obj.name][node_name].append(os_obj)
+ all_os[name][nname] = []
+ all_os[name][node_name].append((path, status, diagnose))
return all_os
def Exec(self, feedback_fn):
@@ -1842,11 +1842,12 @@ class LUDiagnoseOS(NoHooksLU):
if field == "name":
val = os_name
elif field == "valid":
- val = utils.all([osl and osl[0] for osl in os_data.values()])
+ val = utils.all([osl and osl[0][1] for osl in os_data.values()])
elif field == "node_status":
+ # this is just a copy of the dict
val = {}
- for node_name, nos_list in os_data.iteritems():
- val[node_name] = [(v.status, v.path) for v in nos_list]
+ for node_name, nos_list in os_data.items():
+ val[node_name] = nos_list
else:
raise errors.ParameterError(field)
row.append(val)
@@ -3156,10 +3157,11 @@ class LUReinstallInstance(LogicalUnit):
raise errors.OpPrereqError("Primary node '%s' is unknown" %
self.op.pnode)
result = self.rpc.call_os_get(pnode.name, self.op.os_type)
- result.Raise()
- if not isinstance(result.data, objects.OS):
+ msg = result.RemoteFailMsg()
+ if msg:
raise errors.OpPrereqError("OS '%s' not in supported OS list for"
- " primary node" % self.op.os_type)
+ " primary node %s: %s" %
+ (self.op.os_type, pnode.pname, msg))
self.instance = instance
@@ -4843,10 +4845,11 @@ class LUCreateInstance(LogicalUnit):
# os verification
result = self.rpc.call_os_get(pnode.name, self.op.os_type)
- result.Raise()
- if not isinstance(result.data, objects.OS):
+ msg = result.RemoteFailMsg()
+ if msg:
raise errors.OpPrereqError("OS '%s' not in supported os list for"
- " primary node" % self.op.os_type)
+ " primary node %s: %s" %
+ (self.op.os_type, pnode.name, msg))
_CheckNicsBridgesExist(self, self.nics, self.pnode.name)
diff --git a/scripts/gnt-os b/scripts/gnt-os
index a0c09a7ab..4cfa07bf0 100755
--- a/scripts/gnt-os
+++ b/scripts/gnt-os
@@ -64,6 +64,22 @@ def ListOS(opts, args):
return 0
+def _OsStatus(status, diagnose):
+ """Beautifier function for OS status.
+
+ @type status: boolean
+ @param status: is the OS valid
+ @type diagnose: string
+ @param diagnose: the error message for invalid OSes
+ @rtype: string
+ @return: a formatted status
+
+ """
+ if status:
+ return "valid"
+ else:
+ return "invalid - %s" % diagnose
+
def DiagnoseOS(opts, args):
"""Analyse all OSes on this cluster.
@@ -91,16 +107,17 @@ def DiagnoseOS(opts, args):
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_status == constants.OS_VALID_STATUS:
+ first_os_path, first_os_status, first_os_msg = node_info.pop(0)
+ first_os_msg = ("%s (path: %s)" % (_OsStatus(first_os_status,
+ first_os_msg),
+ first_os_path))
+ if first_os_status:
nodes_valid[node_name] = first_os_msg
else:
nodes_bad[node_name] = first_os_msg
- for hstatus, hpath in node_info:
+ for hpath, hstatus, hmsg in node_info:
nodes_hidden[node_name].append(" [hidden] path: %s, status: %s" %
- (hpath, hstatus))
+ (hpath, _OsStatus(hstatus, hmsg)))
else:
nodes_bad[node_name] = "OS not found"
--
GitLab