Sahina Bose has uploaded a new change for review. Change subject: engine: Correct remove-brick flow ......................................................................
engine: Correct remove-brick flow Stop remove-brick and commit remove brick should not be monitored as separate jobs. Once migration of data is completed for remove-brick the step status is updated. Corrected parameters passed to commit brick and the step message that is updated. Change-Id: I8f7a99f8ae87186c42e7346508b6abf0a2d1b7be Bug-Url: https://bugzilla.redhat.com/?????? Signed-off-by: Sahina Bose <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CommitRemoveGlusterVolumeBricksCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterAsyncCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJob.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/CommitRemoveGlusterVolumeBricksCommandTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/constants/gluster/GlusterConstants.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/AbstractGlusterBrokerCommand.java 7 files changed, 37 insertions(+), 22 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/23/20023/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CommitRemoveGlusterVolumeBricksCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CommitRemoveGlusterVolumeBricksCommand.java index 5cd5223..c65f930 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CommitRemoveGlusterVolumeBricksCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CommitRemoveGlusterVolumeBricksCommand.java @@ -1,5 +1,7 @@ package org.ovirt.engine.core.bll.gluster; +import java.util.Map; + import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute; import org.ovirt.engine.core.bll.gluster.tasks.GlusterTaskUtils; import org.ovirt.engine.core.bll.interfaces.BackendInternal; @@ -7,6 +9,7 @@ import org.ovirt.engine.core.common.action.gluster.GlusterVolumeRemoveBricksParameters; import org.ovirt.engine.core.common.asynctasks.gluster.GlusterTaskType; import org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeEntity; +import org.ovirt.engine.core.common.constants.gluster.GlusterConstants; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.job.JobExecutionStatus; import org.ovirt.engine.core.common.job.StepEnum; @@ -60,7 +63,7 @@ runVdsCommand(VDSCommandType.CommitRemoveGlusterVolumeBricks, new GlusterVolumeRemoveBricksVDSParameters(getUpServer().getId(), volume.getName(), - volume.getBricks())); + getParameters().getBricks())); setSucceeded(returnValue.getSucceeded()); if (!getSucceeded()) { handleVdsError(AuditLogType.GLUSTER_VOLUME_REMOVE_BRICKS_COMMIT_FAILED, returnValue.getVdsError() @@ -68,12 +71,19 @@ return; } - endStepJob(); + endStepJob(JobExecutionStatus.FINISHED, getStepMessageMap(JobExecutionStatus.FINISHED)); releaseVolumeLock(); getReturnValue().setActionReturnValue(returnValue.getReturnValue()); } @Override + protected Map<String, String> getStepMessageMap(JobExecutionStatus status) { + Map<String, String> stepMessageMap = super.getStepMessageMap(status); + stepMessageMap.put(GlusterConstants.JOB_STATUS, "COMMITTED"); + return stepMessageMap; + } + + @Override public AuditLogType getAuditLogTypeValue() { if (getSucceeded()) { return AuditLogType.GLUSTER_VOLUME_REMOVE_BRICKS_COMMIT; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterAsyncCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterAsyncCommandBase.java index 464f7cc..b3cb7e7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterAsyncCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterAsyncCommandBase.java @@ -56,8 +56,8 @@ Map<String, String> values = new HashMap<String, String>(); values.put(GlusterConstants.CLUSTER, getVdsGroupName()); values.put(GlusterConstants.VOLUME, getGlusterVolumeName()); - values.put("status", status.toString()); - values.put("info", " "); + values.put(GlusterConstants.JOB_STATUS, status.toString()); + values.put(GlusterConstants.JOB_INFO, " "); return values; } @@ -77,12 +77,16 @@ } protected void endStepJob() { + endStepJob(JobExecutionStatus.ABORTED, getStepMessageMap(JobExecutionStatus.ABORTED)); + } + + protected void endStepJob(JobExecutionStatus status, Map<String, String> stepMessageMap) { GlusterAsyncTask asyncTask = getGlusterVolume().getAsyncTask(); - // Gluster Volume Rebalance Task will have only one step ( REBALANCING_VOLUME ) + // Gluster Task will be associated with only one step ( REBALANCING_VOLUME or REMOVING_BRICK) Step step = getStepDao().getStepsByExternalId(asyncTask.getTaskId()).get(0); - step.markStepEnded(JobExecutionStatus.ABORTED); + step.markStepEnded(status); step.setDescription(ExecutionMessageDirector.resolveStepMessage(getStepType(), - getStepMessageMap(JobExecutionStatus.ABORTED))); + stepMessageMap)); JobRepositoryFactory.getJobRepository().updateStep(step); ExecutionContext finalContext = ExecutionHandler.createFinalizingContext(step.getId()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJob.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJob.java index 184390d..a3c2d93 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJob.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterTasksSyncJob.java @@ -113,6 +113,7 @@ continue; } step.setDescription(getTaskMessage(cluster,step.getStepType(),task)); + step.setStatus(task.getStatus()); if (hasTaskCompleted(task)) { step.markStepEnded(task.getStatus()); endStepJob(step); @@ -245,8 +246,8 @@ Map<String, String> values = new HashMap<String, String>(); values.put(GlusterConstants.CLUSTER, cluster.getName()); values.put(GlusterConstants.VOLUME, task.getTaskParameters().getVolumeName()); - values.put("status", task.getStatus().toString()); - values.put("info", task.getMessage()); + values.put(GlusterConstants.JOB_STATUS, task.getStatus().toString()); + values.put(GlusterConstants.JOB_INFO, task.getMessage()); return values; } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/CommitRemoveGlusterVolumeBricksCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/CommitRemoveGlusterVolumeBricksCommandTest.java index 346f1a1..792b316 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/CommitRemoveGlusterVolumeBricksCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/CommitRemoveGlusterVolumeBricksCommandTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.argThat; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doNothing; @@ -14,7 +15,9 @@ import static org.mockito.Mockito.when; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; @@ -144,25 +147,18 @@ return bricks; } + @SuppressWarnings("unchecked") private void mockBackend(boolean succeeded, VdcBllErrors errorCode) { when(cmd.getBackend()).thenReturn(backend); when(backend.getResourceManager()).thenReturn(vdsBrokerFrontend); - doNothing().when(cmd).endStepJob(); + doReturn(new HashMap<String, String>()).when(cmd).getStepMessageMap(any(JobExecutionStatus.class)); + doNothing().when(cmd).endStepJob(any(JobExecutionStatus.class), any(Map.class)); doNothing().when(cmd).releaseVolumeLock(); VDSReturnValue vdsReturnValue = new VDSReturnValue(); vdsReturnValue.setSucceeded(succeeded); if (!succeeded) { vdsReturnValue.setVdsError(new VDSError(errorCode, "")); - } else { - GlusterAsyncTask task = new GlusterAsyncTask(); - task.setMessage("successful"); - task.setStatus(JobExecutionStatus.FINISHED); - task.setStepId(Guid.newGuid()); - task.setTaskId(Guid.newGuid()); - task.setType(GlusterTaskType.REMOVE_BRICK); - - vdsReturnValue.setReturnValue(task); } when(vdsBrokerFrontend.RunVdsCommand(eq(VDSCommandType.CommitRemoveGlusterVolumeBricks), @@ -182,6 +178,7 @@ }; } + @SuppressWarnings("unchecked") @Test public void testExecuteCommand() { cmd = @@ -192,7 +189,7 @@ assertTrue(cmd.canDoAction()); cmd.executeCommand(); - verify(cmd, times(1)).endStepJob(); + verify(cmd, times(1)).endStepJob(any(JobExecutionStatus.class),any(Map.class)); verify(cmd, times(1)).releaseVolumeLock(); assertEquals(cmd.getAuditLogTypeValue(), AuditLogType.GLUSTER_VOLUME_REMOVE_BRICKS_COMMIT); } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java index bcc3daf..9873e37 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java @@ -264,8 +264,8 @@ ManageGlusterService(1420, ActionGroup.MANIPULATE_GLUSTER_SERVICE, QuotaDependency.NONE), StopRebalanceGlusterVolume(1421, ActionGroup.MANIPULATE_GLUSTER_VOLUME,false, QuotaDependency.NONE), StartRemoveGlusterVolumeBricks(1422, ActionGroup.MANIPULATE_GLUSTER_VOLUME, QuotaDependency.NONE), - StopRemoveGlusterVolumeBricks(1423, ActionGroup.MANIPULATE_GLUSTER_VOLUME, QuotaDependency.NONE), - CommitRemoveGlusterVolumeBricks(1424, ActionGroup.MANIPULATE_GLUSTER_VOLUME, QuotaDependency.NONE), + StopRemoveGlusterVolumeBricks(1423, ActionGroup.MANIPULATE_GLUSTER_VOLUME, false,QuotaDependency.NONE), + CommitRemoveGlusterVolumeBricks(1424, ActionGroup.MANIPULATE_GLUSTER_VOLUME,false, QuotaDependency.NONE), // Cluster Policy AddClusterPolicy(1450, ActionGroup.EDIT_STORAGE_POOL_CONFIGURATION, false, QuotaDependency.NONE), EditClusterPolicy(1451, ActionGroup.EDIT_STORAGE_POOL_CONFIGURATION, false, QuotaDependency.NONE), diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/constants/gluster/GlusterConstants.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/constants/gluster/GlusterConstants.java index 86451c2..bb3da5b 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/constants/gluster/GlusterConstants.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/constants/gluster/GlusterConstants.java @@ -41,4 +41,6 @@ public static final String HOOK_NAME = "glusterhookname"; public static final String FAILURE_MESSAGE = "failuremessage"; + public static final String JOB_STATUS = "status"; + public static final String JOB_INFO = "info"; } diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/AbstractGlusterBrokerCommand.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/AbstractGlusterBrokerCommand.java index bb38dad..2691270 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/AbstractGlusterBrokerCommand.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/AbstractGlusterBrokerCommand.java @@ -62,6 +62,7 @@ case GlusterVolumeStatusAllFailedException: case GlusterVolumeRebalanceStatusFailedException: case GlusterVolumeRemoveBrickStatusFailed: + case GlusterVolumeRemoveBricksCommitFailed: // Capture error from gluster command and record failure getVDSReturnValue().setVdsError(new VDSError(returnStatus, getReturnStatus().mMessage)); getVDSReturnValue().setSucceeded(false); -- To view, visit http://gerrit.ovirt.org/20023 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8f7a99f8ae87186c42e7346508b6abf0a2d1b7be Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sahina Bose <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
