Shireesh Anjal has uploaded a new change for review. Change subject: gluster: Improved gluster validation in add host ......................................................................
gluster: Improved gluster validation in add host When checking if the host is part of another gluster cluster, - Ignore if the command (gluster peer status) fails, to handle cases where glusterfs is not installed or glusterd is down - Do not perform the check if the cluster in which the host is being added is currently empty i.e. when the host being added will be the first host of the cluster. Change-Id: I06a2ee4621b175aa70f1ccdcf7a740d830494acf Bug-Url: https://bugzilla.redhat.com/956077 Signed-off-by: Shireesh Anjal <san...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java 2 files changed, 47 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/24/14224/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java index 3d9f710..68d8100 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java @@ -383,12 +383,7 @@ sshclient.connect(); sshclient.authenticate(); - if (isGlusterSupportEnabled()) { - // Must not allow adding a server that already is part of another gluster cluster - if (getGlusterUtil().hasPeers(sshclient)) { - return failCanDoAction(VdcBllMessages.SERVER_ALREADY_PART_OF_ANOTHER_CLUSTER); - } - } + return isValidGlusterPeer(sshclient); } catch (AuthenticationException e) { log.errorFormat( "Failed to authenticate session with host {0}", @@ -411,7 +406,34 @@ } } } + return true; + } + /** + * Checks if the server can be a valid gluster peer. Fails if it is already part of another cluster (when current + * cluster is not empty). This is done by executing the 'gluster peer status' command on the server.<br> + * Note: In case glusterd is down or not installed on the server (which is a possibility on a new server), the + * command can fail. In such cases, we just log it as a debug message and return true. + * + * @param sshclient + * SSH client that can be used to execute 'gluster peer status' command on the server + * @return true if the server is good to be added to a gluster cluster, else false. + */ + private boolean isValidGlusterPeer(SSHClient sshclient) { + if (isGlusterSupportEnabled() && clusterHasServers()) { + try { + // Must not allow adding a server that already is part of another gluster cluster + if (getGlusterUtil().hasPeers(sshclient)) { + return failCanDoAction(VdcBllMessages.SERVER_ALREADY_PART_OF_ANOTHER_CLUSTER); + } + } catch (Exception e) { + // This can happen if glusterd is not running on the server. Ignore it and let the server get added. + // Peer probe will anyway fail later and the server will then go to non-operational status. + log.debugFormat("Could not check if server {0} is already part of another gluster cluster. Will allow adding it.", + sshclient.getHost(), + e); + } + } return true; } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java index 117cee9..93d98d9 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java @@ -6,6 +6,7 @@ import static org.mockito.Mockito.when; import static org.mockito.Mockito.doCallRealMethod; import static org.mockito.Mockito.doNothing; + import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; @@ -24,6 +25,7 @@ import org.ovirt.engine.core.dao.VdsGroupDAO; import org.ovirt.engine.core.utils.MockConfigRule; import org.ovirt.engine.core.utils.gluster.GlusterUtil; +import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.ssh.SSHClient; @RunWith(MockitoJUnitRunner.class) @@ -43,6 +45,8 @@ private GlusterUtil glusterUtil; @Mock private SSHClient sshClient; + @Mock + private Log log; @ClassRule public static MockConfigRule configRule = @@ -104,6 +108,8 @@ when(clusterUtils.hasServers(any(Guid.class))).thenReturn(clusterHasServers); when(clusterUtils.getUpServer(any(Guid.class))).thenReturn(upServer); + + commandMock.log = this.log; } @Test @@ -113,6 +119,19 @@ } @Test + public void canDoActionSucceedsOnEmptyClusterEvenWhenGlusterServerHasPeers() throws Exception { + setupGlusterMock(false, null, true); + assertTrue(commandMock.canDoAction()); + } + + @Test + public void canDoActionSucceedsWhenHasPeersThrowsException() throws Exception { + setupGlusterMock(true, new VDS(), true); + when(glusterUtil.hasPeers(any(SSHClient.class))).thenThrow(new RuntimeException()); + assertTrue(commandMock.canDoAction()); + } + + @Test public void canDoActionFailsWhenGlusterServerHasPeers() throws Exception { setupGlusterMock(true, new VDS(), true); assertFalse(commandMock.canDoAction()); -- To view, visit http://gerrit.ovirt.org/14224 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I06a2ee4621b175aa70f1ccdcf7a740d830494acf Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shireesh Anjal <san...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
