ONOS-1404 - Rerouting delayed due to flows being removed out of order

ONOS-1409 - handle all links to a switch going away and coming back

Change-Id: Ib7216530fa8eef41b1088c5122a40bf9265481ca

@@ -15,7 +15,15 @@
  */
  */
 package org.onosproject.net.intent.impl;
 package org.onosproject.net.intent.impl;
 
 
-import com.google.common.collect.ImmutableList;
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.stream.Collectors;
+
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -48,15 +56,7 @@ import org.onosproject.net.intent.impl.phase.IntentProcessPhase;
 import org.onosproject.net.intent.impl.phase.IntentWorker;
 import org.onosproject.net.intent.impl.phase.IntentWorker;
 import org.slf4j.Logger;
 import org.slf4j.Logger;
 
 
-import java.util.Collection;
-import java.util.Collections;
-import java.util.EnumSet;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Future;
-import java.util.stream.Collectors;
+import com.google.common.collect.ImmutableList;
 
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.util.concurrent.Executors.newFixedThreadPool;
 import static java.util.concurrent.Executors.newFixedThreadPool;
@@ -359,97 +359,92 @@ public class IntentManager
         }
         }
 
 
         @Override
         @Override
-        public void install(IntentData data) {
-            IntentManager.this.install(data);
+        public void apply(IntentData toUninstall, IntentData toInstall) {
+            IntentManager.this.apply(toUninstall, toInstall);
         }
         }
-
-        @Override
-        public void uninstall(IntentData data) {
-            IntentManager.this.uninstall(data);
-        }
-
     }
     }
 
 
-    private void install(IntentData data) {
-        // need to consider if FlowRuleIntent is only one as installable intent or not
-        List<Intent> installables = data.installables();
-        if (!installables.stream().allMatch(x -> x instanceof FlowRuleIntent)) {
-            throw new IllegalStateException("installable intents must be FlowRuleIntent");
-        }
+    private enum Direction {
+        ADD,
+        REMOVE
+    }
 
 
-        trackerService.addTrackedResources(data.key(), data.intent().resources());
-        installables.forEach(installable ->
-                 trackerService.addTrackedResources(data.key(), installable.resources()));
+    private void applyIntentData(IntentData data,
+                                 FlowRuleOperations.Builder builder,
+                                 Direction direction) {
+        if (data != null) {
+            List<Intent> intentsToApply = data.installables();
+            if (!intentsToApply.stream().allMatch(x -> x instanceof FlowRuleIntent)) {
+                throw new IllegalStateException("installable intents must be FlowRuleIntent");
+            }
 
 
-        List<Collection<FlowRule>> stages = installables.stream()
-                .map(x -> (FlowRuleIntent) x)
-                .map(FlowRuleIntent::flowRules)
-                .collect(Collectors.toList());
+            if (direction == Direction.ADD) {
+                trackerService.addTrackedResources(data.key(), data.intent().resources());
+                intentsToApply.forEach(installable ->
+                        trackerService.addTrackedResources(data.key(), installable.resources()));
+            } else {
+                trackerService.removeTrackedResources(data.key(), data.intent().resources());
+                intentsToApply.forEach(installable ->
+                        trackerService.removeTrackedResources(data.intent().key(),
+                                installable.resources()));
+            }
 
 
-        FlowRuleOperations.Builder builder = FlowRuleOperations.builder();
-        for (Collection<FlowRule> rules : stages) {
-            rules.forEach(builder::add);
+            // FIXME do FlowRuleIntents have stages??? Can we do uninstall work in parallel? I think so.
             builder.newStage();
             builder.newStage();
-        }
 
 
-        FlowRuleOperations operations = builder.build(new FlowRuleOperationsContext() {
-            @Override
-            public void onSuccess(FlowRuleOperations ops) {
-                log.debug("Completed installing: {}", data.key());
-                data.setState(INSTALLED);
-                store.write(data);
-            }
+            List<Collection<FlowRule>> stages = intentsToApply.stream()
+                    .map(x -> (FlowRuleIntent) x)
+                    .map(FlowRuleIntent::flowRules)
+                    .collect(Collectors.toList());
 
 
-            @Override
-            public void onError(FlowRuleOperations ops) {
-                log.warn("Failed installation: {} {} on {}", data.key(), data.intent(), ops);
-                data.setState(FAILED);
-                store.write(data);
+            for (Collection<FlowRule> rules : stages) {
+                if (direction == Direction.ADD) {
+                    rules.forEach(builder::add);
+                } else {
+                    rules.forEach(builder::remove);
+                }
             }
             }
-        });
-
-        flowRuleService.apply(operations);
-    }
-
-    private void uninstall(IntentData data) {
-        List<Intent> installables = data.installables();
-        if (!installables.stream().allMatch(x -> x instanceof FlowRuleIntent)) {
-            throw new IllegalStateException("installable intents must be FlowRuleIntent");
         }
         }
 
 
-        trackerService.removeTrackedResources(data.key(), data.intent().resources());
-        installables.forEach(installable ->
-                 trackerService.removeTrackedResources(data.intent().key(),
-                                                       installable.resources()));
+    }
 
 
-        List<Collection<FlowRule>> stages = installables.stream()
-                .map(x -> (FlowRuleIntent) x)
-                .map(FlowRuleIntent::flowRules)
-                .collect(Collectors.toList());
+    private void apply(IntentData toUninstall, IntentData toInstall) {
+        // need to consider if FlowRuleIntent is only one as installable intent or not
 
 
         FlowRuleOperations.Builder builder = FlowRuleOperations.builder();
         FlowRuleOperations.Builder builder = FlowRuleOperations.builder();
-        for (Collection<FlowRule> rules : stages) {
-            rules.forEach(builder::remove);
-            builder.newStage();
-        }
+        applyIntentData(toUninstall, builder, Direction.REMOVE);
+        applyIntentData(toInstall, builder, Direction.ADD);
 
 
         FlowRuleOperations operations = builder.build(new FlowRuleOperationsContext() {
         FlowRuleOperations operations = builder.build(new FlowRuleOperationsContext() {
             @Override
             @Override
             public void onSuccess(FlowRuleOperations ops) {
             public void onSuccess(FlowRuleOperations ops) {
-                log.debug("Completed withdrawing: {}", data.key());
-                data.setState(WITHDRAWN);
-                data.setInstallables(Collections.emptyList());
-                store.write(data);
+                if (toInstall != null) {
+                    log.debug("Completed installing: {}", toInstall.key());
+                    //FIXME state depends on install.... we might want to pass this in.
+                    toInstall.setState(INSTALLED);
+                    store.write(toInstall);
+                } else if (toUninstall != null) {
+                    log.debug("Completed withdrawing: {}", toUninstall.key());
+                    //FIXME state depends on install.... we might want to pass this in.
+                    toUninstall.setState(WITHDRAWN);
+                    store.write(toUninstall);
+                }
             }
             }
 
 
             @Override
             @Override
             public void onError(FlowRuleOperations ops) {
             public void onError(FlowRuleOperations ops) {
-                log.warn("Failed withdraw: {}", data.key());
-                data.setState(FAILED);
-                store.write(data);
+                if (toInstall != null) {
+                    log.warn("Failed installation: {} {} on {}", toInstall.key(), toInstall.intent(), ops);
+                    //FIXME
+                    toInstall.setState(FAILED);
+                    store.write(toInstall);
+                }
+                // if toInstall was cause of error, then recompile (manage/increment counter, when exceeded -> CORRUPT)
+                // if toUninstall was cause of error, then CORRUPT (another job will lean this up)
             }
             }
         });
         });
 
 
         flowRuleService.apply(operations);
         flowRuleService.apply(operations);
     }
     }
+
 }
 }

@@ -38,16 +38,8 @@ public interface IntentProcessor {
     List<Intent> compile(Intent intent, List<Intent> previousInstallables);
     List<Intent> compile(Intent intent, List<Intent> previousInstallables);
 
 
     /**
     /**
-     * Installs an intent included in the specified intent data.
-     *
-     * @param data intent data containing an intent to be installed
-     */
-    void install(IntentData data);
-
-    /**
-     * Uninstalls an intent included in the specified intent data.
-     *
-     * @param data intent data containing an intent to be uninstalled
+     * @param toUninstall Intent data describing flows to uninstall. May be null.
+     * @param toInstall Intent data describing flows to install. May be null.
      */
      */
-    void uninstall(IntentData data);
+    void apply(IntentData toUninstall, IntentData toInstall);
 }
 }

@@ -50,7 +50,7 @@ final class Compiling implements IntentProcessPhase {
     public Optional<IntentProcessPhase> execute() {
     public Optional<IntentProcessPhase> execute() {
         try {
         try {
             data.setInstallables(processor.compile(data.intent(), null));
             data.setInstallables(processor.compile(data.intent(), null));
-            return Optional.of(new Installing(processor, data));
+            return Optional.of(new Installing(processor, data, null));
         } catch (IntentException e) {
         } catch (IntentException e) {
             log.debug("Unable to compile intent {} due to: {}", data.intent(), e);
             log.debug("Unable to compile intent {} due to: {}", data.intent(), e);
             return Optional.of(new CompileFailed(data));
             return Optional.of(new CompileFailed(data));

@@ -28,6 +28,7 @@ class Installing extends FinalIntentProcessPhase {
 
 
     private final IntentProcessor processor;
     private final IntentProcessor processor;
     private final IntentData data;
     private final IntentData data;
+    private final IntentData stored;
 
 
     /**
     /**
      * Create an installing phase.
      * Create an installing phase.
@@ -35,15 +36,16 @@ class Installing extends FinalIntentProcessPhase {
      * @param processor intent processor that does work for installing
      * @param processor intent processor that does work for installing
      * @param data      intent data containing an intent to be installed
      * @param data      intent data containing an intent to be installed
      */
      */
-    Installing(IntentProcessor processor, IntentData data) {
+    Installing(IntentProcessor processor, IntentData data, IntentData stored) {
         this.processor = checkNotNull(processor);
         this.processor = checkNotNull(processor);
         this.data = checkNotNull(data);
         this.data = checkNotNull(data);
         this.data.setState(INSTALLING);
         this.data.setState(INSTALLING);
+        this.stored = stored;
     }
     }
 
 
     @Override
     @Override
     public void preExecute() {
     public void preExecute() {
-        processor.install(data);
+        processor.apply(stored, data);
     }
     }
 
 
     @Override
     @Override

@@ -17,7 +17,10 @@ package org.onosproject.net.intent.impl.phase;
 
 
 import org.onosproject.net.intent.Intent;
 import org.onosproject.net.intent.Intent;
 import org.onosproject.net.intent.IntentData;
 import org.onosproject.net.intent.IntentData;
+import org.onosproject.net.intent.IntentException;
 import org.onosproject.net.intent.impl.IntentProcessor;
 import org.onosproject.net.intent.impl.IntentProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 
 import java.util.List;
 import java.util.List;
 import java.util.Optional;
 import java.util.Optional;
@@ -29,6 +32,8 @@ import static com.google.common.base.Preconditions.checkNotNull;
  */
  */
 class Recompiling implements IntentProcessPhase {
 class Recompiling implements IntentProcessPhase {
 
 
+    private static final Logger log = LoggerFactory.getLogger(Recompiling.class);
+
     private final IntentProcessor processor;
     private final IntentProcessor processor;
     private final IntentData data;
     private final IntentData data;
     private final IntentData stored;
     private final IntentData stored;
@@ -48,8 +53,14 @@ class Recompiling implements IntentProcessPhase {
 
 
     @Override
     @Override
     public Optional<IntentProcessPhase> execute() {
     public Optional<IntentProcessPhase> execute() {
-        List<Intent> compiled = processor.compile(data.intent(), stored.installables());
-        data.setInstallables(compiled);
-        return Optional.of(new Replacing(processor, data, stored));
+        try {
+            List<Intent> compiled = processor.compile(data.intent(), stored.installables());
+            data.setInstallables(compiled);
+            return Optional.of(new Installing(processor, data, stored));
+        } catch (IntentException e) {
+            log.debug("Unable to recompile intent {} due to: {}", data.intent(), e);
+            // FIXME we need to removed orphaned flows and deallocate resources
+            return Optional.of(new CompileFailed(data));
+        }
     }
     }
 }
 }

-/*
- * Copyright 2015 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.intent.impl.phase;
-
-import org.onosproject.net.intent.IntentData;
-import org.onosproject.net.intent.IntentException;
-import org.onosproject.net.intent.impl.IntentProcessor;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.util.Optional;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-
-/**
- * Represents a phase to replace an intent.
- */
-final class Replacing implements IntentProcessPhase {
-
-    private static final Logger log = LoggerFactory.getLogger(Replacing.class);
-
-    private final IntentProcessor processor;
-    private final IntentData data;
-    private final IntentData stored;
-
-    /**
-     * Creates a replacing phase.
-     *
-     * @param processor intent processor that does work for replacing
-     * @param data      intent data containing an intent to be replaced
-     * @param stored    intent data stored in the store
-     */
-    Replacing(IntentProcessor processor, IntentData data, IntentData stored) {
-        this.processor = checkNotNull(processor);
-        this.data = checkNotNull(data);
-        this.stored = checkNotNull(stored);
-    }
-
-    @Override
-    public Optional<IntentProcessPhase> execute() {
-        try {
-            processor.uninstall(stored);
-            return Optional.of(new Installing(processor, data));
-        } catch (IntentException e) {
-            log.warn("Unable to generate a FlowRuleOperations from intent {} due to:", data.intent().id(), e);
-            return Optional.of(new ReplaceFailed(data));
-        }
-    }
-}

@@ -44,7 +44,7 @@ class Withdrawing extends FinalIntentProcessPhase {
 
 
     @Override
     @Override
     protected void preExecute() {
     protected void preExecute() {
-        processor.uninstall(data);
+        processor.apply(data, null);
     }
     }
 
 
     @Override
     @Override

-/*
- * Copyright 2015 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.intent.impl.phase;
-
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.onosproject.TestApplicationId;
-import org.onosproject.core.ApplicationId;
-import org.onosproject.core.IdGenerator;
-import org.onosproject.net.ConnectPoint;
-import org.onosproject.net.DefaultLink;
-import org.onosproject.net.DefaultPath;
-import org.onosproject.net.Link;
-import org.onosproject.net.Path;
-import org.onosproject.net.flow.DefaultTrafficSelector;
-import org.onosproject.net.flow.DefaultTrafficTreatment;
-import org.onosproject.net.flow.TrafficSelector;
-import org.onosproject.net.flow.TrafficTreatment;
-import org.onosproject.net.intent.Intent;
-import org.onosproject.net.intent.IntentData;
-import org.onosproject.net.intent.MockIdGenerator;
-import org.onosproject.net.intent.PathIntent;
-import org.onosproject.net.intent.PointToPointIntent;
-import org.onosproject.net.intent.impl.IntentInstallationException;
-import org.onosproject.net.intent.impl.IntentProcessor;
-import org.onosproject.net.provider.ProviderId;
-import org.onosproject.store.Timestamp;
-
-import java.util.Arrays;
-import java.util.List;
-import java.util.Optional;
-
-import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.expectLastCall;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.verify;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertThat;
-import static org.onosproject.net.DeviceId.deviceId;
-import static org.onosproject.net.Link.Type.DIRECT;
-import static org.onosproject.net.PortNumber.portNumber;
-import static org.onosproject.net.intent.IntentState.INSTALLED;
-import static org.onosproject.net.intent.IntentState.INSTALL_REQ;
-
-/**
- * Unit tests for InstallingCoordinating phase.
- */
-public class ReplacingTest {
-
-    private final ApplicationId appId = new TestApplicationId("test");
-    private final ProviderId pid = new ProviderId("of", "test");
-    private final TrafficSelector selector = DefaultTrafficSelector.emptySelector();
-    private final TrafficTreatment treatment = DefaultTrafficTreatment.emptyTreatment();
-    private final ConnectPoint cp1 = new ConnectPoint(deviceId("1"), portNumber(1));
-    private final ConnectPoint cp2 = new ConnectPoint(deviceId("1"), portNumber(2));
-    private final ConnectPoint cp3 = new ConnectPoint(deviceId("2"), portNumber(1));
-    private final ConnectPoint cp4 = new ConnectPoint(deviceId("2"), portNumber(2));
-
-    private final List<Link> links = Arrays.asList(new DefaultLink(pid, cp2, cp4, DIRECT));
-    private final Path path = new DefaultPath(pid, links, 10);
-
-    private PointToPointIntent input;
-    private PathIntent compiled;
-
-    private IdGenerator idGenerator;
-    private IntentProcessor processor;
-    private Timestamp version;
-
-    @Before
-    public void setUp() {
-        processor = createMock(IntentProcessor.class);
-        version = createMock(Timestamp.class);
-
-        idGenerator = new MockIdGenerator();
-
-        Intent.bindIdGenerator(idGenerator);
-
-        // Intent creation should be placed after binding an ID generator
-        input = PointToPointIntent.builder()
-                .appId(appId)
-                .selector(selector)
-                .treatment(treatment)
-                .ingressPoint(cp1)
-                .egressPoint(cp3)
-                .build();
-        compiled = PathIntent.builder()
-                .appId(appId)
-                .selector(selector)
-                .treatment(treatment)
-                .path(path)
-                .build();
-    }
-
-
-    @After
-    public void tearDown() {
-        Intent.unbindIdGenerator(idGenerator);
-    }
-
-    /**
-     * Tests a next phase when no exception occurs.
-     */
-    @Test
-    public void testMoveToNextPhaseWithoutError() {
-        IntentData pending = new IntentData(input, INSTALL_REQ, version);
-        pending.setInstallables(Arrays.asList(compiled));
-
-        IntentData current = new IntentData(input, INSTALLED, version);
-        current.setInstallables(Arrays.asList(compiled));
-
-        processor.uninstall(current);
-        replay(processor);
-
-        Replacing sut = new Replacing(processor, pending, current);
-
-        Optional<IntentProcessPhase> executed = sut.execute();
-
-        verify(processor);
-        assertThat(executed.get(), is(instanceOf(Installing.class)));
-    }
-
-    /**
-     * Tests a next phase when IntentInstallationException occurs.
-     */
-    @Test
-    public void testWhenIntentInstallExceptionOccurs() {
-        IntentData pending = new IntentData(input, INSTALL_REQ, version);
-        pending.setInstallables(Arrays.asList(compiled));
-
-        IntentData current = new IntentData(input, INSTALLED, version);
-
-        processor.uninstall(current);
-        expectLastCall().andThrow(new IntentInstallationException());
-        replay(processor);
-
-        Replacing sut = new Replacing(processor, pending, current);
-
-        Optional<IntentProcessPhase> executed = sut.execute();
-
-        verify(processor);
-        assertThat(executed.get(), is(instanceOf(ReplaceFailed.class)));
-    }
-}