cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From d...@apache.org
Subject [25/50] [abbrv] git commit: updated refs/heads/vpc-refactor to a8cb7c1
Date Fri, 03 Oct 2014 11:12:03 GMT
Refactor and test NetworkHelper#sendCommandsToRouterWithNoAnswers


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/8061c5ca
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/8061c5ca
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/8061c5ca

Branch: refs/heads/vpc-refactor
Commit: 8061c5cadbbda6ce51b30d55ca36de7efe35b961
Parents: 51ce935
Author: Antonio Fornie <afornie@schubergphilis.com>
Authored: Thu Aug 21 03:31:02 2014 -0500
Committer: Wilder Rodrigues <wrodrigues@schubergphilis.com>
Committed: Fri Oct 3 06:21:27 2014 +0200

----------------------------------------------------------------------
 .../spring-server-core-managers-context.xml     |   3 -
 .../cloud/network/router/NetworkHelperImpl.java |  18 +--
 .../network/router/RouterControlHelper.java     |   2 +-
 .../VpcVirtualNetworkApplianceManagerImpl.java  |   8 +-
 .../topology/AdvancedNetworkTopology.java       |   1 +
 .../network/topology/BasicNetworkTopology.java  |   1 +
 .../network/router/NetworkHelperImplTest.java   | 156 +++++++++++++++++++
 .../network/router/RouterControlHelperTest.java | 105 +++++++++++++
 8 files changed, 274 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8061c5ca/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
----------------------------------------------------------------------
diff --git a/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
b/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
index 2b46ca3..7a9044b 100644
--- a/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
+++ b/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
@@ -206,9 +206,6 @@
     <bean id="vpcNetworkHelper"
         class="com.cloud.network.router.VpcNetworkHelperImpl" />
         
-    <bean id="vpcVirtualNetworkHelper"
-        class="com.cloud.network.router.VpcVirtualNetworkHelperImpl" />
-        
     <bean id="nicProfileHelper"
         class="com.cloud.network.router.NicProfileHelperImpl" />
         

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8061c5ca/server/src/com/cloud/network/router/NetworkHelperImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/network/router/NetworkHelperImpl.java b/server/src/com/cloud/network/router/NetworkHelperImpl.java
index ece0c0f..689ef3b 100644
--- a/server/src/com/cloud/network/router/NetworkHelperImpl.java
+++ b/server/src/com/cloud/network/router/NetworkHelperImpl.java
@@ -195,22 +195,16 @@ public class NetworkHelperImpl implements NetworkHelper {
             throw new AgentUnavailableException("Unable to send commands to virtual router
", router.getHostId(), e);
         }
 
-        if (answers == null) {
-            return false;
-        }
-
-        if (answers.length != cmds.size()) {
+        if (answers == null || answers.length != cmds.size()) {
             return false;
         }
 
         // FIXME: Have to return state for individual command in the future
         boolean result = true;
-        if (answers.length > 0) {
-            for (final Answer answer : answers) {
-                if (!answer.getResult()) {
-                    result = false;
-                    break;
-                }
+        for (final Answer answer : answers) {
+            if (!answer.getResult()) {
+                result = false;
+                break;
             }
         }
         return result;
@@ -253,7 +247,7 @@ public class NetworkHelperImpl implements NetworkHelper {
         final int connRouterPR = getRealPriority(connectedRouter);
         final int disconnRouterPR = getRealPriority(disconnectedRouter);
         if (connRouterPR < disconnRouterPR) {
-            //connRouterPR < disconnRouterPR, they won't equal at anytime
+            //connRouterPR < disconnRouterPR, they won't equal at any time
             if (!connectedRouter.getIsPriorityBumpUp()) {
                 final BumpUpPriorityCommand command = new BumpUpPriorityCommand();
                 command.setAccessDetail(NetworkElementCommand.ROUTER_IP, getRouterControlIp(connectedRouter.getId()));

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8061c5ca/server/src/com/cloud/network/router/RouterControlHelper.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/network/router/RouterControlHelper.java b/server/src/com/cloud/network/router/RouterControlHelper.java
index 583bd58..68fd6e3 100644
--- a/server/src/com/cloud/network/router/RouterControlHelper.java
+++ b/server/src/com/cloud/network/router/RouterControlHelper.java
@@ -35,7 +35,7 @@ public class RouterControlHelper {
     private static final Logger logger = Logger.getLogger(RouterControlHelper.class);
 
     @Inject
-    private final DomainRouterDao routerDao = null;
+    private DomainRouterDao routerDao;
 
     @Inject
     private NetworkDao networkDao;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8061c5ca/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
index 99ed723..a7edfe3 100644
--- a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
+++ b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
@@ -135,7 +135,7 @@ public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplian
     private EntityManager _entityMgr;
 
     @Inject
-    private NicProfileHelper vpcHelper;
+    private NicProfileHelper nicProfileHelper;
 
     @Override
     public boolean configure(final String name, final Map<String, Object> params) throws
ConfigurationException {
@@ -323,7 +323,7 @@ public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplian
             int i = 0;
 
             for (final PublicIpAddress ipAddr : ipAddrList) {
-                boolean add = (ipAddr.getState() == IpAddress.State.Releasing ? false : true);
+                boolean add = ipAddr.getState() == IpAddress.State.Releasing ? false : true;
 
                 String macAddress = vlanMacAddress.get(BroadcastDomainType.getValue(BroadcastDomainType.fromString(ipAddr.getVlanTag())));
 
@@ -431,7 +431,7 @@ public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplian
     public boolean finalizeCommandsOnStart(final Commands cmds, final VirtualMachineProfile
profile) {
         DomainRouterVO router = _routerDao.findById(profile.getId());
 
-        boolean isVpc = (router.getVpcId() != null);
+        boolean isVpc = router.getVpcId() != null;
         if (!isVpc) {
             return super.finalizeCommandsOnStart(cmds, profile);
         }
@@ -621,7 +621,7 @@ public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplian
         if (router.getVpcId() != null) {
             if (_networkModel.isProviderSupportServiceInNetwork(guestNetworkId, Service.NetworkACL,
Provider.VPCVirtualRouter)) {
                 List<NetworkACLItemVO> networkACLs = _networkACLMgr.listNetworkACLItems(guestNetworkId);
-                if ((networkACLs != null) && !networkACLs.isEmpty()) {
+                if (networkACLs != null && !networkACLs.isEmpty()) {
                     s_logger.debug("Found " + networkACLs.size() + " network ACLs to apply
as a part of VPC VR " + router + " start for guest network id=" +
                             guestNetworkId);
                     createNetworkACLsCommands(networkACLs, router, cmds, guestNetworkId,
false);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8061c5ca/server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java
----------------------------------------------------------------------
diff --git a/server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java
b/server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java
index 8b2a5a6..892a81a 100644
--- a/server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java
+++ b/server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java
@@ -285,6 +285,7 @@ public class AdvancedNetworkTopology extends BasicNetworkTopology {
         }
 
         if (!connectedRouters.isEmpty()) {
+            // Shouldn't we include this check inside the method?
             if (!isZoneBasic && !disconnectedRouters.isEmpty() && disconnectedRouters.get(0).getIsRedundantRouter())
{
                 // These disconnected redundant virtual routers are out of sync
                 // now, stop them for synchronization

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8061c5ca/server/src/org/apache/cloudstack/network/topology/BasicNetworkTopology.java
----------------------------------------------------------------------
diff --git a/server/src/org/apache/cloudstack/network/topology/BasicNetworkTopology.java b/server/src/org/apache/cloudstack/network/topology/BasicNetworkTopology.java
index 0873d9a..ec52911 100644
--- a/server/src/org/apache/cloudstack/network/topology/BasicNetworkTopology.java
+++ b/server/src/org/apache/cloudstack/network/topology/BasicNetworkTopology.java
@@ -412,6 +412,7 @@ public class BasicNetworkTopology implements NetworkTopology {
         }
 
         if (!connectedRouters.isEmpty()) {
+            // Shouldn't we include this check inside the method?
             if (!isZoneBasic && !disconnectedRouters.isEmpty() && disconnectedRouters.get(0).getIsRedundantRouter())
{
                 // These disconnected redundant virtual routers are out of sync
                 // now, stop them for synchronization

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8061c5ca/server/test/com/cloud/network/router/NetworkHelperImplTest.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/network/router/NetworkHelperImplTest.java b/server/test/com/cloud/network/router/NetworkHelperImplTest.java
new file mode 100644
index 0000000..04c5067
--- /dev/null
+++ b/server/test/com/cloud/network/router/NetworkHelperImplTest.java
@@ -0,0 +1,156 @@
+package com.cloud.network.router;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Matchers;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.Command;
+import com.cloud.agent.manager.Commands;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+
+@RunWith(MockitoJUnitRunner.class)
+public class NetworkHelperImplTest {
+
+    private static final long HOST_ID = 10L;
+
+    @Mock
+    protected AgentManager agentManager;
+
+    @InjectMocks
+    protected NetworkHelperImpl nwHelper = new NetworkHelperImpl();
+
+    @Test(expected=CloudRuntimeException.class)
+    public void testSendCommandsToRouterWrongRouterVersion()
+            throws AgentUnavailableException, OperationTimedoutException {
+        // Prepare
+        NetworkHelperImpl nwHelperUT = spy(this.nwHelper);
+        VirtualRouter vr = mock(VirtualRouter.class);
+        doReturn(false).when(nwHelperUT).checkRouterVersion(vr);
+
+        // Execute
+        nwHelperUT.sendCommandsToRouter(vr, null);
+
+        // Assert
+        verify(this.agentManager, times(0)).send((Long) Matchers.anyObject(), (Command) Matchers.anyObject());
+    }
+
+    @Test
+    public void testSendCommandsToRouter()
+            throws AgentUnavailableException, OperationTimedoutException {
+        // Prepare
+        NetworkHelperImpl nwHelperUT = spy(this.nwHelper);
+        VirtualRouter vr = mock(VirtualRouter.class);
+        when(vr.getHostId()).thenReturn(HOST_ID);
+        doReturn(true).when(nwHelperUT).checkRouterVersion(vr);
+
+        Commands commands = mock(Commands.class);
+        when(commands.size()).thenReturn(3);
+        Answer answer1 = mock(Answer.class);
+        Answer answer2 = mock(Answer.class);
+        Answer answer3 = mock(Answer.class);
+        // In the second iteration it should match and return, without invoking the third
+        Answer[] answers = {answer1, answer2, answer3};
+        when(answer1.getResult()).thenReturn(true);
+        when(answer2.getResult()).thenReturn(false);
+        when(answer3.getResult()).thenReturn(false);
+        when(this.agentManager.send(HOST_ID, commands)).thenReturn(answers);
+
+        // Execute
+        final boolean result = nwHelperUT.sendCommandsToRouter(vr, commands);
+
+        // Assert
+        verify(this.agentManager, times(1)).send(HOST_ID, commands);
+        verify(answer1, times(1)).getResult();
+        verify(answer2, times(1)).getResult();
+        verify(answer3, times(0)).getResult();
+        assertFalse(result);
+    }
+
+    /**
+     * The only way result can be true is if each and every command receive a true result
+     *
+     * @throws AgentUnavailableException
+     * @throws OperationTimedoutException
+     */
+    @Test
+    public void testSendCommandsToRouterWithTrueResult()
+            throws AgentUnavailableException, OperationTimedoutException {
+        // Prepare
+        NetworkHelperImpl nwHelperUT = spy(this.nwHelper);
+        VirtualRouter vr = mock(VirtualRouter.class);
+        when(vr.getHostId()).thenReturn(HOST_ID);
+        doReturn(true).when(nwHelperUT).checkRouterVersion(vr);
+
+        Commands commands = mock(Commands.class);
+        when(commands.size()).thenReturn(3);
+        Answer answer1 = mock(Answer.class);
+        Answer answer2 = mock(Answer.class);
+        Answer answer3 = mock(Answer.class);
+        // In the second iteration it should match and return, without invoking the third
+        Answer[] answers = {answer1, answer2, answer3};
+        when(answer1.getResult()).thenReturn(true);
+        when(answer2.getResult()).thenReturn(true);
+        when(answer3.getResult()).thenReturn(true);
+        when(this.agentManager.send(HOST_ID, commands)).thenReturn(answers);
+
+        // Execute
+        final boolean result = nwHelperUT.sendCommandsToRouter(vr, commands);
+
+        // Assert
+        verify(this.agentManager, times(1)).send(HOST_ID, commands);
+        verify(answer1, times(1)).getResult();
+        verify(answer2, times(1)).getResult();
+        verify(answer3, times(1)).getResult();
+        assertTrue(result);
+    }
+
+    /**
+     * If the number of answers is different to the number of commands the result is false
+     *
+     * @throws AgentUnavailableException
+     * @throws OperationTimedoutException
+     */
+    @Test
+    public void testSendCommandsToRouterWithNoAnswers()
+            throws AgentUnavailableException, OperationTimedoutException {
+        // Prepare
+        NetworkHelperImpl nwHelperUT = spy(this.nwHelper);
+        VirtualRouter vr = mock(VirtualRouter.class);
+        when(vr.getHostId()).thenReturn(HOST_ID);
+        doReturn(true).when(nwHelperUT).checkRouterVersion(vr);
+
+        Commands commands = mock(Commands.class);
+        when(commands.size()).thenReturn(3);
+        Answer answer1 = mock(Answer.class);
+        Answer answer2 = mock(Answer.class);
+        // In the second iteration it should match and return, without invoking the third
+        Answer[] answers = {answer1, answer2};
+        when(this.agentManager.send(HOST_ID, commands)).thenReturn(answers);
+
+        // Execute
+        final boolean result = nwHelperUT.sendCommandsToRouter(vr, commands);
+
+        // Assert
+        verify(this.agentManager, times(1)).send(HOST_ID, commands);
+        verify(answer1, times(0)).getResult();
+        assertFalse(result);
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8061c5ca/server/test/com/cloud/network/router/RouterControlHelperTest.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/network/router/RouterControlHelperTest.java b/server/test/com/cloud/network/router/RouterControlHelperTest.java
new file mode 100644
index 0000000..46d0fb2
--- /dev/null
+++ b/server/test/com/cloud/network/router/RouterControlHelperTest.java
@@ -0,0 +1,105 @@
+package com.cloud.network.router;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import com.cloud.network.Networks.TrafficType;
+import com.cloud.network.dao.NetworkDao;
+import com.cloud.network.dao.NetworkVO;
+import com.cloud.vm.DomainRouterVO;
+import com.cloud.vm.NicVO;
+import com.cloud.vm.dao.DomainRouterDao;
+import com.cloud.vm.dao.NicDao;
+
+@RunWith(MockitoJUnitRunner.class)
+public class RouterControlHelperTest {
+
+    private static final String DIDN_T_GET_THE_EXPECTED_IP4_ADDRESS = "Didn't get the expected
IP4 address";
+    private static final String IP4_ADDRES1 = "IP4Addres1";
+    private static final String IP4_ADDRES2 = "IP4Addres2";
+    protected static final long ROUTER_ID = 1L;
+    protected static final long NW_ID_1 = 11L;
+    protected static final long NW_ID_2 = 12L;
+    protected static final long NW_ID_3 = 13L;
+
+    @Mock
+    protected NicDao nicDao;
+    @Mock
+    protected NetworkDao nwDao;
+    @Mock
+    protected DomainRouterDao routerDao;
+
+    @InjectMocks
+    protected RouterControlHelper routerControlHelper = new RouterControlHelper();
+
+    @Test
+    public void testGetRouterControlIp() {
+        // Prepare
+        List<NicVO> nics = new ArrayList<>();
+        NicVO nic1 = mock(NicVO.class);
+        NicVO nic2 = mock(NicVO.class);
+        // Actually the third one will never be used, but we must assert that is not
+        NicVO nic3 = mock(NicVO.class);
+        when(nic1.getNetworkId()).thenReturn(NW_ID_1);
+        when(nic2.getNetworkId()).thenReturn(NW_ID_2);
+        when(nic2.getIp4Address()).thenReturn(IP4_ADDRES1);
+        when(nic3.getNetworkId()).thenReturn(NW_ID_3);
+        when(nic3.getIp4Address()).thenReturn(IP4_ADDRES2);
+        nics.add(nic1);
+        nics.add(nic2);
+        nics.add(nic3);
+        when(this.nicDao.listByVmId(ROUTER_ID)).thenReturn(nics);
+
+        NetworkVO nw1 = mock(NetworkVO.class);
+        when(nw1.getTrafficType()).thenReturn(TrafficType.Public);
+        NetworkVO nw2 = mock(NetworkVO.class);
+        when(nw2.getTrafficType()).thenReturn(TrafficType.Control);
+        NetworkVO nw3 = mock(NetworkVO.class);
+        when(nw3.getTrafficType()).thenReturn(TrafficType.Control);
+        when(this.nwDao.findById(NW_ID_1)).thenReturn(nw1);
+        when(this.nwDao.findById(NW_ID_2)).thenReturn(nw2);
+        when(this.nwDao.findById(NW_ID_3)).thenReturn(nw3);
+
+        // Execute
+        final String ip4address = this.routerControlHelper.getRouterControlIp(ROUTER_ID);
+
+        // Assert
+        assertEquals(DIDN_T_GET_THE_EXPECTED_IP4_ADDRESS, IP4_ADDRES1, ip4address);
+    }
+
+    @Test
+    public void testGetRouterControlIpWithRouterIp() {
+        // Prepare
+        List<NicVO> nics = new ArrayList<>();
+        NicVO nic1 = mock(NicVO.class);
+        when(nic1.getNetworkId()).thenReturn(NW_ID_1);
+        when(nic1.getIp4Address()).thenReturn(null);
+        nics.add(nic1);
+        when(this.nicDao.listByVmId(ROUTER_ID)).thenReturn(nics);
+
+        NetworkVO nw1 = mock(NetworkVO.class);
+        when(nw1.getTrafficType()).thenReturn(TrafficType.Public);
+        when(this.nwDao.findById(NW_ID_1)).thenReturn(nw1);
+
+        DomainRouterVO router = mock(DomainRouterVO.class);
+        when(this.routerDao.findById(ROUTER_ID)).thenReturn(router);
+        when(router.getPrivateIpAddress()).thenReturn(IP4_ADDRES1);
+
+        // Execute
+        final String ip4address = this.routerControlHelper.getRouterControlIp(ROUTER_ID);
+
+        // Assert
+        assertEquals(DIDN_T_GET_THE_EXPECTED_IP4_ADDRESS, IP4_ADDRES1, ip4address);
+    }
+
+}


Mime
View raw message