Small cleanups for vRouter app
Change-Id: Ibee46d3b95ee76dd3547e11d046c4620b3b3306d
diff --git a/apps/routing/src/main/java/org/onosproject/routing/impl/ControlPlaneRedirectManager.java b/apps/routing/src/main/java/org/onosproject/routing/impl/ControlPlaneRedirectManager.java
index a82d4ff..4430f5e 100644
--- a/apps/routing/src/main/java/org/onosproject/routing/impl/ControlPlaneRedirectManager.java
+++ b/apps/routing/src/main/java/org/onosproject/routing/impl/ControlPlaneRedirectManager.java
@@ -16,16 +16,8 @@
package org.onosproject.routing.impl;
-import static com.google.common.base.Preconditions.checkState;
-import static org.slf4j.LoggerFactory.getLogger;
-
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Set;
-
+import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.Maps;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
@@ -69,8 +61,15 @@
import org.onosproject.routing.config.RouterConfig;
import org.slf4j.Logger;
-import com.google.common.collect.ImmutableSortedSet;
-import com.google.common.collect.Maps;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+import static com.google.common.base.Preconditions.checkState;
+import static org.slf4j.LoggerFactory.getLogger;
/**
* Manages connectivity between peers redirecting control traffic to a routing
@@ -183,7 +182,7 @@
updateOspfForwarding(intf, true);
}
/**
- * Install or removes the basic forwarding flows for each interface
+ * Installs or removes the basic forwarding flows for each interface
* based on the flag used.
*
* @param intf the Interface on which event is received
@@ -266,9 +265,9 @@
}
/**
- * Install or removes ospf forwarding rules.
+ * Installs or removes OSPF forwarding rules.
*
- * @param intf the Interface on which event is received
+ * @param intf the interface on which event is received
* @param install true to create an add objective, false to create a remove
* objective
**/
@@ -293,7 +292,7 @@
cpNextId = modifyNextObjective(deviceId, controlPlanePort,
intf.vlan(), false, install);
}
- log.debug("ospf flows intf:{} nextid:{}", intf, cpNextId);
+ log.debug("OSPF flows intf:{} nextid:{}", intf, cpNextId);
flowObjectiveService.forward(controlPlaneConnectPoint.deviceId(),
buildForwardingObjective(toSelector, null, cpNextId, install ? ospfEnabled : install));
}
@@ -331,7 +330,7 @@
nextObjBuilder.withMeta(metabuilder.build());
nextObjBuilder.addTreatment(ttBuilder.build());
- log.debug("Submited next objective {} in device {} for port/vlan {}/{}",
+ log.debug("Submitted next objective {} in device {} for port/vlan {}/{}",
nextId, deviceId, portNumber, vlanId);
if (install) {
flowObjectiveService.next(deviceId, nextObjBuilder.add());
@@ -385,7 +384,6 @@
log.info("Device connected {}", event.subject().id());
updateDevice();
}
-
break;
case DEVICE_UPDATED:
case DEVICE_REMOVED:
diff --git a/apps/routing/src/main/java/org/onosproject/routing/impl/SingleSwitchFibInstaller.java b/apps/routing/src/main/java/org/onosproject/routing/impl/SingleSwitchFibInstaller.java
index b548492..a78e9b4 100644
--- a/apps/routing/src/main/java/org/onosproject/routing/impl/SingleSwitchFibInstaller.java
+++ b/apps/routing/src/main/java/org/onosproject/routing/impl/SingleSwitchFibInstaller.java
@@ -60,9 +60,8 @@
import org.onosproject.net.flowobjective.FlowObjectiveService;
import org.onosproject.net.flowobjective.ForwardingObjective;
import org.onosproject.net.flowobjective.NextObjective;
-import org.onosproject.net.flowobjective.Objective;
import org.onosproject.net.flowobjective.ObjectiveContext;
-import org.onosproject.net.flowobjective.ObjectiveError;
+import org.onosproject.net.flowobjective.DefaultObjectiveContext;
import org.onosproject.routing.FibEntry;
import org.onosproject.routing.FibListener;
import org.onosproject.routing.FibUpdate;
@@ -442,43 +441,15 @@
private void sendFilteringObjective(boolean install, FilteringObjective.Builder fob,
Interface intf) {
- if (install) {
- flowObjectiveService.filter(
- deviceId,
- fob.add(new ObjectiveContext() {
- @Override
- public void onSuccess(Objective objective) {
- log.info("Successfully installed interface based "
- + "filtering objectives for intf {}", intf);
- }
- @Override
- public void onError(Objective objective,
- ObjectiveError error) {
- log.error("Failed to install interface filters for intf {}: {}",
- intf, error);
- // TODO something more than just logging
- }
- }));
- } else {
- flowObjectiveService.filter(
- deviceId,
- fob.remove(new ObjectiveContext() {
- @Override
- public void onSuccess(Objective objective) {
- log.info("Successfully removed interface based "
- + "filtering objectives for intf {}", intf);
- }
+ ObjectiveContext context = new DefaultObjectiveContext(
+ (objective) -> log.info("Installed filter for interface {}", intf),
+ (objective, error) ->
+ log.error("Failed to install filter for interface {}: {}", intf, error));
- @Override
- public void onError(Objective objective,
- ObjectiveError error) {
- log.error("Failed to install interface filters for intf {}: {}",
- intf, error);
- // TODO something more than just logging
- }
- }));
- }
+ FilteringObjective filter = install ? fob.add(context) : fob.remove(context);
+
+ flowObjectiveService.filter(deviceId, filter);
}
private class InternalFibListener implements FibListener {
diff --git a/apps/routing/src/test/java/org/onosproject/routing/impl/ControlPlaneRedirectManagerTest.java b/apps/routing/src/test/java/org/onosproject/routing/impl/ControlPlaneRedirectManagerTest.java
index f305563..db71982 100644
--- a/apps/routing/src/test/java/org/onosproject/routing/impl/ControlPlaneRedirectManagerTest.java
+++ b/apps/routing/src/test/java/org/onosproject/routing/impl/ControlPlaneRedirectManagerTest.java
@@ -15,19 +15,7 @@
*/
package org.onosproject.routing.impl;
-import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.expectLastCall;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.verify;
-import static org.slf4j.LoggerFactory.getLogger;
-
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Objects;
-import java.util.Set;
-
+import com.google.common.collect.Sets;
import org.easymock.EasyMock;
import org.junit.Before;
import org.junit.Test;
@@ -70,23 +58,29 @@
import org.onosproject.net.flowobjective.FlowObjectiveService;
import org.onosproject.net.flowobjective.ForwardingObjective;
import org.onosproject.net.flowobjective.NextObjective;
-import org.onosproject.net.host.HostListener;
import org.onosproject.net.host.HostService;
-import org.onosproject.net.host.HostServiceAdapter;
import org.onosproject.net.host.InterfaceIpAddress;
-import org.onosproject.net.intent.AbstractIntentTest;
import org.onosproject.routing.RoutingService;
import org.onosproject.routing.config.RouterConfig;
-import org.slf4j.Logger;
-import com.google.common.collect.Sets;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
/**
* UnitTests for ControlPlaneRedirectManager.
*/
-public class ControlPlaneRedirectManagerTest extends AbstractIntentTest {
+public class ControlPlaneRedirectManagerTest {
- private final Logger log = getLogger(getClass());
private DeviceService deviceService;
private FlowObjectiveService flowObjectiveService;
private NetworkConfigService networkConfigService;
@@ -97,39 +91,34 @@
private InterfaceService interfaceService;
private static final ApplicationId APPID = TestApplicationId.create("org.onosproject.cpredirect");
- /**
- * Interface Configuration.
- *
- **/
- private ConnectPoint controlPlaneConnectPoint = new ConnectPoint(DeviceId.deviceId("of:0000000000000001"),
- PortNumber.portNumber(1));
- private static final ConnectPoint SW1_ETH1 = new ConnectPoint(DeviceId.deviceId("of:0000000000000001"),
+ private static final DeviceId DEVICE_ID = DeviceId.deviceId("of:0000000000000001");
+
+ private ConnectPoint controlPlaneConnectPoint = new ConnectPoint(DEVICE_ID,
PortNumber.portNumber(1));
- private static final ConnectPoint SW1_ETH2 = new ConnectPoint(DeviceId.deviceId("of:0000000000000001"),
+ private static final ConnectPoint SW1_ETH1 = new ConnectPoint(DEVICE_ID,
+ PortNumber.portNumber(1));
+
+ private static final ConnectPoint SW1_ETH2 = new ConnectPoint(DEVICE_ID,
PortNumber.portNumber(2));
- private static final ConnectPoint SW1_ETH3 = new ConnectPoint(DeviceId.deviceId("of:0000000000000001"),
+ private static final ConnectPoint SW1_ETH3 = new ConnectPoint(DEVICE_ID,
PortNumber.portNumber(3));
- protected HostService hostService;
- private ControlPlaneRedirectManager controlPlaneRedirectManager = new ControlPlaneRedirectManager();;
+ private ControlPlaneRedirectManager controlPlaneRedirectManager = new ControlPlaneRedirectManager();
private RouterConfig routerConfig = new TestRouterConfig();
private NetworkConfigListener networkConfigListener;
private DeviceListener deviceListener;
private MastershipService mastershipService = new InternalMastershipServiceTest();
- private HostListener hostListener;
private InterfaceListener interfaceListener;
- @Override
+
@Before
public void setUp() {
networkConfigListener = createMock(NetworkConfigListener.class);
- hostService = new TestHostService();
deviceService = new TestDeviceService();
deviceListener = createMock(DeviceListener.class);
- hostListener = createMock(HostListener.class);
+
interfaceListener = createMock(InterfaceListener.class);
- hostService.addListener(hostListener);
deviceService.addListener(deviceListener);
setUpInterfaceService();
interfaceService = new InternalInterfaceService();
@@ -143,7 +132,7 @@
controlPlaneRedirectManager.networkConfigService = networkConfigService;
controlPlaneRedirectManager.interfaceService = interfaceService;
controlPlaneRedirectManager.deviceService = deviceService;
- controlPlaneRedirectManager.hostService = hostService;
+ controlPlaneRedirectManager.hostService = createNiceMock(HostService.class);
controlPlaneRedirectManager.mastershipService = mastershipService;
controlPlaneRedirectManager.activate();
verify(flowObjectiveService);
@@ -154,7 +143,7 @@
*/
@Test
public void testAddDevice() {
- ConnectPoint sw1eth4 = new ConnectPoint(DeviceId.deviceId("of:0000000000000001"), PortNumber.portNumber(4));
+ ConnectPoint sw1eth4 = new ConnectPoint(DEVICE_ID, PortNumber.portNumber(4));
Set<InterfaceIpAddress> interfaceIpAddresses4 = Sets.newHashSet();
interfaceIpAddresses4
.add(new InterfaceIpAddress(IpAddress.valueOf("192.168.40.101"), IpPrefix.valueOf("192.168.40.0/24")));
@@ -173,7 +162,7 @@
*/
@Test
public void testUpdateNetworkConfig() {
- ConnectPoint sw1eth4 = new ConnectPoint(DeviceId.deviceId("of:0000000000000001"), PortNumber.portNumber(4));
+ ConnectPoint sw1eth4 = new ConnectPoint(DEVICE_ID, PortNumber.portNumber(4));
Set<InterfaceIpAddress> interfaceIpAddresses4 = Sets.newHashSet();
interfaceIpAddresses4
.add(new InterfaceIpAddress(IpAddress.valueOf("192.168.40.101"), IpPrefix.valueOf("192.168.40.0/24")));
@@ -194,7 +183,7 @@
*/
@Test
public void testAddInterface() {
- ConnectPoint sw1eth4 = new ConnectPoint(DeviceId.deviceId("of:0000000000000001"), PortNumber.portNumber(4));
+ ConnectPoint sw1eth4 = new ConnectPoint(DEVICE_ID, PortNumber.portNumber(4));
Set<InterfaceIpAddress> interfaceIpAddresses4 = Sets.newHashSet();
interfaceIpAddresses4
.add(new InterfaceIpAddress(IpAddress.valueOf("192.168.40.101"), IpPrefix.valueOf("192.168.40.0/24")));
@@ -208,14 +197,13 @@
setUpInterfaceConfiguration(sw1Eth4, true);
replay(flowObjectiveService);
- interfaceListener.event(new InterfaceEvent(
- org.onosproject.incubator.net.intf.InterfaceEvent.Type.INTERFACE_ADDED, sw1Eth4, 500L));
+ interfaceListener.event(new InterfaceEvent(InterfaceEvent.Type.INTERFACE_ADDED, sw1Eth4, 500L));
verify(flowObjectiveService);
}
@Test
public void testRemoveInterface() {
- ConnectPoint sw1eth4 = new ConnectPoint(DeviceId.deviceId("of:0000000000000001"), PortNumber.portNumber(4));
+ ConnectPoint sw1eth4 = new ConnectPoint(DEVICE_ID, PortNumber.portNumber(4));
Set<InterfaceIpAddress> interfaceIpAddresses4 = Sets.newHashSet();
interfaceIpAddresses4
.add(new InterfaceIpAddress(IpAddress.valueOf("192.168.40.101"), IpPrefix.valueOf("192.168.40.0/24")));
@@ -227,8 +215,7 @@
setUpInterfaceConfiguration(sw1Eth4, false);
replay(flowObjectiveService);
- interfaceListener.event(new InterfaceEvent(
- org.onosproject.incubator.net.intf.InterfaceEvent.Type.INTERFACE_REMOVED, sw1Eth4, 500L));
+ interfaceListener.event(new InterfaceEvent(InterfaceEvent.Type.INTERFACE_REMOVED, sw1Eth4, 500L));
verify(flowObjectiveService);
}
@@ -407,15 +394,6 @@
}
- private class TestHostService extends HostServiceAdapter {
-
- @Override
- public void addListener(HostListener listener) {
- ControlPlaneRedirectManagerTest.this.hostListener = listener;
- }
-
- }
-
private class TestRouterConfig extends RouterConfig {
@Override
diff --git a/apps/routing/src/test/java/org/onosproject/routing/impl/SingleSwitchFibInstallerTest.java b/apps/routing/src/test/java/org/onosproject/routing/impl/SingleSwitchFibInstallerTest.java
index e8b15f9..4a2d041 100644
--- a/apps/routing/src/test/java/org/onosproject/routing/impl/SingleSwitchFibInstallerTest.java
+++ b/apps/routing/src/test/java/org/onosproject/routing/impl/SingleSwitchFibInstallerTest.java
@@ -15,20 +15,8 @@
*/
package org.onosproject.routing.impl;
-import static org.easymock.EasyMock.anyObject;
-import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.reset;
-import static org.easymock.EasyMock.verify;
-
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Dictionary;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import org.easymock.EasyMock;
import org.junit.Before;
import org.junit.Test;
@@ -40,11 +28,10 @@
import org.onlab.packet.MacAddress;
import org.onlab.packet.VlanId;
import org.onosproject.TestApplicationId;
+import org.onosproject.cfg.ComponentConfigService;
import org.onosproject.core.ApplicationId;
import org.onosproject.core.CoreService;
import org.onosproject.core.CoreServiceAdapter;
-import org.osgi.service.component.ComponentContext;
-import org.onosproject.cfg.ComponentConfigService;
import org.onosproject.incubator.net.intf.Interface;
import org.onosproject.incubator.net.intf.InterfaceListener;
import org.onosproject.incubator.net.intf.InterfaceService;
@@ -73,8 +60,21 @@
import org.onosproject.routing.RoutingService;
import org.onosproject.routing.RoutingServiceAdapter;
import org.onosproject.routing.config.RouterConfig;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
+import org.osgi.service.component.ComponentContext;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Dictionary;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.reset;
+import static org.easymock.EasyMock.verify;
/**
* Unit tests for SingleSwitchFibInstaller.
@@ -198,7 +198,7 @@
/*
* Sets up NetworkConfigService.
- */
+ */
private void setUpNetworkConfigService() {
ApplicationId routerAppId = coreService.registerApplication(RoutingService.ROUTER_APP_ID);
expect(networkConfigService.getConfig(routerAppId, RoutingService.ROUTER_CONFIG_CLASS)).
@@ -423,13 +423,12 @@
verify(flowObjectiveService);
}
- /**
+ /**
* Tests deleting a FIB entry.
*
* We verify that the flowObjectiveService records the correct state and that the
* correct flow is withdrawn from the flowObjectiveService.
*/
-
@Test
public void testFibDelete() {
// Firstly add a route
diff --git a/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultObjectiveContext.java b/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultObjectiveContext.java
new file mode 100644
index 0000000..533f761
--- /dev/null
+++ b/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultObjectiveContext.java
@@ -0,0 +1,75 @@
+/*
+ * Copyright 2016 Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.net.flowobjective;
+
+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
+
+/**
+ * Implementation of objective context that delegates calls to provided
+ * consumers.
+ */
+public class DefaultObjectiveContext implements ObjectiveContext {
+
+ private final Consumer<Objective> onSuccess;
+ private final BiConsumer<Objective, ObjectiveError> onError;
+
+ /**
+ * Creates a new objective context using the given success and error
+ * consumers.
+ *
+ * @param onSuccess consumer to be called on success
+ * @param onError consumer to be called on error
+ */
+ public DefaultObjectiveContext(Consumer<Objective> onSuccess,
+ BiConsumer<Objective, ObjectiveError> onError) {
+ this.onSuccess = onSuccess;
+ this.onError = onError;
+ }
+
+ /**
+ * Creates a new objective context using the given success consumer.
+ *
+ * @param onSuccess consumer to be called on success
+ */
+ public DefaultObjectiveContext(Consumer<Objective> onSuccess) {
+ this(onSuccess, null);
+ }
+
+ /**
+ * Creates a new objective context using the given error consumer.
+ *
+ * @param onError consumer to be called on error
+ */
+ public DefaultObjectiveContext(BiConsumer<Objective, ObjectiveError> onError) {
+ this(null, onError);
+ }
+
+ @Override
+ public void onSuccess(Objective objective) {
+ if (onSuccess != null) {
+ onSuccess.accept(objective);
+ }
+ }
+
+ @Override
+ public void onError(Objective objective, ObjectiveError error) {
+ if (onError != null) {
+ onError.accept(objective, error);
+ }
+ }
+}