[ONOS] SR resilency, CR resilency and delete problem , old tunnels not deleting issue

Change-Id: Ib4b9732c28a1f87a8a76347a5936593b1b5bf72a

[ONOS] SR resilency, CR resilency and delete problem , old tunnels not deleting issue
Change-Id: Ib4b9732c28a1f87a8a76347a5936593b1b5bf72a
Priyanka B · Gerrit Code Review
Commit 7bc6419db3d9e31a50814cb132b517b7ff12899d 7bc6419d 1 parent 8d9203e3
Showing 7 changed files with 109 additions and 50 deletions
apps/pce/app/src/main/java/org/onosproject/pce/pceservice/BasicPceccHandler.java
apps/pce/app/src/main/java/org/onosproject/pce/pceservice/PceManager.java
apps/pce/app/src/test/java/org/onosproject/pce/pceservice/PceManagerTest.java
providers/bgp/topology/src/main/java/org/onosproject/provider/bgp/topology/impl/BgpTopologyProvider.java
providers/bgp/topology/src/test/java/org/onosproject/provider/bgp/topology/impl/BgpTopologyProviderTest.java
providers/pcep/tunnel/src/main/java/org/onosproject/provider/pcep/tunnel/impl/PcepTunnelProvider.java
providers/pcep/tunnel/src/test/java/org/onosproject/provider/pcep/tunnel/impl/PcepTunnelAddedTest.java
--- a/apps/pce/app/src/main/java/org/onosproject/pce/pceservice/BasicPceccHandler.java
View file @7bc6419
+++ b/apps/pce/app/src/main/java/org/onosproject/pce/pceservice/BasicPceccHandler.java
View file @7bc6419
@@ -325,7 +325,7 @@ public final class BasicPceccHandler {
    }
     // Install a rule for pushing local labels to the device which is specific to path.
-    private void installLocalLabelRule(DeviceId deviceId, LabelResourceId labelId,
+    private synchronized void installLocalLabelRule(DeviceId deviceId, LabelResourceId labelId,
                                        PortNumber portNum, TunnelId tunnelId,
                                        Boolean isBos, Long labelType,
                                        Objective.Operation type) {
--- a/apps/pce/app/src/main/java/org/onosproject/pce/pceservice/PceManager.java
View file @7bc6419
+++ b/apps/pce/app/src/main/java/org/onosproject/pce/pceservice/PceManager.java
View file @7bc6419
@@ -31,7 +31,6 @@ import java.util.concurrent.ScheduledExecutorService;
 import org.onlab.packet.Ethernet;
 import org.onlab.packet.IPv4;
-
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -110,14 +109,14 @@ import org.slf4j.LoggerFactory;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import static org.onosproject.incubator.net.tunnel.Tunnel.Type.MPLS;
+import static org.onosproject.incubator.net.tunnel.Tunnel.State.ACTIVE;
 import static org.onosproject.incubator.net.tunnel.Tunnel.State.INIT;
 import static org.onosproject.incubator.net.tunnel.Tunnel.State.ESTABLISHED;
 import static org.onosproject.incubator.net.tunnel.Tunnel.State.UNSTABLE;
+import static org.onosproject.incubator.net.tunnel.Tunnel.Type.MPLS;
 import static org.onosproject.pce.pceservice.LspType.WITH_SIGNALLING;
 import static org.onosproject.pce.pceservice.LspType.SR_WITHOUT_SIGNALLING;
 import static org.onosproject.pce.pceservice.LspType.WITHOUT_SIGNALLING_AND_WITHOUT_SR;
-
 import static org.onosproject.pce.pceservice.PcepAnnotationKeys.BANDWIDTH;
 import static org.onosproject.pce.pceservice.PcepAnnotationKeys.LOCAL_LSP_ID;
 import static org.onosproject.pce.pceservice.PcepAnnotationKeys.LSP_SIG_TYPE;
@@ -621,6 +620,12 @@ public class PceManager implements PceService {
             return false;
         }
+        LspType lspType = LspType.valueOf(tunnel.annotations().value(LSP_SIG_TYPE));
+        // Release basic PCECC labels.
+        if (lspType == WITHOUT_SIGNALLING_AND_WITHOUT_SR) {
+            crHandler.releaseLabel(tunnel);
+        }
+
         // 2. Call tunnel service.
         return tunnelService.downTunnel(appId, tunnel.tunnelId());
     }
@@ -1148,6 +1153,22 @@ public class PceManager implements PceService {
                     }
                 }
+                //In CR case, release labels when new tunnel for it is updated.
+                if (lspType == WITHOUT_SIGNALLING_AND_WITHOUT_SR && tunnel.state() == ACTIVE
+                        && mastershipService.getLocalRole(tunnel.path().src().deviceId()) == MastershipRole.MASTER) {
+                    Collection<Tunnel> tunnels = tunnelService.queryTunnel(tunnel.src(), tunnel.dst());
+
+                    for (Tunnel t : tunnels) {
+                          if (tunnel.annotations().value(PLSP_ID).equals(t.annotations().value(PLSP_ID))
+                              && !tunnel.annotations().value(LOCAL_LSP_ID)
+                                  .equals(t.annotations().value(LOCAL_LSP_ID))) {
+                              // Release basic PCECC labels.
+                              crHandler.releaseLabel(t);
+                              break;
+                          }
+                    }
+                }
+
                 if (tunnel.state() == UNSTABLE) {
                     /*
                      * During LSP DB sync if PCC doesn't report LSP which was PCE initiated, it's state is turned into
@@ -1183,23 +1204,16 @@ public class PceManager implements PceService {
                 if (lspType != WITH_SIGNALLING) {
                     localLspIdFreeList.add(Short.valueOf(tunnel.annotations().value(LOCAL_LSP_ID)));
                 }
-
                 // If not zero bandwidth, and delegated (initiated LSPs will also be delegated).
-                if (bwConstraintValue != 0) {
+                if (Double.parseDouble(tunnel.annotations().value(BANDWIDTH)) != 0.0
-                    releaseBandwidth(event.subject());
+                        && mastershipService.getLocalRole(tunnel.path().src().deviceId()) == MastershipRole.MASTER) {
-
+                    releaseBandwidth(tunnel);
-                    // Release basic PCECC labels.
-                    if (lspType == WITHOUT_SIGNALLING_AND_WITHOUT_SR) {
-                        // Delete stored tunnel consumer id from PCE store (while still retaining label list.)
-                        PceccTunnelInfo pceccTunnelInfo = pceStore.getTunnelInfo(tunnel.tunnelId());
-                        pceccTunnelInfo.tunnelConsumerId(null);
-                        if (mastershipService.getLocalRole(tunnel.path().src().deviceId()) == MastershipRole.MASTER) {
-                            crHandler.releaseLabel(tunnel);
                 }
-                    } else {
+
+                if (pceStore.getTunnelInfo(tunnel.tunnelId()) != null) {
                     pceStore.removeTunnelInfo(tunnel.tunnelId());
                 }
-                }
+
                 break;
             default:
--- a/apps/pce/app/src/test/java/org/onosproject/pce/pceservice/PceManagerTest.java
View file @7bc6419
+++ b/apps/pce/app/src/test/java/org/onosproject/pce/pceservice/PceManagerTest.java
View file @7bc6419
@@ -641,11 +641,14 @@ public class PceManagerTest {
         build4RouterTopo(false, false, false, false, 5);
         List<Constraint> constraints = new LinkedList<Constraint>();
         CostConstraint costConstraint = new CostConstraint(TE_COST);
+        BandwidthConstraint bwConst = new BandwidthConstraint(Bandwidth.bps(3));
+        constraints.add(bwConst);
         constraints.add(costConstraint);
         pceManager.setupPath(D1.deviceId(), D2.deviceId(), "T123", constraints, WITH_SIGNALLING);
         Collection<Tunnel> tunnels = (Collection<Tunnel>) pceManager.queryAllPath();
+
         assertThat(tunnels.size(), is(1));
         boolean result;
         for (Tunnel tunnel : tunnels) {
--- a/providers/bgp/topology/src/main/java/org/onosproject/provider/bgp/topology/impl/BgpTopologyProvider.java
View file @7bc6419
+++ b/providers/bgp/topology/src/main/java/org/onosproject/provider/bgp/topology/impl/BgpTopologyProvider.java
View file @7bc6419
@@ -63,8 +63,6 @@ import org.onosproject.bgpio.types.attr.BgpLinkAttrTeDefaultMetric;
 import org.onosproject.core.CoreService;
 import org.onosproject.incubator.net.resource.label.LabelResourceAdminService;
 import org.onosproject.incubator.net.resource.label.LabelResourceId;
-import org.onosproject.mastership.MastershipEvent;
-import org.onosproject.mastership.MastershipListener;
 import org.onosproject.mastership.MastershipService;
 import org.onosproject.net.AnnotationKeys;
 import org.onosproject.net.ConnectPoint;
@@ -79,6 +77,8 @@ import org.onosproject.net.config.basics.BandwidthCapacity;
 import org.onosproject.net.device.DefaultDeviceDescription;
 import org.onosproject.net.device.DefaultPortDescription;
 import org.onosproject.net.device.DeviceDescription;
+import org.onosproject.net.device.DeviceEvent;
+import org.onosproject.net.device.DeviceListener;
 import org.onosproject.net.device.DeviceProvider;
 import org.onosproject.net.device.DeviceProviderRegistry;
 import org.onosproject.net.device.DeviceProviderService;
@@ -140,7 +140,7 @@ public class BgpTopologyProvider extends AbstractProvider implements DeviceProvi
     private DeviceProviderService deviceProviderService;
     private LinkProviderService linkProviderService;
-    private InternalMastershipListener masterListener = new InternalMastershipListener();
+    private DeviceListener deviceListener = new InternalDeviceListener();
     private InternalBgpProvider listener = new InternalBgpProvider();
     private static final String UNKNOWN = "unknown";
     public static final long IDENTIFIER_SET = 0x100000000L;
@@ -169,7 +169,7 @@ public class BgpTopologyProvider extends AbstractProvider implements DeviceProvi
         deviceProviderService = deviceProviderRegistry.register(this);
         linkProviderService = linkProviderRegistry.register(this);
         controller.addListener(listener);
-        mastershipService.addListener(masterListener);
+        deviceService.addListener(deviceListener);
         controller.addLinkListener(listener);
     }
@@ -182,25 +182,28 @@ public class BgpTopologyProvider extends AbstractProvider implements DeviceProvi
         linkProviderService = null;
         controller.removeListener(listener);
         controller.removeLinkListener(listener);
-        mastershipService.removeListener(masterListener);
+        deviceService.removeListener(deviceListener);
     }
-    private class InternalMastershipListener implements MastershipListener {
+    private class InternalDeviceListener implements DeviceListener {
         @Override
-        public void event(MastershipEvent event) {
+        public void event(DeviceEvent event) {
-            if (event.type() == MastershipEvent.Type.MASTER_CHANGED) {
+            Device device = event.subject();
-                if (mastershipService.getMasterFor(event.subject()) != null) {
+
-                    //Only for L3 device create label pool for that device
+            switch (event.type()) {
-                    Device device = deviceService.getDevice(event.subject());
+                case DEVICE_ADDED:
-                    if (device == null) {
+                    if (!mastershipService.isLocalMaster(device.id())) {
-                        log.debug("Device {} doesn't exist", event.subject());
+                        break;
-                        return;
                     }
-                    //Reserve device label pool for L3 devices
+
+                    // Reserve device label pool for L3 devices
                     if (device.annotations().value(LSRID) != null) {
-                        createDevicePool(event.subject());
+                        createDevicePool(device.id());
-                    }
                     }
+                    break;
+
+                default:
+                    break;
             }
         }
     }
@@ -444,7 +447,6 @@ public class BgpTopologyProvider extends AbstractProvider implements DeviceProvi
         @Override
         public void deleteLink(BgpLinkLsNlriVer4 linkNlri) throws BgpParseException {
             log.debug("Delete link {}", linkNlri.toString());
-
             if (linkProviderService == null) {
                 return;
             }
@@ -461,6 +463,10 @@ public class BgpTopologyProvider extends AbstractProvider implements DeviceProvi
             }
             linkProviderService.linkVanished(linkDes);
+
+            linkDes = new DefaultLinkDescription(linkDes.dst(), linkDes.src(), Link.Type.DIRECT,
+                    false, linkDes.annotations());
+            linkProviderService.linkVanished(linkDes);
         }
     }
--- a/providers/bgp/topology/src/test/java/org/onosproject/provider/bgp/topology/impl/BgpTopologyProviderTest.java
View file @7bc6419
+++ b/providers/bgp/topology/src/test/java/org/onosproject/provider/bgp/topology/impl/BgpTopologyProviderTest.java
View file @7bc6419
@@ -53,10 +53,7 @@ import org.onosproject.bgpio.types.LinkStateAttributes;
 import org.onosproject.incubator.net.resource.label.LabelResourceAdminService;
 import org.onosproject.incubator.net.resource.label.LabelResourceId;
 import org.onosproject.incubator.net.resource.label.LabelResourcePool;
-import org.onosproject.mastership.MastershipEvent;
-import org.onosproject.mastership.MastershipListener;
 import org.onosproject.mastership.MastershipServiceAdapter;
-import org.onosproject.mastership.MastershipEvent.Type;
 import org.onosproject.net.link.LinkServiceAdapter;
 import org.onosproject.bgpio.types.LinkLocalRemoteIdentifiersTlv;
 import org.onosproject.bgpio.types.RouteDistinguisher;
@@ -68,8 +65,8 @@ import org.onosproject.bgpio.types.attr.BgpLinkAttrMaxLinkBandwidth;
 import org.onosproject.bgpio.types.attr.BgpLinkAttrTeDefaultMetric;
 import org.onosproject.bgpio.util.Constants;
 import org.onosproject.cluster.NodeId;
-import org.onosproject.cluster.RoleInfo;
 import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.DefaultAnnotations;
 import org.onosproject.net.DefaultDevice;
 import org.onosproject.net.DefaultLink;
 import org.onosproject.net.Device;
@@ -82,6 +79,8 @@ import org.onosproject.net.config.ConfigApplyDelegate;
 import org.onosproject.net.config.ConfigFactory;
 import org.onosproject.net.config.NetworkConfigRegistryAdapter;
 import org.onosproject.net.device.DeviceDescription;
+import org.onosproject.net.device.DeviceEvent;
+import org.onosproject.net.device.DeviceListener;
 import org.onosproject.net.device.DeviceProvider;
 import org.onosproject.net.device.DeviceProviderRegistry;
 import org.onosproject.net.device.DeviceProviderService;
@@ -122,7 +121,7 @@ public class BgpTopologyProviderTest {
     private MockNetConfigRegistryAdapter networkConfigService = new MockNetConfigRegistryAdapter();
     private MockLabelResourceService labelResourceAdminService = new MockLabelResourceService();
     private Map<DeviceId, Device> deviceMap = new HashMap<>();
-    private MastershipListener listener;
+    private DeviceListener listener;
     @Before
     public void startUp() throws TestUtilsException {
@@ -134,7 +133,7 @@ public class BgpTopologyProviderTest {
         provider.labelResourceAdminService = labelResourceAdminService;
         provider.mastershipService = mastershipService;
         provider.networkConfigService = networkConfigService;
-        listener = TestUtils.getField(provider, "masterListener");
+        listener = TestUtils.getField(provider, "deviceListener");
         provider.activate();
         assertThat("device provider should be registered", not(nodeRegistry.provider));
         assertThat("link provider should be registered", not(linkRegistry.linkProvider));
@@ -887,10 +886,14 @@ public class BgpTopologyProviderTest {
             l.addNode(nodeNlri, details);
             assertThat(nodeRegistry.connected.size(), is(1));
         }
+        DefaultAnnotations.Builder newBuilder = DefaultAnnotations.builder();
-        MastershipEvent event = new MastershipEvent(Type.MASTER_CHANGED, nodeRegistry.connected.iterator().next(),
+        newBuilder.set("lsrId", "1.1.1.1");
-                new RoleInfo(NodeId.nodeId("Node1"), new LinkedList<>()));
+        Device device = new DefaultDevice(BgpTopologyProviderTest.providerId, nodeRegistry.connected.iterator().next(),
+                Device.Type.ROUTER, UNKNOWN, UNKNOWN, UNKNOWN, UNKNOWN, new ChassisId(), newBuilder.build());
+
+        DeviceEvent event = new DeviceEvent(DeviceEvent.Type.DEVICE_ADDED, device);
         listener.event(event);
         assertThat(labelResourceAdminService.resourcePool.keySet().size(), is(1));
     }
@@ -926,9 +929,16 @@ public class BgpTopologyProviderTest {
         for (BgpNodeListener l : controller.nodeListener) {
             l.addNode(nodeNlri, details);
             assertThat(nodeRegistry.connected.size(), is(1));
-            // Check label resource reserved for that device
+
-            MastershipEvent event = new MastershipEvent(Type.MASTER_CHANGED, nodeRegistry.connected.iterator().next(),
+            DefaultAnnotations.Builder newBuilder = DefaultAnnotations.builder();
-                    new RoleInfo(NodeId.nodeId("Node1"), new LinkedList<>()));
+
+            newBuilder.set("lsrId", "1.1.1.1");
+
+            Device device = new DefaultDevice(BgpTopologyProviderTest.providerId,
+                   nodeRegistry.connected.iterator().next(), Device.Type.ROUTER, UNKNOWN,
+                   UNKNOWN, UNKNOWN, UNKNOWN, new ChassisId(), newBuilder.build());
+
+            DeviceEvent event = new DeviceEvent(DeviceEvent.Type.DEVICE_ADDED, device);
             listener.event(event);
             assertThat(labelResourceAdminService.resourcePool.keySet().size(), is(1));
@@ -988,8 +998,15 @@ public class BgpTopologyProviderTest {
             l.addNode(nodeNlri, details);
             assertThat(nodeRegistry.connected.size(), is(1));
             //Check label resource reserved for that device
-            MastershipEvent event = new MastershipEvent(Type.MASTER_CHANGED, nodeRegistry.connected.iterator().next(),
+            DefaultAnnotations.Builder newBuilder = DefaultAnnotations.builder();
-                    new RoleInfo(NodeId.nodeId("Node1"), new LinkedList<>()));
+
+            newBuilder.set("lsrId", "1.1.1.1");
+
+            Device device = new DefaultDevice(BgpTopologyProviderTest.providerId,
+                    nodeRegistry.connected.iterator().next(), Device.Type.ROUTER,
+                    UNKNOWN, UNKNOWN, UNKNOWN, UNKNOWN, new ChassisId(), newBuilder.build());
+
+            DeviceEvent event = new DeviceEvent(DeviceEvent.Type.DEVICE_ADDED, device);
             listener.event(event);
             assertThat(labelResourceAdminService.resourcePool.keySet().size(), is(1));
             l.addNode(remNodeNlri, details);
--- a/providers/pcep/tunnel/src/main/java/org/onosproject/provider/pcep/tunnel/impl/PcepTunnelProvider.java
View file @7bc6419
+++ b/providers/pcep/tunnel/src/main/java/org/onosproject/provider/pcep/tunnel/impl/PcepTunnelProvider.java
View file @7bc6419
--- a/providers/pcep/tunnel/src/test/java/org/onosproject/provider/pcep/tunnel/impl/PcepTunnelAddedTest.java
View file @7bc6419
+++ b/providers/pcep/tunnel/src/test/java/org/onosproject/provider/pcep/tunnel/impl/PcepTunnelAddedTest.java
View file @7bc6419
@@ -36,6 +36,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -88,6 +89,7 @@ import org.onosproject.pcep.controller.ClientCapability;
 import org.onosproject.pcep.controller.LspKey;
 import org.onosproject.pcep.controller.PccId;
+import com.esotericsoftware.minlog.Log;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableSet;
@@ -223,6 +225,7 @@ public class PcepTunnelAddedTest {
                         tunnel.path(),
                         tunnel.resource(),
                         tunnel.annotations());
+                Log.info("tunnel.annotations().toString() " + tunnel.annotations().toString());
                 tunnelService.tunnelIdAsKeyStore.put(id, storedTunnel);
                 return id;
             }
@@ -237,6 +240,18 @@ public class PcepTunnelAddedTest {
             @Override
             public void tunnelUpdated(TunnelDescription tunnel, State state) {
+                TunnelId id = TunnelId.valueOf(String.valueOf(++tunnelIdCounter));
+                Tunnel storedTunnel = new DefaultTunnel(ProviderId.NONE,
+                        tunnel.src(), tunnel.dst(),
+                        tunnel.type(),
+                        tunnel.groupId(),
+                        id,
+                        tunnel.tunnelName(),
+                        tunnel.path(),
+                        tunnel.resource(),
+                        tunnel.annotations());
+                tunnelService.tunnelIdAsKeyStore.put(id, storedTunnel);
+                Log.info("tunnelService.tunnelIdAsKeyStore ++ " + tunnelService.tunnelIdAsKeyStore.toString());
             }
             @Override
@@ -254,6 +269,7 @@ public class PcepTunnelAddedTest {
         public TunnelId setupTunnel(ApplicationId producerId, ElementId srcElementId, Tunnel tunnel, Path path) {
             TunnelId tunnelId = TunnelId.valueOf(String.valueOf(++tunnelIdCounter));
             tunnelIdAsKeyStore.put(tunnelId, tunnel);
+            Log.info("tunnelIdAsKeyStore insetup :: " + tunnelIdAsKeyStore.toString());
             return tunnelId;
         }
@@ -592,8 +608,11 @@ public class PcepTunnelAddedTest {
         controller.processClientMessage(PccId.pccId(IpAddress.valueOf("1.1.1.1")), message);
         TimeUnit.MILLISECONDS.sleep(4000);
-        assertThat(registry.tunnelIdCounter, is((long) 1));
+        assertThat(registry.tunnelIdCounter, is((long) 2));
-        assertThat(tunnelService.tunnelIdAsKeyStore.values().iterator().next().annotations().value(DELEGATE),
+
+        Iterator<Tunnel> iterator = tunnelService.tunnelIdAsKeyStore.values().iterator();
+        iterator.next();
+        assertThat(iterator.next().annotations().value(DELEGATE),
                 is("true"));
     }