Shireesh Anjal has uploaded a new change for review. Change subject: engine: Improved duplicate host validation ......................................................................
engine: Improved duplicate host validation Currently, the validation in AddVdsCommand#canDoAction() checks if the address entered by user is present in the DB (hostname field in DB). However it is possible that the same host was earlier added by providing hostname the address field, and is now being added using ip address. In such case, it is required to compare the provided address (which could be hostname or ip) with ip addresses of network interfaces in db. This patch does this additional check to ensure that existing host cannot be added again, irrespective of hostname or ip address being used. Change-Id: I37c5ee3546a21400678c2f1f7d1e54bb726988b3 Bug-Url: https://bugzilla.redhat.com/916095 Signed-off-by: Shireesh Anjal <[email protected]> --- 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, 52 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/02/14402/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 a398461..84b29e6 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 @@ -1,6 +1,8 @@ package org.ovirt.engine.core.bll; import java.io.File; +import java.net.InetAddress; +import java.net.UnknownHostException; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -52,6 +54,7 @@ import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dal.job.ExecutionMessageDirector; import org.ovirt.engine.core.dao.gluster.GlusterDBUtils; +import org.ovirt.engine.core.dao.network.InterfaceDao; import org.ovirt.engine.core.utils.gluster.GlusterUtil; import org.ovirt.engine.core.utils.ssh.SSHClient; import org.ovirt.engine.core.utils.threadpool.ThreadPoolUtil; @@ -319,6 +322,7 @@ addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST); returnValue = false; } else { + returnValue = returnValue && serverWithIpAddressExists(vds.getVdsGroupId(), hostName); returnValue = returnValue && validateSingleHostAttachedToLocalStorage(); if (Config.<Boolean> GetValue(ConfigValues.UseSecureConnectionWithServers) @@ -350,6 +354,25 @@ return returnValue; } + private boolean serverWithIpAddressExists(Guid vdsGroupId, String hostName) { + try { + if (getInterfaceDao().getAllInterfacesWithIpAddress(vdsGroupId, getHostAddress(hostName)).size() != 0) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST); + } + } catch (UnknownHostException e) { + return failCanDoAction(VdcBllMessages.VDS_CANNOT_CONNECT_TO_SERVER); + } + return true; + } + + protected String getHostAddress(String hostName) throws UnknownHostException { + return InetAddress.getByName(hostName).getHostAddress(); + } + + protected InterfaceDao getInterfaceDao() { + return DbFacade.getInstance().getInterfaceDao(); + } + protected boolean isPowerManagementLegal() { return IsPowerManagementLegal(getParameters().getVdsStaticData(), getVdsGroup() .getcompatibility_version().toString()); 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 cf13de5..a8dfeae 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 @@ -20,6 +20,7 @@ import org.ovirt.engine.core.bll.utils.ClusterUtils; import org.ovirt.engine.core.common.action.AddVdsActionParameters; import org.ovirt.engine.core.common.businessentities.VDS; +import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.Version; @@ -27,6 +28,7 @@ import org.ovirt.engine.core.dao.VdsDAO; import org.ovirt.engine.core.dao.VdsGroupDAO; import org.ovirt.engine.core.dao.gluster.GlusterDBUtils; +import org.ovirt.engine.core.dao.network.InterfaceDao; import org.ovirt.engine.core.utils.MockConfigRule; import org.ovirt.engine.core.utils.gluster.GlusterUtil; import org.ovirt.engine.core.utils.log.Log; @@ -34,6 +36,9 @@ @RunWith(MockitoJUnitRunner.class) public class AddVdsCommandTest { + private static final String VDS_HOSTNAME = "BUZZ"; + private static final String NEW_IP = "10.20.30.40"; + private static final String EXISTING_IP = "10.20.30.50"; private static final String PEER_1 = "peer1"; private static final Guid vdsId = Guid.NewGuid(); private AddVdsActionParameters parameters; @@ -44,6 +49,8 @@ private VdsGroupDAO groupDAOMock; @Mock private VdsDAO vdsDaoMock; + @Mock + private InterfaceDao interfaceDaoMock; @Mock private ClusterUtils clusterUtils; @Mock @@ -63,7 +70,7 @@ private VDS makeTestVds(Guid vdsId) { VDS newVdsData = new VDS(); - newVdsData.setHostName("BUZZ"); + newVdsData.setHostName(VDS_HOSTNAME); newVdsData.setVdsName("BAR"); newVdsData.setVdsGroupCompatibilityVersion(new Version("1.2.3")); newVdsData.setVdsGroupId(new Guid()); @@ -84,6 +91,10 @@ when(commandMock.canConnect(Mockito.any(VDS.class))).thenCallRealMethod(); when(commandMock.getParameters()).thenReturn(parameters); + when(commandMock.createReturnValue()).thenCallRealMethod(); + when(commandMock.getReturnValue()).thenCallRealMethod(); + doCallRealMethod().when(commandMock).addCanDoActionMessage(any(VdcBllMessages.class)); + when(commandMock.isGlusterSupportEnabled()).thenReturn(glusterEnabled); when(commandMock.getVdsGroupDAO()).thenReturn(groupDAOMock); when(commandMock.getClusterUtils()).thenReturn(clusterUtils); @@ -97,6 +108,12 @@ when(commandMock.getSSHClient(any(String.class))).thenReturn(sshClient); doNothing().when(sshClient).connect(); doNothing().when(sshClient).authenticate(); + + when(commandMock.getInterfaceDao()).thenReturn(interfaceDaoMock); + when(interfaceDaoMock.getAllInterfacesWithIpAddress(any(Guid.class), eq(NEW_IP))).thenReturn(Collections.EMPTY_LIST); + when(interfaceDaoMock.getAllInterfacesWithIpAddress(any(Guid.class), eq(EXISTING_IP))).thenReturn(Collections.singletonList(new VdsNetworkInterface())); + + when(commandMock.getHostAddress(VDS_HOSTNAME)).thenReturn(NEW_IP); } private void setupVirtMock() throws Exception { @@ -106,10 +123,6 @@ @SuppressWarnings("unchecked") private void setupGlusterMock(boolean clusterHasServers, VDS upServer, boolean hasPeers) throws Exception { setupCommonMock(true); - - when(commandMock.createReturnValue()).thenCallRealMethod(); - when(commandMock.getReturnValue()).thenCallRealMethod(); - doCallRealMethod().when(commandMock).addCanDoActionMessage(any(VdcBllMessages.class)); when(commandMock.getGlusterUtil()).thenReturn(glusterUtil); when(glusterUtil.getPeers(any(SSHClient.class))).thenReturn(hasPeers ? Collections.singleton(PEER_1) @@ -130,6 +143,17 @@ } @Test + public void canDoActionFailsWhenServerExists() throws Exception { + setupVirtMock(); + when(commandMock.getHostAddress(VDS_HOSTNAME)).thenReturn(EXISTING_IP); + + assertFalse(commandMock.canDoAction()); + assertTrue(commandMock.getReturnValue() + .getCanDoActionMessages() + .contains(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST.toString())); + } + + @Test public void canDoActionSucceedsOnEmptyClusterEvenWhenGlusterServerHasPeers() throws Exception { setupGlusterMock(false, null, true); assertTrue(commandMock.canDoAction()); -- To view, visit http://gerrit.ovirt.org/14402 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I37c5ee3546a21400678c2f1f7d1e54bb726988b3 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shireesh Anjal <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
