Fixed a few intent synchronization issues.

Also added a CLI command to test SDN-IP primary switchover.

Change-Id: Id31f79262a2b607f987adad2fdb3eb54eb939fea

@@ -46,6 +46,7 @@ import org.onlab.onos.net.host.HostListener;
 import org.onlab.onos.net.host.HostService;
 import org.onlab.onos.net.intent.Intent;
 import org.onlab.onos.net.intent.IntentService;
+import org.onlab.onos.net.intent.IntentState;
 import org.onlab.onos.net.intent.MultiPointToSinglePointIntent;
 import org.onlab.onos.sdnip.config.BgpPeer;
 import org.onlab.onos.sdnip.config.Interface;
@@ -145,11 +146,6 @@ public class Router implements RouteListener {
      * Starts the Router.
      */
     public void start() {
-
-        // TODO hack to enable SDN-IP now for testing
-        isElectedLeader = true;
-        isActivatedLeader = true;
-
         bgpUpdatesExecutor.execute(new Runnable() {
             @Override
             public void run() {
@@ -277,30 +273,21 @@ public class Router implements RouteListener {
             // based on the matching prefix.
             //
             for (Intent intent : intentService.getIntents()) {
-                //
-                // TODO: Ignore all intents that are not installed by
-                // the SDN-IP application.
-                //
-                if (!(intent instanceof MultiPointToSinglePointIntent)) {
+
+                if (!(intent instanceof MultiPointToSinglePointIntent)
+                        || !intent.appId().equals(appId)) {
                     continue;
                 }
                 MultiPointToSinglePointIntent mp2pIntent =
                         (MultiPointToSinglePointIntent) intent;
-                /*Match match = mp2pIntent.getMatch();
-                if (!(match instanceof PacketMatch)) {
-                    continue;
-                }
-                PacketMatch packetMatch = (PacketMatch) match;
-                Ip4Prefix prefix = packetMatch.getDstIpAddress();
-                if (prefix == null) {
-                    continue;
-                }
-                fetchedIntents.put(prefix, mp2pIntent);*/
-                for (Criterion criterion : mp2pIntent.selector().criteria()) {
-                    if (criterion.type() == Type.IPV4_DST) {
-                        IPCriterion ipCriterion = (IPCriterion) criterion;
-                        fetchedIntents.put(ipCriterion.ip(), mp2pIntent);
-                    }
+
+                Criterion c = mp2pIntent.selector().getCriterion(Type.IPV4_DST);
+                if (c != null && c instanceof IPCriterion) {
+                    IPCriterion ipCriterion = (IPCriterion) c;
+                    fetchedIntents.put(ipCriterion.ip(), mp2pIntent);
+                } else {
+                    log.warn("No IPV4_DST criterion found for intent {}",
+                            mp2pIntent.id());
                 }
 
             }
@@ -344,6 +331,14 @@ public class Router implements RouteListener {
                     continue;
                 }
 
+                IntentState state = intentService.getIntentState(fetchedIntent.id());
+                if (state == IntentState.WITHDRAWING ||
+                        state == IntentState.WITHDRAWN) {
+                    // The intent has been withdrawn but according to our route
+                    // table it should be installed. We'll reinstall it.
+                    addIntents.add(Pair.of(prefix, inMemoryIntent));
+                }
+
                 //
                 // If IN-MEMORY Intent is same as the FETCHED Intent,
                 // store the FETCHED Intent in the local memory.
@@ -434,19 +429,7 @@ public class Router implements RouteListener {
     private boolean compareMultiPointToSinglePointIntents(
             MultiPointToSinglePointIntent intent1,
             MultiPointToSinglePointIntent intent2) {
-        /*Match match1 = intent1.getMatch();
-        Match match2 = intent2.getMatch();
-        Action action1 = intent1.getAction();
-        Action action2 = intent2.getAction();
-        Set<SwitchPort> ingressPorts1 = intent1.getIngressPorts();
-        Set<SwitchPort> ingressPorts2 = intent2.getIngressPorts();
-        SwitchPort egressPort1 = intent1.getEgressPort();
-        SwitchPort egressPort2 = intent2.getEgressPort();
-
-        return Objects.equal(match1, match2) &&
-            Objects.equal(action1, action2) &&
-            Objects.equal(egressPort1, egressPort2) &&
-            Objects.equal(ingressPorts1, ingressPorts2);*/
+
         return Objects.equal(intent1.appId(), intent2.appId()) &&
                 Objects.equal(intent1.selector(), intent2.selector()) &&
                 Objects.equal(intent1.treatment(), intent2.treatment()) &&

@@ -77,6 +77,10 @@ public class SdnIp implements SdnIpService {
         router = new Router(appId, intentService, hostService, config, interfaceService);
         router.start();
 
+        // Manually set the router as the leader to allow testing
+        // TODO change this when we get a leader election
+        router.leaderChanged(true);
+
         bgpSessionManager = new BgpSessionManager(router);
         bgpSessionManager.startUp(2000); // TODO
 
@@ -99,6 +103,11 @@ public class SdnIp implements SdnIpService {
         return router.getRoutes();
     }
 
+    @Override
+    public void modifyPrimary(boolean isPrimary) {
+        router.leaderChanged(isPrimary);
+    }
+
     static String dpidToUri(String dpid) {
         return "of:" + dpid.replace(":", "");
     }

@@ -36,4 +36,12 @@ public interface SdnIpService {
      * @return the SDN-IP routes
      */
     public Collection<RouteEntry> getRoutes();
+
+    /**
+     * Changes whether this SDN-IP instance is the primary or not based on the
+     * boolean parameter.
+     *
+     * @param isPrimary true if the instance is primary, false if it is not
+     */
+    public void modifyPrimary(boolean isPrimary);
 }

@@ -1594,7 +1594,7 @@ public class BgpSession extends SimpleChannelHandler {
         //
         // Parse the KEEPALIVE message: nothing to do
         //
-        log.debug("BGP RX KEEPALIVE message from {}", remoteAddress);
+        log.trace("BGP RX KEEPALIVE message from {}", remoteAddress);
 
         // Start the Session Timeout timer
         restartSessionTimeoutTimer(ctx);

+package org.onlab.onos.sdnip.cli;
+
+import org.apache.karaf.shell.commands.Argument;
+import org.apache.karaf.shell.commands.Command;
+import org.onlab.onos.cli.AbstractShellCommand;
+import org.onlab.onos.sdnip.SdnIpService;
+
+/**
+ * Command to change whether this SDNIP instance is primary or not.
+ */
+@Command(scope = "onos", name = "sdnip-set-primary",
+         description = "Changes the primary status of this SDN-IP instance")
+public class PrimaryChangeCommand extends AbstractShellCommand {
+
+    @Argument(index = 0, name = "isPrimary",
+            description = "True if this instance should be primary, false if not",
+            required = true, multiValued = false)
+    boolean isPrimary = false;
+
+    @Override
+    protected void execute() {
+        get(SdnIpService.class).modifyPrimary(isPrimary);
+    }
+
+}

@@ -22,5 +22,8 @@
     <command>
       <action class="org.onlab.onos.sdnip.cli.RoutesListCommand"/>
     </command>
+    <command>
+      <action class="org.onlab.onos.sdnip.cli.PrimaryChangeCommand"/>
+    </command>
   </command-bundle>
 </blueprint>

@@ -8,6 +8,7 @@ import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.reset;
 import static org.easymock.EasyMock.verify;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import java.util.HashSet;
@@ -35,6 +36,7 @@ import org.onlab.onos.net.host.HostService;
 import org.onlab.onos.net.host.InterfaceIpAddress;
 import org.onlab.onos.net.intent.Intent;
 import org.onlab.onos.net.intent.IntentService;
+import org.onlab.onos.net.intent.IntentState;
 import org.onlab.onos.net.intent.MultiPointToSinglePointIntent;
 import org.onlab.onos.net.provider.ProviderId;
 import org.onlab.onos.sdnip.config.Interface;
@@ -234,6 +236,10 @@ public class IntentSyncTest {
                 IpPrefix.valueOf("6.6.6.0/24"),
                 IpAddress.valueOf("192.168.10.1"));
 
+        RouteEntry routeEntry7 = new RouteEntry(
+                IpPrefix.valueOf("7.7.7.0/24"),
+                IpAddress.valueOf("192.168.10.1"));
+
         MultiPointToSinglePointIntent intent1 = intentBuilder(
                 routeEntry1.prefix(), "00:00:00:00:00:01", SW1_ETH1);
         MultiPointToSinglePointIntent intent2 = intentBuilder(
@@ -246,6 +252,8 @@ public class IntentSyncTest {
                 routeEntry4Update.prefix(), "00:00:00:00:00:02", SW2_ETH1);
         MultiPointToSinglePointIntent intent5 = intentBuilder(
                 routeEntry5.prefix(), "00:00:00:00:00:01",  SW1_ETH1);
+        MultiPointToSinglePointIntent intent7 = intentBuilder(
+                routeEntry7.prefix(), "00:00:00:00:00:01",  SW1_ETH1);
 
         // Compose a intent, which is equal to intent5 but the id is different.
         MultiPointToSinglePointIntent intent5New =
@@ -255,7 +263,7 @@ public class IntentSyncTest {
                 new Class<?>[] {MultiPointToSinglePointIntent.class,
                 MultiPointToSinglePointIntent.class},
                 intent5, intent5New).equals(true));
-        assertTrue(!intent5.equals(intent5New));
+        assertFalse(intent5.equals(intent5New));
 
         MultiPointToSinglePointIntent intent6 = intentBuilder(
                 routeEntry6.prefix(), "00:00:00:00:00:01",  SW1_ETH1);
@@ -274,6 +282,8 @@ public class IntentSyncTest {
                 routeEntry5);
         bgpRoutes.put(RouteEntry.createBinaryString(routeEntry6.prefix()),
                 routeEntry6);
+        bgpRoutes.put(RouteEntry.createBinaryString(routeEntry7.prefix()),
+                routeEntry7);
         TestUtils.setField(router, "bgpRoutes", bgpRoutes);
 
         ConcurrentHashMap<IpPrefix, MultiPointToSinglePointIntent>
@@ -283,15 +293,27 @@ public class IntentSyncTest {
         pushedRouteIntents.put(routeEntry4Update.prefix(), intent4Update);
         pushedRouteIntents.put(routeEntry5.prefix(), intent5New);
         pushedRouteIntents.put(routeEntry6.prefix(), intent6);
+        pushedRouteIntents.put(routeEntry7.prefix(), intent7);
         TestUtils.setField(router, "pushedRouteIntents", pushedRouteIntents);
 
         // Set up expectation
         reset(intentService);
         Set<Intent> intents = new HashSet<Intent>();
         intents.add(intent1);
+        expect(intentService.getIntentState(intent1.id()))
+                .andReturn(IntentState.INSTALLED).anyTimes();
         intents.add(intent2);
+        expect(intentService.getIntentState(intent2.id()))
+                .andReturn(IntentState.INSTALLED).anyTimes();
         intents.add(intent4);
+        expect(intentService.getIntentState(intent4.id()))
+                .andReturn(IntentState.INSTALLED).anyTimes();
         intents.add(intent5);
+        expect(intentService.getIntentState(intent5.id()))
+                .andReturn(IntentState.INSTALLED).anyTimes();
+        intents.add(intent7);
+        expect(intentService.getIntentState(intent7.id()))
+                .andReturn(IntentState.WITHDRAWING).anyTimes();
         expect(intentService.getIntents()).andReturn(intents).anyTimes();
 
         intentService.withdraw(intent2);
@@ -299,6 +321,7 @@ public class IntentSyncTest {
         intentService.withdraw(intent4);
         intentService.submit(intent4Update);
         intentService.submit(intent6);
+        intentService.submit(intent7);
         replay(intentService);
 
         // Start the test
@@ -306,14 +329,14 @@ public class IntentSyncTest {
         TestUtils.callMethod(router, "syncIntents", new Class<?>[] {});
 
         // Verify
-        assertEquals(router.getRoutes().size(), 5);
+        assertEquals(router.getRoutes().size(), 6);
         assertTrue(router.getRoutes().contains(routeEntry1));
         assertTrue(router.getRoutes().contains(routeEntry3));
         assertTrue(router.getRoutes().contains(routeEntry4Update));
         assertTrue(router.getRoutes().contains(routeEntry5));
         assertTrue(router.getRoutes().contains(routeEntry6));
 
-        assertEquals(router.getPushedRouteIntents().size(), 5);
+        assertEquals(router.getPushedRouteIntents().size(), 6);
         assertTrue(router.getPushedRouteIntents().contains(intent1));
         assertTrue(router.getPushedRouteIntents().contains(intent3));
         assertTrue(router.getPushedRouteIntents().contains(intent4Update));