From 8217c3243a6dc674ed91515b2cbdbcab7d37847c Mon Sep 17 00:00:00 2001 From: Hideyuki Tai Date: Mon, 30 Sep 2013 14:26:29 -0400 Subject: [PATCH] Changed TopologyManager to ignore edges that contain invalid node connectors. - Changed TopologyManager to add only edges whose tail and head node connectors exist. - Added a method to ISwitchManager to check whether a node connector exists. - Added tests for the above changes. Change-Id: Ib17ce94afea9ba16379ce454f48d5d8b1a1970b2 Signed-off-by: Hideyuki Tai --- .../switchmanager/ISwitchManager.java | 8 + .../switchmanager/internal/SwitchManager.java | 6 + .../internal/SwitchManagerTest.java | 5 + .../topologymanager/implementation/pom.xml | 6 + .../topologymanager/internal/Activator.java | 5 + .../internal/TopologyManagerImpl.java | 55 +++- .../internal/TopologyManagerImplTest.java | 292 +++++++++++++++++- 7 files changed, 362 insertions(+), 15 deletions(-) diff --git a/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/ISwitchManager.java b/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/ISwitchManager.java index f1b9585d02..9ac0b5055f 100644 --- a/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/ISwitchManager.java +++ b/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/ISwitchManager.java @@ -335,6 +335,14 @@ public interface ISwitchManager { */ public Boolean isNodeConnectorEnabled(NodeConnector nodeConnector); + /** + * Test whether the given node connector exists. + * + * @param nc {@link org.opendaylight.controller.sal.core.NodeConnector} + * @return True if exists, false otherwise. + */ + public boolean doesNodeConnectorExist(NodeConnector nc); + /** * Return controller MAC address * diff --git a/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java b/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java index 5d0e364116..d3f440090d 100644 --- a/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java +++ b/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java @@ -1831,6 +1831,12 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa && (state != null) && (state.getValue() == State.EDGE_UP)); } + @Override + public boolean doesNodeConnectorExist(NodeConnector nc) { + return (nc != null && nodeConnectorProps != null + && nodeConnectorProps.containsKey(nc)); + } + @Override public String getHelp() { StringBuffer help = new StringBuffer(); diff --git a/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java b/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java index 97747e9a22..e7dac9c143 100644 --- a/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java +++ b/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java @@ -134,10 +134,15 @@ public class SwitchManagerTest { switchmgr.updateNode(tailnc[i - 1].getNode(), UpdateType.ADDED, props); + Assert.assertFalse(switchmgr.doesNodeConnectorExist(headnc[i - 1])); switchmgr.updateNodeConnector(headnc[i - 1], UpdateType.ADDED, props); + Assert.assertTrue(switchmgr.doesNodeConnectorExist(headnc[i - 1])); + + Assert.assertFalse(switchmgr.doesNodeConnectorExist(tailnc[i - 1])); switchmgr.updateNodeConnector(tailnc[i - 1], UpdateType.ADDED, props); + Assert.assertTrue(switchmgr.doesNodeConnectorExist(tailnc[i - 1])); } for (int i = 0; i < 5; i++) { diff --git a/opendaylight/topologymanager/implementation/pom.xml b/opendaylight/topologymanager/implementation/pom.xml index c4a82fe82f..f4f4924e44 100755 --- a/opendaylight/topologymanager/implementation/pom.xml +++ b/opendaylight/topologymanager/implementation/pom.xml @@ -38,6 +38,7 @@ org.opendaylight.controller.sal.topology, org.opendaylight.controller.configuration, org.opendaylight.controller.clustering.services, + org.opendaylight.controller.switchmanager, org.osgi.service.component, org.slf4j, org.apache.felix.dm, @@ -71,5 +72,10 @@ configuration 0.4.1-SNAPSHOT + + org.opendaylight.controller + switchmanager + 0.5.1-SNAPSHOT + diff --git a/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/Activator.java b/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/Activator.java index 30a9445b1d..b2a096aa7b 100644 --- a/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/Activator.java +++ b/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/Activator.java @@ -21,6 +21,7 @@ import org.opendaylight.controller.configuration.IConfigurationContainerAware; import org.opendaylight.controller.sal.core.ComponentActivatorAbstractBase; import org.opendaylight.controller.sal.topology.IListenTopoUpdates; import org.opendaylight.controller.sal.topology.ITopologyService; +import org.opendaylight.controller.switchmanager.ISwitchManager; import org.opendaylight.controller.topologymanager.ITopologyManager; import org.opendaylight.controller.topologymanager.ITopologyManagerAware; import org.opendaylight.controller.topologymanager.ITopologyManagerClusterWideAware; @@ -78,6 +79,10 @@ public class Activator extends ComponentActivatorAbstractBase { ITopologyService.class).setCallbacks("setTopoService", "unsetTopoService").setRequired(true)); + c.add(createContainerServiceDependency(containerName).setService( + ISwitchManager.class).setCallbacks("setSwitchManager", + "unsetSwitchManager").setRequired(true)); + // These are all the listeners for a topology manager // updates, there could be many or none c.add(createContainerServiceDependency(containerName).setService( diff --git a/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImpl.java b/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImpl.java index 5ecddcfc88..b905a8982e 100644 --- a/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImpl.java +++ b/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImpl.java @@ -53,6 +53,7 @@ import org.opendaylight.controller.sal.utils.ObjectReader; import org.opendaylight.controller.sal.utils.ObjectWriter; import org.opendaylight.controller.sal.utils.Status; import org.opendaylight.controller.sal.utils.StatusCode; +import org.opendaylight.controller.switchmanager.ISwitchManager; import org.opendaylight.controller.topologymanager.ITopologyManager; import org.opendaylight.controller.topologymanager.ITopologyManagerAware; import org.opendaylight.controller.topologymanager.ITopologyManagerClusterWideAware; @@ -81,6 +82,7 @@ public class TopologyManagerImpl implements private static final Logger log = LoggerFactory.getLogger(TopologyManagerImpl.class); private ITopologyService topoService; private IClusterContainerServices clusterContainerService; + private ISwitchManager switchManager; // DB of all the Edges with properties which constitute our topology private ConcurrentMap> edgesDB; // DB of all NodeConnector which are part of ISL Edges, meaning they @@ -161,6 +163,18 @@ public class TopologyManagerImpl implements } } + void setSwitchManager(ISwitchManager s) { + log.debug("Adding ISwitchManager: {}", s); + this.switchManager = s; + } + + void unsetSwitchManager(ISwitchManager s) { + if (this.switchManager == s) { + log.debug("Removing ISwitchManager: {}", s); + this.switchManager = null; + } + } + /** * Function called by the dependency manager when all the required * dependencies are satisfied @@ -507,9 +521,22 @@ public class TopologyManagerImpl implements } } + private boolean nodeConnectorsExist(Edge e) { + NodeConnector head = e.getHeadNodeConnector(); + NodeConnector tail = e.getTailNodeConnector(); + return (switchManager.doesNodeConnectorExist(head) && + switchManager.doesNodeConnectorExist(tail)); + } + private TopoEdgeUpdate edgeUpdate(Edge e, UpdateType type, Set props) { switch (type) { case ADDED: + // Ensure that both tail and head node connectors exist. + if (!nodeConnectorsExist(e)) { + log.warn("Ignore edge that contains invalid node connector: {}", e); + return null; + } + // Make sure the props are non-null if (props == null) { props = new HashSet(); @@ -631,18 +658,21 @@ public class TopologyManagerImpl implements Set p = topoedgeupdateList.get(i).getProperty(); UpdateType type = topoedgeupdateList.get(i).getUpdateType(); TopoEdgeUpdate teu = edgeUpdate(e, type, p); - teuList.add(teu); + if (teu != null) { + teuList.add(teu); + } } - // Now update the listeners - for (ITopologyManagerAware s : this.topologyManagerAware) { - try { - s.edgeUpdate(teuList); - } catch (Exception exc) { - log.error("Exception on edge update:", exc); + if (!teuList.isEmpty()) { + // Now update the listeners + for (ITopologyManagerAware s : this.topologyManagerAware) { + try { + s.edgeUpdate(teuList); + } catch (Exception exc) { + log.error("Exception on edge update:", exc); + } } } - } private Edge getReverseLinkTuple(TopologyUserLinkConfig link) { @@ -692,7 +722,14 @@ public class TopologyManagerImpl implements Edge linkTuple = getLinkTuple(userLink); if (linkTuple != null) { if (!isProductionLink(linkTuple)) { - edgeUpdate(linkTuple, UpdateType.ADDED, new HashSet()); + TopoEdgeUpdate teu = edgeUpdate(linkTuple, UpdateType.ADDED, + new HashSet()); + if (teu == null) { + userLinksDB.remove(userLink.getName()); + return new Status(StatusCode.NOTFOUND, + "Link configuration contains invalid node connector: " + + userLink); + } } linkTuple = getReverseLinkTuple(userLink); diff --git a/opendaylight/topologymanager/implementation/src/test/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImplTest.java b/opendaylight/topologymanager/implementation/src/test/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImplTest.java index 65487a614a..13888646bc 100644 --- a/opendaylight/topologymanager/implementation/src/test/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImplTest.java +++ b/opendaylight/topologymanager/implementation/src/test/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImplTest.java @@ -11,6 +11,7 @@ package org.opendaylight.controller.topologymanager.internal; import java.net.InetAddress; import java.net.UnknownHostException; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -28,17 +29,270 @@ import org.opendaylight.controller.sal.core.Latency; import org.opendaylight.controller.sal.core.Node; import org.opendaylight.controller.sal.core.Node.NodeIDType; import org.opendaylight.controller.sal.core.NodeConnector; +import org.opendaylight.controller.sal.core.NodeConnector.NodeConnectorIDType; import org.opendaylight.controller.sal.core.Property; import org.opendaylight.controller.sal.core.State; import org.opendaylight.controller.sal.core.UpdateType; import org.opendaylight.controller.sal.packet.address.EthernetAddress; import org.opendaylight.controller.sal.topology.TopoEdgeUpdate; +import org.opendaylight.controller.sal.utils.Status; import org.opendaylight.controller.sal.utils.StatusCode; import org.opendaylight.controller.sal.utils.NodeConnectorCreator; import org.opendaylight.controller.sal.utils.NodeCreator; +import org.opendaylight.controller.switchmanager.ISwitchManager; +import org.opendaylight.controller.switchmanager.SpanConfig; +import org.opendaylight.controller.switchmanager.Subnet; +import org.opendaylight.controller.switchmanager.SubnetConfig; +import org.opendaylight.controller.switchmanager.Switch; +import org.opendaylight.controller.switchmanager.SwitchConfig; import org.opendaylight.controller.topologymanager.TopologyUserLinkConfig; public class TopologyManagerImplTest { + /** + * Mockup of switch manager that only maintains existence of node + * connector. + */ + private final class TestSwitchManager implements ISwitchManager { + private final Set nodeSet = new HashSet(); + private final Set nodeConnectorSet = + new HashSet(); + + private void addNodeConnectors(NodeConnector ... connectors) { + for (NodeConnector nc: connectors) { + if (nc != null) { + nodeSet.add(nc.getNode()); + nodeConnectorSet.add(nc); + } + } + } + + private void addNodeConnectors(TopologyUserLinkConfig ... links) { + for (TopologyUserLinkConfig link: links) { + NodeConnector src = + NodeConnector.fromString(link.getSrcNodeConnector()); + NodeConnector dst = + NodeConnector.fromString(link.getDstNodeConnector()); + addNodeConnectors(src, dst); + } + } + + @Override + public Status addSubnet(SubnetConfig configObject) { + return null; + } + + @Override + public Status removeSubnet(SubnetConfig configObject) { + return null; + } + + @Override + public Status modifySubnet(SubnetConfig configObject) { + return null; + } + + @Override + public Status removeSubnet(String name) { + return null; + } + + @Override + public List getNetworkDevices() { + return null; + } + + @Override + public List getSubnetsConfigList() { + return null; + } + + @Override + public SubnetConfig getSubnetConfig(String subnet) { + return null; + } + + @Override + public Subnet getSubnetByNetworkAddress(InetAddress networkAddress) { + return null; + } + + @Override + public Status saveSwitchConfig() { + return null; + } + + @Override + public Status addSpanConfig(SpanConfig configObject) { + return null; + } + + @Override + public Status removeSpanConfig(SpanConfig cfgObject) { + return null; + } + + @Override + public List getSpanConfigList() { + return null; + } + + @Override + public List getSpanPorts(Node node) { + return null; + } + + @Override + public void updateSwitchConfig(SwitchConfig cfgObject) { + } + + @Override + public Status updateNodeConfig(SwitchConfig switchConfig) { + return null; + } + + @Override + public Status removeNodeConfig(String nodeId) { + return null; + } + + @Override + public SwitchConfig getSwitchConfig(String nodeId) { + return null; + } + + @Override + public Status addPortsToSubnet(String name, List nodeConnectors) { + return null; + } + + @Override + public Status removePortsFromSubnet(String name, List nodeConnectors) { + return null; + } + + @Override + public Set getNodes() { + return new HashSet(nodeSet); + } + + @Override + public Map getNodeProps(Node node) { + return new HashMap(); + } + + @Override + public Property getNodeProp(Node node, String propName) { + return null; + } + + @Override + public void setNodeProp(Node node, Property prop) { + } + + @Override + public Status removeNodeProp(Node node, String propName) { + return null; + } + + @Override + public Status removeNodeAllProps(Node node) { + return null; + } + + @Override + public Set getUpNodeConnectors(Node node) { + return getNodeConnectors(node); + } + + @Override + public Set getNodeConnectors(Node node) { + Set set = new HashSet(); + for (NodeConnector nc: nodeConnectorSet) { + if (nc.getNode().equals(node)) { + set.add(nc); + } + } + + return set; + } + + @Override + public Set getPhysicalNodeConnectors(Node node) { + return getNodeConnectors(node); + } + + @Override + public Map getNodeConnectorProps(NodeConnector nodeConnector) { + return new HashMap(); + } + + @Override + public Property getNodeConnectorProp(NodeConnector nodeConnector, String propName) { + return null; + } + + @Override + public Status addNodeConnectorProp(NodeConnector nodeConnector, Property prop) { + return null; + } + + @Override + public Status removeNodeConnectorProp(NodeConnector nc, String propName) { + return null; + } + + @Override + public Status removeNodeConnectorAllProps(NodeConnector nodeConnector) { + return null; + } + + @Override + public NodeConnector getNodeConnector(Node node, String nodeConnectorName) { + return null; + } + + @Override + public boolean isSpecial(NodeConnector p) { + String type = p.getType(); + return (type.equals(NodeConnectorIDType.CONTROLLER) + || type.equals(NodeConnectorIDType.ALL) + || type.equals(NodeConnectorIDType.SWSTACK) + || type.equals(NodeConnectorIDType.HWPATH)); + } + + @Override + public Boolean isNodeConnectorEnabled(NodeConnector nodeConnector) { + if (doesNodeConnectorExist(nodeConnector)) { + return Boolean.TRUE; + } + return Boolean.FALSE; + } + + @Override + public boolean doesNodeConnectorExist(NodeConnector nc) { + return (nc != null && nodeConnectorSet.contains(nc)); + } + + @Override + public byte[] getControllerMAC() { + return new byte[6]; + } + + @Override + public byte[] getNodeMAC(Node node) { + return new byte[6]; + } + + @Override + public Property createProperty(String propName, String propValue) { + return null; + } + + @Override + public String getNodeDescription(Node node) { + return null; + } + } /* * Sets the node, edges and properties for edges here: Edge > nodeEdgeMap = topoManagerImpl.getNodeEdges(); for (Iterator>> i = nodeEdgeMap.entrySet() @@ -126,7 +382,9 @@ public class TopologyManagerImplTest { @Test public void testGetEdges() throws ConstructionException { TopologyManagerImpl topoManagerImpl = new TopologyManagerImpl(); - setNodeEdges(topoManagerImpl); + TestSwitchManager swMgr = new TestSwitchManager(); + topoManagerImpl.setSwitchManager(swMgr); + setNodeEdges(topoManagerImpl, swMgr); Map> edgeProperty = topoManagerImpl.getEdges(); @@ -208,8 +466,12 @@ public class TopologyManagerImplTest { "OF|10@OF|20", "OF|10@OF|20"); TopologyManagerImpl topoManagerImpl = new TopologyManagerImpl(); + TestSwitchManager swMgr = new TestSwitchManager(); + topoManagerImpl.setSwitchManager(swMgr); topoManagerImpl.nonClusterObjectCreate(); + swMgr.addNodeConnectors(link1, link2, link3, link4); + Assert.assertTrue(topoManagerImpl.addUserLink(link1).isSuccess()); Assert.assertTrue(topoManagerImpl.addUserLink(link2).getCode() == StatusCode.CONFLICT); Assert.assertTrue(topoManagerImpl.addUserLink(link3).getCode() == StatusCode.CONFLICT); @@ -222,6 +484,14 @@ public class TopologyManagerImplTest { .isSuccess()); Assert.assertTrue(topoManagerImpl.getUserLinks().isEmpty()); + TopologyUserLinkConfig badlink1 = + new TopologyUserLinkConfig("bad1", "OF|1@OF|2", "OF|1@OF|3"); + TopologyUserLinkConfig badlink2 = + new TopologyUserLinkConfig("bad2", "OF|10@OF|21", "OF|10@OF|20"); + Assert.assertEquals(StatusCode.NOTFOUND, + topoManagerImpl.addUserLink(badlink1).getCode()); + Assert.assertEquals(StatusCode.NOTFOUND, + topoManagerImpl.addUserLink(badlink2).getCode()); } @Test @@ -229,6 +499,8 @@ public class TopologyManagerImplTest { TopologyUserLinkConfig[] link = new TopologyUserLinkConfig[5]; TopologyUserLinkConfig[] reverseLink = new TopologyUserLinkConfig[5]; TopologyManagerImpl topoManagerImpl = new TopologyManagerImpl(); + TestSwitchManager swMgr = new TestSwitchManager(); + topoManagerImpl.setSwitchManager(swMgr); topoManagerImpl.nonClusterObjectCreate(); String name = "Test"; @@ -285,7 +557,11 @@ public class TopologyManagerImplTest { Assert.assertTrue(link[i].isValid() == true); reverseLink[i] = new TopologyUserLinkConfig(name, dstNodeConnector, srcNodeConnector); - topoManagerImpl.addUserLink(link[i]); + + Assert.assertEquals(StatusCode.NOTFOUND, + topoManagerImpl.addUserLink(link[i]).getCode()); + swMgr.addNodeConnectors(link[i]); + Assert.assertTrue(topoManagerImpl.addUserLink(link[i]).isSuccess()); } ConcurrentMap userLinks = topoManagerImpl @@ -308,6 +584,8 @@ public class TopologyManagerImplTest { public void testHostLinkMethods() throws ConstructionException, UnknownHostException { TopologyManagerImpl topoManagerImpl = new TopologyManagerImpl(); + TestSwitchManager swMgr = new TestSwitchManager(); + topoManagerImpl.setSwitchManager(swMgr); topoManagerImpl.nonClusterObjectCreate(); int hostCounter = 0; @@ -369,6 +647,8 @@ public class TopologyManagerImplTest { public void testGetNodesWithNodeConnectorHost() throws ConstructionException, UnknownHostException { TopologyManagerImpl topoManagerImpl = new TopologyManagerImpl(); + TestSwitchManager swMgr = new TestSwitchManager(); + topoManagerImpl.setSwitchManager(swMgr); topoManagerImpl.nonClusterObjectCreate(); int hostCounter = 0; -- 2.36.6