Remove dependency on LinkResourceService from MplsPathIntentCompiler

In addition, revise test to make the situation more realistic Change-Id: If23648b77f2eddbe27873711dab3b196a4a2729d

Remove dependency on LinkResourceService from MplsPathIntentCompiler
In addition, revise test to make the situation more realistic Change-Id: If23648b77f2eddbe27873711dab3b196a4a2729d
Sho SHIMIZU · Ray Milkey
Commit f26e692cf310a8ebfc2e681b5d87f594e6cb8092 f26e692c 1 parent e39f49a1
Showing 3 changed files with 160 additions and 66 deletions
core/net/src/main/java/org/onosproject/net/intent/impl/compiler/MplsPathIntentCompiler.java
core/net/src/test/java/org/onosproject/net/intent/impl/compiler/MockResourceService.java
core/net/src/test/java/org/onosproject/net/intent/impl/compiler/MplsPathIntentCompilerTest.java
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/MplsPathIntentCompiler.java
View file @f26e692
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/MplsPathIntentCompiler.java
View file @f26e692
@@ -24,12 +24,14 @@ import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.onlab.packet.EthType;
 import org.onlab.packet.Ethernet;
+import org.onlab.packet.MplsLabel;
 import org.onlab.packet.VlanId;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Link;
+import org.onosproject.net.LinkKey;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.flow.DefaultFlowRule;
 import org.onosproject.net.flow.DefaultTrafficSelector;
@@ -46,17 +48,9 @@ import org.onosproject.net.intent.Intent;
 import org.onosproject.net.intent.IntentCompiler;
 import org.onosproject.net.intent.IntentExtensionService;
 import org.onosproject.net.intent.MplsPathIntent;
-import org.onosproject.net.link.LinkStore;
+import org.onosproject.net.newresource.ResourcePath;
-import org.onosproject.net.resource.ResourceRequest;
+import org.onosproject.net.newresource.ResourceService;
-import org.onosproject.net.resource.link.DefaultLinkResourceRequest;
 import org.onosproject.net.resource.link.LinkResourceAllocations;
-import org.onosproject.net.resource.link.LinkResourceRequest;
-import org.onosproject.net.resource.link.LinkResourceService;
-import org.onosproject.net.resource.link.MplsLabel;
-import org.onosproject.net.resource.link.MplsLabelResourceAllocation;
-import org.onosproject.net.resource.ResourceAllocation;
-import org.onosproject.net.resource.ResourceType;
-import org.onosproject.net.resource.link.MplsLabelResourceRequest;
 import org.slf4j.Logger;
 import java.util.Collections;
@@ -67,9 +61,10 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.stream.StreamSupport;
+import java.util.stream.Collectors;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.onosproject.net.LinkKey.linkKey;
 import static org.slf4j.LoggerFactory.getLogger;
 @Component(immediate = true)
@@ -84,18 +79,15 @@ public class MplsPathIntentCompiler implements IntentCompiler<MplsPathIntent> {
     protected CoreService coreService;
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
-    protected LinkResourceService resourceService;
+    protected ResourceService resourceService;
-
-    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
-    protected LinkStore linkStore;
     protected ApplicationId appId;
     @Override
     public List<Intent> compile(MplsPathIntent intent, List<Intent> installable,
                                 Set<LinkResourceAllocations> resources) {
-        LinkResourceAllocations allocations = assignMplsLabel(intent);
+        Map<LinkKey, MplsLabel> labels = assignMplsLabel(intent);
-        List<FlowRule> rules = generateRules(intent, allocations);
+        List<FlowRule> rules = generateRules(intent, labels);
         return Collections.singletonList(new FlowRuleIntent(appId, rules, intent.resources()));
     }
@@ -111,30 +103,38 @@ public class MplsPathIntentCompiler implements IntentCompiler<MplsPathIntent> {
         intentExtensionService.unregisterCompiler(MplsPathIntent.class);
     }
-    private LinkResourceAllocations assignMplsLabel(MplsPathIntent intent) {
+    private Map<LinkKey, MplsLabel> assignMplsLabel(MplsPathIntent intent) {
         // TODO: do it better... Suggestions?
-        Set<Link> linkRequest = Sets.newHashSetWithExpectedSize(intent.path()
+        Set<LinkKey> linkRequest = Sets.newHashSetWithExpectedSize(intent.path()
                 .links().size() - 2);
         for (int i = 1; i <= intent.path().links().size() - 2; i++) {
-            Link link = intent.path().links().get(i);
+            LinkKey link = linkKey(intent.path().links().get(i));
             linkRequest.add(link);
             // add the inverse link. I want that the label is reserved both for
             // the direct and inverse link
-            linkRequest.add(linkStore.getLink(link.dst(), link.src()));
+            linkRequest.add(linkKey(link.dst(), link.src()));
         }
-        Map<Link, MplsLabel> labels = findMplsLabels(linkRequest);
+        Map<LinkKey, MplsLabel> labels = findMplsLabels(linkRequest);
+        if (labels.isEmpty()) {
+            return Collections.emptyMap();
+        }
-        LinkResourceRequest request = DefaultLinkResourceRequest
+        List<ResourcePath> resources = labels.entrySet().stream()
-                .builder(intent.id(), linkRequest)
+                .map(x -> new ResourcePath(linkKey(x.getKey().src(), x.getKey().src()), x.getValue()))
-                .addMplsRequest(labels)
+                .collect(Collectors.toList());
-                .build();
+        List<org.onosproject.net.newresource.ResourceAllocation> allocations =
-        return resourceService.requestResources(request);
+                resourceService.allocate(intent.id(), resources);
+        if (allocations.isEmpty()) {
+            Collections.emptyMap();
+        }
+
+        return labels;
     }
-    private Map<Link, MplsLabel> findMplsLabels(Set<Link> links) {
+    private Map<LinkKey, MplsLabel> findMplsLabels(Set<LinkKey> links) {
-        Map<Link, MplsLabel> labels = new HashMap<>();
+        Map<LinkKey, MplsLabel> labels = new HashMap<>();
-        for (Link link : links) {
+        for (LinkKey link : links) {
             Optional<MplsLabel> label = findMplsLabel(link);
             if (label.isPresent()) {
                 labels.put(link, label.get());
@@ -144,29 +144,19 @@ public class MplsPathIntentCompiler implements IntentCompiler<MplsPathIntent> {
         return labels;
     }
-    private Optional<MplsLabel> findMplsLabel(Link link) {
+    private Optional<MplsLabel> findMplsLabel(LinkKey link) {
-        Iterable<ResourceRequest> freeLabels = resourceService.getAvailableResources(link);
+        return resourceService.getAvailableResources(new ResourcePath(link)).stream()
-        return StreamSupport.stream(freeLabels.spliterator(), false)
+                .filter(x -> x.lastComponent() instanceof MplsLabel)
-                .filter(x -> x instanceof MplsLabelResourceRequest)
+                .map(x -> (MplsLabel) x.lastComponent())
-                .map(x -> (MplsLabelResourceRequest) x)
-                .map(MplsLabelResourceRequest::mplsLabel)
                 .findFirst();
     }
-    private MplsLabel getMplsLabel(LinkResourceAllocations allocations, Link link) {
+    private MplsLabel getMplsLabel(Map<LinkKey, MplsLabel> labels, LinkKey link) {
-        for (ResourceAllocation allocation : allocations
+        return labels.get(link);
-                .getResourceAllocation(link)) {
-            if (allocation.type() == ResourceType.MPLS_LABEL) {
-                return ((MplsLabelResourceAllocation) allocation).mplsLabel();
-
-            }
-        }
-        log.warn("MPLS label was not assigned successfully");
-        return null;
     }
     private List<FlowRule> generateRules(MplsPathIntent intent,
-                                         LinkResourceAllocations allocations) {
+                                         Map<LinkKey, MplsLabel> labels) {
         Iterator<Link> links = intent.path().links().iterator();
         Link srcLink = links.next();
@@ -178,7 +168,7 @@ public class MplsPathIntentCompiler implements IntentCompiler<MplsPathIntent> {
         // Ingress traffic
         // Get the new MPLS label
-        MplsLabel mpls = getMplsLabel(allocations, link);
+        MplsLabel mpls = getMplsLabel(labels, linkKey(link));
         checkNotNull(mpls);
         MplsLabel prevLabel = mpls;
         rules.add(ingressFlow(prev.port(), link, intent, mpls));
@@ -192,7 +182,7 @@ public class MplsPathIntentCompiler implements IntentCompiler<MplsPathIntent> {
             if (links.hasNext()) {
                 // Transit traffic
                 // Get the new MPLS label
-                mpls = getMplsLabel(allocations, link);
+                mpls = getMplsLabel(labels, linkKey(link));
                 checkNotNull(mpls);
                 rules.add(transitFlow(prev.port(), link, intent,
                         prevLabel, mpls));
@@ -210,7 +200,8 @@ public class MplsPathIntentCompiler implements IntentCompiler<MplsPathIntent> {
     }
     private FlowRule ingressFlow(PortNumber inPort, Link link,
-                                 MplsPathIntent intent, MplsLabel label) {
+                                 MplsPathIntent intent,
+                                 MplsLabel label) {
         TrafficSelector.Builder ingressSelector = DefaultTrafficSelector
                 .builder(intent.selector());
@@ -222,10 +213,10 @@ public class MplsPathIntentCompiler implements IntentCompiler<MplsPathIntent> {
                     .matchMplsLabel(intent.ingressLabel().get());
             // Swap the MPLS label
-            treat.setMpls(label.label());
+            treat.setMpls(label);
         } else {
             // Push and set the MPLS label
-            treat.pushMpls().setMpls(label.label());
+            treat.pushMpls().setMpls(label);
         }
         // Add the output action
         treat.setOutput(link.src().port());
@@ -234,21 +225,21 @@ public class MplsPathIntentCompiler implements IntentCompiler<MplsPathIntent> {
     }
     private FlowRule transitFlow(PortNumber inPort, Link link,
-                                          MplsPathIntent intent,
+                                 MplsPathIntent intent,
-                                          MplsLabel prevLabel,
+                                 MplsLabel prevLabel,
-                                          MplsLabel outLabel) {
+                                 MplsLabel outLabel) {
         // Ignore the ingress Traffic Selector and use only the MPLS label
         // assigned in the previous link
         TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
         selector.matchInPort(inPort).matchEthType(Ethernet.MPLS_UNICAST)
-                .matchMplsLabel(prevLabel.label());
+                .matchMplsLabel(prevLabel);
         TrafficTreatment.Builder treat = DefaultTrafficTreatment.builder();
         // Set the new label only if the label on the packet is
         // different
         if (!prevLabel.equals(outLabel)) {
-            treat.setMpls(outLabel.label());
+            treat.setMpls(outLabel);
         }
         treat.setOutput(link.src().port());
@@ -256,14 +247,14 @@ public class MplsPathIntentCompiler implements IntentCompiler<MplsPathIntent> {
     }
     private FlowRule egressFlow(PortNumber inPort, Link link,
-                                         MplsPathIntent intent,
+                                MplsPathIntent intent,
-                                         MplsLabel prevLabel) {
+                                MplsLabel prevLabel) {
         // egress point: either set the egress MPLS label or pop the
         // MPLS label based on the intent annotations
         TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
         selector.matchInPort(inPort).matchEthType(Ethernet.MPLS_UNICAST)
-                .matchMplsLabel(prevLabel.label());
+                .matchMplsLabel(prevLabel);
         // apply the intent's treatments
         TrafficTreatment.Builder treat = DefaultTrafficTreatment.builder(intent
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/MockResourceService.java 0 → 100644
View file @f26e692
+++ b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/MockResourceService.java 0 → 100644
View file @f26e692
+/*
+ * 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.compiler;
+
+import com.google.common.collect.ImmutableList;
+import org.onlab.packet.MplsLabel;
+import org.onosproject.net.newresource.ResourceAllocation;
+import org.onosproject.net.newresource.ResourceConsumer;
+import org.onosproject.net.newresource.ResourcePath;
+import org.onosproject.net.newresource.ResourceService;
+
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+class MockResourceService implements ResourceService {
+
+    private final Map<ResourcePath, ResourceConsumer> assignment = new HashMap<>();
+
+    @Override
+    public List<ResourceAllocation> allocate(ResourceConsumer consumer, List<ResourcePath> resources) {
+        assignment.putAll(
+                resources.stream().collect(Collectors.toMap(x -> x, x -> consumer))
+        );
+
+        return resources.stream()
+                .map(x -> new ResourceAllocation(x, consumer))
+                .collect(Collectors.toList());
+    }
+
+    @Override
+    public boolean release(List<ResourceAllocation> allocations) {
+        allocations.forEach(x -> assignment.remove(x.resource()));
+
+        return true;
+    }
+
+    @Override
+    public boolean release(ResourceConsumer consumer) {
+        List<ResourcePath> resources = assignment.entrySet().stream()
+                .filter(x -> x.getValue().equals(consumer))
+                .map(Map.Entry::getKey)
+                .collect(Collectors.toList());
+        List<ResourceAllocation> allocations = resources.stream()
+                .map(x -> new ResourceAllocation(x, consumer))
+                .collect(Collectors.toList());
+
+        return release(allocations);
+    }
+
+    @Override
+    public Optional<ResourceAllocation> getResourceAllocation(ResourcePath resource) {
+        return Optional.ofNullable(assignment.get(resource))
+                .map(x -> new ResourceAllocation(resource, x));
+    }
+
+    @Override
+    public <T> Collection<ResourceAllocation> getResourceAllocations(ResourcePath parent, Class<T> cls) {
+        return assignment.entrySet().stream()
+                .filter(x -> x.getKey().parent().isPresent())
+                .filter(x -> x.getKey().parent().get().equals(parent))
+                .map(x -> new ResourceAllocation(x.getKey(), x.getValue()))
+                .collect(Collectors.toList());
+    }
+
+    @Override
+    public Collection<ResourceAllocation> getResourceAllocations(ResourceConsumer consumer) {
+        return assignment.entrySet().stream()
+                .filter(x -> x.getValue().equals(consumer))
+                .map(x -> new ResourceAllocation(x.getKey(), x.getValue()))
+                .collect(Collectors.toList());
+    }
+
+    @Override
+    public Collection<ResourcePath> getAvailableResources(ResourcePath parent) {
+        ResourcePath resource = ResourcePath.child(parent, MplsLabel.mplsLabel(10));
+        return ImmutableList.of(resource);
+    }
+
+    @Override
+    public boolean isAvailable(ResourcePath resource) {
+        return true;
+    }
+}
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/MplsPathIntentCompilerTest.java
View file @f26e692
+++ b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/MplsPathIntentCompilerTest.java
View file @f26e692
@@ -41,10 +41,8 @@ import org.onosproject.net.flow.TrafficTreatment;
 import org.onosproject.net.intent.FlowRuleIntent;
 import org.onosproject.net.intent.Intent;
 import org.onosproject.net.intent.IntentExtensionService;
-import org.onosproject.net.intent.IntentTestsMocks;
 import org.onosproject.net.intent.MockIdGenerator;
 import org.onosproject.net.intent.MplsPathIntent;
-import org.onosproject.store.trivial.SimpleLinkStore;
 import static org.easymock.EasyMock.createMock;
 import static org.easymock.EasyMock.expect;
@@ -52,6 +50,7 @@ import static org.easymock.EasyMock.replay;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
+import static org.onosproject.net.DefaultEdgeLink.createEdgeLink;
 import static org.onosproject.net.Link.Type.DIRECT;
 import static org.onosproject.net.NetTestTools.APP_ID;
 import static org.onosproject.net.NetTestTools.PID;
@@ -61,10 +60,12 @@ public class MplsPathIntentCompilerTest {
     private final ApplicationId appId = new TestApplicationId("test");
+    private final ConnectPoint d1pi = connectPoint("s1", 100);
     private final ConnectPoint d1p1 = connectPoint("s1", 0);
     private final ConnectPoint d2p0 = connectPoint("s2", 0);
     private final ConnectPoint d2p1 = connectPoint("s2", 1);
     private final ConnectPoint d3p1 = connectPoint("s3", 1);
+    private final ConnectPoint d3pe = connectPoint("s3", 100);
     private final TrafficSelector selector = DefaultTrafficSelector.builder().build();
     private final TrafficTreatment treatment = DefaultTrafficTreatment.builder().build();
@@ -75,8 +76,10 @@ public class MplsPathIntentCompilerTest {
             Optional.of(MplsLabel.mplsLabel(20));
     private final List<Link> links = Arrays.asList(
+            createEdgeLink(d1pi, true),
             new DefaultLink(PID, d1p1, d2p0, DIRECT),
-            new DefaultLink(PID, d2p1, d3p1, DIRECT)
+            new DefaultLink(PID, d2p1, d3p1, DIRECT),
+            createEdgeLink(d3pe, false)
     );
     private IdGenerator idGenerator = new MockIdGenerator();
@@ -92,8 +95,7 @@ public class MplsPathIntentCompilerTest {
         expect(coreService.registerApplication("org.onosproject.net.intent"))
                 .andReturn(appId);
         sut.coreService = coreService;
-        sut.linkStore = new SimpleLinkStore();
+        sut.resourceService = new MockResourceService();
-        sut.resourceService = new IntentTestsMocks.MockResourceService();
         Intent.bindIdGenerator(idGenerator);
@@ -128,7 +130,7 @@ public class MplsPathIntentCompilerTest {
         assertThat(compiled, hasSize(1));
         Collection<FlowRule> rules = ((FlowRuleIntent) compiled.get(0)).flowRules();
-        assertThat(rules, hasSize(1));
+        assertThat(rules, hasSize(3));
         FlowRule rule = rules.stream()
                 .filter(x -> x.deviceId().equals(d2p0.deviceId()))
@@ -139,4 +141,5 @@ public class MplsPathIntentCompilerTest {
         sut.deactivate();
     }
+
 }