From 895f3e89b3c2554e704f74c764d23d5b79bf0862 Mon Sep 17 00:00:00 2001 From: Gilles Thouenon Date: Wed, 26 Jan 2022 09:14:05 +0100 Subject: [PATCH] Fix bug in PCE picking wrong client port - Instead of systematically picking the first available client port compatible with the service, pick the client port provided by the input request parameters when specified. - Update existing node validation to pick only endpoints with tail - Fix a bug in otn intermediate switch functional test suite. - Remove from tapi-connectivity impl few lines of strange code that seems being wrong since it was replacing the client port-name specified in the request by its associated network port-name, what was making Ethernet service creation failing. JIRA: TRNSPRTPCE-460 Signed-off-by: Gilles Thouenon Change-Id: Ic3f1203f62fa19db46935524200c47f74611187c --- .../pce/networkanalyzer/PceCalculation.java | 47 +++++++++++++---- .../pce/networkanalyzer/PceOtnNode.java | 11 ++-- .../transportpce/pce/graph/PceGraphTest.java | 4 +- .../pce/networkanalyzer/PceLinkTest.java | 3 +- .../pce/networkanalyzer/PceOtnNodeTest.java | 22 ++++---- .../tapi/connectivity/ConnectivityUtils.java | 51 ------------------- ...15_otn_end2end_with_intermediate_switch.py | 4 +- 7 files changed, 63 insertions(+), 79 deletions(-) diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceCalculation.java b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceCalculation.java index d2f3305f1..32d9068f3 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceCalculation.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceCalculation.java @@ -449,13 +449,26 @@ public class PceCalculation { if (validateNodeConstraints(pceNode).equals(ConstraintTypes.HARD_EXCLUDE)) { return; } - if (endPceNode(nodeType, pceNode.getNodeId(), pceNode) && this.aendPceNode == null - && isAZendPceNode(this.serviceFormatA, pceNode, anodeId, "A")) { - this.aendPceNode = pceNode; - } - if (endPceNode(nodeType, pceNode.getNodeId(), pceNode) && this.zendPceNode == null - && isAZendPceNode(this.serviceFormatZ, pceNode, znodeId, "Z")) { - this.zendPceNode = pceNode; + + if (endPceNode(nodeType, pceNode.getNodeId(), pceNode)) { + if (this.aendPceNode == null && isAZendPceNode(this.serviceFormatA, pceNode, anodeId, "A")) { + // Added to ensure A-node has a addlink in the topology + List links = this.allLinks.stream() + .filter(x -> x.getSource().getSourceNode().getValue().contains(pceNode.getNodeId().getValue())) + .collect(Collectors.toList()); + if (links.size() > 0) { + this.aendPceNode = pceNode; + } + } + if (this.zendPceNode == null && isAZendPceNode(this.serviceFormatZ, pceNode, znodeId, "Z")) { + // Added to ensure Z-node has a droplink in the topology + List links = this.allLinks.stream() + .filter(x -> x.getDestination().getDestNode().getValue().contains(pceNode.getNodeId().getValue())) + .collect(Collectors.toList()); + if (links.size() > 0) { + this.zendPceNode = pceNode; + } + } } allPceNodes.put(pceNode.getNodeId(), pceNode); @@ -496,8 +509,24 @@ public class PceCalculation { } OpenroadmNodeType nodeType = node.augmentation(Node1.class).getNodeType(); - - PceOtnNode pceOtnNode = new PceOtnNode(node, nodeType, node.getNodeId(), "otn", serviceType); + String clientPort = null; + if (node.getNodeId().getValue().equals(anodeId) + && this.aendPceNode == null + && input.getServiceAEnd() != null + && input.getServiceAEnd().getRxDirection() != null + && input.getServiceAEnd().getRxDirection().getPort() != null + && input.getServiceAEnd().getRxDirection().getPort().getPortName() != null) { + clientPort = input.getServiceAEnd().getRxDirection().getPort().getPortName(); + } else if (node.getNodeId().getValue().equals(znodeId) + && this.zendPceNode == null + && input.getServiceZEnd() != null + && input.getServiceZEnd().getRxDirection() != null + && input.getServiceZEnd().getRxDirection().getPort() != null + && input.getServiceZEnd().getRxDirection().getPort().getPortName() != null) { + clientPort = input.getServiceZEnd().getRxDirection().getPort().getPortName(); + } + + PceOtnNode pceOtnNode = new PceOtnNode(node, nodeType, node.getNodeId(), "otn", serviceType, clientPort); pceOtnNode.validateXponder(anodeId, znodeId); if (!pceOtnNode.isValid()) { diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNode.java b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNode.java index 35276d5e0..2393f2f92 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNode.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNode.java @@ -75,8 +75,10 @@ public class PceOtnNode implements PceNode { private List outgoingLinks = new ArrayList<>(); private Map clientPerNwTp = new HashMap<>(); + private String clientPort; - public PceOtnNode(Node node, OpenroadmNodeType nodeType, NodeId nodeId, String pceNodeType, String serviceType) { + public PceOtnNode(Node node, OpenroadmNodeType nodeType, NodeId nodeId, String pceNodeType, String serviceType, + String clientPort) { this.node = node; this.nodeId = nodeId; this.nodeType = nodeType; @@ -99,6 +101,7 @@ public class PceOtnNode implements PceNode { checkAvailableTribPort(); this.tpAvailableTribSlot.clear(); checkAvailableTribSlot(); + this.clientPort = clientPort; if (node == null || nodeId == null || nodeType != OpenroadmNodeType.MUXPDR && nodeType != OpenroadmNodeType.SWITCH && nodeType != OpenroadmNodeType.TPDR) { LOG.error("PceOtnNode: one of parameters is not populated : nodeId, node type"); @@ -240,8 +243,10 @@ public class PceOtnNode implements PceNode { usableXpdrNWTps.add(nwTp); } if (usableXpdrClientTps.size() >= nbClient && usableXpdrNWTps.size() >= nbNetw) { - clientPerNwTp.put(nwTp.getValue(), clTp.getValue()); - return true; + if (this.clientPort == null || this.clientPort.equals(clTp.getValue())) { + clientPerNwTp.put(nwTp.getValue(), clTp.getValue()); + return true; + } } } } diff --git a/pce/src/test/java/org/opendaylight/transportpce/pce/graph/PceGraphTest.java b/pce/src/test/java/org/opendaylight/transportpce/pce/graph/PceGraphTest.java index cd5abb985..5ccd1d9ce 100644 --- a/pce/src/test/java/org/opendaylight/transportpce/pce/graph/PceGraphTest.java +++ b/pce/src/test/java/org/opendaylight/transportpce/pce/graph/PceGraphTest.java @@ -140,7 +140,7 @@ public class PceGraphTest { node = NodeUtils.getOTNNodeBuilder(NodeUtils.geSupportingNodes(), OpenroadmTpType.XPONDERNETWORK).build(); PceOtnNode pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.MUXPDR, - new NodeId("optical"), ServiceFormat.OTU.getName(), "DEGREE"); + new NodeId("optical"), ServiceFormat.OTU.getName(), "serviceType", null); pceOtnNode.validateXponder("optical", "sl"); pceOtnNode.validateXponder("not optical", "sl"); pceOtnNode.initXndrTps("AZ"); @@ -149,7 +149,7 @@ public class PceGraphTest { PceOtnNode pceOtnNode2 = new PceOtnNode(node, OpenroadmNodeType.MUXPDR, - new NodeId("optical2"), ServiceFormat.OTU.getName(), "DEGREE"); + new NodeId("optical2"), ServiceFormat.OTU.getName(), "serviceType", null); pceOtnNode2.validateXponder("optical", "sl"); pceOtnNode2.validateXponder("not optical", "sl"); pceOtnNode2.initXndrTps("AZ"); diff --git a/pce/src/test/java/org/opendaylight/transportpce/pce/networkanalyzer/PceLinkTest.java b/pce/src/test/java/org/opendaylight/transportpce/pce/networkanalyzer/PceLinkTest.java index e17183fa4..c2ff8ac86 100644 --- a/pce/src/test/java/org/opendaylight/transportpce/pce/networkanalyzer/PceLinkTest.java +++ b/pce/src/test/java/org/opendaylight/transportpce/pce/networkanalyzer/PceLinkTest.java @@ -174,7 +174,8 @@ public class PceLinkTest extends AbstractTest { Assert.assertNotNull(node.augmentation(Node1.class)); // OpenroadmNodeType nodeType = node.augmentation(Node1.class).; - PceOtnNode pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.SRG, node.getNodeId(), "otn", "serviceType"); + PceOtnNode pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.SRG, node.getNodeId(), "otn", "serviceType", + null); } diff --git a/pce/src/test/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNodeTest.java b/pce/src/test/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNodeTest.java index 646fea65e..09803b5c9 100644 --- a/pce/src/test/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNodeTest.java +++ b/pce/src/test/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNodeTest.java @@ -74,7 +74,7 @@ public class PceOtnNodeTest extends AbstractTest { @Test public void testInitXndrTpsODU4() { pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.MUXPDR, - new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_ODU4); + new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_ODU4, null); pceOtnNode.initXndrTps("AZ"); pceOtnNode.checkAvailableTribPort(); pceOtnNode.checkAvailableTribSlot(); @@ -85,7 +85,7 @@ public class PceOtnNodeTest extends AbstractTest { @Test public void testInitXndrTps10GE() { pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.MUXPDR, - new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_10GE); + new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_10GE, null); pceOtnNode.initXndrTps("mode"); pceOtnNode.checkAvailableTribPort(); pceOtnNode.checkAvailableTribSlot(); @@ -96,7 +96,7 @@ public class PceOtnNodeTest extends AbstractTest { public void testInitXndrTps10GXponderClient1() { node = getNodeBuilder(geSupportingNodes(), OpenroadmTpType.XPONDERCLIENT).build(); pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.ROADM, - new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_10GE); + new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_10GE, null); pceOtnNode.initXndrTps("mode"); pceOtnNode.checkAvailableTribPort(); pceOtnNode.checkAvailableTribSlot(); @@ -110,7 +110,7 @@ public class PceOtnNodeTest extends AbstractTest { public void testInitXndrTps1GXponderClient() { node = getNodeBuilder(geSupportingNodes(), OpenroadmTpType.XPONDERCLIENT).build(); pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.MUXPDR, - new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_1GE); + new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_1GE, null); pceOtnNode.initXndrTps("mode"); pceOtnNode.checkAvailableTribPort(); pceOtnNode.checkAvailableTribSlot(); @@ -120,7 +120,7 @@ public class PceOtnNodeTest extends AbstractTest { @Test public void testInitXndrTps10GXponderClient() { pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.MUXPDR, - new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_10GE); + new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_10GE, null); pceOtnNode.validateXponder("optical", "sl"); pceOtnNode.validateXponder("not optical", "sl"); pceOtnNode.initXndrTps("AZ"); @@ -134,7 +134,7 @@ public class PceOtnNodeTest extends AbstractTest { @Test public void testIsPceOtnNodeValid() { pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.MUXPDR, - new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_10GE); + new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_10GE, null); pceOtnNode.initXndrTps("AZ"); pceOtnNode.checkAvailableTribPort(); pceOtnNode.checkAvailableTribSlot(); @@ -144,7 +144,7 @@ public class PceOtnNodeTest extends AbstractTest { @Test public void testIsPceOtnNodeValidNode() { pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.DEGREE, - new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_100GE_M); + new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_100GE_M, null); pceOtnNode.initXndrTps("AZ"); pceOtnNode.checkAvailableTribPort(); pceOtnNode.checkAvailableTribSlot(); @@ -157,7 +157,7 @@ public class PceOtnNodeTest extends AbstractTest { @Test public void testIsPceOtnNodeValidNodeTypeNull() { pceOtnNode = new PceOtnNode(node, null, - new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_100GE_M); + new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_100GE_M, null); pceOtnNode.initXndrTps("AZ"); pceOtnNode.checkAvailableTribPort(); pceOtnNode.checkAvailableTribSlot(); @@ -167,7 +167,7 @@ public class PceOtnNodeTest extends AbstractTest { @Test public void testIsPceOtnNodeValidNodeTypeDeg() { pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.DEGREE, - new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_100GE_M); + new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_100GE_M, null); pceOtnNode.initXndrTps("AZ"); Assert.assertFalse("not valid node , its type isn't one of MUXPDR or SWITCH or TPDR" , pceOtnNode.isPceOtnNodeValid(pceOtnNode)); @@ -176,7 +176,7 @@ public class PceOtnNodeTest extends AbstractTest { @Test public void testIsPceOtnNodeValidTrue() { pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.MUXPDR, - new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_ODU4); + new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_ODU4, null); pceOtnNode.initXndrTps("AZ"); pceOtnNode.checkAvailableTribPort(); pceOtnNode.checkAvailableTribSlot(); @@ -187,7 +187,7 @@ public class PceOtnNodeTest extends AbstractTest { public void testIsPceOtnNodeValidChecksw() { node = getNodeBuilder(geSupportingNodes(), OpenroadmTpType.XPONDERCLIENT).build(); pceOtnNode = new PceOtnNode(node, OpenroadmNodeType.MUXPDR, - new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_1GE); + new NodeId("optical"), ServiceFormat.OMS.getName(), StringConstants.SERVICE_TYPE_1GE, null); pceOtnNode.initXndrTps("mode"); Assert.assertFalse("not valid otn service Type" , pceOtnNode.isPceOtnNodeValid(pceOtnNode)); } diff --git a/tapi/src/main/java/org/opendaylight/transportpce/tapi/connectivity/ConnectivityUtils.java b/tapi/src/main/java/org/opendaylight/transportpce/tapi/connectivity/ConnectivityUtils.java index 04d2eba46..1884cb3b5 100644 --- a/tapi/src/main/java/org/opendaylight/transportpce/tapi/connectivity/ConnectivityUtils.java +++ b/tapi/src/main/java/org/opendaylight/transportpce/tapi/connectivity/ConnectivityUtils.java @@ -125,7 +125,6 @@ public final class ConnectivityUtils { .onf.otcc.yang.tapi.connectivity.rev181210.connectivity.context.ConnectionKey, org.opendaylight.yang.gen.v1.urn.onf.otcc.yang.tapi.connectivity.rev181210.connectivity.context.Connection> connectionFullMap; // this variable is for complete connection objects - private Map> networkMap = new HashMap<>(); private final NetworkTransactionService networkTransactionService; // TODO -> handle cases for which node id is ROADM-A1 and not ROADMA01 or XPDR-A1 and not XPDRA01 @@ -1428,19 +1427,6 @@ public final class ConnectivityUtils { String rxPortName = txPortName; LOG.debug("Node z id = {}, txportDeviceName = {}, txPortName = {}", nodeid, txPortDeviceName, txPortName); LOG.debug("Node z id = {}, rxportDeviceName = {}, rxPortName = {}", nodeid, rxPortDeviceName, rxPortName); - if (serviceFormat.equals(ServiceFormat.ODU)) { - // TODO --> populate network map - populateNetworkMap(nodeid, txPortName); - } - if (serviceFormat.equals(ServiceFormat.Ethernet)) { - // TODO --> choose from network Map un network port which hasnt been used yet by another service. - // Set boolean to true and update txportName and so on - String updTxName = findFreeConfiguredNetworkPort(nodeid); - if (updTxName != null) { - txPortName = updTxName; - rxPortName = txPortName; - } - } // TODO --> get clli from datastore? String clli = "NodeSC"; LOG.info("Node z id = {}, txportDeviceName = {}, txPortName = {}", nodeid, txPortDeviceName, txPortName); @@ -1540,19 +1526,6 @@ public final class ConnectivityUtils { String rxPortName = txPortName; LOG.debug("Node a id = {}, txportDeviceName = {}, txPortName = {}", nodeid, txPortDeviceName, txPortName); LOG.debug("Node a id = {}, rxportDeviceName = {}, rxPortName = {}", nodeid, rxPortDeviceName, rxPortName); - if (serviceFormat.equals(ServiceFormat.ODU)) { - // TODO --> populate network map - populateNetworkMap(nodeid, txPortName); - } - if (serviceFormat.equals(ServiceFormat.Ethernet)) { - // TODO --> choose from network Map un network port which hasnt been used yet by another service. - // Set boolean to true and update txportName and so on - String updTxName = findFreeConfiguredNetworkPort(nodeid); - if (updTxName != null) { - txPortName = updTxName; - rxPortName = txPortName; - } - } // TODO --> get clli from datastore? String clli = "NodeSA"; LOG.info("Node a id = {}, txportDeviceName = {}, txPortName = {}", nodeid, txPortDeviceName, txPortName); @@ -1611,30 +1584,6 @@ public final class ConnectivityUtils { return serviceAEndBuilder.build(); } - private String findFreeConfiguredNetworkPort(String nodeid) { - if (!this.networkMap.containsKey(nodeid)) { - return null; - } - Map netMap = this.networkMap.get(nodeid); - for (Map.Entry entry : netMap.entrySet()) { - if (!entry.getValue()) { - this.networkMap.get(nodeid).put(entry.getKey(), true); - return entry.getKey(); - } - } - return null; - } - - private void populateNetworkMap(String nodeid, String txPortName) { - Map netMap = new HashMap<>(); - netMap.put(txPortName, false); - if (!this.networkMap.containsKey(nodeid)) { - this.networkMap.put(nodeid, netMap); - } else if (!this.networkMap.get(nodeid).containsKey(txPortName)) { - this.networkMap.get(nodeid).putAll(netMap); - } - } - private ConnectionType getConnectionTypePhtnc(Collection endPoints) { if (endPoints.stream().anyMatch(ep -> ep.getName().values().stream() diff --git a/tests/transportpce_tests/2.2.1/test15_otn_end2end_with_intermediate_switch.py b/tests/transportpce_tests/2.2.1/test15_otn_end2end_with_intermediate_switch.py index 3cc1e32bf..3fb038830 100644 --- a/tests/transportpce_tests/2.2.1/test15_otn_end2end_with_intermediate_switch.py +++ b/tests/transportpce_tests/2.2.1/test15_otn_end2end_with_intermediate_switch.py @@ -739,8 +739,8 @@ class TransportPCEtesting(unittest.TestCase): self.cr_serv_sample_data["input"]["service-z-end"]["service-rate"] = "10" self.cr_serv_sample_data["input"]["service-z-end"]["service-format"] = "Ethernet" del self.cr_serv_sample_data["input"]["service-z-end"]["odu-service-rate"] - self.cr_serv_sample_data["input"]["service-z-end"]["tx-direction"]["port"]["port-name"] = "XPDR1-NETWORK1" - self.cr_serv_sample_data["input"]["service-z-end"]["rx-direction"]["port"]["port-name"] = "XPDR1-NETWORK1" + self.cr_serv_sample_data["input"]["service-z-end"]["tx-direction"]["port"]["port-name"] = "XPDR1-CLIENT1" + self.cr_serv_sample_data["input"]["service-z-end"]["rx-direction"]["port"]["port-name"] = "XPDR1-CLIENT1" response = test_utils.service_create_request(self.cr_serv_sample_data) self.assertEqual(response.status_code, requests.codes.ok) res = response.json() -- 2.36.6