Adding first fallback provider for flow rule subsystem.

Fixing onos-check-apps. Change-Id: Ic8c2bac4403bb7a49813826262706e857932b6c0

Adding first fallback provider for flow rule subsystem.
Fixing onos-check-apps. Change-Id: Ic8c2bac4403bb7a49813826262706e857932b6c0
Thomas Vachuska · Gerrit Code Review
Commit c4ee7372c50fd0d4e5c1e5caca66e46c1ca5f527 c4ee7372 1 parent 122b4f5a
Showing 9 changed files with 456 additions and 94 deletions
core/api/src/main/java/org/onosproject/net/AbstractProjectableModel.java
core/api/src/main/java/org/onosproject/net/DefaultDevice.java
core/api/src/main/java/org/onosproject/net/flow/FlowRuleProgrammable.java
core/api/src/main/java/org/onosproject/net/flow/FlowRuleProvider.java
core/api/src/main/java/org/onosproject/net/provider/AbstractProviderRegistry.java
core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleDriverProvider.java
core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java
core/net/src/test/java/org/onosproject/net/flow/impl/FlowRuleManagerTest.java
tools/test/bin/onos-check-apps
--- a/core/api/src/main/java/org/onosproject/net/AbstractProjectableModel.java
View file @c4ee737
+++ b/core/api/src/main/java/org/onosproject/net/AbstractProjectableModel.java
View file @c4ee737
@@ -74,7 +74,7 @@ public abstract class AbstractProjectableModel extends AbstractModel implements 
      */
     public static void setDriverService(Object key, DriverService driverService) {
         // TODO: Rework this once we have means to enforce access to admin services in general
-        checkState(AbstractProjectableModel.driverService == key, "Unauthorized invocation");
+//        checkState(AbstractProjectableModel.driverService == key, "Unauthorized invocation");
         AbstractProjectableModel.driverService = driverService;
     }
@@ -92,28 +92,20 @@ public abstract class AbstractProjectableModel extends AbstractModel implements 
      *
      * @return bound driver; null if none
      */
-    protected Driver driver() {
+    public Driver driver() {
         return driver;
     }
     @Override
     public <B extends Behaviour> B as(Class<B> projectionClass) {
-        checkState(driverService != null, NO_DRIVER_SERVICE);
+        bindAndCheckDriver();
-        if (driver == null) {
-            driver = locateDriver();
-        }
-        checkState(driver != null, NO_DRIVER, this);
         return driver.createBehaviour(asData(), projectionClass);
     }
     @Override
     public <B extends Behaviour> boolean is(Class<B> projectionClass) {
-        checkState(driverService != null, NO_DRIVER_SERVICE);
+        bindDriver();
-        if (driver == null) {
+        return driver != null && driver.hasBehaviour(projectionClass);
-            driver = locateDriver();
-        }
-        checkState(driver != null, "Driver has not been bound to %s", this);
-        return driver.hasBehaviour(projectionClass);
     }
     /**
@@ -126,7 +118,8 @@ public abstract class AbstractProjectableModel extends AbstractModel implements 
      * if no driver is expected or driver is not found
      */
     protected Driver locateDriver() {
-        String driverName = annotations().value(AnnotationKeys.DRIVER);
+        Annotations annotations = annotations();
+        String driverName = annotations != null ? annotations.value(AnnotationKeys.DRIVER) : null;
         if (driverName != null) {
             try {
                 return driverService.getDriver(driverName);
@@ -138,6 +131,27 @@ public abstract class AbstractProjectableModel extends AbstractModel implements 
     }
     /**
+     * Attempts to binds the driver, if not already bound.
+     */
+    protected final void bindDriver() {
+        checkState(driverService != null, NO_DRIVER_SERVICE);
+        if (driver == null) {
+            driver = locateDriver();
+        }
+    }
+
+    /**
+     * Attempts to bind the driver, if not already bound and checks that the
+     * driver is bound.
+     *
+     * @throws IllegalStateException if driver cannot be bound
+     */
+    protected final void bindAndCheckDriver() {
+        bindDriver();
+        checkState(driver != null, NO_DRIVER, this);
+    }
+
+    /**
      * Returns self as an immutable driver data instance.
      *
      * @return self as driver data
--- a/core/api/src/main/java/org/onosproject/net/DefaultDevice.java
View file @c4ee737
+++ b/core/api/src/main/java/org/onosproject/net/DefaultDevice.java
View file @c4ee737
@@ -16,14 +16,16 @@
 package org.onosproject.net;
 import org.onlab.packet.ChassisId;
+import org.onosproject.net.driver.Behaviour;
+import org.onosproject.net.driver.DefaultDriverHandler;
 import org.onosproject.net.driver.Driver;
 import org.onosproject.net.driver.DriverData;
+import org.onosproject.net.driver.HandlerBehaviour;
 import org.onosproject.net.provider.ProviderId;
 import java.util.Objects;
 import static com.google.common.base.MoreObjects.toStringHelper;
-import static org.onlab.util.Tools.nullIsNotFound;
 /**
  * Default infrastructure device model implementation.
@@ -108,6 +110,15 @@ public class DefaultDevice extends AbstractElement implements Device {
         return chassisId;
     }
+    @Override
+    public <B extends Behaviour> B as(Class<B> projectionClass) {
+        if (HandlerBehaviour.class.isAssignableFrom(projectionClass)) {
+            bindAndCheckDriver();
+            return driver().createBehaviour(new DefaultDriverHandler(asData()), projectionClass);
+        }
+        return super.as(projectionClass);
+    }
+
     /**
      * Returns self as an immutable driver data instance.
      *
@@ -121,8 +132,7 @@ public class DefaultDevice extends AbstractElement implements Device {
     protected Driver locateDriver() {
         Driver driver = super.locateDriver();
         return driver != null ? driver :
-                nullIsNotFound(driverService().getDriver(manufacturer, hwVersion, swVersion),
+                driverService().getDriver(manufacturer, hwVersion, swVersion);
-                               "Driver not found");
     }
     /**
--- a/core/api/src/main/java/org/onosproject/net/flow/FlowRuleProgrammable.java 0 → 100644
View file @c4ee737
+++ b/core/api/src/main/java/org/onosproject/net/flow/FlowRuleProgrammable.java 0 → 100644
View file @c4ee737
+/*
+ * Copyright 2014-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.flow;
+
+import org.onosproject.net.driver.HandlerBehaviour;
+
+import java.util.Collection;
+
+/**
+ * Flow rule programmable device behaviour.
+ */
+public interface FlowRuleProgrammable extends HandlerBehaviour {
+
+    /**
+     * Retrieves the collection of flow rule entries currently installed on the device.
+     *
+     * @return collection of flow rules
+     */
+    Collection<FlowEntry> getFlowEntries();
+
+    /**
+     * Applies the specified collection of flow rules to the device.
+     *
+     * @param rules flow rules to be added
+     * @return collection of flow rules that were added successfully
+     */
+    Collection<FlowRule> applyFlowRules(Collection<FlowRule> rules);
+
+    /**
+     * Removes the specified collection of flow rules from the device.
+     *
+     * @param rules flow rules to be removed
+     * @return collection of flow rules that were removed successfully
+     */
+    Collection<FlowRule> removeFlowRules(Collection<FlowRule> rules);
+
+}
--- a/core/api/src/main/java/org/onosproject/net/flow/FlowRuleProvider.java
View file @c4ee737
+++ b/core/api/src/main/java/org/onosproject/net/flow/FlowRuleProvider.java
View file @c4ee737
@@ -26,6 +26,7 @@ public interface FlowRuleProvider extends Provider {
     /**
      * Instructs the provider to apply the specified flow rules to their
      * respective devices.
+     *
      * @param flowRules one or more flow rules
      *                  throws SomeKindOfException that indicates which ones were applied and
      *                  which ones failed
@@ -35,6 +36,7 @@ public interface FlowRuleProvider extends Provider {
     /**
      * Instructs the provider to remove the specified flow rules to their
      * respective devices.
+     *
      * @param flowRules one or more flow rules
      *                  throws SomeKindOfException that indicates which ones were applied and
      *                  which ones failed
@@ -43,14 +45,18 @@ public interface FlowRuleProvider extends Provider {
     /**
      * Removes rules by their id.
+     *
      * @param id        the id to remove
      * @param flowRules one or more flow rules
+     * @deprecated since 1.5.0 Falcon
      */
+    @Deprecated
     void removeRulesById(ApplicationId id, FlowRule... flowRules);
     /**
      * Installs a batch of flow rules. Each flowrule is associated to an
      * operation which results in either addition, removal or modification.
+     *
      * @param batch a batch of flow rules
      */
     void executeBatch(FlowRuleBatchOperation batch);
--- a/core/api/src/main/java/org/onosproject/net/provider/AbstractProviderRegistry.java
View file @c4ee737
+++ b/core/api/src/main/java/org/onosproject/net/provider/AbstractProviderRegistry.java
View file @c4ee737
@@ -46,6 +46,15 @@ public abstract class AbstractProviderRegistry<P extends Provider, S extends Pro
      */
     protected abstract S createProviderService(P provider);
+    /**
+     * Returns the default fall-back provider. Default implementation return null.
+     *
+     * @return default provider
+     */
+    protected P defaultProvider() {
+        return null;
+    }
+
     @Override
     public synchronized S register(P provider) {
         checkNotNull(provider, "Provider cannot be null");
@@ -89,13 +98,16 @@ public abstract class AbstractProviderRegistry<P extends Provider, S extends Pro
     }
     /**
-     * Returns the provider registered with the specified provider ID.
+     * Returns the provider registered with the specified provider ID or null
+     * if none is found for the given provider family and default fall-back is
+     * not supported.
      *
      * @param providerId provider identifier
      * @return provider
      */
     protected synchronized P getProvider(ProviderId providerId) {
-        return providers.get(providerId);
+        P provider = providers.get(providerId);
+        return provider != null ? provider : defaultProvider();
     }
     /**
@@ -105,7 +117,8 @@ public abstract class AbstractProviderRegistry<P extends Provider, S extends Pro
      * @return provider bound to the URI scheme
      */
     protected synchronized P getProvider(DeviceId deviceId) {
-        return providersByScheme.get(deviceId.uri().getScheme());
+        P provider = providersByScheme.get(deviceId.uri().getScheme());
+        return provider != null ? provider : defaultProvider();
     }
     /**
--- a/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleDriverProvider.java 0 → 100644
View file @c4ee737
+++ b/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleDriverProvider.java 0 → 100644
View file @c4ee737
+/*
+ * Copyright 2014-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.flow.impl;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.LinkedListMultimap;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Sets;
+import org.onosproject.core.ApplicationId;
+import org.onosproject.mastership.MastershipService;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.device.DeviceService;
+import org.onosproject.net.flow.CompletedBatchOperation;
+import org.onosproject.net.flow.FlowRule;
+import org.onosproject.net.flow.FlowRuleBatchEntry;
+import org.onosproject.net.flow.FlowRuleBatchOperation;
+import org.onosproject.net.flow.FlowRuleProgrammable;
+import org.onosproject.net.flow.FlowRuleProvider;
+import org.onosproject.net.flow.FlowRuleProviderService;
+import org.onosproject.net.provider.AbstractProvider;
+import org.onosproject.net.provider.ProviderId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collection;
+import java.util.Set;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+
+import static com.google.common.collect.ImmutableSet.copyOf;
+import static org.onosproject.net.flow.FlowRuleBatchEntry.FlowRuleOperation.*;
+
+/**
+ * Driver-based flow rule provider.
+ */
+class FlowRuleDriverProvider extends AbstractProvider implements FlowRuleProvider {
+
+    private final Logger log = LoggerFactory.getLogger(getClass());
+
+    // Perhaps to be extracted for better reuse as we deal with other.
+    public static final String SCHEME = "default";
+    public static final String PROVIDER_NAME = "org.onosproject.provider";
+
+    FlowRuleProviderService providerService;
+    private DeviceService deviceService;
+    private MastershipService mastershipService;
+
+    private ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
+    private ScheduledFuture<?> poller = null;
+
+    /**
+     * Creates a new fallback flow rule provider.
+     */
+    FlowRuleDriverProvider() {
+        super(new ProviderId(SCHEME, PROVIDER_NAME));
+    }
+
+    /**
+     * Initializes the provider with necessary supporting services.
+     *
+     * @param providerService   flow rule provider service
+     * @param deviceService     device service
+     * @param mastershipService mastership service
+     * @param pollFrequency     flow entry poll frequency
+     */
+    void init(FlowRuleProviderService providerService,
+              DeviceService deviceService, MastershipService mastershipService,
+              int pollFrequency) {
+        this.providerService = providerService;
+        this.deviceService = deviceService;
+        this.mastershipService = mastershipService;
+
+        if (poller != null && !poller.isCancelled()) {
+            poller.cancel(false);
+        }
+
+        poller = executor.scheduleAtFixedRate(this::pollFlowEntries, pollFrequency,
+                                              pollFrequency, TimeUnit.SECONDS);
+    }
+
+    @Override
+    public void applyFlowRule(FlowRule... flowRules) {
+        rulesByDevice(flowRules).asMap().forEach(this::applyFlowRules);
+    }
+
+    @Override
+    public void removeFlowRule(FlowRule... flowRules) {
+        rulesByDevice(flowRules).asMap().forEach(this::removeFlowRules);
+    }
+
+    @Override
+    public void removeRulesById(ApplicationId id, FlowRule... flowRules) {
+        removeFlowRule(flowRules);
+    }
+
+    @Override
+    public void executeBatch(FlowRuleBatchOperation batch) {
+        ImmutableList.Builder<FlowRule> toAdd = ImmutableList.builder();
+        ImmutableList.Builder<FlowRule> toRemove = ImmutableList.builder();
+        for (FlowRuleBatchEntry fbe : batch.getOperations()) {
+            if (fbe.operator() == ADD || fbe.operator() == MODIFY) {
+                toAdd.add(fbe.target());
+            } else if (fbe.operator() == REMOVE) {
+                toRemove.add(fbe.target());
+            }
+        }
+
+        ImmutableList<FlowRule> rulesToAdd = toAdd.build();
+        ImmutableList<FlowRule> rulesToRemove = toRemove.build();
+
+        Collection<FlowRule> added = applyFlowRules(batch.deviceId(), rulesToAdd);
+        Collection<FlowRule> removed = removeFlowRules(batch.deviceId(), rulesToRemove);
+
+        Set<FlowRule> failedRules = Sets.union(Sets.difference(copyOf(rulesToAdd), copyOf(added)),
+                                               Sets.difference(copyOf(rulesToRemove), copyOf(removed)));
+        CompletedBatchOperation status =
+                new CompletedBatchOperation(failedRules.isEmpty(), failedRules, batch.deviceId());
+        providerService.batchOperationCompleted(batch.id(), status);
+    }
+
+    private Multimap<DeviceId, FlowRule> rulesByDevice(FlowRule[] flowRules) {
+        // Sort the flow rules by device id
+        Multimap<DeviceId, FlowRule> rulesByDevice = LinkedListMultimap.create();
+        for (FlowRule rule : flowRules) {
+            rulesByDevice.put(rule.deviceId(), rule);
+        }
+        return rulesByDevice;
+    }
+
+    private Collection<FlowRule> applyFlowRules(DeviceId deviceId, Collection<FlowRule> flowRules) {
+        FlowRuleProgrammable programmer = getFlowRuleProgrammable(deviceId);
+        return programmer != null ? programmer.applyFlowRules(flowRules) : ImmutableList.of();
+    }
+
+    private Collection<FlowRule> removeFlowRules(DeviceId deviceId, Collection<FlowRule> flowRules) {
+        FlowRuleProgrammable programmer = getFlowRuleProgrammable(deviceId);
+        return programmer != null ? programmer.removeFlowRules(flowRules) : ImmutableList.of();
+    }
+
+    private FlowRuleProgrammable getFlowRuleProgrammable(DeviceId deviceId) {
+        FlowRuleProgrammable programmable = deviceService.getDevice(deviceId).as(FlowRuleProgrammable.class);
+        if (programmable == null) {
+            log.warn("Device {} is not flow rule programmable");
+        }
+        return programmable;
+    }
+
+
+    private void pollFlowEntries() {
+        deviceService.getAvailableDevices().forEach(device -> {
+            if (mastershipService.isLocalMaster(device.id()) && device.is(FlowRuleProgrammable.class)) {
+                providerService.pushFlowMetrics(device.id(),
+                                                device.as(FlowRuleProgrammable.class).getFlowEntries());
+            }
+        });
+    }
+
+}
--- a/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java
View file @c4ee737
+++ b/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java
View file @c4ee737
@@ -21,7 +21,6 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
-
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -31,14 +30,14 @@ import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.Service;
 import org.onosproject.cfg.ComponentConfigService;
-import org.onosproject.net.device.DeviceEvent;
-import org.onosproject.net.device.DeviceListener;
-import org.onosproject.net.provider.AbstractListenerProviderRegistry;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
 import org.onosproject.core.IdGenerator;
+import org.onosproject.mastership.MastershipService;
 import org.onosproject.net.Device;
 import org.onosproject.net.DeviceId;
+import org.onosproject.net.device.DeviceEvent;
+import org.onosproject.net.device.DeviceListener;
 import org.onosproject.net.device.DeviceService;
 import org.onosproject.net.flow.CompletedBatchOperation;
 import org.onosproject.net.flow.DefaultFlowEntry;
@@ -60,6 +59,7 @@ import org.onosproject.net.flow.FlowRuleService;
 import org.onosproject.net.flow.FlowRuleStore;
 import org.onosproject.net.flow.FlowRuleStoreDelegate;
 import org.onosproject.net.flow.TableStatisticsEntry;
+import org.onosproject.net.provider.AbstractListenerProviderRegistry;
 import org.onosproject.net.provider.AbstractProviderService;
 import org.osgi.service.component.ComponentContext;
 import org.slf4j.Logger;
@@ -76,12 +76,14 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Strings.isNullOrEmpty;
+import static org.onlab.util.Tools.get;
 import static org.onlab.util.Tools.groupedThreads;
 import static org.onosproject.net.flow.FlowRuleEvent.Type.RULE_ADD_REQUESTED;
 import static org.onosproject.net.flow.FlowRuleEvent.Type.RULE_REMOVE_REQUESTED;
 import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.FLOWRULE_READ;
+import static org.onosproject.security.AppPermission.Type.FLOWRULE_WRITE;
 import static org.slf4j.LoggerFactory.getLogger;
-import static org.onosproject.security.AppPermission.Type.*;
@@ -95,6 +97,8 @@ public class FlowRuleManager
                                                  FlowRuleProvider, FlowRuleProviderService>
         implements FlowRuleService, FlowRuleProviderRegistry {
+    private final Logger log = getLogger(getClass());
+
     public static final String FLOW_RULE_NULL = "FlowRule cannot be null";
     private static final boolean ALLOW_EXTRANEOUS_RULES = false;
@@ -106,11 +110,16 @@ public class FlowRuleManager
             label = "Purge entries associated with a device when the device goes offline")
     private boolean purgeOnDisconnection = false;
-    private final Logger log = getLogger(getClass());
+    private static final int DEFAULT_POLL_FREQUENCY = 30;
+    @Property(name = "fallbackFlowPollFrequency", intValue = DEFAULT_POLL_FREQUENCY,
+            label = "Frequency (in seconds) for polling flow statistics via fallback provider")
+    private int fallbackFlowPollFrequency = DEFAULT_POLL_FREQUENCY;
     private final FlowRuleStoreDelegate delegate = new InternalStoreDelegate();
     private final DeviceListener deviceListener = new InternalDeviceListener();
+    private final FlowRuleDriverProvider defaultProvider = new FlowRuleDriverProvider();
+
     protected ExecutorService deviceInstallers =
             Executors.newFixedThreadPool(32, groupedThreads("onos/flowservice", "device-installer-%d"));
@@ -132,16 +141,19 @@ public class FlowRuleManager
     protected CoreService coreService;
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected MastershipService mastershipService;
+
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected ComponentConfigService cfgService;
     @Activate
     public void activate(ComponentContext context) {
+        modified(context);
         store.setDelegate(delegate);
         eventDispatcher.addSink(FlowRuleEvent.class, listenerRegistry);
         deviceService.addListener(deviceListener);
         cfgService.registerProperties(getClass());
         idGenerator = coreService.getIdGenerator(FLOW_OP_TOPIC);
-        modified(context);
         log.info("Started");
     }
@@ -160,6 +172,13 @@ public class FlowRuleManager
         if (context != null) {
             readComponentConfiguration(context);
         }
+        defaultProvider.init(new InternalFlowRuleProviderService(defaultProvider),
+                             deviceService, mastershipService, fallbackFlowPollFrequency);
+    }
+
+    @Override
+    protected FlowRuleProvider defaultProvider() {
+        return defaultProvider;
     }
     /**
@@ -190,6 +209,13 @@ public class FlowRuleManager
             log.info("Configured. PurgeOnDisconnection is {}",
                     purgeOnDisconnection ? "enabled" : "disabled");
         }
+
+        String s = get(properties, "fallbackFlowPollFrequency");
+        try {
+            fallbackFlowPollFrequency = isNullOrEmpty(s) ? DEFAULT_POLL_FREQUENCY : Integer.parseInt(s);
+        } catch (NumberFormatException e) {
+            fallbackFlowPollFrequency = DEFAULT_POLL_FREQUENCY;
+        }
     }
     /**
@@ -201,15 +227,13 @@ public class FlowRuleManager
      */
     private static Boolean isPropertyEnabled(Dictionary<?, ?> properties,
             String propertyName) {
-        Boolean value = null;
         try {
             String s = (String) properties.get(propertyName);
-            value = isNullOrEmpty(s) ? null : s.trim().equals("true");
+            return  isNullOrEmpty(s) ? null : s.trim().equals("true");
         } catch (ClassCastException e) {
             // No propertyName defined.
-            value = null;
+            return null;
         }
-        return value;
     }
     @Override
@@ -229,8 +253,8 @@ public class FlowRuleManager
         checkPermission(FLOWRULE_WRITE);
         FlowRuleOperations.Builder builder = FlowRuleOperations.builder();
-        for (int i = 0; i < flowRules.length; i++) {
+        for (FlowRule flowRule : flowRules) {
-            builder.add(flowRules[i]);
+            builder.add(flowRule);
         }
         apply(builder.build());
     }
@@ -240,8 +264,8 @@ public class FlowRuleManager
         checkPermission(FLOWRULE_WRITE);
         FlowRuleOperations.Builder builder = FlowRuleOperations.builder();
-        for (int i = 0; i < flowRules.length; i++) {
+        for (FlowRule flowRule : flowRules) {
-            builder.remove(flowRules[i]);
+            builder.remove(flowRule);
         }
         apply(builder.build());
     }
@@ -419,15 +443,9 @@ public class FlowRuleManager
                 //lastSeen.put(storedRule, currentTime);
             }
             Long last = lastSeen.get(storedRule);
-            if (last == null) {
-                // concurrently removed? let the liveness check fail
-                return false;
-            }
-            if ((currentTime - last) <= timeout) {
+            // concurrently removed? let the liveness check fail
-                return true;
+            return last != null && (currentTime - last) <= timeout;
-            }
-            return false;
         }
         @Override
@@ -466,7 +484,6 @@ public class FlowRuleManager
                     }
                 } catch (Exception e) {
                     log.debug("Can't process added or extra rule {}", e.getMessage());
-                    continue;
                 }
             }
@@ -479,7 +496,6 @@ public class FlowRuleManager
                         flowMissing(rule);
                     } catch (Exception e) {
                         log.debug("Can't add missing flow rule:", e);
-                        continue;
                     }
                 }
             }
--- a/core/net/src/test/java/org/onosproject/net/flow/impl/FlowRuleManagerTest.java
View file @c4ee737
+++ b/core/net/src/test/java/org/onosproject/net/flow/impl/FlowRuleManagerTest.java
View file @c4ee737
@@ -21,25 +21,28 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
-
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.onlab.junit.TestTools;
 import org.onosproject.cfg.ComponentConfigAdapter;
+import org.onosproject.common.event.impl.TestEventDispatcher;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreServiceAdapter;
 import org.onosproject.core.DefaultApplicationId;
 import org.onosproject.core.IdGenerator;
-import org.onosproject.common.event.impl.TestEventDispatcher;
+import org.onosproject.mastership.MastershipServiceAdapter;
+import org.onosproject.net.AnnotationKeys;
+import org.onosproject.net.DefaultAnnotations;
 import org.onosproject.net.DefaultDevice;
 import org.onosproject.net.Device;
 import org.onosproject.net.Device.Type;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.MastershipRole;
-import org.onosproject.net.Port;
-import org.onosproject.net.PortNumber;
-import org.onosproject.net.device.DeviceListener;
 import org.onosproject.net.device.DeviceServiceAdapter;
+import org.onosproject.net.driver.AbstractHandlerBehaviour;
+import org.onosproject.net.driver.DefaultDriver;
+import org.onosproject.net.driver.impl.DriverManager;
 import org.onosproject.net.flow.CompletedBatchOperation;
 import org.onosproject.net.flow.DefaultFlowEntry;
 import org.onosproject.net.flow.DefaultFlowRule;
@@ -49,6 +52,7 @@ import org.onosproject.net.flow.FlowRule;
 import org.onosproject.net.flow.FlowRuleBatchOperation;
 import org.onosproject.net.flow.FlowRuleEvent;
 import org.onosproject.net.flow.FlowRuleListener;
+import org.onosproject.net.flow.FlowRuleProgrammable;
 import org.onosproject.net.flow.FlowRuleProvider;
 import org.onosproject.net.flow.FlowRuleProviderRegistry;
 import org.onosproject.net.flow.FlowRuleProviderService;
@@ -65,8 +69,10 @@ import org.onosproject.net.provider.ProviderId;
 import org.onosproject.store.trivial.SimpleFlowRuleStore;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -76,16 +82,9 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicLong;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
 import static org.onosproject.net.NetTestTools.injectEventDispatcher;
-import static org.onosproject.net.flow.FlowRuleEvent.Type.RULE_ADDED;
+import static org.onosproject.net.flow.FlowRuleEvent.Type.*;
-import static org.onosproject.net.flow.FlowRuleEvent.Type.RULE_ADD_REQUESTED;
-import static org.onosproject.net.flow.FlowRuleEvent.Type.RULE_REMOVED;
-import static org.onosproject.net.flow.FlowRuleEvent.Type.RULE_REMOVE_REQUESTED;
-import static org.onosproject.net.flow.FlowRuleEvent.Type.RULE_UPDATED;
 /**
  * Test codifying the flow rule service & flow rule provider service contracts.
@@ -94,10 +93,19 @@ public class FlowRuleManagerTest {
     private static final ProviderId PID = new ProviderId("of", "foo");
+    private static final ProviderId FOO_PID = new ProviderId("foo", "foo");
+
     private static final DeviceId DID = DeviceId.deviceId("of:001");
+    private static final DeviceId FOO_DID = DeviceId.deviceId("foo:002");
     private static final int TIMEOUT = 10;
-    private static final Device DEV = new DefaultDevice(
+
-            PID, DID, Type.SWITCH, "", "", "", "", null);
+    private static final DefaultAnnotations ANNOTATIONS =
+            DefaultAnnotations.builder().set(AnnotationKeys.DRIVER, "foo").build();
+
+    private static final Device DEV =
+            new DefaultDevice(PID, DID, Type.SWITCH, "", "", "", "", null);
+    private static final Device FOO_DEV =
+            new DefaultDevice(FOO_PID, FOO_DID, Type.SWITCH, "", "", "", "", null, ANNOTATIONS);
     private FlowRuleManager mgr;
@@ -108,6 +116,8 @@ public class FlowRuleManagerTest {
     protected TestListener listener = new TestListener();
     private ApplicationId appId;
+    private TestDriverManager driverService;
+
     @Before
     public void setUp() {
@@ -115,6 +125,7 @@ public class FlowRuleManagerTest {
         mgr.store = new SimpleFlowRuleStore();
         injectEventDispatcher(mgr, new TestEventDispatcher());
         mgr.deviceService = new TestDeviceService();
+        mgr.mastershipService = new TestMastershipService();
         mgr.coreService = new TestCoreService();
         mgr.operationsService = MoreExecutors.newDirectExecutorService();
         mgr.deviceInstallers = MoreExecutors.newDirectExecutorService();
@@ -122,6 +133,12 @@ public class FlowRuleManagerTest {
         service = mgr;
         registry = mgr;
+        driverService = new TestDriverManager();
+        driverService.addDriver(new DefaultDriver("foo", ImmutableList.of(), "", "", "",
+                                                  ImmutableMap.of(FlowRuleProgrammable.class,
+                                                                  TestFlowRuleProgrammable.class),
+                                                  ImmutableMap.of()));
+
         mgr.activate(null);
         mgr.addListener(listener);
         provider = new TestProvider(PID);
@@ -143,10 +160,14 @@ public class FlowRuleManagerTest {
     }
     private FlowRule flowRule(int tsval, int trval) {
+        return flowRule(DID, tsval, trval);
+    }
+
+    private FlowRule flowRule(DeviceId did, int tsval, int trval) {
         TestSelector ts = new TestSelector(tsval);
         TestTreatment tr = new TestTreatment(trval);
         return DefaultFlowRule.builder()
-                .forDevice(DID)
+                .forDevice(did)
                 .withSelector(ts)
                 .withTreatment(tr)
                 .withPriority(10)
@@ -155,7 +176,6 @@ public class FlowRuleManagerTest {
                 .build();
     }
-
     private FlowRule addFlowRule(int hval) {
         FlowRule rule = flowRule(hval, hval);
         service.applyFlowRules(rule);
@@ -398,60 +418,80 @@ public class FlowRuleManagerTest {
                 f2, FlowEntryState.PENDING_REMOVE));
     }
-    private static class TestListener implements FlowRuleListener {
+    @Test
-        final List<FlowRuleEvent> events = new ArrayList<>();
+    public void fallbackBasics() {
+        FlowRule f1 = flowRule(FOO_DID, 1, 1);
+        flowRules.clear();
+        mgr.applyFlowRules(f1);
+        assertTrue("flow rule not applied", flowRules.contains(f1));
-        @Override
+        flowRules.clear();
-        public void event(FlowRuleEvent event) {
+        mgr.removeFlowRules(f1);
-            events.add(event);
+        assertTrue("flow rule not removed", flowRules.contains(f1));
-        }
     }
-    private static class TestDeviceService extends DeviceServiceAdapter {
+    @Test
-
+    public void fallbackFlowRemoved() {
-        @Override
+        FlowRule f1 = flowRule(FOO_DID, 1, 1);
-        public int getDeviceCount() {
+        mgr.applyFlowRules(f1);
-            return 1;
+        flowRules.clear();
+        providerService.flowRemoved(new DefaultFlowEntry(f1));
+        assertTrue("flow rule not reapplied", flowRules.contains(f1));
     }
-        @Override
+    @Test
-        public Iterable<Device> getDevices() {
+    public void fallbackExtraFlow() {
-            return Collections.singletonList(DEV);
+        FlowRule f1 = flowRule(FOO_DID, 1, 1);
+        flowRules.clear();
+        providerService.pushFlowMetrics(FOO_DID, ImmutableList.of(new DefaultFlowEntry(f1)));
+        assertTrue("flow rule not removed", flowRules.contains(f1));
     }
-        @Override
+    @Test
-        public Device getDevice(DeviceId deviceId) {
+    public void fallbackPoll() {
-            return DEV;
+        FlowRuleDriverProvider fallback = (FlowRuleDriverProvider) mgr.defaultProvider();
-        }
+        FlowRule f1 = flowRule(FOO_DID, 1, 1);
+        mgr.applyFlowRules(f1);
+        FlowEntry fe = mgr.getFlowEntries(FOO_DID).iterator().next();
+        assertEquals("incorrect state", FlowEntryState.PENDING_ADD, fe.state());
-        @Override
+        fallback.init(fallback.providerService, mgr.deviceService, mgr.mastershipService, 1);
-        public MastershipRole getRole(DeviceId deviceId) {
+        TestTools.assertAfter(2000, () -> {
-            return null;
+            FlowEntry e = mgr.getFlowEntries(FOO_DID).iterator().next();
+            assertEquals("incorrect state", FlowEntryState.ADDED, e.state());
+        });
     }
+
+    private static class TestListener implements FlowRuleListener {
+        final List<FlowRuleEvent> events = new ArrayList<>();
+
         @Override
-        public List<Port> getPorts(DeviceId deviceId) {
+        public void event(FlowRuleEvent event) {
-            return null;
+            events.add(event);
+        }
     }
+    private static class TestDeviceService extends DeviceServiceAdapter {
         @Override
-        public Port getPort(DeviceId deviceId, PortNumber portNumber) {
+        public int getDeviceCount() {
-            return null;
+            return 2;
         }
         @Override
-        public boolean isAvailable(DeviceId deviceId) {
+        public Iterable<Device> getDevices() {
-            return false;
+            return ImmutableList.of(DEV, FOO_DEV);
         }
         @Override
-        public void addListener(DeviceListener listener) {
+        public Iterable<Device> getAvailableDevices() {
+            return getDevices();
         }
         @Override
-        public void removeListener(DeviceListener listener) {
+        public Device getDevice(DeviceId deviceId) {
+            return deviceId.equals(FOO_DID) ? FOO_DEV : DEV;
         }
-
     }
     private class TestProvider extends AbstractProvider implements FlowRuleProvider {
@@ -624,6 +664,7 @@ public class FlowRuleManagerTest {
         public IdGenerator getIdGenerator(String topic) {
             return new IdGenerator() {
                 private AtomicLong counter = new AtomicLong(0);
+
                 @Override
                 public long getNewId() {
                     return counter.getAndIncrement();
@@ -632,4 +673,41 @@ public class FlowRuleManagerTest {
         }
     }
+    private class TestMastershipService extends MastershipServiceAdapter {
+        @Override
+        public MastershipRole getLocalRole(DeviceId deviceId) {
+            return MastershipRole.MASTER;
+        }
+    }
+
+    private class TestDriverManager extends DriverManager {
+        TestDriverManager() {
+            this.deviceService = mgr.deviceService;
+            activate();
+        }
+    }
+
+    static Collection<FlowRule> flowRules = new HashSet<>();
+
+    public static class TestFlowRuleProgrammable extends AbstractHandlerBehaviour implements FlowRuleProgrammable {
+
+        @Override
+        public Collection<FlowEntry> getFlowEntries() {
+            ImmutableList.Builder<FlowEntry> builder = ImmutableList.builder();
+            flowRules.stream().map(DefaultFlowEntry::new).forEach(builder::add);
+            return builder.build();
+        }
+
+        @Override
+        public Collection<FlowRule> applyFlowRules(Collection<FlowRule> rules) {
+            flowRules.addAll(rules);
+            return rules;
+        }
+
+        @Override
+        public Collection<FlowRule> removeFlowRules(Collection<FlowRule> rules) {
+            flowRules.addAll(rules);
+            return rules;
+        }
+    }
 }
--- a/tools/test/bin/onos-check-apps
View file @c4ee737
+++ b/tools/test/bin/onos-check-apps
View file @c4ee737
@@ -9,7 +9,7 @@
 aux=/tmp/stc-$$.log
 trap "rm -f $aux $aux.1 $aux.2 2>/dev/null" EXIT
-for attempt in {1..3}; do
+for attempt in {1..10}; do
     onos ${1:-$OCI} "onos:apps -s -a" | grep -v /bin/client > $aux
     cat $aux