Fix bug in PCE picking wrong client port 15/99615/1
authorGilles Thouenon <gilles.thouenon@orange.com>
Wed, 26 Jan 2022 08:14:05 +0000 (09:14 +0100)
committerChristophe BETOULE <christophe.betoule@orange.com>
Mon, 14 Feb 2022 14:56:48 +0000 (14:56 +0000)
- 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 <gilles.thouenon@orange.com>
Change-Id: Ic3f1203f62fa19db46935524200c47f74611187c
(cherry picked from commit 895f3e89b3c2554e704f74c764d23d5b79bf0862)

pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceCalculation.java
pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNode.java
pce/src/test/java/org/opendaylight/transportpce/pce/graph/PceGraphTest.java
pce/src/test/java/org/opendaylight/transportpce/pce/networkanalyzer/PceLinkTest.java
pce/src/test/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNodeTest.java
tapi/src/main/java/org/opendaylight/transportpce/tapi/connectivity/ConnectivityUtils.java
tests/transportpce_tests/2.2.1/test15_otn_end2end_with_intermediate_switch.py

index d2f3305f1c321ff37bf23f323d9cc78dfee70329..32d9068f33632bcd716e5204eef94865fc3202b2 100644 (file)
@@ -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<Link> 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<Link> 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()) {
index 35276d5e0c2b4e86e60f0c00e2a1e3162817170f..2393f2f92b86d98827df089c28c8a659a871f8e1 100644 (file)
@@ -75,8 +75,10 @@ public class PceOtnNode implements PceNode {
 
     private List<PceLink> outgoingLinks = new ArrayList<>();
     private Map<String, String> 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;
+                            }
                         }
                     }
                 }
index cd5abb985deeb07fb594c835599883d088903c49..5ccd1d9ce32ac34b8832d4019b78e057402221be 100644 (file)
@@ -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");
index e17183fa43fb5f7174f704cd68ca1f68db18a70c..c2ff8ac86292bf95d1071f4f49c06e15ef1cf090 100644 (file)
@@ -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);
 
     }
 
index 646fea65ec14ad2d028a2b49e020cbceb9bfecca..09803b5c94c539eabd3c9129626d4a9f7a9043c3 100644 (file)
@@ -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));
     }
index 04d2eba46fb4190ac80e0b53e62a22fabaa643da..1884cb3b577a288e23ce0ff5e4555446f0246a9e 100644 (file)
@@ -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<String, Map<String, Boolean>> 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<String, Boolean> netMap = this.networkMap.get(nodeid);
-        for (Map.Entry<String, Boolean> 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<String, Boolean> 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<org.opendaylight.yang.gen.v1.urn
             .onf.otcc.yang.tapi.connectivity.rev181210.create.connectivity.service.input.EndPoint> endPoints) {
         if (endPoints.stream().anyMatch(ep -> ep.getName().values().stream()
index 3cc1e32bfb0fd63c38ac444ddaeb59cbfce1a0e7..3fb038830bd7d037bea50d512812eb02c3d870bb 100644 (file)
@@ -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()