Keith Robertson has uploaded a new change for review.
Change subject: tools: Better API error handling
......................................................................
tools: Better API error handling
Print the actual exception string when attempting
to create an instance of the API and fix
the 'NoneType' error that sometimes occurs when
API creation fails.
Change-Id: Ib51002532b726cb7becdd4e4289d385119fa1ea3
Signed-off-by: Keith Robertson <krobe...@redhat.com>
---
M src/engine-iso-uploader.py
1 file changed, 77 insertions(+), 68 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-iso-uploader
refs/changes/54/8954/1
diff --git a/src/engine-iso-uploader.py b/src/engine-iso-uploader.py
index 05a26f5..5ebe0be 100644
--- a/src/engine-iso-uploader.py
+++ b/src/engine-iso-uploader.py
@@ -32,21 +32,21 @@
NFS_UMOUNT_OPTS = '-t nfs -f '
NFS_USER = 'vdsm'
NUMERIC_VDSM_ID = 36
-SUDO='/usr/bin/sudo'
-MOUNT='/bin/mount'
-UMOUNT='/bin/umount'
-SSH='/usr/bin/ssh'
-SCP='/usr/bin/scp'
-CP='/bin/cp'
-RM='/bin/rm -fv'
-MV='/bin/mv -fv'
-CHOWN='/bin/chown'
-CHMOD='/bin/chmod'
-TEST='/usr/bin/test'
-DEFAULT_CONFIGURATION_FILE='/etc/ovirt-engine/isouploader.conf'
-DEFAULT_LOG_FILE='/var/log/ovirt-engine/engine-iso-uploader.log'
-PERMS_MASK='640'
-PYTHON='/usr/bin/python'
+SUDO = '/usr/bin/sudo'
+MOUNT = '/bin/mount'
+UMOUNT = '/bin/umount'
+SSH = '/usr/bin/ssh'
+SCP = '/usr/bin/scp'
+CP = '/bin/cp'
+RM = '/bin/rm -fv'
+MV = '/bin/mv -fv'
+CHOWN = '/bin/chown'
+CHMOD = '/bin/chmod'
+TEST = '/usr/bin/test'
+DEFAULT_CONFIGURATION_FILE = '/etc/ovirt-engine/isouploader.conf'
+DEFAULT_LOG_FILE = '/var/log/ovirt-engine/engine-iso-uploader.log'
+PERMS_MASK = '640'
+PYTHON = '/usr/bin/python'
def multilog(logger, msg):
@@ -64,12 +64,12 @@
"""
A simple psudo-enumeration class to hold the current and future exit codes
"""
- NOERR=0
- CRITICAL=1
- LIST_ISO_ERR=2
- UPLOAD_ERR=3
- CLEANUP_ERR=4
- exit_code=NOERR
+ NOERR = 0
+ CRITICAL = 1
+ LIST_ISO_ERR = 2
+ UPLOAD_ERR = 3
+ CLEANUP_ERR = 4
+ exit_code = NOERR
class Commands():
"""
@@ -77,7 +77,7 @@
"""
LIST = 'list'
UPLOAD = 'upload'
- #DELETE = 'delete'
+ # DELETE = 'delete'
ARY = [LIST, UPLOAD]
class Caller(object):
@@ -106,7 +106,7 @@
logging.debug("STDERR(%s)" % stderr)
if returncode == 0:
- return (stdout,returncode)
+ return (stdout, returncode)
else:
raise Exception(stderr)
@@ -210,8 +210,8 @@
# we want the items from the ISOUploader section only
try:
- opts = ["--%s=%s" % (k,v)
- for k,v in cp.items("ISOUploader")]
+ opts = ["--%s=%s" % (k, v)
+ for k, v in cp.items("ISOUploader")]
(new_options, args) = self.parser.parse_args(args=opts,
values=self.options)
self.from_option_groups(new_options, self.parser)
self.from_options(new_options, self.parser)
@@ -355,6 +355,9 @@
url = "https://"; + self.configuration.get("engine") + "/api"
try:
+ logging.debug('user(%s) ca(%s) insecure(%s)' %
(self.configuration.get("user"),
+
self.configuration.get("engine_ca"),
+
self.configuration.get("insecure")))
self.api = API(url=url,
username=self.configuration.get("user"),
password=self.configuration.get("passwd"),
@@ -366,21 +369,22 @@
vrm = '%s.%s.%s' % (pi.get_version().get_major(),
pi.get_version().get_minor(),
pi.get_version().get_revision())
- logging.debug("API Vendor(%s)\tAPI Version(%s)" %
(pi.get_vendor(), vrm))
+ logging.debug("API Vendor(%s)\tAPI Version(%s)" %
(pi.get_vendor(), vrm))
else:
logging.error(_("Unable to connect to REST API."))
return False
except RequestError, re:
- logging.error(_("Unable to connect to REST API. Reason: %s")
% re.reason)
+ logging.error(_("Unable to connect to REST API. Reason: %s")
% re.reason)
return False
- except ConnectionError:
- logging.error(_("Problem connecting to the REST API. Is the
service available and does the CA certificate exist?"))
+ except ConnectionError, ce:
+ logging.error(_("Problem connecting to the REST API. \
+Is the service available and does the CA certificate exist? Error: %s") %
str(ce))
return False
except NoCertificatesError:
logging.error(_("Problem connecting to the REST API. The CA
is invalid. To override use the \'insecure\' option."))
return False
except Exception, e:
- logging.error(_("Unable to connect to REST API. Message: %s")
% e)
+ logging.error(_("Unable to connect to REST API. Message: %s")
% e)
return False
return True
@@ -421,10 +425,10 @@
print "\n".join(fmt % (name, dcName, status)
for name, dcName, status in isoAry)
else:
- ExitCodes.exit_code=ExitCodes.LIST_ISO_ERR
+ ExitCodes.exit_code = ExitCodes.LIST_ISO_ERR
logging.error(_("There are no ISO storage domains."))
else:
- ExitCodes.exit_code=ExitCodes.LIST_ISO_ERR
+ ExitCodes.exit_code = ExitCodes.LIST_ISO_ERR
logging.error(_("There are no datacenters with ISO storage
domains."))
def get_host_and_path_from_ISO_domain(self, isodomain):
@@ -435,7 +439,8 @@
(host, id, path)
"""
if not self._initialize_api():
- return
+ return (None, None, None)
+
sd = self.api.storagedomains.get(isodomain)
if sd is not None:
if sd.get_type() != 'iso':
@@ -498,11 +503,11 @@
"""
cmd = self.format_ssh_command()
- cmd += ' %s%s "%s -e %s"' % (user, address, TEST, file)
+ cmd += ' %s%s "%s -e %s"' % (user, address, TEST, file)
logging.debug(cmd)
returncode = 1
try:
- stdout,returncode = self.caller.call(cmd)
+ stdout, returncode = self.caller.call(cmd)
except:
pass
@@ -513,7 +518,7 @@
logging.debug("exists returning false")
return False
- def space_test_ssh(self, user, address,dir, file):
+ def space_test_ssh(self, user, address, dir, file):
"""
Function to test if the given file will fit on the given
remote directory. This function will return the available
@@ -525,8 +530,8 @@
cmd += """ %s%s "%s -c 'import os; dir_stat = os.statvfs(\\"%s\\");
print (dir_stat.f_bavail * dir_stat.f_frsize)'" """ % (user, address, PYTHON,
dir)
logging.debug('Mount point size test command is (%s)' % cmd)
try:
- dir_size,returncode = self.caller.call(cmd)
- except Exception,e:
+ dir_size, returncode = self.caller.call(cmd)
+ except Exception, e:
pass
if returncode == 0 and dir_size is not None:
@@ -534,9 +539,9 @@
dir_size = dir_size.strip()
file_size = os.path.getsize(file)
logging.debug("Size of %s:\t%s bytes\t%.1f 1K-blocks\t%.1f MB" %
- (file, file_size, file_size/1024,
(file_size/1024)/1024))
- logging.debug("Available space in %s:\t%s bytes\t%.1f
1K-blocks\t%.1f MB" %
- (dir, dir_size, float(dir_size)/1024,
(float(dir_size)/1024)/1024))
+ (file, file_size, file_size / 1024, (file_size /
1024) / 1024))
+ logging.debug("Available space in %s:\t%s bytes\t%.1f
1K-blocks\t%.1f MB" %
+ (dir, dir_size, float(dir_size) / 1024,
(float(dir_size) / 1024) / 1024))
return (dir_size, file_size)
else:
raise Exception("unable to test the available space on %s" % dir)
@@ -560,12 +565,12 @@
dir_size = (dir_stat.f_bavail * dir_stat.f_frsize)
file_size = os.path.getsize(file)
logging.debug("Size of %s:\t%s bytes\t%.1f 1K-blocks\t%.1f MB" %
- (file, file_size, file_size/1024, (file_size/1024)/1024))
- logging.debug("Available space in %s:\t%s bytes\t%.1f 1K-blocks\t%.1f
MB" %
- (dir, dir_size, dir_size/1024, (dir_size/1024)/1024))
+ (file, file_size, file_size / 1024, (file_size / 1024) /
1024))
+ logging.debug("Available space in %s:\t%s bytes\t%.1f 1K-blocks\t%.1f
MB" %
+ (dir, dir_size, dir_size / 1024, (dir_size / 1024) /
1024))
return (dir_size, file_size)
- def copy_file(self, src_file_name,dest_file_name, uid, gid):
+ def copy_file(self, src_file_name, dest_file_name, uid, gid):
"""
Copy a file from source to dest via file handles. The destination
file will be opened and written to as the UID and GID provided.
@@ -575,7 +580,7 @@
"""
retVal = True
logging.debug("euid(%s) egid(%s)" % (os.geteuid(), os.getegid()))
- umask_save = os.umask(0137) # Set to 660
+ umask_save = os.umask(0137) # Set to 660
try:
src = open(src_file_name, 'r')
os.setegid(gid)
@@ -594,7 +599,7 @@
dest.close()
return retVal
- def rename_file_nfs(self, src_file_name,dest_file_name, uid, gid):
+ def rename_file_nfs(self, src_file_name, dest_file_name, uid, gid):
"""
Rename a file from source to dest as the UID and GID provided.
This method will set the euid and egid to that which is provided
@@ -621,8 +626,8 @@
cmd += """ %s%s "%s %s %s" """ % (user, address, MV, src_file_name,
dest_file_name)
logging.debug('Rename file command is (%s)' % cmd)
try:
- stdout,returncode = self.caller.call(cmd)
- except Exception,e:
+ stdout, returncode = self.caller.call(cmd)
+ except Exception, e:
raise Exception("unable to move file from %s to %s" %
(src_file_name, dest_file_name))
def remove_file_nfs(self, file_name, uid, gid):
@@ -644,7 +649,7 @@
os.seteuid(0)
os.setegid(0)
- def remove_file_ssh(self,user, address, file):
+ def remove_file_ssh(self, user, address, file):
"""
This method will remove a file via SSH.
"""
@@ -653,8 +658,8 @@
cmd += """ %s%s "%s %s" """ % (user, address, RM, file)
logging.debug('Remove file command is (%s)' % cmd)
try:
- stdout,returncode = self.caller.call(cmd)
- except Exception,e:
+ stdout, returncode = self.caller.call(cmd)
+ except Exception, e:
raise Exception("unable to remove %s" % file)
def refresh_iso_domain(self, id):
@@ -669,8 +674,8 @@
sd = self.api.storagedomains.get(id=id)
if sd is not None and sd.files is not None:
sd.files.list()
- except Exception,e:
- logging.warn(_("failed to refresh the list of files available in
the %s ISO storage domain. Please refresh the list manually using the 'Refresh'
button in the oVirt Webadmin console." %
+ except Exception, e:
+ logging.warn(_("failed to refresh the list of files available in
the %s ISO storage domain. Please refresh the list manually using the 'Refresh'
button in the oVirt Webadmin console." %
self.configuration.get('iso_domain')))
logging.debug(e)
@@ -688,7 +693,11 @@
elif self.configuration.get('iso_domain'):
# Discover the hostname and path from the ISO domain.
(id, address, path) =
self.get_host_and_path_from_ISO_domain(self.configuration.get('iso_domain'))
- remote_path = os.path.join(id,DEFAULT_IMAGES_DIR)
+ if id:
+ remote_path = os.path.join(id, DEFAULT_IMAGES_DIR)
+ else:
+ logging.debug('Unable to get host and path information from
API.')
+ return
elif self.configuration.get('nfs_server'):
mnt = self.configuration.get('nfs_server')
(address, sep, path) = mnt.partition(':')
@@ -746,11 +755,11 @@
logging.error(_('There is not enough space in %s
(%s bytes) for %s (%s bytes)' %
(path, dir_size, file, file_size)))
else:
- ExitCodes.exit_code=ExitCodes.UPLOAD_ERR
+ ExitCodes.exit_code = ExitCodes.UPLOAD_ERR
logging.error(_('%s exists on %s. Either remove it or
supply the --force option to overwrite it.')
- % (file,address))
+ % (file, address))
except Exception, e:
- ExitCodes.exit_code=ExitCodes.UPLOAD_ERR
+ ExitCodes.exit_code = ExitCodes.UPLOAD_ERR
logging.error(_('Unable to copy %s to ISO storage
domain on %s.'
% (file,
self.configuration.get('iso_domain'))))
@@ -764,15 +773,15 @@
self.caller.call(cmd)
passwd = getpwnam(NFS_USER)
for file in self.configuration.files:
- dest_dir = os.path.join(tmpDir,remote_path)
+ dest_dir = os.path.join(tmpDir, remote_path)
dest_file = os.path.join(dest_dir, os.path.basename(file))
- retVal = self.exists_nfs(dest_file,NUMERIC_VDSM_ID,
NUMERIC_VDSM_ID)
+ retVal = self.exists_nfs(dest_file, NUMERIC_VDSM_ID,
NUMERIC_VDSM_ID)
if conf.get('force') or not retVal:
try:
# Remove the file if it exists before checking
space.
if retVal:
self.remove_file_nfs(dest_file,
NUMERIC_VDSM_ID, NUMERIC_VDSM_ID)
- (dir_size, file_size) =
self.space_test_nfs(dest_dir,file, NUMERIC_VDSM_ID, NUMERIC_VDSM_ID)
+ (dir_size, file_size) =
self.space_test_nfs(dest_dir, file, NUMERIC_VDSM_ID, NUMERIC_VDSM_ID)
if (dir_size > file_size):
temp_dest_file = os.path.join(dest_dir, '.%s'
% os.path.basename(file))
if self.copy_file(file,
@@ -791,7 +800,7 @@
logging.error(_('There is not enough space in
%s (%s bytes) for %s (%s bytes)' %
(path, dir_size, file,
file_size)))
except Exception, e:
- ExitCodes.exit_code=ExitCodes.UPLOAD_ERR
+ ExitCodes.exit_code = ExitCodes.UPLOAD_ERR
logging.error(_('Unable to copy %s to ISO storage
domain on %s.'
% (file,
self.configuration.get('iso_domain') if
@@ -799,18 +808,18 @@
self.configuration.get('nfs_server'))))
logging.error(_('Error message is "%s"') %
str(e).strip())
else:
- ExitCodes.exit_code=ExitCodes.UPLOAD_ERR
+ ExitCodes.exit_code = ExitCodes.UPLOAD_ERR
logging.error(_('%s exists on %s. Either remove it or
supply the --force option to overwrite it.')
- % (file,address))
+ % (file, address))
except KeyError, k:
- ExitCodes.exit_code=ExitCodes.CRITICAL
+ ExitCodes.exit_code = ExitCodes.CRITICAL
logging.error(_("A user named %s with a UID and GID of %d must
be defined on the system to mount the ISO storage domain on %s as Read/Write"
% (NFS_USER,
NUMERIC_VDSM_ID,
self.configuration.get('iso_domain'))))
except Exception, e:
- ExitCodes.exit_code=ExitCodes.CRITICAL
+ ExitCodes.exit_code = ExitCodes.CRITICAL
logging.error(e)
finally:
try:
@@ -819,7 +828,7 @@
self.caller.call(cmd)
shutil.rmtree(tmpDir)
except Exception, e:
- ExitCodes.exit_code=ExitCodes.CLEANUP_ERR
+ ExitCodes.exit_code = ExitCodes.CLEANUP_ERR
logging.debug(e)
if __name__ == '__main__':
@@ -962,7 +971,7 @@
if conf and (conf.get("verbose")):
logging.debug(_("Configuration:"))
logging.debug(_("command: %s") % conf.command)
- #multilog(logging.debug, pprint.pformat(conf))
+ # multilog(logging.debug, pprint.pformat(conf))
multilog(logging.debug, traceback.format_exc())
sys.exit(ExitCodes.CRITICAL)
--
To view, visit http://gerrit.ovirt.org/8954
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib51002532b726cb7becdd4e4289d385119fa1ea3
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-iso-uploader
Gerrit-Branch: master
Gerrit-Owner: Keith Robertson <krobe...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
