Fixed a few intent synchronization issues.
Also added a CLI command to test SDN-IP primary switchover.
Change-Id: Id31f79262a2b607f987adad2fdb3eb54eb939fea
diff --git a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/Router.java b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/Router.java
index 1dcd9d3..79491c5 100644
--- a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/Router.java
+++ b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/Router.java
@@ -46,6 +46,7 @@
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 @@
* 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 @@
// 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 @@
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 @@
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()) &&
diff --git a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/SdnIp.java b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/SdnIp.java
index 4b67546..0f6e38a 100644
--- a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/SdnIp.java
+++ b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/SdnIp.java
@@ -77,6 +77,10 @@
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 @@
return router.getRoutes();
}
+ @Override
+ public void modifyPrimary(boolean isPrimary) {
+ router.leaderChanged(isPrimary);
+ }
+
static String dpidToUri(String dpid) {
return "of:" + dpid.replace(":", "");
}
diff --git a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/SdnIpService.java b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/SdnIpService.java
index d0b3434..936b7e8 100644
--- a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/SdnIpService.java
+++ b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/SdnIpService.java
@@ -36,4 +36,12 @@
* @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);
}
diff --git a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/bgp/BgpSession.java b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/bgp/BgpSession.java
index c4a2a0c..a6f3329 100644
--- a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/bgp/BgpSession.java
+++ b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/bgp/BgpSession.java
@@ -1594,7 +1594,7 @@
//
// 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);
diff --git a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/cli/PrimaryChangeCommand.java b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/cli/PrimaryChangeCommand.java
new file mode 100644
index 0000000..5b20238
--- /dev/null
+++ b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/cli/PrimaryChangeCommand.java
@@ -0,0 +1,25 @@
+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);
+ }
+
+}
diff --git a/apps/sdnip/src/main/resources/OSGI-INF/blueprint/shell-config.xml b/apps/sdnip/src/main/resources/OSGI-INF/blueprint/shell-config.xml
index a06342e..773ead6 100644
--- a/apps/sdnip/src/main/resources/OSGI-INF/blueprint/shell-config.xml
+++ b/apps/sdnip/src/main/resources/OSGI-INF/blueprint/shell-config.xml
@@ -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>
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java
index 18cd58a..45b0c90 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java
@@ -8,6 +8,7 @@
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.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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
intentService.withdraw(intent4);
intentService.submit(intent4Update);
intentService.submit(intent6);
+ intentService.submit(intent7);
replay(intentService);
// Start the test
@@ -306,14 +329,14 @@
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));