Repository: cloudstack Updated Branches: refs/heads/master 282c9da68 -> 94ab870d4
null pointer dereference fix Signed-off-by: Laszlo Hornyak <laszlo.horn...@gmail.com> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/94ab870d Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/94ab870d Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/94ab870d Branch: refs/heads/master Commit: 94ab870d423fa168773469221929281f2a21a919 Parents: 282c9da Author: Laszlo Hornyak <laszlo.horn...@gmail.com> Authored: Mon Apr 28 20:52:32 2014 +0200 Committer: Laszlo Hornyak <laszlo.horn...@gmail.com> Committed: Tue Apr 29 21:46:16 2014 +0200 ---------------------------------------------------------------------- .../ExternalLoadBalancerDeviceManagerImpl.java | 19 +- ...ternalLoadBalancerDeviceManagerImplTest.java | 210 +++++++++++++++++++ 2 files changed, 221 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/94ab870d/server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java b/server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java index 2a22ac2..a608db8 100644 --- a/server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java +++ b/server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java @@ -691,7 +691,9 @@ public abstract class ExternalLoadBalancerDeviceManagerImpl extends AdapterBase try { answer = (DestroyLoadBalancerApplianceAnswer)_agentMgr.easySend(lbDevice.getParentHostId(), lbDeleteCmd); if (answer == null || !answer.getResult()) { - s_logger.warn("Failed to destoy load balancer appliance used by the network" + guestConfig.getId() + " due to " + answer.getDetails()); + s_logger.warn("Failed to destoy load balancer appliance used by the network" + + guestConfig.getId() + " due to " + answer == null ? "communication error with agent" + : answer.getDetails()); } } catch (Exception e) { s_logger.warn("Failed to destroy load balancer appliance used by the network" + guestConfig.getId() + " due to " + e.getMessage()); @@ -1145,7 +1147,6 @@ public abstract class ExternalLoadBalancerDeviceManagerImpl extends AdapterBase // Find the external load balancer in this zone long zoneId = network.getDataCenterId(); DataCenterVO zone = _dcDao.findById(zoneId); - HealthCheckLBConfigAnswer answer = null; if (loadBalancingRules == null || loadBalancingRules.isEmpty()) { return null; @@ -1169,10 +1170,8 @@ public abstract class ExternalLoadBalancerDeviceManagerImpl extends AdapterBase List<LoadBalancerTO> loadBalancersToApply = new ArrayList<LoadBalancerTO>(); List<MappingState> mappingStates = new ArrayList<MappingState>(); - for (int i = 0; i < loadBalancingRules.size(); i++) { - LoadBalancingRule rule = loadBalancingRules.get(i); - - boolean revoked = (rule.getState().equals(FirewallRule.State.Revoke)); + for (final LoadBalancingRule rule : loadBalancingRules) { + boolean revoked = (FirewallRule.State.Revoke.equals(rule.getState())); String protocol = rule.getProtocol(); String algorithm = rule.getAlgorithm(); String uuid = rule.getUuid(); @@ -1213,12 +1212,16 @@ public abstract class ExternalLoadBalancerDeviceManagerImpl extends AdapterBase long guestVlanTag = Integer.parseInt(BroadcastDomainType.getValue(network.getBroadcastUri())); cmd.setAccessDetail(NetworkElementCommand.GUEST_VLAN_TAG, String.valueOf(guestVlanTag)); - answer = (HealthCheckLBConfigAnswer)_agentMgr.easySend(externalLoadBalancer.getId(), cmd); + HealthCheckLBConfigAnswer answer = (HealthCheckLBConfigAnswer) _agentMgr + .easySend(externalLoadBalancer.getId(), cmd); + // easySend will return null on error + return answer == null ? null : answer.getLoadBalancers(); } } catch (Exception ex) { s_logger.error("Exception Occured ", ex); } - return answer.getLoadBalancers(); + //null return is handled by clients + return null; } private NicVO getPlaceholderNic(Network network) { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/94ab870d/server/test/com/cloud/network/ExternalLoadBalancerDeviceManagerImplTest.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/network/ExternalLoadBalancerDeviceManagerImplTest.java b/server/test/com/cloud/network/ExternalLoadBalancerDeviceManagerImplTest.java new file mode 100644 index 0000000..046bdb6 --- /dev/null +++ b/server/test/com/cloud/network/ExternalLoadBalancerDeviceManagerImplTest.java @@ -0,0 +1,210 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package com.cloud.network; + +import java.lang.reflect.Field; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Arrays; +import java.util.Collections; + +import javax.inject.Inject; + +import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; + +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Command; +import com.cloud.agent.api.routing.HealthCheckLBConfigAnswer; +import com.cloud.agent.api.to.LoadBalancerTO; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.HostPodDao; +import com.cloud.dc.dao.VlanDao; +import com.cloud.exception.ResourceUnavailableException; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; +import com.cloud.host.dao.HostDetailsDao; +import com.cloud.network.dao.ExternalFirewallDeviceDao; +import com.cloud.network.dao.ExternalLoadBalancerDeviceDao; +import com.cloud.network.dao.ExternalLoadBalancerDeviceVO; +import com.cloud.network.dao.IPAddressDao; +import com.cloud.network.dao.InlineLoadBalancerNicMapDao; +import com.cloud.network.dao.LoadBalancerDao; +import com.cloud.network.dao.NetworkDao; +import com.cloud.network.dao.NetworkExternalFirewallDao; +import com.cloud.network.dao.NetworkExternalLoadBalancerDao; +import com.cloud.network.dao.NetworkExternalLoadBalancerVO; +import com.cloud.network.dao.NetworkServiceMapDao; +import com.cloud.network.dao.PhysicalNetworkDao; +import com.cloud.network.dao.PhysicalNetworkServiceProviderDao; +import com.cloud.network.lb.LoadBalancingRule; +import com.cloud.network.rules.dao.PortForwardingRulesDao; +import com.cloud.offerings.dao.NetworkOfferingDao; +import com.cloud.resource.ResourceManager; +import com.cloud.user.AccountManager; +import com.cloud.user.dao.AccountDao; +import com.cloud.user.dao.UserStatisticsDao; +import com.cloud.utils.net.Ip; +import com.cloud.vm.dao.DomainRouterDao; +import com.cloud.vm.dao.NicDao; + +@RunWith(MockitoJUnitRunner.class) +public class ExternalLoadBalancerDeviceManagerImplTest { + + @Mock + NetworkExternalLoadBalancerDao _networkExternalLBDao; + @Mock + ExternalLoadBalancerDeviceDao _externalLoadBalancerDeviceDao; + @Mock + HostDao _hostDao; + @Mock + DataCenterDao _dcDao; + @Mock + NetworkModel _networkModel; + @Mock + NetworkOrchestrationService _networkMgr; + @Mock + InlineLoadBalancerNicMapDao _inlineLoadBalancerNicMapDao; + @Mock + NicDao _nicDao; + @Mock + AgentManager _agentMgr; + @Mock + ResourceManager _resourceMgr; + @Mock + IPAddressDao _ipAddressDao; + @Mock + VlanDao _vlanDao; + @Mock + NetworkOfferingDao _networkOfferingDao; + @Mock + AccountDao _accountDao; + @Mock + PhysicalNetworkDao _physicalNetworkDao; + @Mock + PhysicalNetworkServiceProviderDao _physicalNetworkServiceProviderDao; + @Mock + AccountManager _accountMgr; + @Mock + UserStatisticsDao _userStatsDao; + @Mock + NetworkDao _networkDao; + @Mock + DomainRouterDao _routerDao; + @Mock + LoadBalancerDao _loadBalancerDao; + @Mock + PortForwardingRulesDao _portForwardingRulesDao; + @Mock + ConfigurationDao _configDao; + @Mock + HostDetailsDao _hostDetailDao; + @Mock + NetworkExternalLoadBalancerDao _networkLBDao; + @Mock + NetworkServiceMapDao _ntwkSrvcProviderDao; + @Mock + NetworkExternalFirewallDao _networkExternalFirewallDao; + @Mock + ExternalFirewallDeviceDao _externalFirewallDeviceDao; + @Mock + protected HostPodDao _podDao = null; + @Mock + IpAddressManager _ipAddrMgr; + + @Mock + Network network; + + @Mock + LoadBalancingRule rule; + + ExternalLoadBalancerDeviceManagerImpl externalLoadBalancerDeviceManager; + + @Before + public void setup() throws IllegalArgumentException, + IllegalAccessException, NoSuchFieldException, SecurityException { + externalLoadBalancerDeviceManager = new ExternalLoadBalancerDeviceManagerImpl() { + }; + for (Field fieldToInject : ExternalLoadBalancerDeviceManagerImpl.class + .getDeclaredFields()) { + if (fieldToInject.isAnnotationPresent(Inject.class)) { + fieldToInject.setAccessible(true); + fieldToInject.set(externalLoadBalancerDeviceManager, this + .getClass().getDeclaredField(fieldToInject.getName()) + .get(this)); + } + } + } + + @Test + public void getLBHealthChecks() throws ResourceUnavailableException, + URISyntaxException { + setupLBHealthChecksMocks(); + + HealthCheckLBConfigAnswer answer = Mockito + .mock(HealthCheckLBConfigAnswer.class); + Mockito.when(answer.getLoadBalancers()).thenReturn( + Collections.<LoadBalancerTO> emptyList()); + Mockito.when( + _agentMgr.easySend(Mockito.anyLong(), + Mockito.any(Command.class))).thenReturn(answer); + + Assert.assertNotNull(externalLoadBalancerDeviceManager + .getLBHealthChecks(network, Arrays.asList(rule))); + } + + @Test + public void getLBHealthChecksNullAnswer() throws ResourceUnavailableException, + URISyntaxException { + setupLBHealthChecksMocks(); + + Mockito.when( + _agentMgr.easySend(Mockito.anyLong(), + Mockito.any(Command.class))).thenReturn(null); + + Assert.assertNull(externalLoadBalancerDeviceManager + .getLBHealthChecks(network, Arrays.asList(rule))); + } + + private void setupLBHealthChecksMocks() throws URISyntaxException { + Mockito.when(network.getId()).thenReturn(42l); + Mockito.when(network.getBroadcastUri()).thenReturn(new URI("vlan://1")); + NetworkExternalLoadBalancerVO externalLb = Mockito + .mock(NetworkExternalLoadBalancerVO.class); + Mockito.when(externalLb.getExternalLBDeviceId()).thenReturn(66l); + Mockito.when(_networkExternalLBDao.findByNetworkId(42)).thenReturn( + externalLb); + ExternalLoadBalancerDeviceVO lbDevice = Mockito + .mock(ExternalLoadBalancerDeviceVO.class); + Mockito.when(_externalLoadBalancerDeviceDao.findById(66l)).thenReturn( + lbDevice); + Mockito.when(rule.getAlgorithm()).thenReturn("TEST"); + Mockito.when(rule.getProtocol()).thenReturn("TEST"); + Mockito.when(rule.getSourceIp()).thenReturn(new Ip(1l)); + Mockito.when(lbDevice.getHostId()).thenReturn(99l); + HostVO hostVo = Mockito.mock(HostVO.class); + Mockito.when(_hostDao.findById(Mockito.anyLong())).thenReturn(hostVo); + } +}
