anmolbabu has uploaded a new change for review. Change subject: webadmin, engine : Error handling in geo-rep create ......................................................................
webadmin, engine : Error handling in geo-rep create The failure message of a command will be set in 1. canDoAction failure FrontendActionAsyncResult#getReturnValue#getCanDoActionMessages 2. Command failure from Vds FrontendActionAsyncResult#getReturnValue#getFault#getMessage 3. UI error handling assumed the only source of error to be 2 1, 2 and 3 necessiated to allow UI framework to handle erroneous conditions as it is capable of doing this conditional handling as in Frontend#handleActionResult Along with the above there was an error in propogation of error from internal commands to the command invoking them and hence removed error handling from internal commands and propogated(using CommandBase#propagateFailure)them to the command utilising them. Change-Id: I3f7beec04869f7417f6f691aa33cd8ba67c72e75 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1230353 Signed-off-by: Anmol Babu <anb...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CreateGlusterVolumeGeoRepSessionCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/SetupGlusterGeoRepMountBrokerInternalCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/UpdateGlusterHostPubKeyToSlaveInternalCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcReturnValueBase.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeGeoRepListModel.java 6 files changed, 68 insertions(+), 42 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/25/42325/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java index 85cf613..ba0298f 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java @@ -2195,6 +2195,18 @@ getReturnValue().setCanDoAction(internalReturnValue.getCanDoAction()); } + protected VdcReturnValueBase convertToVdcReturnValueBase(final VDSReturnValue vdsReturnValue) { + VdcReturnValueBase returnValue = new VdcReturnValueBase(); + returnValue.setSucceeded(false); + returnValue.setActionReturnValue(vdsReturnValue.getReturnValue()); + returnValue.setExecuteFailedMessages(new ArrayList<String>() + {{ + add(vdsReturnValue.getVdsError().getMessage()); + }} + ); + return returnValue; + } + public void persistCommand(VdcActionType parentCommand) { persistCommand(parentCommand, getContext(), false); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CreateGlusterVolumeGeoRepSessionCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CreateGlusterVolumeGeoRepSessionCommand.java index 9121629..19021ae 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CreateGlusterVolumeGeoRepSessionCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CreateGlusterVolumeGeoRepSessionCommand.java @@ -1,6 +1,7 @@ package org.ovirt.engine.core.bll.gluster; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -11,6 +12,7 @@ import org.ovirt.engine.core.bll.utils.GlusterUtil; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.VdcActionType; +import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.action.gluster.GlusterVolumeGeoRepSessionParameters; import org.ovirt.engine.core.common.action.gluster.SetUpMountBrokerParameters; import org.ovirt.engine.core.common.action.gluster.UpdateGlusterHostPubKeyToSlaveParameters; @@ -127,9 +129,16 @@ Set<Guid> remoteServerIds = getServerIds(remoteServersSet); Guid slaveHostId = getParameters().getSlaveHostId(); if (!rootSession) { - succeeded = setUpMountBrokerOnSlave(); + VdcReturnValueBase mountBrokerOnSlaveReturnValue = setUpMountBrokerOnSlave(); + succeeded = + evaluateReturnValues(AuditLogType.GLUSTER_GEOREP_SETUP_MOUNT_BROKER_FAILED, + Collections.singletonList(mountBrokerOnSlaveReturnValue)); remoteServerIds.remove(slaveHostId); - succeeded = setUpPartialMountBrokerOnSlaves(remoteServerIds); + if (succeeded) { + succeeded = + evaluateReturnValues(AuditLogType.GLUSTER_GEOREP_SETUP_MOUNT_BROKER_FAILED, + setUpPartialMountBrokerOnSlaves(remoteServerIds)); + } } if (succeeded) { remoteServerIds.add(slaveHostId); @@ -158,71 +167,62 @@ return remoteServerIds; } - protected boolean setUpMountBrokerOnSlave() { - return getBackend().runInternalAction(VdcActionType.SetupGlusterGeoRepMountBrokerInternal, + protected VdcReturnValueBase setUpMountBrokerOnSlave() { + VdcReturnValueBase mountBrokerOnSlaveReturnValue = getBackend().runInternalAction(VdcActionType.SetupGlusterGeoRepMountBrokerInternal, new SetUpMountBrokerParameters(getParameters().getSlaveHostId(), getParameters().getSlaveVolumeName(), getParameters().getUserName(), - getParameters().getUserGroup())).getSucceeded(); + getParameters().getUserGroup())); + return mountBrokerOnSlaveReturnValue; } - protected boolean setUpPartialMountBrokerOnSlaves(Set<Guid> remoteServerIds) { + protected List<VdcReturnValueBase> setUpPartialMountBrokerOnSlaves(Set<Guid> remoteServerIds) { if (remoteServerIds == null || remoteServerIds.isEmpty()) { - return true; + return null; } - List<Callable<Boolean>> mountBrokerPartialSetupReturnStatuses = new ArrayList<Callable<Boolean>>(); + List<Callable<VdcReturnValueBase>> mountBrokerPartialSetupReturnStatuses = new ArrayList<Callable<VdcReturnValueBase>>(); for (final Guid currentSlaveServerId : remoteServerIds) { - mountBrokerPartialSetupReturnStatuses.add(new Callable<Boolean>() { + mountBrokerPartialSetupReturnStatuses.add(new Callable<VdcReturnValueBase>() { @Override - public Boolean call() throws Exception { + public VdcReturnValueBase call() throws Exception { return getBackend().runInternalAction(VdcActionType.SetupGlusterGeoRepMountBrokerInternal, new SetUpMountBrokerParameters(currentSlaveServerId, getParameters().getSlaveVolumeName(), getParameters().getUserName(), getParameters().getUserGroup(), - true)).getSucceeded(); + true)); } }); } - List<Boolean> returnStatuses = ThreadPoolUtil.invokeAll(mountBrokerPartialSetupReturnStatuses); - for (Boolean currentReturnStatus : returnStatuses) { - if (!currentReturnStatus) { - return false; - } - } - return true; + List<VdcReturnValueBase> returnStatuses = ThreadPoolUtil.invokeAll(mountBrokerPartialSetupReturnStatuses); + return returnStatuses; } protected boolean setUpPasswordlessSSH(Guid masterUpServerId, Set<Guid> remoteServerSet, String userName) { List<String> pubKeys = readPubKey(masterUpServerId); boolean canProceed = pubKeys != null && pubKeys.size() > 0; if (canProceed) { - canProceed = updatePubKeysToRemoteHosts(pubKeys, remoteServerSet, userName); + canProceed = evaluateReturnValues(AuditLogType.GLUSTER_GEOREP_PUBLIC_KEY_WRITE_FAILED, updatePubKeysToRemoteHosts(pubKeys, remoteServerSet, userName)); } return canProceed; } - private boolean updatePubKeysToRemoteHosts(final List<String> pubKeys, + private List<VdcReturnValueBase> updatePubKeysToRemoteHosts(final List<String> pubKeys, Set<Guid> remoteServersSet, final String userName) { - List<Callable<Boolean>> slaveWritePubKeyList = new ArrayList<Callable<Boolean>>(); + List<Callable<VdcReturnValueBase>> slaveWritePubKeyList = new ArrayList<Callable<VdcReturnValueBase>>(); for (final Guid currentRemoteHostId : remoteServersSet) { - slaveWritePubKeyList.add(new Callable<Boolean>() { + slaveWritePubKeyList.add(new Callable<VdcReturnValueBase>() { @Override - public Boolean call() throws Exception { + public VdcReturnValueBase call() throws Exception { return getBackend().runInternalAction(VdcActionType.UpdateGlusterHostPubKeyToSlaveInternal, new UpdateGlusterHostPubKeyToSlaveParameters(currentRemoteHostId, - pubKeys, userName)).getSucceeded(); + pubKeys, userName)); } }); } - List<Boolean> returnStatuses = ThreadPoolUtil.invokeAll(slaveWritePubKeyList); - for (Boolean currentReturnStatus : returnStatuses) { - if (!currentReturnStatus) { - return false; - } - } - return true; + List<VdcReturnValueBase> returnStatuses = ThreadPoolUtil.invokeAll(slaveWritePubKeyList); + return returnStatuses; } @SuppressWarnings("unchecked") @@ -279,4 +279,20 @@ protected GlusterUtil getGlusterUtil() { return GlusterUtil.getInstance(); } + + private boolean evaluateReturnValues(AuditLogType auditLogType, List<VdcReturnValueBase> returnValues) { + boolean succeeded = true; + List<String> errors = new ArrayList<>(); + for (VdcReturnValueBase currentReturnValue : returnValues) { + boolean currentExecutionStatus = currentReturnValue.getSucceeded(); + succeeded = succeeded && currentExecutionStatus; + if (!currentExecutionStatus) { + errors.addAll(currentReturnValue.getExecuteFailedMessages()); + } + } + if (!succeeded) { + handleVdsErrors(auditLogType, errors); + } + return succeeded; + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/SetupGlusterGeoRepMountBrokerInternalCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/SetupGlusterGeoRepMountBrokerInternalCommand.java index 3cb3dbe1..89d3890 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/SetupGlusterGeoRepMountBrokerInternalCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/SetupGlusterGeoRepMountBrokerInternalCommand.java @@ -101,15 +101,13 @@ if (mountBrokerReturnValue.getSucceeded()) { VDSReturnValue restartGlusterdReturnValue = restartGlusterd(getParameters().getId()); if (!restartGlusterdReturnValue.getSucceeded()) { - handleVdsError(AuditLogType.GLUSTER_GEOREP_SETUP_MOUNT_BROKER_FAILED, - restartGlusterdReturnValue.getVdsError().getMessage()); + propagateFailure(convertToVdcReturnValueBase(restartGlusterdReturnValue)); return; } else { setSucceeded(restartGlusterdReturnValue.getSucceeded()); } } else { - handleVdsError(AuditLogType.GLUSTER_GEOREP_SETUP_MOUNT_BROKER_FAILED, mountBrokerReturnValue.getVdsError() - .getMessage()); + propagateFailure(convertToVdcReturnValueBase(mountBrokerReturnValue)); return; } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/UpdateGlusterHostPubKeyToSlaveInternalCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/UpdateGlusterHostPubKeyToSlaveInternalCommand.java index de94d85..2d0b563 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/UpdateGlusterHostPubKeyToSlaveInternalCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/UpdateGlusterHostPubKeyToSlaveInternalCommand.java @@ -56,17 +56,15 @@ @Override protected void executeCommand() { - VDSReturnValue writePubKeysReturnValue = + final VDSReturnValue writePubKeysReturnValue = runVdsCommand(VDSCommandType.UpdateGlusterGeoRepKeys, new UpdateGlusterGeoRepKeysVDSParameters(getParameters().getId(), getParameters().getPubKeys(), getParameters().getRemoteUserName())); setSucceeded(writePubKeysReturnValue.getSucceeded()); if (!writePubKeysReturnValue.getSucceeded()) { - handleVdsError(AuditLogType.GLUSTER_GEOREP_PUBLIC_KEY_WRITE_FAILED, writePubKeysReturnValue.getVdsError() - .getMessage()); + propagateFailure(convertToVdcReturnValueBase(writePubKeysReturnValue)); return; } } - } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcReturnValueBase.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcReturnValueBase.java index 26d5891..a36b137 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcReturnValueBase.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcReturnValueBase.java @@ -138,6 +138,10 @@ return endActionTryAgain; } + public void setExecuteFailedMessages(ArrayList<String> value) { + executeFailedMessages = value; + } + public void setEndActionTryAgain(boolean value) { endActionTryAgain = value; } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeGeoRepListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeGeoRepListModel.java index 1894f12..5a556ec 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeGeoRepListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeGeoRepListModel.java @@ -471,13 +471,11 @@ VolumeGeoRepListModel.this, false); } - } else { - createModel.setQueryFailureMessage(result.getReturnValue().getFault().getMessage()); } } }, this, - false); + true); } private void createGeoRepSession() { -- To view, visit https://gerrit.ovirt.org/42325 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3f7beec04869f7417f6f691aa33cd8ba67c72e75 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5-gluster Gerrit-Owner: anmolbabu <anb...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
