Sahina Bose has uploaded a new change for review.
Change subject: engine: Added canDoAction checks in case of gluster host removal
......................................................................
engine: Added canDoAction checks in case of gluster host removal
Remove host should fail in following cases if host being removed is
part of gluster cluster:
* If force remove is not set
- host has bricks on a volume
- there is no up server in cluster, if host being removed is not
last server in cluster
* If force remove is set, but if host being removed has bricks and
there is another up server in cluster.
Bug-Url: https://bugzilla.redhat.com/957859
Change-Id: I42e1a5452e7c40ae770108cadaf69118b5e77719
Signed-off-by: Sahina Bose <[email protected]>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVdsCommand.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveVdsCommandTest.java
2 files changed, 38 insertions(+), 13 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/01/15401/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVdsCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVdsCommand.java
index d0e2b28..a136c2d 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVdsCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVdsCommand.java
@@ -8,13 +8,13 @@
import org.ovirt.engine.core.bll.utils.ClusterUtils;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.action.RemoveVdsParameters;
+import org.ovirt.engine.core.common.businessentities.StoragePool;
import org.ovirt.engine.core.common.businessentities.StorageType;
import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.businessentities.VDSGroup;
import org.ovirt.engine.core.common.businessentities.VDSStatus;
-import org.ovirt.engine.core.common.businessentities.StoragePool;
-import org.ovirt.engine.core.common.locks.LockingGroup;
import org.ovirt.engine.core.common.errors.VdcBllMessages;
+import org.ovirt.engine.core.common.locks.LockingGroup;
import org.ovirt.engine.core.common.utils.Pair;
import org.ovirt.engine.core.common.vdscommands.RemoveVdsVDSCommandParameters;
import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
@@ -82,18 +82,25 @@
}
}
- // Perform volume bricks on server and up server null check only when
force action is false
- if (returnValue && isGlusterEnabled() &&
!getParameters().isForceAction()) {
- if (hasVolumeBricksOnServer()) {
-
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_REMOVE_HOST_HAVING_GLUSTER_VOLUME);
- returnValue = false;
- }
-
- if (clusterHasMultipleHosts()) {
- upServer = getClusterUtils().getUpServer(getVdsGroupId());
- if (upServer == null) {
+ // Perform volume bricks on server and up server null check
+ if (returnValue && isGlusterEnabled()) {
+ upServer = getClusterUtils().getUpServer(getVdsGroupId());
+ if (!getParameters().isForceAction()) {
+ // if host has bricks on a volume
+ // if there is no up server in cluster, if host being removed
is not
+ // last server in cluster
+ if (hasVolumeBricksOnServer()) {
+
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_REMOVE_HOST_HAVING_GLUSTER_VOLUME);
+ returnValue = false;
+ } else if (upServer == null && clusterHasMultipleHosts()) {
addCanDoActionMessage(String.format("$clusterName %1$s",
getVdsGroup().getname()));
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NO_UP_SERVER_FOUND);
+ returnValue = false;
+ }
+ } else {
+ // cannot remove if there are bricks on server and there is an
up server.
+ if (hasVolumeBricksOnServer() && upServer != null ) {
+
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_REMOVE_HOST_HAVING_GLUSTER_VOLUME);
returnValue = false;
}
}
@@ -191,7 +198,7 @@
}
private void glusterHostRemove() {
- if (isGlusterEnabled() && clusterHasMultipleHosts() &&
!hasVolumeBricksOnServer()) {
+ if (clusterHasMultipleHosts() && !hasVolumeBricksOnServer()) {
VDSReturnValue returnValue =
runVdsCommand(
VDSCommandType.RemoveGlusterServer,
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveVdsCommandTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveVdsCommandTest.java
index b1c6a44..9582a0e 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveVdsCommandTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveVdsCommandTest.java
@@ -82,6 +82,11 @@
when(clusterUtils.getUpServer(clusterId)).thenReturn(getVds(VDSStatus.Up));
}
+ private void mockHasMultipleClusters(Boolean isMultiple) {
+ when(command.getVdsGroupId()).thenReturn(clusterId);
+
when(clusterUtils.hasMultipleServers(clusterId)).thenReturn(isMultiple);
+ }
+
private VDS getVds(VDSStatus status) {
VDS vds = new VDS();
vds.setId(Guid.NewGuid());
@@ -121,6 +126,19 @@
}
@Test
+ public void canDoActionFailsWhenGlusterMultipleHostHasVolumesWithForce()
throws Exception {
+ command = spy(new RemoveVdsCommand<RemoveVdsParameters>(new
RemoveVdsParameters(Guid.NewGuid(), true)));
+ prepareMocks();
+ mockVdsWithStatus(VDSStatus.Maintenance);
+ mockHasMultipleClusters(true);
+ mockIsGlusterEnabled(true);
+ mockHasVolumeOnServer(true);
+
+ CanDoActionTestUtils.runAndAssertCanDoActionFailure(command,
+ VdcBllMessages.VDS_CANNOT_REMOVE_HOST_HAVING_GLUSTER_VOLUME);
+ }
+
+ @Test
public void canDoActionSucceedsWithForceOption() throws Exception {
command = spy(new RemoveVdsCommand<RemoveVdsParameters>(new
RemoveVdsParameters(Guid.NewGuid(), true)));
prepareMocks();
--
To view, visit http://gerrit.ovirt.org/15401
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I42e1a5452e7c40ae770108cadaf69118b5e77719
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
