From 4cb9b33e90d32d140b9db2545df4c8076531e330 Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Thu, 14 Jul 2016 19:02:35 -0300 Subject: [PATCH] Fixing wok_log redundancy and i18n.py error messages This patch removes wok_log calls when they are just reproducing the same message of a call to a wok.exception (NotFoundError, OperationFailed ...) class. Wok exceptions already write the messages into the log and calling wok_log is redundant. Any other use of wok_log was preserved (logging a message that didn't appear to the user, for example). i18n.py messages were fixed along the way (wrong use of message strings, typos, etc). Affected files: model/swaps.py, model/users.py, model/utils.py, model/vol_group.py Signed-off-by: Daniel Henrique Barboza --- i18n.py | 37 ++++++------ model/swaps.py | 10 +--- model/users.py | 70 ++++------------------- model/utils.py | 119 ++------------------------------------- model/vol_group.py | 17 ++---- tests/test_swap.py | 20 ++----- tests/test_user_model.py | 68 ++++------------------ 7 files changed, 59 insertions(+), 282 deletions(-) diff --git a/i18n.py b/i18n.py index 2b41ab43..23c39da7 100644 --- a/i18n.py +++ b/i18n.py @@ -121,12 +121,12 @@ "GINUSER0006E": _("Could not add user '%(user)s' to kvm group."), "GINUSER0007E": _("Could not add user '%(user)s' to sudoers list, error: '%(err)s'."), "GINUSER0008E": _("User/Login '%(user)s' already in use."), - "GINUSER0009E": _("There was a problem while creating user '%(user)s', error: '%(err)s'."), - "GINUSER0010E": _("There was a problem while deleting user '%(user)s', error: '%(err)s'."), + "GINUSER0009E": _("There was a problem while creating user '%(user)s', error: user name is empty."), + "GINUSER0010E": _("There was a problem while deleting user '%(user)s', error: user name is empty."), "GINUSER0011E": _("User '%(user)s' does not exist."), - "GINUSER0012E": _("There was a problem while deleting group '%(group)s', error: '%(err)s'."), + "GINUSER0012E": _("There was a problem while deleting group '%(group)s'. Group name is an empty string."), "GINUSER0013E": _("Failed to remove user '%(user)s' from sudoers list"), - "GINUSER0014E": _("There was a problem while creating group '%(group)s', error: %(err)s'."), + "GINUSER0014E": _("There was a problem while creating group '%(group)s'. Group name is an empty string."), "GINUSER0015E": _("User 'no_login' should be boolean."), 'GINUSER0016E': _("Deletion is not supported for users found in '/etc/sudoers' file. User: %(user)s"), 'GINUSER0017E': _("Deletion is not supported for group found in '/etc/sudoers' file. Group: %(group)s"), @@ -137,6 +137,9 @@ "GINUSER0022E": _("There was a problem while reading users details, see log for details"), "GINUSER0023E": _("There was a problem while fetching user profile, see log for details"), "GINUSER0024E": _("Failed to change password for user '%(user)s', error: '%(err)s'."), + "GINUSER0025E": _("There was a problem while creating user '%(user)s', error: group id %(gid)s is not of integer type."), + "GINUSER0026E": _("There was a problem while creating user '%(user)s', error: no_login id %(no_login)s is not of boolean type."), + "GINUSER0027E": _("Failed to remove user '%(user)s' from group '%(group)s'. Error: group name is an empty string."), "GINFW0001E": _("Cannot update system firmware while running VMs."), "GINFW0002E": _("Firmware image unpack failed: rc = %(rc)s. " @@ -212,21 +215,22 @@ "GINSP00003E": _("Size is mandatory while creating file type swap device."), "GINSP00004E": _("Incorrect swap type, only 'device' and 'file' are allowed."), "GINSP00005E": _("Error creating a swap device. %(err)s "), - "GINSP00006E": _("Error deleting a swap file."), + "GINSP00006E": _("Error deleting a swap file: %(err)s"), "GINSP00007E": _("Error deleting a swap device. %(err)s"), "GINSP00008E": _("Swap device not found. %(name)s"), - "GINSP00009E": _("Error deleting a swap file. %(err)s"), + "GINSP00009E": _("Error deleting a swap device. %(err)s"), "GINSP00010E": _("Error parsing /proc/swaps file. %(err)s"), "GINSP00011E": _("Error creating a flat file. %(err)s"), "GINSP00012E": _("Unable to format swap device. %(err)s"), "GINSP00013E": _("Unable to activate swap device. %(err)s"), "GINSP00014E": _("Unable to parse 'swapon -s' output. %(err)s"), "GINSP00015E": _("Unable to get single swap device info. %(err)s"), - "GINSP00016E": _("Unable to activate swap device. %(err)s"), - "GINSP00017E": _("No partitions found for disk . %(disk)s"), + "GINSP00016E": _("Unable to deactivate swap device. %(err)s"), + "GINSP00017E": _("No partitions found for disk %(disk)s"), "GINSP00018E": _("Single swap device %(swap)s not found."), "GINSP00019E": _("Unable to get single swap device info: directory /proc/swaps not found."), - "GINSP00020E": _("File already in use."), + "GINSP00020E": _("File %(file)s already in use."), + "GINSP00021E": _("Unable to change the partition type. Error: %(err)s"), "GINPART00001E": _("Fetching list of partitions failed. Error: %(err)s"), "GINPART00002E": _("Create partition failed. Error: %(err)s"), @@ -242,7 +246,7 @@ "GINPART00013E": _("No partitions found"), "GINPART00014E": _("Partition %(name)s not found."), - "GINLVM0001E": _("Error parsing output of 'lvm version' command.. %(err)s"), + "GINLVM0001E": _("Error parsing output of 'lvm version' command: %(err)s"), "GINLVM0002E": _("Error executing 'lvm version' command. %(err)s"), "GINLVM0003E": _("Incompatible output from 'lvm version' command. %(err)s"), @@ -258,12 +262,12 @@ "GINPV00010E": _("Remove failed: device %(dev)s not found."), "GINPV00011E": _("Unable to find device %(dev)s ."), - "GINVG00001E": _("Failed to create VG"), - "GINVG00002E": _("Failed to list VGs"), - "GINVG00003E": _("Failed to fetch VG details"), - "GINVG00004E": _("Failed to delete VG"), - "GINVG00005E": _("Failed to extend VG"), - "GINVG00006E": _("Failed to reduce VG"), + "GINVG00001E": _("Failed to create VG %(name)s. Error: %(err)s"), + "GINVG00002E": _("Failed to list VGs. Error: %(err)s"), + "GINVG00003E": _("Failed to fetch VG %(name)s details."), + "GINVG00004E": _("Failed to delete VG %(name)s."), + "GINVG00005E": _("Failed to extend VG %(name)s."), + "GINVG00006E": _("Failed to reduce VG %(name)s."), "GINVG00007E": _("vgs command failed"), "GINVG00008E": _("vgs command for given VG failed"), "GINVG00009E": _("vgcreate command failed"), @@ -301,6 +305,7 @@ "GINDASD0012E": _("Error in executing 'lscss -d' command : %(err)s"), "GINDASD0013E": _("Error in parsing 'lscss -d' command : %(err)s"), "GINDASD0014E": _("No more than %(max_dasd_fmt)s concurrent DASD format operations are permitted."), + "GINDASD0015E": _("Header is empty for given pattern."), "GINDASDPAR0005E": _("Require name to create DASD device partition"), "GINDASDPAR0006E": _("Require partition size to create DASD device partition"), diff --git a/model/swaps.py b/model/swaps.py index 942e5182..a67e99a7 100644 --- a/model/swaps.py +++ b/model/swaps.py @@ -27,7 +27,7 @@ from wok.exception import InvalidParameter, NotFoundError, OperationFailed from wok.model.tasks import TaskModel from wok.rollbackcontext import RollbackContext -from wok.utils import add_task, run_command, wok_log +from wok.utils import add_task, run_command class SwapsModel(object): @@ -44,15 +44,12 @@ def create(self, params): file_loc = '' if 'file_loc' not in params or not params['file_loc']: - wok_log.error("File location required for creating a swap device.") raise InvalidParameter('GINSP00001E') if 'type' not in params: - wok_log.error("Type required for creating a swap device.") raise InvalidParameter('GINSP00002E') else: if params['type'] == 'file' and 'size' not in params: - wok_log.error("Size is required file type swap device.") raise InvalidParameter('GINSP00003E') if params['type'] == 'device' or params['type'] == 'file': @@ -60,7 +57,6 @@ def create(self, params): self._create_task, self.objstore, params) return self.task.lookup(taskid) else: - wok_log.error("Incorrect swap type.") raise InvalidParameter('GINSP00004E') def _create_task(self, cb, params): @@ -131,8 +127,7 @@ def delete_swap_file(file_loc): os.remove(file_loc) except Exception as e: - wok_log.error("Error deleting a swap file, %s", e.message) - raise OperationFailed('GINSP00006E') + raise OperationFailed('GINSP00006E', {'err': e.message}) def get_list(self): out, err, rc = run_command(["cat", "/proc/swaps"]) @@ -166,5 +161,4 @@ def delete(self, name): else: utils._swapoff_device(name) except Exception as e: - wok_log.error("Error deleting a swap device, %s", e.message) raise OperationFailed("GINSP00009E", {'err': e.message}) diff --git a/model/users.py b/model/users.py index 0e6e81b4..48c213c7 100644 --- a/model/users.py +++ b/model/users.py @@ -68,13 +68,8 @@ def create_group(groupname): :param groupname: non-empty string :return: group id(gid) """ - wok_log.info('in create_group() method. group name: %s' % groupname) if not isinstance(groupname, str) or not groupname.strip(): - wok_log.error('group name is not non-empty string. ' - 'group name %s' % groupname) - raise InvalidParameter('GINUSER0014E', - {'group': groupname, - 'err': 'see log for details'}) + raise InvalidParameter('GINUSER0014E', {'group': groupname}) adm = libuser.admin() if adm.lookupGroupByName(groupname): raise OperationFailed('GINUSER0018E', {'group': groupname}) @@ -83,7 +78,6 @@ def create_group(groupname): if new_group[libuser.GIDNUMBER][0] < 1000: new_group.set(libuser.GIDNUMBER, adm.getFirstUnusedGid(1000)) adm.addGroup(new_group) - wok_log.info('successfully created group. group name: %s.' % groupname) return new_group[libuser.GIDNUMBER][0] except Exception as e: @@ -91,13 +85,9 @@ def create_group(groupname): def delete_group(groupname): - wok_log.info('in delete_group(). group name: %s' % groupname) if not isinstance(groupname, str) or not groupname.strip(): - wok_log.error('group name is not non-empty string. ' - 'group name %s' % groupname) raise InvalidParameter('GINUSER0012E', - {'group': groupname, - 'err': 'see log for details'}) + {'group': groupname}) adm = libuser.admin() group_id = int(get_group_gid(groupname)) @@ -167,21 +157,13 @@ def create_user(name, plain_passwd, gid, no_login=False): :param gid: primary group id for user :param no_login: True/False for log in shell """ - wok_log.info('in create_user() method') if not isinstance(name, str) or not name.strip(): - wok_log.error('user name is not non-empty string. name: %s' % name) - raise InvalidParameter( - 'GINUSER0009E', {'user': name, 'err': 'see log for details'}) + raise InvalidParameter('GINUSER0009E', {'user': name}) if not type(gid) in [int, long]: - wok_log.error('group id is not of integer type. gid: %s' % gid) - raise InvalidParameter('GINUSER0009E', {'user': name, - 'err': 'see log for details'}) + raise InvalidParameter('GINUSER0025E', {'user': name, 'gid': gid}) if not isinstance(no_login, bool): - wok_log.error('no_login id is not of boolean type.' - ' no_login: %s' % no_login) - raise InvalidParameter('GINUSER0009E', - {'user': name, - 'err': 'see log for details'}) + raise InvalidParameter('GINUSER0026E', {'user': name, + 'no_login': no_login}) adm = libuser.admin() if adm.lookupUserByName(name): raise OperationFailed('GINUSER0008E', {'user': name}) @@ -229,11 +211,8 @@ def delete_user(username): method to delete user :param username: user name """ - wok_log.info('in delete_user() method') if not isinstance(username, str) or not username.strip(): - wok_log.error('username is not non-empty string. name: %s' % username) - raise InvalidParameter('GINUSER0010E', {'user': username, - 'err': 'see log for details'}) + raise InvalidParameter('GINUSER0010E', {'user': username}) if username in get_sudoers(admin_check=False): raise OperationFailed('GINUSER0016E', {'user': username}) @@ -253,7 +232,6 @@ def delete_user(username): try: os.unlink(f) except Exception as e: - wok_log.error('Error removing file "%s": %s' % (f, e)) raise OperationFailed('GINUSER0013E', {'user': username}) try: adm.deleteUser(user_obj, True, True) @@ -268,16 +246,11 @@ def remove_user_from_group(user, group): :param user: user name :param group: group name """ - wok_log.info('in remove_user_from_group() method') if not isinstance(user, str) or not user.strip(): - wok_log.error('user name is not non-empty string. name: %s' % user) - raise InvalidParameter('GINUSER0010E', - {'user': user, 'err': 'see log for details'}) + raise InvalidParameter('GINUSER0010E', {'user': user}) if not isinstance(group, str) or not group.strip(): - wok_log.error('group name is not non-empty string. ' - 'group name %s' % group) - raise InvalidParameter('GINUSER0010E', - {'user': user, 'err': 'see log for details'}) + raise InvalidParameter('GINUSER0027E', + {'user': user, 'group': group}) try: adm = libuser.admin() grpobj = adm.lookupGroupByName(group) @@ -300,7 +273,6 @@ def get_sudoers(admin_check=True): :param admin_check: True/False (to check admin/just part of sudoers file) :return: list of users and groups """ - wok_log.info('in get_sudoers() method') sudoers = [] try: parser = augeas.Augeas() @@ -340,7 +312,6 @@ def get_users_info(users): :param users: list of users :return: list of dictionary with user details """ - wok_log.info('in get_users_info(). Users: %s' % users) if not isinstance(users, list): wok_log.error('Users is not of type list. Users: %s' % users) raise InvalidParameter('GINUSER0022E') @@ -381,7 +352,6 @@ def get_user_profile(user, sudoers=None): :return: admin, virtuser or regularuser """ if not isinstance(user, str) or not user.strip(): - wok_log.error('username is not non-empty string. name: %s' % user) raise InvalidParameter('GINUSER0002E') if sudoers and not isinstance(sudoers, list): wok_log.error('Sudoers is not of type list. Sudoers: %s' % sudoers) @@ -452,10 +422,7 @@ def _validate_create_params(self, params): :return: params dictionary with keys: name, password, profile, group and no_login """ - wok_log.info('in _validate_create_params(). params: %s' % params) if not isinstance(params, dict): - wok_log.error( - 'Params is not of type dictionary. Params: %s' % params) raise InvalidParameter('GINUSER0001E') keys = params.keys() if isinstance(params['name'], unicode): @@ -496,17 +463,12 @@ def _validate_create_params(self, params): params['profile'] = 'regularuser' params['no_login'] = False - wok_log.info( - 'End of _validate_create_params(). Return params: %s' % params) return params def _add_user_to_kvm_group(self, user): # Add new user to KVM group if not isinstance(user, str) or not user.strip(): - wok_log.error('username is not non-empty string. name: %s' % user) - raise InvalidParameter('GINUSER0009E', - {'user': user, - 'err': 'see log for details'}) + raise InvalidParameter('GINUSER0009E', {'user': user}) adm = libuser.admin() kvmgrp = get_group_obj('kvm') if isinstance(user, unicode): @@ -518,10 +480,7 @@ def _add_user_to_kvm_group(self, user): def _add_user_to_sudoers(self, user): if not isinstance(user, str) or not user.strip(): - wok_log.error('username is not non-empty string. name: %s' % user) - raise InvalidParameter('GINUSER0009E', - {'user': user, - 'err': 'see log for details'}) + raise InvalidParameter('GINUSER0009E', {'user': user}) try: # Creates the file in /etc/sudoers.d with proper user permission with open(SUDOERS_FILE % user, 'w') as f: @@ -570,7 +529,6 @@ def lookup(self, user): return user_info[0] def chpasswd(self, user, password): - wok_log.info("In UserModel.chpasswd method") if isinstance(user, unicode): user = user.encode('utf-8') if not isinstance(user, str) or not user.strip(): @@ -582,7 +540,6 @@ def chpasswd(self, user, password): adm = libuser.admin() user_obj = adm.lookupUserByName(user) if not user_obj: - wok_log.error("User '%s' not found" % user) raise NotFoundError('GINUSER0011E', {'user': user}) try: adm.setpassUser(user_obj, password, False) @@ -590,8 +547,5 @@ def chpasswd(self, user, password): wok_log.info("Successfully changed password for user '%s'" % user) except Exception as e: err_msg = e.message if e.message else e - wok_log.error("Failed to change password for user '%s'. " - "Error: '%s'" % (user, err_msg)) raise OperationFailed( 'GINUSER0024E', {'user': user, 'err': err_msg}) - wok_log.info("End of UserModel.chpasswd method") diff --git a/model/utils.py b/model/utils.py index 12dce6a2..02c39655 100644 --- a/model/utils.py +++ b/model/utils.py @@ -81,7 +81,6 @@ def _get_swapdev_list_parser(output): dev_name = swapdev.split()[0] output_list.append(dev_name) except Exception as e: - wok_log.error("Error parsing /proc/swaps file.") raise OperationFailed("GINSP00010E", {'err', e.message}) return output_list @@ -99,9 +98,7 @@ def _create_file(size, file_loc): if rc != 0: if "Text file busy" in err: - wok_log.error("file already in use. %s", file_loc) - raise InvalidParameter("GINSP00020E") - wok_log.error("Error creating a flat file. %s", file_loc) + raise InvalidParameter("GINSP00020E", {'file': file_loc}) raise OperationFailed("GINSP00011E", {'err': err}) # So that only root can see the content of the swap @@ -121,7 +118,6 @@ def _make_swap(file_loc): out, err, rc = run_command(["mkswap", file_loc]) if rc != 0: - wok_log.error("Unable to format swap device. %s", file_loc) raise OperationFailed("GINSP00012E", {'err': err}) return @@ -134,7 +130,6 @@ def _activate_swap(file_loc): """ out, err, rc = run_command(["swapon", file_loc]) if rc != 0: - wok_log.error("Unable to activate swap device. %s", file_loc) raise OperationFailed("GINSP00013E", {'err': err}) return @@ -153,7 +148,6 @@ def _parse_swapon_output(output): output_dict['used'] = int(output_list[3]) output_dict['priority'] = output_list[4] except Exception as e: - wok_log.error("Unable to parse 'swapon -s' output") raise OperationFailed("GINSP00014E", {'err': e.message}) return output_dict @@ -167,16 +161,12 @@ def _get_swap_output(device_name): out, err, rc = run_command(["grep", "-w", device_name, "/proc/swaps"]) if rc == 1: - wok_log.error("Single swap device %s not found.", device_name) raise NotFoundError("GINSP00018E", {'swap': device_name}) elif rc == 2: - wok_log.error("Unable to get single swap device info: /proc/swaps " - "dir not found.") raise OperationFailed("GINSP00019E") elif rc != 0: - wok_log.error("Unable to get single swap device info. %s", device_name) raise OperationFailed("GINSP00015E", {'err': err}) return _parse_swapon_output(out) @@ -191,11 +181,8 @@ def _swapoff_device(device_name): out, err, rc = run_command(["swapoff", device_name]) if rc != 0: - wok_log.error("Unable to deactivate swap device. %s", device_name) raise OperationFailed("GINSP00016E", {'err': err}) - return - def get_dm_name(devname): """ @@ -236,9 +223,7 @@ def change_part_type(part, type_hex): elif len(parts) > 1: typ_str = '\nt\n' + partnum + '\n' + type_hex + '\n' + 'w\n' else: - wok_log.error("No partitions found for disk, %s", disk) - raise OperationFailed("GINSP00017E", - {'disk': "No partitions found for disk " + disk}) + raise OperationFailed("GINSP00017E", {'disk': disk}) t1_out = subprocess.Popen(["echo", "-e", "\'", typ_str, "\'"], stdout=subprocess.PIPE) @@ -249,8 +234,7 @@ def change_part_type(part, type_hex): out, err = t2_out.communicate() if t2_out.returncode != 0: - wok_log.error("Unable to change the partition type.") - raise OperationFailed("change type failed", err) + raise OperationFailed("GINSP00021E", {'err': err}) return part @@ -740,7 +724,6 @@ def get_disks_by_id_out(): cmd = ['ls', '-l', '/dev/disk/by-id'] out, err, rc = run_command(cmd) if rc != 0: - wok_log.error("Error executing 'ls -l /dev/disk/by-id.") raise OperationFailed("GINSD00001E", {'err': err}) return out @@ -761,7 +744,6 @@ def get_lsblk_keypair_out(transport=True): out, err, rc = run_command(cmd) if rc != 0: - wok_log.error("Error executing 'lsblk -Po") raise OperationFailed("GINSD00002E", {'err': err}) return out @@ -811,7 +793,6 @@ def parse_ll_out(ll_out): else: return_id_dict[disk_id] = [name] except Exception as e: - wok_log.error("Error parsing 'ls -l /dev/disk/by-id'") raise OperationFailed("GINSD00003E", {'err': e.message}) return return_dict, return_id_dict @@ -847,7 +828,6 @@ def parse_lsblk_out(lsblk_out): return_dict[disk_attrs[0].split("=")[1][1:-1]] = disk_info except Exception as e: - wok_log.error("Error parsing 'lsblk -Po") raise OperationFailed("GINSD00004E", {'err': e.message}) return return_dict @@ -913,7 +893,6 @@ def get_final_list(): else: final_list.append(final_dict) except Exception as e: - wok_log.error("Error getting list of storage devices") raise OperationFailed("GINSD00005E", {'err': e.message}) return final_list @@ -955,89 +934,6 @@ def get_fc_path_elements(): return fc_blk_dict -def get_storagedevice(device): - """ - get the device info dict for the device passed as parameter. - Raises exception on failure of lscss execution or device is None/blank - :param device: device id for which we need info to be returned - :return: device info dict - """ - device = _validate_device(device) - if device and isinstance(device, unicode): - device = device.encode('utf-8') - device = str(device) - if dasd_utils._is_dasdeckd_device(device) or _is_zfcp_device(device): - command = [lscss, '-d', device] - msg = 'The command is "%s" ' % ' '.join(command) - wok_log.debug(msg) - out, err, rc = run_command(command) - messge = 'The output of command "%s" is %s' % (' '. - join(command), out) - wok_log.debug(messge) - if rc: - err = err.strip().replace("lscss:", '').strip() - wok_log.error(err) - raise OperationFailed("GS390XCMD0001E", - {'command': ' '.join(command), - 'rc': rc, 'reason': err}) - if out.strip(): - device_info = _get_deviceinfo(out, device) - return device_info - wok_log.error("lscss output is either blank or None") - raise OperationFailed("GS390XCMD0001E", - {'command': ' '.join(command), - 'rc': rc, 'reason': out}) - else: - wok_log.error("Given device id is of type dasd-eckd or zfcp. " - "Device: %s" % device) - raise InvalidParameter("GS390XINVINPUT", - {'reason': 'given device is not of type ' - 'dasd-eckd or zfcp. ' - 'Device : %s' % device}) - - -def _validate_device(device): - """ - validate the device id. Valid device Ids should have - ..<4 digit hexadecimalnumber> - or <4 digit hexadecimal number> - :param device: device id - """ - pattern_with_dot = r'^\d\.\d\.[0-9a-fA-F]{4}$' - if device and isinstance(device, unicode): - device = device.encode('utf-8') - device = str(device) - if device and not device.isspace(): - device = device.strip() - if "." in device: - out = re.search(pattern_with_dot, device) - else: - device = '0.0.' + device - out = re.search(pattern_with_dot, device) - if out is None: - wok_log.error("Invalid device id. Device: %s" % device) - raise InvalidParameter("GS390XINVINPUT", - {'reason': 'invalid device id: %s' - % device}) - else: - wok_log.error("Device id is empty. Device: %s" % device) - raise InvalidParameter("GS390XINVINPUT", - {'reason': 'device id is required. Device: %s' - % device}) - return device - - -def _is_zfcp_device(device): - """ - Return True if the device is of type zfcp otherwise False - :param device: device id - """ - zfcp_devices = _get_zfcp_devices() - if device in zfcp_devices: - return True - return False - - def _get_deviceinfo(lscss_out, device): """ :param lscss_out: out of lscss command @@ -1189,9 +1085,7 @@ def get_row_data(command_output, header_pattern, value_pattern): value = re.search(value_pattern, command_output, re.M | re.I) row_data = {} if header is None or not header.group(): - wok_log.error("header is empty for given pattern") - raise OperationFailed("GS390XREG0001E", - {'reason': "header is empty for given pattern"}) + raise OperationFailed("GINDASD0015E") elif value: if (header.group() != value.group()) and \ (len(header.groups()) == len(value.groups())): @@ -1245,8 +1139,6 @@ def _parse_lvm_version(lvm_version_out): try: lvm_version = m.group(1) except Exception as e: - wok_log.error("Output of 'lvm version', %s" % lvm_version_out) - wok_log.error("Error parsing output of 'lvm version' command.") raise OperationFailed("GINLVM0001E", {'err': e.message}) return lvm_version @@ -1261,14 +1153,11 @@ def get_lvm_version(): ["lvm", "version"]) if rc != 0: - wok_log.error("Error executing 'lvm version' command, %s" % err) raise OperationFailed("GINLVM0002E", {'err': err}) try: out = out.splitlines()[0].split(":")[1] except Exception as e: - wok_log.error("Output of 'lvm version', %s" % out) - wok_log.error("Incompatible output from 'lvm version' command.") raise OperationFailed("GINLVM0003E", {'err': e.message}) return _parse_lvm_version(out) diff --git a/model/vol_group.py b/model/vol_group.py index be8acb90..27d70af3 100644 --- a/model/vol_group.py +++ b/model/vol_group.py @@ -23,7 +23,7 @@ from wok.exception import OperationFailed from wok.model.tasks import TaskModel -from wok.utils import add_task, wok_log +from wok.utils import add_task class VolumeGroupsModel(object): @@ -62,9 +62,8 @@ def _create_task(self, cb, params): utils._create_vg(vgname, pv_paths) except (OperationFailed), e: - wok_log.error('failed to create vg') raise OperationFailed('GINVG00001E', - {'vgname': vgname, + {'name': vgname, 'err': e.message}) cb('OK', True) @@ -74,7 +73,6 @@ def get_list(self): try: vg_names = utils._get_vg_list() except OperationFailed as e: - wok_log.error('failed to list VGs') raise NotFoundError("GINVG00002E", {'err': e.message}) @@ -94,7 +92,6 @@ def lookup(self, name): return utils._vgdisplay_out(name) except OperationFailed: - wok_log.error('failed to fetch vg info') raise NotFoundError("GINVG00003E", {'name': name}) def delete(self, name): @@ -102,24 +99,20 @@ def delete(self, name): vg_details = self.lookup(name) if vg_details['Cur LV'] > 0: - wok_log.error('VG %s contains logical volumes.' % name) raise InvalidOperation('GINVG00017E', {'name': name}) utils._remove_vg(name) except OperationFailed: - wok_log.error('failed to delete vg') - raise OperationFailed("GINVG00004E") + raise OperationFailed("GINVG00004E", {'name': name}) def extend(self, name, paths): try: utils._extend_vg(name, paths) except OperationFailed: - wok_log.error('failed to extend vg') - raise OperationFailed("GINVG00005E") + raise OperationFailed("GINVG00005E", {'name': name}) def reduce(self, name, paths): try: utils._reduce_vg(name, paths) except OperationFailed: - wok_log.error('failed to reduce vg') - raise OperationFailed("GINVG00006E") + raise OperationFailed("GINVG00006E", {'name': name}) diff --git a/tests/test_swap.py b/tests/test_swap.py index d32e2901..82bc3fe5 100644 --- a/tests/test_swap.py +++ b/tests/test_swap.py @@ -33,11 +33,10 @@ def setUp(self): objstore_loc = config.get_object_store() + '_ginger' self._objstore = ObjectStore(objstore_loc) - @mock.patch('wok.plugins.ginger.model.swaps.wok_log') @mock.patch('wok.plugins.ginger.model.swaps.add_task') @mock.patch('wok.plugins.ginger.model.swaps.TaskModel') def test_create_swap_missing_file_loc(self, mock_task_model, - mock_add_task, mock_wok_log): + mock_add_task): """ Test case to raise exception in case of missing file location in parameters @@ -55,14 +54,11 @@ def test_create_swap_missing_file_loc(self, mock_task_model, self.assertRaises(InvalidParameter, swap_model.create, params) self.assertFalse(mock_add_task.called, msg='Unexpected call to mock_add_task()') - msg = "File location required for creating a swap device." - mock_wok_log.error.assert_called_once_with(msg) - @mock.patch('wok.plugins.ginger.model.swaps.wok_log') @mock.patch('wok.plugins.ginger.model.swaps.add_task') @mock.patch('wok.plugins.ginger.model.swaps.TaskModel') def test_create_swap_missing_type(self, mock_task_model, - mock_add_task, mock_wok_log): + mock_add_task): """ Test case to raise exception in case of missing type in parameters """ @@ -79,14 +75,11 @@ def test_create_swap_missing_type(self, mock_task_model, self.assertRaises(InvalidParameter, swap_model.create, params) self.assertFalse(mock_add_task.called, msg='Unexpected call to mock_add_task()') - msg = "Type required for creating a swap device." - mock_wok_log.error.assert_called_once_with(msg) - @mock.patch('wok.plugins.ginger.model.swaps.wok_log') @mock.patch('wok.plugins.ginger.model.swaps.add_task') @mock.patch('wok.plugins.ginger.model.swaps.TaskModel') def test_create_swap_missing_size(self, mock_task_model, - mock_add_task, mock_wok_log): + mock_add_task): """ Test case to raise exception in case of missing size in parameters for type 'file' @@ -104,14 +97,11 @@ def test_create_swap_missing_size(self, mock_task_model, self.assertRaises(InvalidParameter, swap_model.create, params) self.assertFalse(mock_add_task.called, msg='Unexpected call to mock_add_task()') - msg = "Size is required file type swap device." - mock_wok_log.error.assert_called_once_with(msg) - @mock.patch('wok.plugins.ginger.model.swaps.wok_log') @mock.patch('wok.plugins.ginger.model.swaps.add_task') @mock.patch('wok.plugins.ginger.model.swaps.TaskModel') def test_create_swap_wrong_type(self, mock_task_model, - mock_add_task, mock_wok_log): + mock_add_task): """ Test case to raise exception in case of wrong type """ @@ -129,8 +119,6 @@ def test_create_swap_wrong_type(self, mock_task_model, self.assertRaises(InvalidParameter, swap_model.create, params) self.assertFalse(mock_add_task.called, msg='Unexpected call to mock_add_task()') - msg = "Incorrect swap type." - mock_wok_log.error.assert_called_once_with(msg) def test_swap_parser(self): """ diff --git a/tests/test_user_model.py b/tests/test_user_model.py index 14b131ba..7b175a02 100644 --- a/tests/test_user_model.py +++ b/tests/test_user_model.py @@ -33,26 +33,18 @@ class CreateGroupUnitTests(unittest.TestCase): """ Unit tests for create_group() method """ - @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) - def test_create_group_empty_name(self, mock_log): + def test_create_group_empty_name(self): """ unittest to validate create_group method with empty group name - mock_log: mock of wok_log imported in model.users """ self.assertRaises(InvalidParameter, create_group, '') - mock_log.error.assert_called_once_with( - 'group name is not non-empty string. group name %s' % '') - @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) - def test_create_group_invalid_input(self, mock_log): + def test_create_group_invalid_input(self): """ unittest to validate create_group method with invalid input for example integer value for group name instead of string - mock_log: mock of wok_log imported in model.users """ self.assertRaises(InvalidParameter, create_group, 12) - mock_log.error.assert_called_once_with( - 'group name is not non-empty string. group name %s' % 12) @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) @mock.patch('plugins.ginger.model.users.libuser', autospec=True) @@ -82,13 +74,11 @@ def test_create_grp_libuser_exception(self, mock_libuser, mock_log): self.assertRaises(OperationFailed, create_group, 'dummy_group') mock_adm.initGroup.assert_called_once_with('dummy_group') - @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) @mock.patch('plugins.ginger.model.users.libuser', autospec=True) - def test_create_grp_success(self, mock_libuser, mock_log): + def test_create_grp_success(self, mock_libuser): """ unittest to validate create_group success case mock_libuser: mock of libuser imported in model.users - mock_log: mock of wok_log imported in model.users """ mock_adm = mock_libuser.admin() mock_libuser.GIDNUMBER.return_value = 1200 @@ -98,34 +88,24 @@ def test_create_grp_success(self, mock_libuser, mock_log): self.assertTrue(mock_adm.addGroup.called, msg='Expected call to mock_adm.addGroup(). ' 'Not called') - mock_log.info.assert_called_with( - 'successfully created group. group name: %s.' % 'dummy_group') class DeleteGroupUnitTests(unittest.TestCase): """ Unit tests for delete_group() method """ - @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) - def test_delete_group_empty_name(self, mock_log): + def test_delete_group_empty_name(self): """ unittest to validate delete_group method with empty group name - mock_log: mock of wok_log imported in model.users """ self.assertRaises(InvalidParameter, delete_group, '') - mock_log.error.assert_called_once_with( - 'group name is not non-empty string. group name %s' % '') - @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) - def test_delete_group_invalid_input(self, mock_log): + def test_delete_group_invalid_input(self): """ unittest to validate delete_group method with invalid input for example integer value for group name instead of string - mock_log: mock of wok_log imported in model.users """ self.assertRaises(InvalidParameter, delete_group, 12) - mock_log.error.assert_called_once_with( - 'group name is not non-empty string. group name %s' % 12) @mock.patch('plugins.ginger.model.users.get_sudoers', autospec=True) @mock.patch('plugins.ginger.model.users.get_group_gid', autospec=True) @@ -213,7 +193,6 @@ def test_delete_group_success(self, mock_libuser, mock_get_gid, mock_get_sudoers): """ unittest to validate delete_group method success case - mock_log: mock of wok_log imported in model.users mock_libuser: mock of libuser imported in model.users mock_get_gid: mock of get_group_gid() method in model.users mock_get_sudoers: mock of get_sudoers() method in model.users @@ -330,26 +309,18 @@ class DeleteUserUnitTests(unittest.TestCase): """ Unit tests for delete_user() method """ - @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) - def test_delete_user_empty_name(self, mock_log): + def test_delete_user_empty_name(self): """ unittest to validate delete_user method with empty user name - mock_log: mock of wok_log imported in model.users """ self.assertRaises(InvalidParameter, delete_user, '') - mock_log.error.assert_called_once_with( - 'username is not non-empty string. name: %s' % '') - @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) - def test_delete_user_invalid_input(self, mock_log): + def test_delete_user_invalid_input(self): """ unittest to validate delete_user method with invalid input for example integer value for user name instead of string - mock_log: mock of wok_log imported in model.users """ self.assertRaises(InvalidParameter, delete_user, 12) - mock_log.error.assert_called_once_with( - 'username is not non-empty string. name: %s' % 12) @mock.patch('plugins.ginger.model.users.remove_user_from_group', autospec=True) @@ -421,13 +392,11 @@ def test_delete_user_not_found(self, mock_log, mock_libuser, mock_os, @mock.patch('plugins.ginger.model.users.get_sudoers', autospec=True) @mock.patch('plugins.ginger.model.users.os', autospec=True) @mock.patch('plugins.ginger.model.users.libuser', autospec=True) - @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) - def test_delete_user_unlink_exception(self, mock_log, mock_libuser, + def test_delete_user_unlink_exception(self, mock_libuser, mock_os, mock_get_sudoers, mock_remove_from_group): """ unittest to validate delete_user for exception in os.unlink - mock_log: mock of wok_log imported in model.users mock_libuser: mock of libuser imported in model.users mock_get_sudoers: mock of get_sudoers() method in model.users mock_remove_from_group: mock of remove_user_from_group() method @@ -441,8 +410,6 @@ def test_delete_user_unlink_exception(self, mock_log, mock_libuser, mock_get_sudoers.return_value = [] self.assertRaises(OperationFailed, delete_user, 'dummy_user') f = '/etc/sudoers.d/dummy_user_conf' - mock_log.error.assert_called_once_with( - 'Error removing file "%s": %s' % (f, 'fail')) mock_get_sudoers.assert_called_once_with(admin_check=False) mock_adm.lookupUserByName.assert_called_once_with('dummy_user') self.assertFalse(mock_adm.deleteUser.called, @@ -767,22 +734,18 @@ def test_user_profile_inv_sudoers(self, mock_log, mock_libuser, mock_os, @mock.patch('plugins.ginger.model.users.get_sudoers', autospec=True) @mock.patch('plugins.ginger.model.users.os', autospec=True) @mock.patch('plugins.ginger.model.users.libuser', autospec=True) - @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) - def test_user_profile_inv_user(self, mock_log, mock_libuser, mock_os, + def test_user_profile_inv_user(self, mock_libuser, mock_os, mock_get_sudoers): """ unittest to validate get_user_profile() method with invalid i/p for user i.e., passing empty string or integer - mock_log: mock of wok_log imported in model.users mock_libuser: mock of libuser imported in model.users mock_os: mock of os imported in model.users mock_get_sudoers: mock of get_sudoers() in model.users """ mock_adm = mock_libuser.admin() self.assertRaises(InvalidParameter, get_user_profile, 235) - mock_log.error.assert_called_once_with( - 'username is not non-empty string. name: %s' % 235) self.assertFalse(mock_adm.enumerateGroupsByUser.called, msg='Unexpected call to ' 'mock_adm.enumerateGroupsByUser') @@ -999,12 +962,10 @@ def test_chpasswd_invalid_password(self, mock_log, mock_libuser): msg='Unexpected call to mock_adm.setpassUser()') @mock.patch('plugins.ginger.model.users.libuser', autospec=True) - @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) - def test_chpasswd_user_notfound(self, mock_log, mock_libuser): + def test_chpasswd_user_notfound(self, mock_libuser): """ unittest to validate chpasswd() method for non-existing user - mock_log: mock of wok_log imported in model.users mock_libuser: mock of libuser imported in model.users """ mock_adm = mock_libuser.admin() @@ -1013,19 +974,15 @@ def test_chpasswd_user_notfound(self, mock_log, mock_libuser): self.assertRaises(NotFoundError, user_model.chpasswd, 'user1', 'password') mock_adm.lookupUserByName.assert_called_once_with('user1') - mock_log.error.assert_called_once_with( - "User '%s' not found" % 'user1') self.assertFalse(mock_adm.setpassUser.called, msg='Unexpected call to mock_adm.setpassUser()') @mock.patch('plugins.ginger.model.users.libuser', autospec=True) - @mock.patch('plugins.ginger.model.users.wok_log', autospec=True) - def test_chpasswd_setpass_exception(self, mock_log, mock_libuser): + def test_chpasswd_setpass_exception(self, mock_libuser): """ unittest to validate chpasswd() method when setpassUser of libuser raises Exception - mock_log: mock of wok_log imported in model.users mock_libuser: mock of libuser imported in model.users """ mock_adm = mock_libuser.admin() @@ -1037,9 +994,6 @@ def test_chpasswd_setpass_exception(self, mock_log, mock_libuser): mock_adm.lookupUserByName.assert_called_once_with('user1') mock_adm.setpassUser.assert_called_once_with( 'dummy_user', 'password', False) - mock_log.error.assert_called_once_with( - "Failed to change password for user '%s'. Error: '%s'" - % ('user1', 'fail')) @mock.patch('plugins.ginger.model.users.libuser', autospec=True) @mock.patch('plugins.ginger.model.users.wok_log', autospec=True)