Updates for SDN-IP:
* Use the new Leadership Service instead of Distributed Lock to
elect the SDN-IP Leader
* Reimplement the SDN-IP Intent Synchronizer. In the new implementation
the Point-to-Point Peer intents are also synchronized by and pushed
only by the Leader (same as the Multipoint-to-SinglePoint Route intents)
* Minor cleanups
Change-Id: I8e142781211a1d0f2d362875bc28fd05d843cd4b
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 4b29ba8..3d9cd23 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
@@ -5,16 +5,23 @@
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.expectLastCall;
import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.reportMatcher;
import static org.easymock.EasyMock.reset;
import static org.easymock.EasyMock.verify;
+import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.commons.collections4.CollectionUtils;
+import org.easymock.IArgumentMatcher;
import org.junit.Before;
import org.junit.Test;
import org.onlab.junit.TestUtils;
@@ -35,10 +42,14 @@
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.IntentId;
+import org.onlab.onos.net.intent.IntentOperation;
+import org.onlab.onos.net.intent.IntentOperations;
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.IntentSynchronizer.IntentKey;
import org.onlab.onos.sdnip.config.Interface;
import org.onlab.packet.Ethernet;
import org.onlab.packet.IpAddress;
@@ -97,8 +108,8 @@
intentService = createMock(IntentService.class);
intentSynchronizer = new IntentSynchronizer(APPID, intentService);
- router = new Router(APPID, intentSynchronizer,
- hostService, null, interfaceService);
+ router = new Router(APPID, intentSynchronizer, null, interfaceService,
+ hostService);
}
/**
@@ -263,17 +274,16 @@
// Compose a intent, which is equal to intent5 but the id is different.
MultiPointToSinglePointIntent intent5New =
staticIntentBuilder(intent5, routeEntry5, "00:00:00:00:00:01");
- assertTrue(TestUtils.callMethod(intentSynchronizer,
- "compareMultiPointToSinglePointIntents",
- new Class<?>[] {MultiPointToSinglePointIntent.class,
- MultiPointToSinglePointIntent.class},
- intent5, intent5New).equals(true));
+ assertThat(IntentSynchronizer.IntentKey.equalIntents(
+ intent5, intent5New),
+ is(true));
assertFalse(intent5.equals(intent5New));
MultiPointToSinglePointIntent intent6 = intentBuilder(
routeEntry6.prefix(), "00:00:00:00:00:01", SW1_ETH1);
- // Set up the bgpRoutes and pushedRouteIntents fields in Router class
+ // Set up the bgpRoutes field in Router class and routeIntents fields
+ // in IntentSynchronizer class
InvertedRadixTree<RouteEntry> bgpRoutes =
new ConcurrentInvertedRadixTree<>(
new DefaultByteArrayNodeFactory());
@@ -292,15 +302,14 @@
TestUtils.setField(router, "bgpRoutes", bgpRoutes);
ConcurrentHashMap<Ip4Prefix, MultiPointToSinglePointIntent>
- pushedRouteIntents = new ConcurrentHashMap<>();
- pushedRouteIntents.put(routeEntry1.prefix(), intent1);
- pushedRouteIntents.put(routeEntry3.prefix(), intent3);
- pushedRouteIntents.put(routeEntry4Update.prefix(), intent4Update);
- pushedRouteIntents.put(routeEntry5.prefix(), intent5New);
- pushedRouteIntents.put(routeEntry6.prefix(), intent6);
- pushedRouteIntents.put(routeEntry7.prefix(), intent7);
- TestUtils.setField(intentSynchronizer, "pushedRouteIntents",
- pushedRouteIntents);
+ routeIntents = new ConcurrentHashMap<>();
+ routeIntents.put(routeEntry1.prefix(), intent1);
+ routeIntents.put(routeEntry3.prefix(), intent3);
+ routeIntents.put(routeEntry4Update.prefix(), intent4Update);
+ routeIntents.put(routeEntry5.prefix(), intent5New);
+ routeIntents.put(routeEntry6.prefix(), intent6);
+ routeIntents.put(routeEntry7.prefix(), intent7);
+ TestUtils.setField(intentSynchronizer, "routeIntents", routeIntents);
// Set up expectation
reset(intentService);
@@ -322,18 +331,26 @@
.andReturn(IntentState.WITHDRAWING).anyTimes();
expect(intentService.getIntents()).andReturn(intents).anyTimes();
- intentService.withdraw(intent2);
- intentService.submit(intent3);
- intentService.withdraw(intent4);
- intentService.submit(intent4Update);
- intentService.submit(intent6);
- intentService.submit(intent7);
+ IntentOperations.Builder builder = IntentOperations.builder();
+ builder.addWithdrawOperation(intent2.id());
+ builder.addWithdrawOperation(intent4.id());
+ intentService.execute(eqExceptId(builder.build()));
+
+ builder = IntentOperations.builder();
+ builder.addSubmitOperation(intent3);
+ builder.addSubmitOperation(intent4Update);
+ builder.addSubmitOperation(intent6);
+ builder.addSubmitOperation(intent7);
+ intentService.execute(eqExceptId(builder.build()));
replay(intentService);
// Start the test
intentSynchronizer.leaderChanged(true);
- TestUtils.callMethod(intentSynchronizer, "syncIntents",
+ /*
+ TestUtils.callMethod(intentSynchronizer, "synchronizeIntents",
new Class<?>[] {});
+ */
+ intentSynchronizer.synchronizeIntents();
// Verify
assertEquals(router.getRoutes().size(), 6);
@@ -343,12 +360,12 @@
assertTrue(router.getRoutes().contains(routeEntry5));
assertTrue(router.getRoutes().contains(routeEntry6));
- assertEquals(intentSynchronizer.getPushedRouteIntents().size(), 6);
- assertTrue(intentSynchronizer.getPushedRouteIntents().contains(intent1));
- assertTrue(intentSynchronizer.getPushedRouteIntents().contains(intent3));
- assertTrue(intentSynchronizer.getPushedRouteIntents().contains(intent4Update));
- assertTrue(intentSynchronizer.getPushedRouteIntents().contains(intent5));
- assertTrue(intentSynchronizer.getPushedRouteIntents().contains(intent6));
+ assertEquals(intentSynchronizer.getRouteIntents().size(), 6);
+ assertTrue(intentSynchronizer.getRouteIntents().contains(intent1));
+ assertTrue(intentSynchronizer.getRouteIntents().contains(intent3));
+ assertTrue(intentSynchronizer.getRouteIntents().contains(intent4Update));
+ assertTrue(intentSynchronizer.getRouteIntents().contains(intent5));
+ assertTrue(intentSynchronizer.getRouteIntents().contains(intent6));
verify(intentService);
}
@@ -410,4 +427,129 @@
"ingressPoints", intent.ingressPoints());
return intentNew;
}
+
+ /*
+ * EasyMock matcher that matches {@link IntenOperations} but
+ * ignores the {@link IntentId} when matching.
+ * <p/>
+ * The normal intent equals method tests that the intent IDs are equal,
+ * however in these tests we can't know what the intent IDs will be in
+ * advance, so we can't set up expected intents with the correct IDs. Thus,
+ * the solution is to use an EasyMock matcher that verifies that all the
+ * value properties of the provided intent match the expected values, but
+ * ignores the intent ID when testing equality.
+ */
+ private static final class IdAgnosticIntentOperationsMatcher implements
+ IArgumentMatcher {
+
+ private final IntentOperations intentOperations;
+ private String providedString;
+
+ /**
+ * Constructor taking the expected intent operations to match against.
+ *
+ * @param intentOperations the expected intent operations
+ */
+ public IdAgnosticIntentOperationsMatcher(
+ IntentOperations intentOperations) {
+ this.intentOperations = intentOperations;
+ }
+
+ @Override
+ public void appendTo(StringBuffer strBuffer) {
+ strBuffer.append("IntentOperationsMatcher unable to match: "
+ + providedString);
+ }
+
+ @Override
+ public boolean matches(Object object) {
+ if (!(object instanceof IntentOperations)) {
+ return false;
+ }
+
+ IntentOperations providedIntentOperations =
+ (IntentOperations) object;
+ providedString = providedIntentOperations.toString();
+
+ List<IntentKey> thisSubmitIntents = new LinkedList<>();
+ List<IntentId> thisWithdrawIntentIds = new LinkedList<>();
+ List<IntentKey> thisReplaceIntents = new LinkedList<>();
+ List<IntentKey> thisUpdateIntents = new LinkedList<>();
+ List<IntentKey> providedSubmitIntents = new LinkedList<>();
+ List<IntentId> providedWithdrawIntentIds = new LinkedList<>();
+ List<IntentKey> providedReplaceIntents = new LinkedList<>();
+ List<IntentKey> providedUpdateIntents = new LinkedList<>();
+
+ extractIntents(intentOperations, thisSubmitIntents,
+ thisWithdrawIntentIds, thisReplaceIntents,
+ thisUpdateIntents);
+ extractIntents(providedIntentOperations, providedSubmitIntents,
+ providedWithdrawIntentIds, providedReplaceIntents,
+ providedUpdateIntents);
+
+ return CollectionUtils.isEqualCollection(thisSubmitIntents,
+ providedSubmitIntents) &&
+ CollectionUtils.isEqualCollection(thisWithdrawIntentIds,
+ providedWithdrawIntentIds) &&
+ CollectionUtils.isEqualCollection(thisUpdateIntents,
+ providedUpdateIntents) &&
+ CollectionUtils.isEqualCollection(thisReplaceIntents,
+ providedReplaceIntents);
+ }
+
+ /**
+ * Extracts the intents per operation type. Each intent is encapsulated
+ * in IntentKey so it can be compared by excluding the Intent ID.
+ *
+ * @param intentOperations the container with the intent operations
+ * to extract the intents from
+ * @param submitIntents the SUBMIT intents
+ * @param withdrawIntentIds the WITHDRAW intents IDs
+ * @param replaceIntents the REPLACE intents
+ * @param updateIntents the UPDATE intens
+ */
+ private void extractIntents(IntentOperations intentOperations,
+ List<IntentKey> submitIntents,
+ List<IntentId> withdrawIntentIds,
+ List<IntentKey> replaceIntents,
+ List<IntentKey> updateIntents) {
+ for (IntentOperation oper : intentOperations.operations()) {
+ IntentId intentId;
+ IntentKey intentKey;
+ switch (oper.type()) {
+ case SUBMIT:
+ intentKey = new IntentKey(oper.intent());
+ submitIntents.add(intentKey);
+ break;
+ case WITHDRAW:
+ intentId = oper.intentId();
+ withdrawIntentIds.add(intentId);
+ break;
+ case REPLACE:
+ intentKey = new IntentKey(oper.intent());
+ replaceIntents.add(intentKey);
+ break;
+ case UPDATE:
+ intentKey = new IntentKey(oper.intent());
+ updateIntents.add(intentKey);
+ break;
+ default:
+ break;
+ }
+ }
+ }
+ }
+
+ /**
+ * Matcher method to set an expected intent to match against (ignoring the
+ * the intent ID).
+ *
+ * @param intent the expected intent
+ * @return something of type IntentOperations
+ */
+ private static IntentOperations eqExceptId(
+ IntentOperations intentOperations) {
+ reportMatcher(new IdAgnosticIntentOperationsMatcher(intentOperations));
+ return intentOperations;
+ }
}
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java
index d21bb2e..b7cdbc0 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java
@@ -20,6 +20,8 @@
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
+import org.onlab.junit.TestUtils;
+import org.onlab.junit.TestUtils.TestUtilsException;
import org.onlab.onos.core.ApplicationId;
import org.onlab.onos.net.ConnectPoint;
import org.onlab.onos.net.DeviceId;
@@ -70,9 +72,10 @@
};
private PeerConnectivityManager peerConnectivityManager;
- private IntentService intentService;
+ private IntentSynchronizer intentSynchronizer;
private SdnIpConfigService configInfoService;
private InterfaceService interfaceService;
+ private IntentService intentService;
private Map<String, BgpSpeaker> bgpSpeakers;
private Map<String, Interface> interfaces;
@@ -525,8 +528,10 @@
/**
* Initializes peer connectivity testing environment.
+ *
+ * @throws TestUtilsException if exceptions when using TestUtils
*/
- private void initPeerConnectivity() {
+ private void initPeerConnectivity() throws TestUtilsException {
configInfoService = createMock(SdnIpConfigService.class);
expect(configInfoService.getBgpPeers()).andReturn(peers).anyTimes();
@@ -536,8 +541,13 @@
intentService = createMock(IntentService.class);
replay(intentService);
- peerConnectivityManager = new PeerConnectivityManager(APPID, configInfoService,
- interfaceService, intentService);
+ intentSynchronizer = new IntentSynchronizer(APPID, intentService);
+ intentSynchronizer.leaderChanged(true);
+ TestUtils.setField(intentSynchronizer, "isActivatedLeader", true);
+
+ peerConnectivityManager =
+ new PeerConnectivityManager(APPID, intentSynchronizer,
+ configInfoService, interfaceService);
}
/*
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTest.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTest.java
index b5beb4a..db21d65 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTest.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTest.java
@@ -115,8 +115,8 @@
intentService = createMock(IntentService.class);
intentSynchronizer = new IntentSynchronizer(APPID, intentService);
- router = new Router(APPID, intentSynchronizer,
- hostService, sdnIpConfigService, interfaceService);
+ router = new Router(APPID, intentSynchronizer, sdnIpConfigService,
+ interfaceService, hostService);
}
/**
@@ -267,8 +267,8 @@
// Verify
assertEquals(router.getRoutes().size(), 1);
assertTrue(router.getRoutes().contains(routeEntry));
- assertEquals(intentSynchronizer.getPushedRouteIntents().size(), 1);
- assertEquals(intentSynchronizer.getPushedRouteIntents().iterator().next(),
+ assertEquals(intentSynchronizer.getRouteIntents().size(), 1);
+ assertEquals(intentSynchronizer.getRouteIntents().iterator().next(),
intent);
verify(intentService);
}
@@ -347,8 +347,8 @@
// Verify
assertEquals(router.getRoutes().size(), 1);
assertTrue(router.getRoutes().contains(routeEntryUpdate));
- assertEquals(intentSynchronizer.getPushedRouteIntents().size(), 1);
- assertEquals(intentSynchronizer.getPushedRouteIntents().iterator().next(),
+ assertEquals(intentSynchronizer.getRouteIntents().size(), 1);
+ assertEquals(intentSynchronizer.getRouteIntents().iterator().next(),
intentNew);
verify(intentService);
}
@@ -397,7 +397,7 @@
// Verify
assertEquals(router.getRoutes().size(), 0);
- assertEquals(intentSynchronizer.getPushedRouteIntents().size(), 0);
+ assertEquals(intentSynchronizer.getRouteIntents().size(), 0);
verify(intentService);
}
@@ -425,7 +425,7 @@
// Verify
assertEquals(router.getRoutes().size(), 1);
assertTrue(router.getRoutes().contains(routeEntry));
- assertEquals(intentSynchronizer.getPushedRouteIntents().size(), 0);
+ assertEquals(intentSynchronizer.getRouteIntents().size(), 0);
verify(intentService);
}
}
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTestWithAsyncArp.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTestWithAsyncArp.java
index 3f147cc..b4389c8 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTestWithAsyncArp.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTestWithAsyncArp.java
@@ -117,7 +117,7 @@
intentSynchronizer = new IntentSynchronizer(APPID, intentService);
router = new Router(APPID, intentSynchronizer,
- hostService, sdnIpConfigService, interfaceService);
+ sdnIpConfigService, interfaceService, hostService);
internalHostListener = router.new InternalHostListener();
}
@@ -229,8 +229,8 @@
// Verify
assertEquals(router.getRoutes().size(), 1);
assertTrue(router.getRoutes().contains(routeEntry));
- assertEquals(intentSynchronizer.getPushedRouteIntents().size(), 1);
- assertEquals(intentSynchronizer.getPushedRouteIntents().iterator().next(),
+ assertEquals(intentSynchronizer.getRouteIntents().size(), 1);
+ assertEquals(intentSynchronizer.getRouteIntents().iterator().next(),
intent);
verify(intentService);
verify(hostService);
@@ -254,9 +254,9 @@
MultiPointToSinglePointIntent intent = staticIntentBuilder();
// Set up the bgpRoutes field of Router class with existing route, and
- // pushedRouteIntents field with the corresponding existing intent
+ // routeIntents field with the corresponding existing intent
setBgpRoutesField(routeEntry);
- setPushedRouteIntentsField(routeEntry, intent);
+ setRouteIntentsField(routeEntry, intent);
// Start to construct a new route entry and new intent
RouteEntry routeEntryUpdate = new RouteEntry(
@@ -312,8 +312,8 @@
// Verify
assertEquals(router.getRoutes().size(), 1);
assertTrue(router.getRoutes().contains(routeEntryUpdate));
- assertEquals(intentSynchronizer.getPushedRouteIntents().size(), 1);
- assertEquals(intentSynchronizer.getPushedRouteIntents().iterator().next(),
+ assertEquals(intentSynchronizer.getRouteIntents().size(), 1);
+ assertEquals(intentSynchronizer.getRouteIntents().iterator().next(),
intentNew);
verify(intentService);
verify(hostService);
@@ -334,9 +334,9 @@
MultiPointToSinglePointIntent intent = staticIntentBuilder();
// Set up the bgpRoutes field of Router class with existing route, and
- // pushedRouteIntents field with the corresponding existing intent
+ // routeIntents field with the corresponding existing intent
setBgpRoutesField(routeEntry);
- setPushedRouteIntentsField(routeEntry, intent);
+ setRouteIntentsField(routeEntry, intent);
// Set up expectation
reset(intentService);
@@ -350,7 +350,7 @@
// Verify
assertEquals(router.getRoutes().size(), 0);
- assertEquals(intentSynchronizer.getPushedRouteIntents().size(), 0);
+ assertEquals(intentSynchronizer.getRouteIntents().size(), 0);
verify(intentService);
}
@@ -397,17 +397,17 @@
}
/**
- * Sets pushedRouteIntentsField in Router class.
+ * Sets routeIntentsField in IntentSynchronizer class.
*
* @throws TestUtilsException
*/
- private void setPushedRouteIntentsField(RouteEntry routeEntry,
+ private void setRouteIntentsField(RouteEntry routeEntry,
MultiPointToSinglePointIntent intent)
throws TestUtilsException {
ConcurrentHashMap<Ip4Prefix, MultiPointToSinglePointIntent>
- pushedRouteIntents = new ConcurrentHashMap<>();
- pushedRouteIntents.put(routeEntry.prefix(), intent);
- TestUtils.setField(router, "pushedRouteIntents", pushedRouteIntents);
+ routeIntents = new ConcurrentHashMap<>();
+ routeIntents.put(routeEntry.prefix(), intent);
+ TestUtils.setField(intentSynchronizer, "routeIntents", routeIntents);
}
}
\ No newline at end of file
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/SdnIpTest.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/SdnIpTest.java
index 0cf706a..f83f846 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/SdnIpTest.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/SdnIpTest.java
@@ -113,8 +113,8 @@
random = new Random();
intentSynchronizer = new IntentSynchronizer(APPID, intentService);
- router = new Router(APPID, intentSynchronizer, hostService,
- sdnIpConfigService, interfaceService);
+ router = new Router(APPID, intentSynchronizer, sdnIpConfigService,
+ interfaceService, hostService);
}
/**
@@ -241,7 +241,7 @@
latch.await(5000, TimeUnit.MILLISECONDS);
assertEquals(router.getRoutes().size(), numRoutes);
- assertEquals(intentSynchronizer.getPushedRouteIntents().size(),
+ assertEquals(intentSynchronizer.getRouteIntents().size(),
numRoutes);
verify(intentService);
@@ -317,7 +317,7 @@
deleteCount.await(5000, TimeUnit.MILLISECONDS);
assertEquals(0, router.getRoutes().size());
- assertEquals(0, intentSynchronizer.getPushedRouteIntents().size());
+ assertEquals(0, intentSynchronizer.getRouteIntents().size());
verify(intentService);
}