Yaniv Bronhaim has uploaded a new change for review. Change subject: engine-manage-domain is missing details on error reports. ......................................................................
engine-manage-domain is missing details on error reports. https://bugzilla.redhat.com/show_bug.cgi?id=818479 Added variable of default message, so if error occurred during varification of the paramerts, the default message will be the output. Change-Id: If21f117208a53a80ebfefbbe4f6a0c188e35a3bd Signed-off-by: Yaniv Bronhaim <ybron...@redhat.com> --- M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ConfigurationProvider.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ManageDomains.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ManageDomainsResult.java 3 files changed, 101 insertions(+), 39 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/02/7202/1 diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ConfigurationProvider.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ConfigurationProvider.java index f67ef72..cffc847 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ConfigurationProvider.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ConfigurationProvider.java @@ -19,9 +19,9 @@ public class ConfigurationProvider { - private EnumMap<ConfigValues, String> configVals = new EnumMap<ConfigValues, String>(ConfigValues.class); - private String engineConfigExecutable; - private String engineConfigProperties; + private final EnumMap<ConfigValues, String> configVals = new EnumMap<ConfigValues, String>(ConfigValues.class); + private final String engineConfigExecutable; + private final String engineConfigProperties; private final static Logger log = Logger.getLogger(ManageDomainsDAOImpl.class); public ConfigurationProvider(String adUserName, @@ -103,13 +103,12 @@ int retVal = engineConfigProcess.waitFor(); if (retVal != 0) { throw new ManageDomainsResult(ManageDomainsResultEnum.FAILED_SETTING_CONFIGURATION_VALUE_FOR_OPTION, + "Verify Connection to Db, Check if DB service is running", enumValue.name()); } - } catch (IOException e) { + } catch (Throwable e) { throw new ManageDomainsResult(ManageDomainsResultEnum.FAILED_SETTING_CONFIGURATION_VALUE_FOR_OPTION_WITH_DETAILS, - new String[] { enumValue.name(), e.getMessage() }); - } catch (InterruptedException e) { - throw new ManageDomainsResult(ManageDomainsResultEnum.FAILED_SETTING_CONFIGURATION_VALUE_FOR_OPTION_WITH_DETAILS, + "Verify Connection to Db, Check if DB service is running", new String[] { enumValue.name(), e.getMessage() }); } finally { diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ManageDomains.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ManageDomains.java index e011669..598af99 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ManageDomains.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ManageDomains.java @@ -16,7 +16,6 @@ import java.util.Scanner; import java.util.Set; -import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; import org.ovirt.engine.core.common.config.ConfigValues; @@ -119,14 +118,16 @@ try { utilityConfiguration = new ManageDomainsConfiguration(configFilePath); - } catch (ConfigurationException e) { - throw new ManageDomainsResult(ManageDomainsResultEnum.FAILED_READING_CONFIGURATION, e.getMessage()); + } catch (Exception e) { + throw new ManageDomainsResult(ManageDomainsResultEnum.FAILED_READING_CONFIGURATION, + "Verify that postgresql service is up and check DB configurations", e.getMessage()); } try { daoImpl = new ManageDomainsDAOImpl(); - } catch (SQLException e) { - throw new ManageDomainsResult(ManageDomainsResultEnum.DB_EXCEPTION, e.getMessage()); + } catch (Exception e) { + throw new ManageDomainsResult(ManageDomainsResultEnum.DB_EXCEPTION, + "Verify that postgresql service is up and check DB configurations" , e.getMessage()); } } @@ -216,12 +217,9 @@ utilityConfiguration.getEngineConfigExecutablePath(), engineConfigProperties); - } catch (IOException e) { - throw new ManageDomainsResult(ManageDomainsResultEnum.FAILED_READING_CURRENT_CONFIGURATION, e.getMessage()); - } catch (InterruptedException e) { - throw new ManageDomainsResult(ManageDomainsResultEnum.FAILED_READING_CURRENT_CONFIGURATION, e.getMessage()); - } catch (FailedReadingConfigValueException e) { - throw new ManageDomainsResult(ManageDomainsResultEnum.FAILED_READING_CURRENT_CONFIGURATION, e.getMessage()); + } catch (Throwable e) { + throw new ManageDomainsResult(ManageDomainsResultEnum.FAILED_READING_CURRENT_CONFIGURATION, + "Verify that postgresql service is up and check DB configurations", e.getMessage()); } } @@ -232,7 +230,7 @@ try { actionType = ActionType.valueOf(action); } catch (IllegalArgumentException ex) { - throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ACTION, action); + throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ACTION, "Invalid action, verify DB connection", action); } if (actionType.equals(ActionType.add)) { @@ -253,7 +251,8 @@ } else if (actionType.equals(ActionType.list)) { getConfiguration(); } else { - throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ACTION, action); + throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ACTION, + "Invalid action, verify DB connection", action); } } @@ -272,7 +271,8 @@ for (LdapProviderType t : LdapProviderType.values()) { sb.append(" " + t.name() + "\n"); } - throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ARGUMENT_FOR_COMMAND, sb.toString()); + throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ARGUMENT_FOR_COMMAND, + "Check Db Connection, getLdapProviderType received empty parameter", sb.toString()); } private String getPasswordInput(CLIParser parser) throws ManageDomainsResult { @@ -283,7 +283,8 @@ String passwordFile = parser.getArg(Arguments.passwordFile.name()); pass = readPasswordFile(passwordFile); } catch (Exception e) { - throw new ManageDomainsResult(ManageDomainsResultEnum.FAILURE_READING_PASSWORD_FILE, e.getMessage()); + throw new ManageDomainsResult(ManageDomainsResultEnum.FAILURE_READING_PASSWORD_FILE, + "Check DB connection. Error during reading password file", e.getMessage()); } } else if (parser.hasArg(Arguments.interactive.name())) { pass = readPasswordInteractively(); @@ -404,7 +405,8 @@ new DomainsConfigurationEntry(currentDomains, DOMAIN_SEPERATOR, null); if (domainNameEntry.doesDomainExist(domainName)) { - throw new ManageDomainsResult(ManageDomainsResultEnum.DOMAIN_ALREADY_EXISTS_IN_CONFIGURATION, domainName); + throw new ManageDomainsResult(ManageDomainsResultEnum.DOMAIN_ALREADY_EXISTS_IN_CONFIGURATION, + "Error during verified domain name. Empty name or DB connection error" ,domainName); } domainNameEntry.setValueForDomain(domainName, null); @@ -481,6 +483,7 @@ return OK_RESULT; } catch (SQLException e) { return new ManageDomainsResult(ManageDomainsResultEnum.FAILURE_WHILE_APPLYING_CHANGES_IN_DATABASE, + "Check connection to DB, failed during applying changes", e.getMessage()); } } @@ -507,7 +510,9 @@ new DomainsConfigurationEntry(currentDomains, DOMAIN_SEPERATOR, null); if (!domainNameEntry.doesDomainExist(domainName)) { - throw new ManageDomainsResult(ManageDomainsResultEnum.DOMAIN_DOESNT_EXIST_IN_CONFIGURATION, domainName); + throw new ManageDomainsResult(ManageDomainsResultEnum.DOMAIN_DOESNT_EXIST_IN_CONFIGURATION, + "Error during verified domain name. Empty name or DB connection error", + domainName); } domainNameEntry.setValueForDomain(domainName, null); @@ -594,6 +599,7 @@ } catch (Exception ex) { ManageDomainsResult result = new ManageDomainsResult(ManageDomainsResultEnum.FAILURE_CREATING_KERBEROS_CONFIGURATION, + "Check network Connection to DB, failed during configure kerberos.", ex.getMessage()); throw result; } @@ -633,7 +639,8 @@ } log.info("Successfully tested kerberos configuration for domain: " + domain); } catch (Exception e) { - ManageDomainsResult result = new ManageDomainsResult(ManageDomainsResultEnum.FAILURE_WHILE_TESTING_DOMAIN, new String[] { + ManageDomainsResult result = new ManageDomainsResult(ManageDomainsResultEnum.FAILURE_WHILE_TESTING_DOMAIN, + "Check DB Connection, failed during testing domain", new String[] { domain, e.getMessage() }); if ((isValidate && reportAllErrors) || ((domainName != null) && !domain.equals(domainName))) { @@ -662,6 +669,7 @@ FileUtil.deleteFile(utilityConfiguration.getkrb5confFilePath() + TESTING_KRB5_CONF_SUFFIX); } catch (IOException e) { throw new ManageDomainsResult(ManageDomainsResultEnum.FAILURE_WHILE_APPLYING_KERBEROS_CONFIGURATION, + "Check Connection to DB, kerberos configuration couldn't been saved", e.getMessage()); } } @@ -676,8 +684,9 @@ ReturnStatus returnStatus = simpleAuthenticationCheck.printUserGuid(domain, userName, password, address, userGuid, ldapProviderType); if (!returnStatus.equals(ReturnStatus.OK)) { - return new ManageDomainsResult(ManageDomainsResultEnum.FAILURE_WHILE_TESTING_DOMAIN, new String[] { domain, - returnStatus.getDetailedMessage() }); + return new ManageDomainsResult(ManageDomainsResultEnum.FAILURE_WHILE_TESTING_DOMAIN, + "Checking domain configuration failed, check network connection.", + new String[] { domain, returnStatus.getDetailedMessage() }); } log.info("Successfully tested domain " + domain); return OK_RESULT; @@ -821,7 +830,8 @@ new DomainsConfigurationEntry(currentDomains, DOMAIN_SEPERATOR, null); if (!domainNameEntry.doesDomainExist(domainName)) { - throw new ManageDomainsResult(ManageDomainsResultEnum.DOMAIN_DOESNT_EXIST_IN_CONFIGURATION, domainName); + throw new ManageDomainsResult(ManageDomainsResultEnum.DOMAIN_DOESNT_EXIST_IN_CONFIGURATION, + "Error during verified domain name. Empty name or DB connection error", domainName); } domainNameEntry.removeValueForDomain(domainName); @@ -879,7 +889,8 @@ try { actionType = ActionType.valueOf(action); } catch (IllegalArgumentException ex) { - throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ACTION, action); + throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ACTION, + "Invalid DB action, check DB connection", action); } if (actionType.equals(ActionType.add)) { requireArgs(parser, Arguments.domain, Arguments.user, Arguments.provider); @@ -916,7 +927,8 @@ private void requireArgs(CLIParser parser, Arguments... args) throws ManageDomainsResult { for (Arguments arg: args) { if (!parser.hasArg(arg.name())) { - throw new ManageDomainsResult(ManageDomainsResultEnum.ARGUMENT_IS_REQUIRED, arg.name()); + throw new ManageDomainsResult(ManageDomainsResultEnum.ARGUMENT_IS_REQUIRED, + "Missing argument for command" , arg.name()); } } } @@ -927,13 +939,15 @@ return; } } - throw new ManageDomainsResult(ManageDomainsResultEnum.ARGUMENT_IS_REQUIRED, Arrays.deepToString(args)); + throw new ManageDomainsResult(ManageDomainsResultEnum.ARGUMENT_IS_REQUIRED, + "Missing argument for command", Arrays.deepToString(args)); } private void checkInvalidArgs(CLIParser parser, Arguments... args) throws ManageDomainsResult { for (Arguments arg : args) { if (parser.hasArg(arg.name())) { - throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ARGUMENT_FOR_COMMAND, arg.name()); + throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ARGUMENT_FOR_COMMAND, + "Empty argument pass. Verify command", arg.name()); } } } diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ManageDomainsResult.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ManageDomainsResult.java index 32cccab..34a2b98 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ManageDomainsResult.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ManageDomainsResult.java @@ -1,27 +1,76 @@ package org.ovirt.engine.core.utils.kerberos; +import org.apache.commons.lang.StringUtils; +import org.apache.log4j.Logger; + +/* + * Exception during configure domains. If this exception occurred its probably because + * DB errors. This class define handling of configure exceptions + */ public class ManageDomainsResult extends Exception { private static final long serialVersionUID = -2897637328396868452L; private ManageDomainsResultEnum enumResult; private int exitCode; private String detailedMessage; + private final static Logger log = Logger.getLogger(ManageDomainsResult.class); + /** + * This constructor is present exception without additional params. + * we use this one without additional info, + * only enum result type with default detailed massage. + * + * @param enumResult - enum result type + */ public ManageDomainsResult(ManageDomainsResultEnum enumResult) { this.exitCode = enumResult.getExitCode(); this.detailedMessage = enumResult.getDetailedMessage(); this.enumResult = enumResult; } - public ManageDomainsResult(ManageDomainsResultEnum enumResult, String param) { - this.exitCode = enumResult.getExitCode(); - this.detailedMessage = String.format(enumResult.getDetailedMessage(), param); - this.enumResult = enumResult; - } + /** + * This constructor gets params for enum result and defaultMsg for a case that params are wrong or + * include misleading variables (like empty string) + * + * @param enumResult - the error to publish + * @param defaultMsg - msg for default use, if something wrong with params + * @param params - enumResult additional params + */ + public ManageDomainsResult(ManageDomainsResultEnum enumResult, String defaultMsg ,String... params) { + boolean validParams = true; - public ManageDomainsResult(ManageDomainsResultEnum enumResult, String... params) { this.exitCode = enumResult.getExitCode(); - this.detailedMessage = String.format(enumResult.getDetailedMessage(), params); this.enumResult = enumResult; + + if (StringUtils.isEmpty(defaultMsg)){ + log.debug("No default param passed."); + this.detailedMessage = enumResult.getDetailedMessage(); + } + + // if we used this constructor we must have params. verified that all params are acceptable + // (not empty string or null) + if (params.length == 0){ + log.debug("Wrong exception's params recevied."); + this.detailedMessage = defaultMsg; + return; + } + + for (String param : params){ + if (StringUtils.isEmpty(param)){ + log.debug("Got null value."); + validParams = false; + } + } + + // use default message - something is wrong with one of the params + if (!validParams){ + this.detailedMessage = defaultMsg; + return; + } + // Create message + else{ + // log.debug("ManageDomainsResult: evferythign good."); + this.detailedMessage = String.format(enumResult.getDetailedMessage(), params); + } } public int getExitCode() { -- To view, visit http://gerrit.ovirt.org/7202 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If21f117208a53a80ebfefbbe4f6a0c188e35a3bd Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
