From 3199de69fe6511cbb424940b7c7d00b52f2ec631 Mon Sep 17 00:00:00 2001 From: wrodrigues Date: Tue, 11 Feb 2014 11:15:55 +0100 Subject: [PATCH] Fixes on Contrail and Mon InMemory plugins; adding comments about the changes. Signed-off-by: Hugo Trippaers --- .../mom/inmemory/InMemoryEventBus.java | 53 ++++-- .../mom/inmemory/InMemoryEventBusTest.java | 162 ++++++++++++++++ .../management/ContrailManagerImpl.java | 68 +++---- .../network/contrail/model/ModelObject.java | 10 +- .../contrail/model/ServiceInstanceModel.java | 21 +- .../contrail/model/VirtualMachineModel.java | 78 +++++--- .../contrail/model/VirtualNetworkModel.java | 179 ++++++++++-------- .../model/VirtualMachineModelTest.java | 9 +- .../model/VirtualNetworkModelTest.java | 143 +++++++++++++- utils/src/com/cloud/utils/UuidUtils.java | 10 +- utils/test/com/cloud/utils/UuidUtilsTest.java | 23 +++ 11 files changed, 571 insertions(+), 185 deletions(-) create mode 100644 plugins/event-bus/inmemory/test/org/apache/cloudstack/mom/inmemory/InMemoryEventBusTest.java create mode 100644 utils/test/com/cloud/utils/UuidUtilsTest.java diff --git a/plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java b/plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java index 99d0a12eb3b..9a57ff067bf 100644 --- a/plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java +++ b/plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java @@ -26,33 +26,36 @@ import java.util.concurrent.ConcurrentHashMap; import javax.ejb.Local; import javax.naming.ConfigurationException; -import com.cloud.utils.Pair; -import org.apache.log4j.Logger; - import org.apache.cloudstack.framework.events.Event; import org.apache.cloudstack.framework.events.EventBus; import org.apache.cloudstack.framework.events.EventBusException; import org.apache.cloudstack.framework.events.EventSubscriber; import org.apache.cloudstack.framework.events.EventTopic; +import org.apache.log4j.Logger; +import com.cloud.utils.Pair; import com.cloud.utils.component.ManagerBase; @Local(value = EventBus.class) public class InMemoryEventBus extends ManagerBase implements EventBus { - private String name; private static final Logger s_logger = Logger.getLogger(InMemoryEventBus.class); - private static ConcurrentHashMap> s_subscribers; + + private final static ConcurrentHashMap> subscribers; + + static { + subscribers = new ConcurrentHashMap>(); + } @Override public boolean configure(String name, Map params) throws ConfigurationException { - s_subscribers = new ConcurrentHashMap>(); + _name = name; return true; } @Override public void setName(String name) { - this.name = name; + _name = name; } @Override @@ -62,29 +65,35 @@ public class InMemoryEventBus extends ManagerBase implements EventBus { } UUID subscriberId = UUID.randomUUID(); - s_subscribers.put(subscriberId, new Pair(topic, subscriber)); + subscribers.put(subscriberId, new Pair(topic, subscriber)); return subscriberId; } @Override public void unsubscribe(UUID subscriberId, EventSubscriber subscriber) throws EventBusException { - if (s_subscribers != null && s_subscribers.isEmpty()) { + if (subscriberId == null) { + throw new EventBusException("Cannot unregister a null subscriberId."); + } + + if (subscribers.isEmpty()) { throw new EventBusException("There are no registered subscribers to unregister."); } - if (s_subscribers.get(subscriberId) == null) { + + if (!subscribers.containsKey(subscriberId)) { throw new EventBusException("No subscriber found with subscriber id " + subscriberId); + } else { + subscribers.remove(subscriberId); } - s_subscribers.remove(subscriberId); } @Override public void publish(Event event) throws EventBusException { - if (s_subscribers == null || s_subscribers.isEmpty()) { + if (subscribers == null || subscribers.isEmpty()) { return; // no subscriber to publish to, so just return } - for (UUID subscriberId : s_subscribers.keySet()) { - Pair subscriberDetails = s_subscribers.get(subscriberId); + for (UUID subscriberId : subscribers.keySet()) { + Pair subscriberDetails = subscribers.get(subscriberId); // if the event matches subscribers interested event topic then call back the subscriber with the event if (isEventMatchesTopic(event, subscriberDetails.first())) { EventSubscriber subscriber = subscriberDetails.second(); @@ -108,6 +117,10 @@ public class InMemoryEventBus extends ManagerBase implements EventBus { return true; } + public int totalSubscribers() { + return subscribers.size(); + } + private String replaceNullWithWildcard(String key) { if (key == null || key.isEmpty()) { return "*"; @@ -122,7 +135,7 @@ public class InMemoryEventBus extends ManagerBase implements EventBus { eventTopicSource = eventTopicSource.replace(".", "-"); String eventSource = replaceNullWithWildcard(event.getEventSource()); eventSource = eventSource.replace(".", "-"); - if (eventTopicSource != "*" && eventSource != "*" && !eventTopicSource.equalsIgnoreCase(eventSource)) { + if (!eventTopicSource.equals("*") && !eventSource.equals("*") && !eventTopicSource.equalsIgnoreCase(eventSource)) { return false; } @@ -130,7 +143,7 @@ public class InMemoryEventBus extends ManagerBase implements EventBus { eventTopicCategory = eventTopicCategory.replace(".", "-"); String eventCategory = replaceNullWithWildcard(event.getEventCategory()); eventCategory = eventCategory.replace(".", "-"); - if (eventTopicCategory != "*" && eventCategory != "*" && !eventTopicCategory.equalsIgnoreCase(eventCategory)) { + if (!eventTopicCategory.equals("*") && !eventCategory.equals("*") && !eventTopicCategory.equalsIgnoreCase(eventCategory)) { return false; } @@ -138,7 +151,7 @@ public class InMemoryEventBus extends ManagerBase implements EventBus { eventTopicType = eventTopicType.replace(".", "-"); String eventType = replaceNullWithWildcard(event.getEventType()); eventType = eventType.replace(".", "-"); - if (eventTopicType != "*" && eventType != "*" && !eventTopicType.equalsIgnoreCase(eventType)) { + if (!eventTopicType.equals("*") && !eventType.equals("*") && !eventTopicType.equalsIgnoreCase(eventType)) { return false; } @@ -146,7 +159,7 @@ public class InMemoryEventBus extends ManagerBase implements EventBus { eventTopicResourceType = eventTopicResourceType.replace(".", "-"); String resourceType = replaceNullWithWildcard(event.getResourceType()); resourceType = resourceType.replace(".", "-"); - if (eventTopicResourceType != "*" && resourceType != "*" && !eventTopicResourceType.equalsIgnoreCase(resourceType)) { + if (!eventTopicResourceType.equals("*") && !resourceType.equals("*") && !eventTopicResourceType.equalsIgnoreCase(resourceType)) { return false; } @@ -154,10 +167,10 @@ public class InMemoryEventBus extends ManagerBase implements EventBus { resourceUuid = resourceUuid.replace(".", "-"); String eventTopicresourceUuid = replaceNullWithWildcard(topic.getResourceUUID()); eventTopicresourceUuid = eventTopicresourceUuid.replace(".", "-"); - if (resourceUuid != "*" && eventTopicresourceUuid != "*" && !resourceUuid.equalsIgnoreCase(eventTopicresourceUuid)) { + if (!resourceUuid.equals("*") && !eventTopicresourceUuid.equals("*") && !resourceUuid.equalsIgnoreCase(eventTopicresourceUuid)) { return false; } return true; } -} +} \ No newline at end of file diff --git a/plugins/event-bus/inmemory/test/org/apache/cloudstack/mom/inmemory/InMemoryEventBusTest.java b/plugins/event-bus/inmemory/test/org/apache/cloudstack/mom/inmemory/InMemoryEventBusTest.java new file mode 100644 index 00000000000..a967f8934ef --- /dev/null +++ b/plugins/event-bus/inmemory/test/org/apache/cloudstack/mom/inmemory/InMemoryEventBusTest.java @@ -0,0 +1,162 @@ +/* + * 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 org.apache.cloudstack.mom.inmemory; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.util.UUID; + +import org.apache.cloudstack.framework.events.Event; +import org.apache.cloudstack.framework.events.EventBusException; +import org.apache.cloudstack.framework.events.EventSubscriber; +import org.apache.cloudstack.framework.events.EventTopic; +import org.junit.Test; + +import com.cloud.utils.UuidUtils; + +public class InMemoryEventBusTest { + + @Test + public void testConfigure() throws Exception { + String name = "test001"; + + InMemoryEventBus bus = new InMemoryEventBus(); + boolean success = bus.configure(name, null); + + assertTrue(success); + assertTrue(name.equals(bus.getName())); + } + + @Test + public void testSubscribe() throws Exception { + EventTopic topic = mock(EventTopic.class); + EventSubscriber subscriber = mock(EventSubscriber.class); + + InMemoryEventBus bus = new InMemoryEventBus(); + + UUID uuid = bus.subscribe(topic, subscriber); + assertNotNull(uuid); + + String uuidStr = uuid.toString(); + assertTrue(UuidUtils.validateUUID(uuidStr)); + assertTrue(bus.totalSubscribers() == 1); + + bus.unsubscribe(uuid, subscriber); + assertTrue(bus.totalSubscribers() == 0); + } + + @Test(expected = EventBusException.class) + public void testSubscribeFailTopic() throws Exception { + EventSubscriber subscriber = mock(EventSubscriber.class); + + InMemoryEventBus bus = new InMemoryEventBus(); + + bus.subscribe(null, subscriber); + } + + @Test(expected = EventBusException.class) + public void testSubscribeFailSubscriber() throws Exception { + EventTopic topic = mock(EventTopic.class); + + InMemoryEventBus bus = new InMemoryEventBus(); + + bus.subscribe(topic, null); + } + + @Test + public void testUnsubscribe() throws Exception { + EventTopic topic = mock(EventTopic.class); + EventSubscriber subscriber = mock(EventSubscriber.class); + + InMemoryEventBus bus = new InMemoryEventBus(); + + UUID uuid = bus.subscribe(topic, subscriber); + assertNotNull(uuid); + + String uuidStr = uuid.toString(); + + assertTrue(UuidUtils.validateUUID(uuidStr)); + assertTrue(bus.totalSubscribers() == 1); + // + bus.unsubscribe(uuid, subscriber); + assertTrue(bus.totalSubscribers() == 0); + } + + @Test(expected = EventBusException.class) + public void testUnsubscribeFailEmpty() throws Exception { + UUID uuid = UUID.randomUUID(); + + InMemoryEventBus bus = new InMemoryEventBus(); + bus.unsubscribe(uuid, null); + } + + @Test(expected = EventBusException.class) + public void testUnsubscribeFailNull() throws Exception { + InMemoryEventBus bus = new InMemoryEventBus(); + bus.unsubscribe(null, null); + } + + @Test(expected = EventBusException.class) + public void testUnsubscribeFailWrongUUID() throws Exception { + EventSubscriber subscriber = mock(EventSubscriber.class); + InMemoryEventBus bus = new InMemoryEventBus(); + UUID uuid = UUID.randomUUID(); + + bus.unsubscribe(uuid, subscriber); + } + + @Test + public void testPublish() throws Exception { + EventTopic topic = mock(EventTopic.class); + EventSubscriber subscriber = mock(EventSubscriber.class); + Event event = mock(Event.class); + + InMemoryEventBus bus = new InMemoryEventBus(); + + UUID uuid = bus.subscribe(topic, subscriber); + assertNotNull(uuid); + + String uuidStr = uuid.toString(); + assertTrue(UuidUtils.validateUUID(uuidStr)); + assertTrue(bus.totalSubscribers() == 1); + + bus.publish(event); + + verify(subscriber, times(1)).onEvent(event); + + bus.unsubscribe(uuid, subscriber); + assertTrue(bus.totalSubscribers() == 0); + } + + @Test + public void testPublishEmpty() throws Exception { + EventSubscriber subscriber = mock(EventSubscriber.class); + Event event = mock(Event.class); + + InMemoryEventBus bus = new InMemoryEventBus(); + bus.publish(event); + + verify(subscriber, times(0)).onEvent(event); + } +} \ No newline at end of file diff --git a/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java b/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java index b25de488f37..01be7dbd66a 100644 --- a/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java +++ b/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java @@ -31,9 +31,9 @@ import java.util.Set; import java.util.Timer; import java.util.TimerTask; +import javax.ejb.Local; import javax.inject.Inject; import javax.naming.ConfigurationException; -import javax.ejb.Local; import net.juniper.contrail.api.ApiConnector; import net.juniper.contrail.api.ApiConnectorFactory; @@ -44,19 +44,16 @@ import net.juniper.contrail.api.types.FloatingIpPool; import net.juniper.contrail.api.types.NetworkPolicy; import net.juniper.contrail.api.types.VirtualNetwork; -import org.apache.commons.lang.StringUtils; -import org.apache.log4j.Logger; - -import com.google.common.collect.ImmutableList; - import org.apache.cloudstack.network.contrail.model.FloatingIpModel; import org.apache.cloudstack.network.contrail.model.FloatingIpPoolModel; import org.apache.cloudstack.network.contrail.model.ModelController; import org.apache.cloudstack.network.contrail.model.VirtualNetworkModel; +import org.apache.commons.io.IOUtils; +import org.apache.commons.lang.StringUtils; +import org.apache.log4j.Logger; import com.cloud.configuration.ConfigurationManager; import com.cloud.configuration.ConfigurationService; -import com.cloud.server.ConfigurationServer; import com.cloud.dc.DataCenter; import com.cloud.dc.dao.DataCenterDao; import com.cloud.dc.dao.VlanDao; @@ -75,20 +72,21 @@ import com.cloud.network.dao.NetworkVO; import com.cloud.network.dao.PhysicalNetworkDao; import com.cloud.network.dao.PhysicalNetworkServiceProviderDao; import com.cloud.network.dao.PhysicalNetworkVO; -import com.cloud.offering.NetworkOffering.Availability; -import com.cloud.offering.NetworkOffering; -import com.cloud.offerings.NetworkOfferingVO; -import com.cloud.offerings.dao.NetworkOfferingDao; -import com.cloud.projects.ProjectVO; -import com.cloud.network.vpc.dao.NetworkACLDao; -import com.cloud.network.vpc.dao.VpcDao; -import com.cloud.network.vpc.dao.VpcOfferingDao; +import com.cloud.network.vpc.NetworkACLVO; import com.cloud.network.vpc.VpcOffering; import com.cloud.network.vpc.VpcOfferingVO; import com.cloud.network.vpc.VpcProvisioningService; import com.cloud.network.vpc.VpcVO; -import com.cloud.network.vpc.NetworkACLVO; +import com.cloud.network.vpc.dao.NetworkACLDao; +import com.cloud.network.vpc.dao.VpcDao; +import com.cloud.network.vpc.dao.VpcOfferingDao; +import com.cloud.offering.NetworkOffering; +import com.cloud.offering.NetworkOffering.Availability; +import com.cloud.offerings.NetworkOfferingVO; +import com.cloud.offerings.dao.NetworkOfferingDao; +import com.cloud.projects.ProjectVO; import com.cloud.projects.dao.ProjectDao; +import com.cloud.server.ConfigurationServer; import com.cloud.user.Account; import com.cloud.user.dao.AccountDao; import com.cloud.utils.PropertiesUtil; @@ -100,6 +98,7 @@ import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; +import com.google.common.collect.ImmutableList; @Local(value = { ContrailManager.class}) public class ContrailManagerImpl extends ManagerBase implements ContrailManager { @@ -159,7 +158,7 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager private Timer _dbSyncTimer; private int _dbSyncInterval = DB_SYNC_INTERVAL_DEFAULT; private final String configuration = "contrail.properties"; - private ModelDatabase _database; + private final ModelDatabase _database; private ModelController _controller; ContrailManagerImpl() { @@ -194,7 +193,7 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager } private NetworkOffering locatePublicNetworkOffering(String offeringName, - String offeringDisplayText, Provider provider) { + String offeringDisplayText, Provider provider) { List offerList = _configService.listNetworkOfferings(TrafficType.Public, false); for (NetworkOffering offer: offerList) { if (offer.getName().equals(offeringName)) { @@ -229,7 +228,7 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager } private NetworkOffering locateNetworkOffering(String offeringName, - String offeringDisplayText, Provider provider) { + String offeringDisplayText, Provider provider) { List offerList = _configService.listNetworkOfferings(TrafficType.Guest, false); for (NetworkOffering offer : offerList) { if (offer.getName().equals(offeringName)) { @@ -248,8 +247,8 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager } ConfigurationManager configMgr = (ConfigurationManager)_configService; NetworkOfferingVO voffer = - configMgr.createNetworkOffering(offeringName, offeringDisplayText, TrafficType.Guest, null, false, Availability.Optional, null, serviceProviderMap, true, - Network.GuestType.Isolated, false, null, false, null, false, true, null, true, null, false); + configMgr.createNetworkOffering(offeringName, offeringDisplayText, TrafficType.Guest, null, false, Availability.Optional, null, serviceProviderMap, true, + Network.GuestType.Isolated, false, null, false, null, false, true, null, true, null, false); voffer.setState(NetworkOffering.State.Enabled); long id = voffer.getId(); @@ -300,6 +299,7 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager public boolean configure(String name, Map params) throws ConfigurationException { File configFile = PropertiesUtil.findConfigFile(configuration); + FileInputStream fileStream = null; try { String hostname = null; int port = 0; @@ -307,10 +307,12 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager return false; } else { final Properties configProps = new Properties(); - configProps.load(new FileInputStream(configFile)); + fileStream = new FileInputStream(configFile); + configProps.load(fileStream); + String value = configProps.getProperty("management.db_sync_interval"); if (value != null) { - _dbSyncInterval = Integer.valueOf(value); + _dbSyncInterval = Integer.parseInt(value); } hostname = configProps.getProperty("api.hostname"); String portStr = configProps.getProperty("api.port"); @@ -326,16 +328,18 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager s_logger.debug("Exception in configure: " + ex); ex.printStackTrace(); throw new ConfigurationException(); + } finally { + IOUtils.closeQuietly(fileStream); } _controller = new ModelController(this, _api, _vmDao, _networksDao, _nicDao, _vlanDao, _ipAddressDao); _routerOffering = locateNetworkOffering(routerOfferingName, routerOfferingDisplayText, - Provider.JuniperContrailRouter); + Provider.JuniperContrailRouter); _routerPublicOffering = locatePublicNetworkOffering(routerPublicOfferingName, routerPublicOfferingDisplayText, - Provider.JuniperContrailRouter); + Provider.JuniperContrailRouter); _vpcRouterOffering = locateNetworkOffering(vpcRouterOfferingName, vpcRouterOfferingDisplayText, - Provider.JuniperContrailVpcRouter); + Provider.JuniperContrailVpcRouter); _vpcOffering = locateVpcOffering(); _eventHandler.subscribe(); @@ -448,7 +452,7 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager DomainVO domain = _domainDao.findById(domainId); if (domain.getId() != Domain.ROOT_DOMAIN) { net.juniper.contrail.api.types.Domain vncDomain = - (net.juniper.contrail.api.types.Domain)_api.findById(net.juniper.contrail.api.types.Domain.class, domain.getUuid()); + (net.juniper.contrail.api.types.Domain)_api.findById(net.juniper.contrail.api.types.Domain.class, domain.getUuid()); return _api.findByName(net.juniper.contrail.api.types.Project.class, vncDomain, VNC_DEFAULT_PROJECT); } return null; @@ -481,8 +485,8 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager @Override public void findInfrastructureNetworks(PhysicalNetworkVO phys, List dbList) { final TrafficType[] ttypes = {TrafficType.Control, // maps to __link_local__ - TrafficType.Management, // maps to ip-fabric - TrafficType.Public, TrafficType.Storage // maps to ip-fabric + TrafficType.Management, // maps to ip-fabric + TrafficType.Public, TrafficType.Storage // maps to ip-fabric }; for (int i = 0; i < ttypes.length; i++) { @@ -528,7 +532,7 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager List net_list = _physicalNetworkDao.listByZone(network.getDataCenterId()); for (PhysicalNetworkVO phys : net_list) { if(_physProviderDao.findByServiceProvider(phys.getId(), Provider.JuniperContrailRouter.getName()) != null || - _physProviderDao.findByServiceProvider(phys.getId(), Provider.JuniperContrailVpcRouter.getName()) != null) { + _physProviderDao.findByServiceProvider(phys.getId(), Provider.JuniperContrailVpcRouter.getName()) != null) { return true; } } @@ -653,7 +657,7 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager for (Iterator iter = phys_list.iterator(); iter.hasNext();) { PhysicalNetworkVO phys = iter.next(); if (_physProviderDao.findByServiceProvider(phys.getId(), provider) != null || - _physProviderDao.findByServiceProvider(phys.getId(), vpcProvider) != null) { + _physProviderDao.findByServiceProvider(phys.getId(), vpcProvider) != null) { List infraNets = new ArrayList(); findInfrastructureNetworks(phys, infraNets); for (NetworkVO net : infraNets) { @@ -886,7 +890,7 @@ public class ContrailManagerImpl extends ManagerBase implements ContrailManager VirtualNetworkModel vnModel = getDatabase().lookupVirtualNetwork(net.getUuid(), getCanonicalName(net), TrafficType.Public); if (vnModel == null) { vnModel = new VirtualNetworkModel(net, net.getUuid(), - getCanonicalName(net), net.getTrafficType()); + getCanonicalName(net), net.getTrafficType()); vnModel.setProperties(getModelController(), net); } try { diff --git a/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ModelObject.java b/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ModelObject.java index 1b048ed440b..c751d75b63c 100644 --- a/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ModelObject.java +++ b/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ModelObject.java @@ -39,8 +39,16 @@ import com.cloud.exception.InternalErrorException; * The update method pushes updates to the contrail API server. */ public interface ModelObject { + public static class ModelReference implements Comparable, Serializable { - WeakReference reference; + + private static final long serialVersionUID = -2019113974956703526L; + + /* + * WeakReference class is not serializable by definition. So, we cannot enforce its serialization unless we write the implementation of + * methods writeObject() and readObject(). Since the code was already not serializing it, it's been marked as transient. + */ + transient WeakReference reference; ModelReference(ModelObject obj) { reference = new WeakReference(obj); diff --git a/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ServiceInstanceModel.java b/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ServiceInstanceModel.java index 0ce22adc421..e79053ca4f3 100644 --- a/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ServiceInstanceModel.java +++ b/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ServiceInstanceModel.java @@ -30,11 +30,10 @@ import net.juniper.contrail.api.types.ServiceInstanceType; import net.juniper.contrail.api.types.ServiceTemplate; import net.juniper.contrail.api.types.ServiceTemplateType; +import org.apache.cloudstack.network.contrail.management.ContrailManager; import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; -import org.apache.cloudstack.network.contrail.management.ContrailManager; - import com.cloud.offering.ServiceOffering; import com.cloud.template.VirtualMachineTemplate; import com.cloud.utils.exception.CloudRuntimeException; @@ -72,8 +71,14 @@ public class ServiceInstanceModel extends ModelObjectBase { String parent_name; if (project != null) { parent_name = StringUtils.join(project.getQualifiedName(), ':'); + + _projectId = project.getUuid(); } else { parent_name = ContrailManager.VNC_ROOT_DOMAIN + ":" + ContrailManager.VNC_DEFAULT_PROJECT; + + //In the original code, if the projectId is null, it will simply throw NPE on the last line (nr. 90) of the method where the projectId.getUuid() is called. + //This was added as a way to avoid NPE. Should we perhaps throw a CloudRuntimeException if the project object is null? + _projectId = UUID.randomUUID().toString(); } _fqName = parent_name + ":" + name; @@ -86,8 +91,6 @@ public class ServiceInstanceModel extends ModelObjectBase { _templateName = template.getName(); _templateId = template.getUuid(); _templateUrl = template.getUrl(); - - _projectId = project.getUuid(); } /** @@ -170,16 +173,16 @@ public class ServiceInstanceModel extends ModelObjectBase { } private void clearServicePolicy(ModelController controller) { - _left.addToNetworkPolicy(null); - _right.addToNetworkPolicy(null); - try { + _left.addToNetworkPolicy(null); + _right.addToNetworkPolicy(null); + try { controller.getManager().getDatabase().getNetworkPolicys().remove(_policy); _policy.delete(controller.getManager().getModelController()); _policy = null; } catch (Exception e) { s_logger.error(e); } - try { + try { _left.update(controller.getManager().getModelController()); _right.update(controller.getManager().getModelController()); } catch (Exception ex) { @@ -194,7 +197,7 @@ public class ServiceInstanceModel extends ModelObjectBase { _right.addToNetworkPolicy(policyModel); List siList = new ArrayList(); siList.add(StringUtils.join(_serviceInstance.getQualifiedName(), ':')); - try { + try { policyModel.build(controller.getManager().getModelController(), _leftName, _rightName, "in-network", siList, "pass"); } catch (Exception e) { s_logger.error(e); diff --git a/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualMachineModel.java b/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualMachineModel.java index 4d0218c8b4a..4b64c808e88 100644 --- a/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualMachineModel.java +++ b/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualMachineModel.java @@ -18,6 +18,7 @@ package org.apache.cloudstack.network.contrail.model; import java.io.IOException; +import java.text.MessageFormat; import java.util.List; import java.util.Map; import java.util.TreeSet; @@ -28,13 +29,14 @@ import net.juniper.contrail.api.types.ServiceInstance; import net.juniper.contrail.api.types.VirtualMachine; import org.apache.cloudstack.network.contrail.management.ContrailManager; -import org.apache.log4j.Logger; import org.apache.commons.lang.StringUtils; +import org.apache.log4j.Logger; import com.cloud.exception.InternalErrorException; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; import com.cloud.uservm.UserVm; +import com.cloud.utils.UuidUtils; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.NicVO; import com.cloud.vm.VMInstanceVO; @@ -83,10 +85,26 @@ public class VirtualMachineModel extends ModelObjectBase { final Gson json = new Gson(); Map kvmap = json.fromJson(userVm.getUserData(), new TypeToken>() { }.getType()); - String data = kvmap.get("service-instance"); - if (data != null) { - /* link the object with the service instance */ - buildServiceInstance(controller, data); + //Renamed "data" to "serviceUuid" because it's clearer. + String serviceUuid = kvmap.get("service-instance"); + if (serviceUuid != null) { + /* + * UUID.fromString() does not validate an UUID properly. I tried, for example, informing less digits in the UUID, where 12 were expected, + * and the UUID.fromstring() did not thrown the exception as expected. However, if you try UUID.fromString("aaa") it breaks, but if you try + * UUID.fromString("3dd4fa6e-2899-4429-b818-d34fe8df5") it doesn't (the last portion should have 12, instead of 9 digits). + * + * In other fix I added the validate UUID method to the UuidUtil classes. + */ + if (UuidUtils.validateUUID(serviceUuid)) { + /* link the object with the service instance */ + buildServiceInstance(controller, serviceUuid); + } else { + // Throw a CloudRuntimeException in case the UUID is not valid. + String message = "Invalid UUID ({0}) given for the service-instance for VM {1}."; + message = MessageFormat.format(message, instance.getId(), serviceUuid); + s_logger.warn(message); + throw new CloudRuntimeException(message); + } } } } @@ -109,22 +127,20 @@ public class VirtualMachineModel extends ModelObjectBase { s_logger.warn("service-instance read", ex); throw new CloudRuntimeException("Unable to read service-instance object", ex); } + ServiceInstanceModel siModel; - if (siObj == null) { + String fqn = StringUtils.join(siObj.getQualifiedName(), ':'); + siModel = manager.getDatabase().lookupServiceInstance(fqn); + if (siModel == null) { siModel = new ServiceInstanceModel(serviceUuid); siModel.build(controller, siObj); manager.getDatabase().getServiceInstances().add(siModel); - } else { - String fqn = StringUtils.join(siObj.getQualifiedName(), ':'); - siModel = manager.getDatabase().lookupServiceInstance(fqn); - if (siModel == null) { - if (siObj == null) { - siModel = new ServiceInstanceModel(serviceUuid); - siModel.build(controller, siObj); - manager.getDatabase().getServiceInstances().add(siModel); - } - } } + /* + * The code that was under the ELSE was never executed and due to that has been removed. + * Also, in the case siObj was null, it was going pass it as parameter to the build() method in the + * siModel object. + */ _serviceModel = siModel; } @@ -344,26 +360,26 @@ public class VirtualMachineModel extends ModelObjectBase { @Override public boolean verify(ModelController controller) { assert _initialized : "initialized is false"; - assert _uuid != null : "uuid is not set"; + assert _uuid != null : "uuid is not set"; - ApiConnector api = controller.getApiAccessor(); + ApiConnector api = controller.getApiAccessor(); - try { - _vm = (VirtualMachine) api.findById(VirtualMachine.class, _uuid); - } catch (IOException e) { - s_logger.error("virtual-machine verify", e); - } + try { + _vm = (VirtualMachine) api.findById(VirtualMachine.class, _uuid); + } catch (IOException e) { + s_logger.error("virtual-machine verify", e); + } - if (_vm == null) { + if (_vm == null) { + return false; + } + + for (ModelObject successor: successors()) { + if (!successor.verify(controller)) { return false; } - - for (ModelObject successor: successors()) { - if (!successor.verify(controller)) { - return false; - } - } - return true; + } + return true; } @Override diff --git a/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualNetworkModel.java b/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualNetworkModel.java index b0505b17db7..7563714528b 100644 --- a/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualNetworkModel.java +++ b/plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualNetworkModel.java @@ -32,9 +32,8 @@ import net.juniper.contrail.api.types.VirtualNetwork; import net.juniper.contrail.api.types.VirtualNetworkPolicyType; import net.juniper.contrail.api.types.VnSubnetsType; -import org.apache.log4j.Logger; - import org.apache.cloudstack.network.contrail.management.ContrailManager; +import org.apache.log4j.Logger; import com.cloud.dc.VlanVO; import com.cloud.dc.dao.VlanDao; @@ -49,7 +48,7 @@ public class VirtualNetworkModel extends ModelObjectBase { private String _uuid; private long _id; - private TrafficType _trafficType; + private final TrafficType _trafficType; /* * current state for object properties @@ -362,92 +361,92 @@ public class VirtualNetworkModel extends ModelObjectBase { @Override public boolean verify(ModelController controller) { assert _initialized : "initialized is false"; - assert _uuid != null : "uuid is not set"; + assert _uuid != null : "uuid is not set"; - ApiConnector api = controller.getApiAccessor(); - VlanDao vlanDao = controller.getVlanDao(); + ApiConnector api = controller.getApiAccessor(); + VlanDao vlanDao = controller.getVlanDao(); - try { - _vn = (VirtualNetwork)api.findById(VirtualNetwork.class, _uuid); - } catch (IOException e) { - e.printStackTrace(); + try { + _vn = (VirtualNetwork)api.findById(VirtualNetwork.class, _uuid); + } catch (IOException e) { + e.printStackTrace(); + } + + if (_vn == null) { + return false; + } + + if (!isDynamicNetwork()) { + return true; + } + + List dbSubnets = new ArrayList(); + if (_trafficType == TrafficType.Public) { + List vlan_list = vlanDao.listVlansByNetworkId(_id); + for (VlanVO vlan : vlan_list) { + String cidr = NetUtils.ipAndNetMaskToCidr(vlan.getVlanGateway(), vlan.getVlanNetmask()); + dbSubnets.add(vlan.getVlanGateway() + cidr); } + } else { + dbSubnets.add(_gateway + _prefix); + } - if (_vn == null) { - return false; - } + List> ipamRefs = _vn.getNetworkIpam(); + List vncSubnets = new ArrayList(); - if (!isDynamicNetwork()) { - return true; - } + if (ipamRefs == null && !dbSubnets.isEmpty()) { + return false; + } - List dbSubnets = new ArrayList(); - if (_trafficType == TrafficType.Public) { - List vlan_list = vlanDao.listVlansByNetworkId(_id); - for (VlanVO vlan : vlan_list) { - String cidr = NetUtils.ipAndNetMaskToCidr(vlan.getVlanGateway(), vlan.getVlanNetmask()); - dbSubnets.add(vlan.getVlanGateway() + cidr); - } - } else { - dbSubnets.add(this._gateway + this._prefix); - } - - List> ipamRefs = _vn.getNetworkIpam(); - List vncSubnets = new ArrayList(); - - if (ipamRefs == null && !dbSubnets.isEmpty()) { - return false; - } - - if (ipamRefs != null) { - for (ObjectReference ref : ipamRefs) { - VnSubnetsType vnSubnetType = ref.getAttr(); - if (vnSubnetType != null) { - List subnets = vnSubnetType.getIpamSubnets(); - if (subnets != null && !subnets.isEmpty()) { - VnSubnetsType.IpamSubnetType ipamSubnet = subnets.get(0); - vncSubnets.add(ipamSubnet.getDefaultGateway() + ipamSubnet.getSubnet().getIpPrefix() + "/" + ipamSubnet.getSubnet().getIpPrefixLen()); - } + if (ipamRefs != null) { + for (ObjectReference ref : ipamRefs) { + VnSubnetsType vnSubnetType = ref.getAttr(); + if (vnSubnetType != null) { + List subnets = vnSubnetType.getIpamSubnets(); + if (subnets != null && !subnets.isEmpty()) { + VnSubnetsType.IpamSubnetType ipamSubnet = subnets.get(0); + vncSubnets.add(ipamSubnet.getDefaultGateway() + ipamSubnet.getSubnet().getIpPrefix() + "/" + ipamSubnet.getSubnet().getIpPrefixLen()); } } } - // unordered, no duplicates hence perform negation operation as set - Set diff = new HashSet(dbSubnets); - diff.removeAll(vncSubnets); + } + // unordered, no duplicates hence perform negation operation as set + Set diff = new HashSet(dbSubnets); + diff.removeAll(vncSubnets); - if (!diff.isEmpty()) { - s_logger.debug("Subnets changed, network: " + this._name + "; db: " + dbSubnets + ", vnc: " + vncSubnets + ", diff: " + diff); + if (!diff.isEmpty()) { + s_logger.debug("Subnets changed, network: " + _name + "; db: " + dbSubnets + ", vnc: " + vncSubnets + ", diff: " + diff); + return false; + } + + List> policyRefs = _vn.getNetworkPolicy(); + if ((policyRefs == null || policyRefs.isEmpty()) && _policyModel != null) { + return false; + } + + if ((policyRefs != null && !policyRefs.isEmpty()) && _policyModel == null) { + return false; + } + + if (policyRefs != null && !policyRefs.isEmpty() && _policyModel != null) { + ObjectReference ref = policyRefs.get(0); + if (!ref.getUuid().equals(_policyModel.getUuid())) { return false; } + } - List> policyRefs = _vn.getNetworkPolicy(); - if ((policyRefs == null || policyRefs.isEmpty()) && _policyModel != null) { + for (ModelObject successor : successors()) { + if (!successor.verify(controller)) { return false; } - - if ((policyRefs != null && !policyRefs.isEmpty()) && _policyModel == null) { - return false; - } - - if (policyRefs != null && !policyRefs.isEmpty() && _policyModel != null) { - ObjectReference ref = policyRefs.get(0); - if (!ref.getUuid().equals(_policyModel.getUuid())) { - return false; - } - } - - for (ModelObject successor : successors()) { - if (!successor.verify(controller)) { - return false; - } - } - return true; + } + return true; } @Override public boolean compare(ModelController controller, ModelObject o) { VirtualNetworkModel latest; - assert this._vn != null : "vnc virtual network current is not initialized"; + assert _vn != null : "vnc virtual network current is not initialized"; try { latest = (VirtualNetworkModel)o; @@ -464,7 +463,7 @@ public class VirtualNetworkModel extends ModelObjectBase { } assert latest._vn != null : "vnc virtual network new is not initialized"; - List> currentIpamRefs = this._vn.getNetworkIpam(); + List> currentIpamRefs = _vn.getNetworkIpam(); List> newIpamRefs = latest._vn.getNetworkIpam(); List currentSubnets = new ArrayList(); List newSubnets = new ArrayList(); @@ -503,42 +502,60 @@ public class VirtualNetworkModel extends ModelObjectBase { diff.removeAll(newSubnets); if (!diff.isEmpty()) { - s_logger.debug("Subnets differ, network: " + this._name + "; db: " + currentSubnets + ", vnc: " + newSubnets + ", diff: " + diff); + s_logger.debug("Subnets differ, network: " + _name + "; db: " + currentSubnets + ", vnc: " + newSubnets + ", diff: " + diff); return false; } - List> currentPolicyRefs = this._vn.getNetworkPolicy(); + List> currentPolicyRefs = _vn.getNetworkPolicy(); List> latestPolicyRefs = latest._vn.getNetworkPolicy(); if (currentPolicyRefs == null && latestPolicyRefs == null) { return true; } - if ((currentPolicyRefs == null && latestPolicyRefs != null) || - (currentPolicyRefs != null && latestPolicyRefs == null) || - (currentPolicyRefs.size() != latestPolicyRefs.size())) { + if ((currentPolicyRefs == null && latestPolicyRefs != null) || (currentPolicyRefs != null + && latestPolicyRefs == null)) { return false; } - if (currentPolicyRefs.isEmpty() && latestPolicyRefs.isEmpty()) { + if ((currentPolicyRefs != null && latestPolicyRefs != null) && (currentPolicyRefs.size() != latestPolicyRefs.size())) { + return false; + } + + if ((currentPolicyRefs != null && latestPolicyRefs != null) && currentPolicyRefs.isEmpty() + && latestPolicyRefs.isEmpty()) { return true; } //both must be non empty lists - ObjectReference ref1 = currentPolicyRefs.get(0); - ObjectReference ref2 = latestPolicyRefs.get(0); + ObjectReference ref1 = null; + + if (currentPolicyRefs != null) { + ref1 = currentPolicyRefs.get(0); + } + + ObjectReference ref2 = null; + + if (latestPolicyRefs != null) { + ref2 = latestPolicyRefs.get(0); + } + + if (ref1 == null && ref2 == null) { + return true; + } if ((ref1 != null && ref2 == null) || (ref1 == null && ref2 != null)) { return false; } - if ((ref1.getUuid() != null && ref2.getUuid() == null) || (ref1.getUuid() == null && ref2.getUuid() != null)) { + if ((ref1 != null && ref2 != null) && ((ref1.getUuid() != null && ref2.getUuid() == null) + || (ref1.getUuid() == null && ref2.getUuid() != null))) { return false; } - if (ref1.getUuid() == null && ref2.getUuid() == null) { + if ((ref1 != null && ref2 != null) && (ref1.getUuid() == null && ref2.getUuid() == null)) { return true; } - if (!ref1.getUuid().equals(ref2.getUuid())) { + if ((ref1 != null && ref2 != null) && !ref1.getUuid().equals(ref2.getUuid())) { return false; } return true; diff --git a/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualMachineModelTest.java b/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualMachineModelTest.java index f85beb62fe6..1e3944a4f26 100644 --- a/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualMachineModelTest.java +++ b/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualMachineModelTest.java @@ -28,11 +28,10 @@ import junit.framework.TestCase; import net.juniper.contrail.api.ApiConnector; import net.juniper.contrail.api.ApiConnectorMock; -import org.apache.log4j.Logger; -import org.junit.Test; - import org.apache.cloudstack.network.contrail.management.ContrailManagerImpl; import org.apache.cloudstack.network.contrail.management.ModelDatabase; +import org.apache.log4j.Logger; +import org.junit.Test; import com.cloud.network.Network; import com.cloud.network.dao.NetworkVO; @@ -118,7 +117,5 @@ public class VirtualMachineModelTest extends TestCase { //verify assertTrue(vmModel.verify(controller)); - } - -} +} \ No newline at end of file diff --git a/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualNetworkModelTest.java b/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualNetworkModelTest.java index b1b5ae1ebff..3a1e147d8c5 100644 --- a/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualNetworkModelTest.java +++ b/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualNetworkModelTest.java @@ -21,26 +21,138 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.UUID; import junit.framework.TestCase; import net.juniper.contrail.api.ApiConnector; import net.juniper.contrail.api.ApiConnectorMock; - -import org.apache.log4j.Logger; -import org.junit.Test; +import net.juniper.contrail.api.ObjectReference; +import net.juniper.contrail.api.types.VirtualNetwork; +import net.juniper.contrail.api.types.VirtualNetworkPolicyType; import org.apache.cloudstack.network.contrail.management.ContrailManager; import org.apache.cloudstack.network.contrail.management.ContrailManagerImpl; import org.apache.cloudstack.network.contrail.management.ModelDatabase; +import org.apache.log4j.Logger; +import org.junit.Before; +import org.junit.Test; +import com.cloud.dc.dao.VlanDao; import com.cloud.network.Network.State; import com.cloud.network.Networks.TrafficType; import com.cloud.network.dao.NetworkVO; public class VirtualNetworkModelTest extends TestCase { + private static final Logger s_logger = Logger.getLogger(VirtualNetworkModelTest.class); + private ModelController controller; + + private VirtualNetworkModel vnModel; + private VirtualNetworkModel vnModel1; + private VirtualNetworkModel vnModel2; + private VirtualNetworkModel vnModel3; + + @Override + @Before + public void setUp() throws IOException { + //Network UUIDs + String uuid = UUID.randomUUID().toString(); + String uuid1 = UUID.randomUUID().toString(); + String uuid2 = UUID.randomUUID().toString(); + String uuid3 = UUID.randomUUID().toString(); + + //ContrailManager + ContrailManagerImpl contrailMgr = mock(ContrailManagerImpl.class); + + controller = mock(ModelController.class); + VlanDao vlanDao = mock(VlanDao.class); + + ApiConnector api = mock(ApiConnectorMock.class); + + //Mock classes/methods + when(controller.getManager()).thenReturn(contrailMgr); + when(controller.getApiAccessor()).thenReturn(api); + when(controller.getVlanDao()).thenReturn(vlanDao); + + //Policy References used by vnModel1 + List> policyRefs1 = new ArrayList>(); + ObjectReference objectReference1 = new ObjectReference(); + policyRefs1.add(objectReference1); + + //Policy References used by vnModel2 + List> policyRefs2 = new ArrayList>(); + ObjectReference objectReference2 = new ObjectReference(); + policyRefs2.add(objectReference2); + + //Policy References used by vnModel3 + List> policyRefs3 = new ArrayList>(); + ObjectReference objectReference3 = new ObjectReference(); + objectReference3.setReference(Arrays.asList(""), null, null, UUID.randomUUID().toString()); + + policyRefs3.add(objectReference3); + + //Network to be compared with + VirtualNetwork vn = mock(VirtualNetwork.class); + when(api.findById(VirtualNetwork.class, uuid)).thenReturn(vn); + + //Network to be compared with + VirtualNetwork vn1 = mock(VirtualNetwork.class); + when(api.findById(VirtualNetwork.class, uuid1)).thenReturn(vn1); + when(vn1.getNetworkPolicy()).thenReturn(policyRefs1); + + //Network to be compared to + VirtualNetwork vn2 = mock(VirtualNetwork.class); + when(api.findById(VirtualNetwork.class, uuid2)).thenReturn(vn2); + when(vn2.getNetworkPolicy()).thenReturn(policyRefs2); + + //Network to be compared to + VirtualNetwork vn3 = mock(VirtualNetwork.class); + when(api.findById(VirtualNetwork.class, uuid3)).thenReturn(vn3); + when(vn3.getNetworkPolicy()).thenReturn(policyRefs3); + + //Virtual-Network 1 + NetworkVO network1 = mock(NetworkVO.class); + when(network1.getName()).thenReturn("testnetwork"); + when(network1.getState()).thenReturn(State.Allocated); + when(network1.getGateway()).thenReturn("10.1.1.1"); + when(network1.getCidr()).thenReturn("10.1.1.0/24"); + when(network1.getPhysicalNetworkId()).thenReturn(42L); + when(network1.getDomainId()).thenReturn(10L); + when(network1.getAccountId()).thenReturn(42L); + + //Virtual-Network 2 + NetworkVO network2 = mock(NetworkVO.class); + when(network2.getName()).thenReturn("Testnetwork"); + when(network2.getState()).thenReturn(State.Allocated); + when(network2.getGateway()).thenReturn("10.1.1.1"); + when(network2.getCidr()).thenReturn("10.1.1.0/24"); + when(network2.getPhysicalNetworkId()).thenReturn(42L); + when(network2.getDomainId()).thenReturn(10L); + when(network2.getAccountId()).thenReturn(42L); + + //Virtual-Network 3 + NetworkVO network3 = mock(NetworkVO.class); + when(network3.getName()).thenReturn("Testnetwork"); + when(network3.getState()).thenReturn(State.Allocated); + when(network3.getGateway()).thenReturn("10.1.1.1"); + when(network3.getCidr()).thenReturn("10.1.1.0/24"); + when(network3.getPhysicalNetworkId()).thenReturn(42L); + when(network3.getDomainId()).thenReturn(10L); + when(network3.getAccountId()).thenReturn(42L); + + when(contrailMgr.getCanonicalName(network1)).thenReturn("testnetwork"); + when(contrailMgr.getProjectId(network1.getDomainId(), network1.getAccountId())).thenReturn("testProjectId"); + + vnModel = new VirtualNetworkModel(network1, uuid, "testnetwork", TrafficType.Guest); + vnModel1 = new VirtualNetworkModel(network1, uuid1, "testnetwork", TrafficType.Guest); + vnModel2 = new VirtualNetworkModel(network2, uuid2, "testnetwork", TrafficType.Guest); + vnModel3 = new VirtualNetworkModel(network3, uuid3, "testnetwork", TrafficType.Guest); + } + @Test public void testDBLookup() { ModelDatabase db = new ModelDatabase(); @@ -99,4 +211,27 @@ public class VirtualNetworkModelTest extends TestCase { assertTrue(vnModel.verify(controller)); } -} + @Test + public void testCompareDifferentVirtualNetwork() throws IOException { + //This one returns false because one network has Policy References + vnModel.read(controller); + + assertFalse(vnModel.compare(controller, vnModel1)); + } + + @Test + public void testCompareSameVirtualNetwork() throws IOException { + //This one returns true because both networks have the same Policy References + vnModel1.read(controller); + + assertTrue(vnModel1.compare(controller, vnModel2)); + } + + @Test + public void testCompareDifferentDeeperVirtualNetwork() throws IOException { + //This one returns false because one network has Policy References + vnModel2.read(controller); + + assertFalse(vnModel2.compare(controller, vnModel3)); + } +} \ No newline at end of file diff --git a/utils/src/com/cloud/utils/UuidUtils.java b/utils/src/com/cloud/utils/UuidUtils.java index 7831beaa753..28b6b1d84ac 100644 --- a/utils/src/com/cloud/utils/UuidUtils.java +++ b/utils/src/com/cloud/utils/UuidUtils.java @@ -16,8 +16,16 @@ // under the License. package com.cloud.utils; +import org.apache.xerces.impl.xpath.regex.RegularExpression; + public class UuidUtils { + public final static String first(String uuid) { return uuid.substring(0, uuid.indexOf('-')); } -} + + public static boolean validateUUID(String uuid) { + RegularExpression regex = new RegularExpression("[0-9a-fA-F]{8}(?:-[0-9a-fA-F]{4}){3}-[0-9a-fA-F]{12}"); + return regex.matches(uuid); + } +} \ No newline at end of file diff --git a/utils/test/com/cloud/utils/UuidUtilsTest.java b/utils/test/com/cloud/utils/UuidUtilsTest.java new file mode 100644 index 00000000000..299e247e7c1 --- /dev/null +++ b/utils/test/com/cloud/utils/UuidUtilsTest.java @@ -0,0 +1,23 @@ +package com.cloud.utils; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +public class UuidUtilsTest { + + @Test + public void testValidateUUIDPass() throws Exception { + String serviceUuid = "f81a9aa3-1f7d-466f-b04b-f2b101486bae"; + + assertTrue(UuidUtils.validateUUID(serviceUuid)); + } + + @Test + public void testValidateUUIDFail() throws Exception { + String serviceUuid = "6fc6ce7-d503-4f95-9e68-c9cd281b13df"; + + assertFalse(UuidUtils.validateUUID(serviceUuid)); + } +} \ No newline at end of file