ONOS-2446: Implement API to declare resource hierarchy
Remove API to define resource boundary
(ResourceAdminService.defineResourceBoundary) to integrate with API for
resource hierarchy
Change-Id: Iffa28dec16320122fe41f4f455000596fa266acb
diff --git a/core/api/src/main/java/org/onosproject/net/newresource/ResourceAdminService.java b/core/api/src/main/java/org/onosproject/net/newresource/ResourceAdminService.java
index 5fcb132..1a13c32 100644
--- a/core/api/src/main/java/org/onosproject/net/newresource/ResourceAdminService.java
+++ b/core/api/src/main/java/org/onosproject/net/newresource/ResourceAdminService.java
@@ -17,7 +17,8 @@
import com.google.common.annotations.Beta;
-import java.util.function.Predicate;
+import java.util.Arrays;
+import java.util.List;
/**
* Service for administering resource service behavior.
@@ -25,13 +26,26 @@
@Beta
public interface ResourceAdminService {
/**
- * Define a boundary of the resource specified by the class.
- * The specified predicate is expected to return true if the supplied value is
- * in the resource boundary and return false if it is out of the boundary.
+ * Register resources as the children of the parent resource path.
*
- * @param cls class of the resource type
- * @param predicate predicate returning true if the value is in the boundary
- * @param <T> type of the resource
+ * @param parent parent resource path under which the resource are registered
+ * @param children resources to be registered as the children of the parent
+ * @param <T> type of resources
+ * @return true if registration is successfully done, false otherwise. Registration
+ * succeeds when each resource is not registered or unallocated.
*/
- <T> void defineResourceBoundary(Class<T> cls, Predicate<T> predicate);
+ default <T> boolean registerResources(ResourcePath parent, T... children) {
+ return registerResources(parent, Arrays.asList(children));
+ }
+
+ /**
+ * Register resources as the children of the parent resource path.
+ *
+ * @param parent parent resource path under which the resource are registered
+ * @param children resources to be registered as the children of the parent
+ * @param <T> type of resources
+ * @return true if registration is successfully done, false otherwise. Registration
+ * succeeds when each resource is not registered or unallocated.
+ */
+ <T> boolean registerResources(ResourcePath parent, List<T> children);
}
diff --git a/core/api/src/main/java/org/onosproject/net/newresource/ResourcePath.java b/core/api/src/main/java/org/onosproject/net/newresource/ResourcePath.java
index 6a7ab94..7c78a9a 100644
--- a/core/api/src/main/java/org/onosproject/net/newresource/ResourcePath.java
+++ b/core/api/src/main/java/org/onosproject/net/newresource/ResourcePath.java
@@ -24,7 +24,6 @@
import java.util.Objects;
import java.util.Optional;
-import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
/**
@@ -42,6 +41,16 @@
private final List<Object> resources;
+ public static final ResourcePath ROOT = new ResourcePath(ImmutableList.of());
+
+ public static ResourcePath child(ResourcePath parent, Object child) {
+ ImmutableList<Object> components = ImmutableList.builder()
+ .addAll(parent.components())
+ .add(child)
+ .build();
+ return new ResourcePath(components);
+ }
+
/**
* Creates an resource path from the specified components.
*
@@ -58,7 +67,6 @@
*/
public ResourcePath(List<Object> components) {
checkNotNull(components);
- checkArgument(components.size() > 0);
this.resources = ImmutableList.copyOf(components);
}
@@ -85,7 +93,7 @@
* If there is no parent, empty instance will be returned.
*/
public Optional<ResourcePath> parent() {
- if (resources.size() >= 2) {
+ if (resources.size() > 0) {
return Optional.of(new ResourcePath(resources.subList(0, resources.size() - 1)));
}
diff --git a/core/api/src/main/java/org/onosproject/net/newresource/ResourceStore.java b/core/api/src/main/java/org/onosproject/net/newresource/ResourceStore.java
index 7280b60..b711f39 100644
--- a/core/api/src/main/java/org/onosproject/net/newresource/ResourceStore.java
+++ b/core/api/src/main/java/org/onosproject/net/newresource/ResourceStore.java
@@ -11,6 +11,19 @@
*/
@Beta
public interface ResourceStore {
+
+ /**
+ * Registers the resources as children of the parent resource in transactional way.
+ * Resource registration is must be done before resource allocation. The state after completion
+ * of this method is all the resources are registered, or no resource is registered.
+ * The whole registration fails when any one of the resource can't be registered.
+ *
+ * @param parent resource which is the parent of the resource to be registered
+ * @param children resources to be registered
+ * @return true if the registration succeeds, false otherwise
+ */
+ boolean register(ResourcePath parent, List<ResourcePath> children);
+
/**
* Allocates the specified resources to the specified consumer in transactional way.
* The state after completion of this method is all the resources are allocated to the consumer,
diff --git a/core/api/src/test/java/org/onosproject/net/newresource/ResourcePathTest.java b/core/api/src/test/java/org/onosproject/net/newresource/ResourcePathTest.java
index 3034048..4a8886a 100644
--- a/core/api/src/test/java/org/onosproject/net/newresource/ResourcePathTest.java
+++ b/core/api/src/test/java/org/onosproject/net/newresource/ResourcePathTest.java
@@ -49,9 +49,11 @@
.testEquals();
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void testCreateWithZeroComponent() {
ResourcePath path = new ResourcePath();
+
+ assertThat(path, is(ResourcePath.ROOT));
}
@Test
@@ -66,7 +68,7 @@
public void testNoParent() {
ResourcePath path = new ResourcePath(LinkKey.linkKey(CP1_1, CP2_1));
- assertThat(path.parent(), is(Optional.empty()));
+ assertThat(path.parent(), is(Optional.of(ResourcePath.ROOT)));
}
@Test
diff --git a/core/net/src/main/java/org/onosproject/net/newresource/impl/ResourceManager.java b/core/net/src/main/java/org/onosproject/net/newresource/impl/ResourceManager.java
index 7ff6514..d2fe4a7 100644
--- a/core/net/src/main/java/org/onosproject/net/newresource/impl/ResourceManager.java
+++ b/core/net/src/main/java/org/onosproject/net/newresource/impl/ResourceManager.java
@@ -17,6 +17,7 @@
import com.google.common.annotations.Beta;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Reference;
import org.apache.felix.scr.annotations.ReferenceCardinality;
@@ -33,9 +34,6 @@
import java.util.Collection;
import java.util.List;
import java.util.Optional;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import java.util.function.Predicate;
import java.util.stream.Collectors;
import static com.google.common.base.Preconditions.checkNotNull;
@@ -48,8 +46,6 @@
@Beta
public final class ResourceManager implements ResourceService, ResourceAdminService {
- private final ConcurrentMap<Class<?>, Predicate<?>> boundaries = new ConcurrentHashMap<>();
-
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected ResourceStore store;
@@ -79,10 +75,6 @@
checkNotNull(consumer);
checkNotNull(resources);
- if (resources.stream().anyMatch(x -> !isValid(x))) {
- return ImmutableList.of();
- }
-
// TODO: implement support of resource hierarchy
// allocation for a particular resource implies allocations for all of the sub-resources need to be done
@@ -177,38 +169,8 @@
}
@Override
- public <T> void defineResourceBoundary(Class<T> cls, Predicate<T> predicate) {
- boundaries.put(cls, predicate);
- }
-
- /**
- * Returns the predicate associated with the specified resource.
- *
- * @param resource resource whose associated predicate is to be returned
- * @param <T> type of the resource
- * @return predicate associated with the resource
- * Null if the resource doesn't have an associated predicate.
- */
- @SuppressWarnings("unchecked")
- private <T> Predicate<T> lookupPredicate(T resource) {
- return (Predicate<T>) boundaries.get(resource.getClass());
- }
-
- /**
- * Returns if the specified resource is in the resource range.
- * E.g. VLAN ID against a link must be within 12 bit address space.
- *
- * @param resource resource to be checked if it is within the resource range
- * @return true if the resource within the range, false otherwise
- */
- boolean isValid(ResourcePath resource) {
- List<Object> flatten = resource.components();
- Object bottom = flatten.get(flatten.size() - 1);
- Predicate<Object> predicate = lookupPredicate(bottom);
- if (predicate == null) {
- return true;
- }
-
- return predicate.test(bottom);
+ public <T> boolean registerResources(ResourcePath parent, List<T> children) {
+ List<ResourcePath> resources = Lists.transform(children, x -> ResourcePath.child(parent, x));
+ return store.register(parent, resources);
}
}
diff --git a/core/net/src/test/java/org/onosproject/net/newresource/impl/ResourceManagerTest.java b/core/net/src/test/java/org/onosproject/net/newresource/impl/ResourceManagerTest.java
deleted file mode 100644
index ae6d103..0000000
--- a/core/net/src/test/java/org/onosproject/net/newresource/impl/ResourceManagerTest.java
+++ /dev/null
@@ -1,101 +0,0 @@
-/*
- * 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.newresource.impl;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.onlab.packet.VlanId;
-import org.onosproject.net.ConnectPoint;
-import org.onosproject.net.DeviceId;
-import org.onosproject.net.LinkKey;
-import org.onosproject.net.PortNumber;
-import org.onosproject.net.newresource.ResourcePath;
-
-import java.util.function.Predicate;
-
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertThat;
-
-/**
- * Unit tests for ResourceManager.
- */
-public class ResourceManagerTest {
-
- private static final DeviceId D1 = DeviceId.deviceId("of:001");
- private static final DeviceId D2 = DeviceId.deviceId("of:002");
- private static final PortNumber P1 = PortNumber.portNumber(1);
- private static final ConnectPoint CP1_1 = new ConnectPoint(D1, P1);
- private static final ConnectPoint CP2_1 = new ConnectPoint(D2, P1);
- private static final short VLAN_LOWER_LIMIT = 0;
- private static final short VLAN_UPPER_LIMIT = 1024;
-
- private final Predicate<VlanId> vlanPredicate =
- x -> x.toShort() >= VLAN_LOWER_LIMIT && x.toShort() < VLAN_UPPER_LIMIT;
- private ResourceManager manager;
-
- @Before
- public void setUp() {
- manager = new ResourceManager();
- }
-
- /**
- * Tests resource boundaries.
- */
- @Test
- public void testBoundary() {
- manager.defineResourceBoundary(VlanId.class, vlanPredicate);
-
- LinkKey linkKey = LinkKey.linkKey(CP1_1, CP2_1);
-
- assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) (VLAN_LOWER_LIMIT - 1)))),
- is(false));
-
- assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId(VLAN_LOWER_LIMIT))),
- is(true));
-
- assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) 100))),
- is(true));
-
- assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) (VLAN_UPPER_LIMIT - 1)))),
- is(true));
-
- assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId(VLAN_UPPER_LIMIT))),
- is(false));
- }
-
- /**
- * Tests the case that a boundary is not set.
- */
- @Test
- public void testWhenBoundaryNotSet() {
- LinkKey linkKey = LinkKey.linkKey(CP1_1, CP2_1);
-
- assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) (VLAN_LOWER_LIMIT - 1)))),
- is(true));
-
- assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId(VLAN_LOWER_LIMIT))),
- is(true));
-
- assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) 100))),
- is(true));
-
- assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) (VLAN_UPPER_LIMIT - 1)))),
- is(true));
-
- assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId(VLAN_UPPER_LIMIT))),
- is(true));
- }
-}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java b/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java
index 9c88c54..a20c2b7 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java
@@ -35,8 +35,12 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.Iterator;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -54,18 +58,25 @@
public class ConsistentResourceStore implements ResourceStore {
private static final Logger log = LoggerFactory.getLogger(ConsistentResourceStore.class);
- private static final String MAP_NAME = "onos-resource-consumers";
- private static final Serializer SERIALIZER = Serializer.using(KryoNamespaces.API);
+ private static final String CONSUMER_MAP = "onos-resource-consumers";
+ private static final String CHILD_MAP = "onos-resource-children";
+ private static final Serializer SERIALIZER = Serializer.using(
+ Arrays.asList(KryoNamespaces.BASIC, KryoNamespaces.API));
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected StorageService service;
- private ConsistentMap<ResourcePath, ResourceConsumer> consumers;
+ private ConsistentMap<ResourcePath, ResourceConsumer> consumerMap;
+ private ConsistentMap<ResourcePath, List<ResourcePath>> childMap;
@Activate
public void activate() {
- consumers = service.<ResourcePath, ResourceConsumer>consistentMapBuilder()
- .withName(MAP_NAME)
+ consumerMap = service.<ResourcePath, ResourceConsumer>consistentMapBuilder()
+ .withName(CONSUMER_MAP)
+ .withSerializer(SERIALIZER)
+ .build();
+ childMap = service.<ResourcePath, List<ResourcePath>>consistentMapBuilder()
+ .withName(CHILD_MAP)
.withSerializer(SERIALIZER)
.build();
}
@@ -74,7 +85,7 @@
public Optional<ResourceConsumer> getConsumer(ResourcePath resource) {
checkNotNull(resource);
- Versioned<ResourceConsumer> consumer = consumers.get(resource);
+ Versioned<ResourceConsumer> consumer = consumerMap.get(resource);
if (consumer == null) {
return Optional.empty();
}
@@ -83,6 +94,33 @@
}
@Override
+ public boolean register(ResourcePath resource, List<ResourcePath> children) {
+ checkNotNull(resource);
+ checkNotNull(children);
+
+ TransactionContext tx = service.transactionContextBuilder().build();
+ tx.begin();
+
+ try {
+ TransactionalMap<ResourcePath, List<ResourcePath>> childTxMap =
+ tx.getTransactionalMap(CHILD_MAP, SERIALIZER);
+
+ if (!isRegistered(childTxMap, resource)) {
+ return abortTransaction(tx);
+ }
+
+ if (!appendValue(childTxMap, resource, children)) {
+ return abortTransaction(tx);
+ }
+
+ return commitTransaction(tx);
+ } catch (TransactionException e) {
+ log.error("Exception thrown, abort the transaction", e);
+ return abortTransaction(tx);
+ }
+ }
+
+ @Override
public boolean allocate(List<ResourcePath> resources, ResourceConsumer consumer) {
checkNotNull(resources);
checkNotNull(consumer);
@@ -91,11 +129,18 @@
tx.begin();
try {
- TransactionalMap<ResourcePath, ResourceConsumer> txMap = tx.getTransactionalMap(MAP_NAME, SERIALIZER);
+ TransactionalMap<ResourcePath, List<ResourcePath>> childTxMap =
+ tx.getTransactionalMap(CHILD_MAP, SERIALIZER);
+ TransactionalMap<ResourcePath, ResourceConsumer> consumerTxMap =
+ tx.getTransactionalMap(CONSUMER_MAP, SERIALIZER);
+
for (ResourcePath resource: resources) {
- ResourceConsumer existing = txMap.putIfAbsent(resource, consumer);
- // if the resource is already allocated to another consumer, the whole allocation fails
- if (existing != null) {
+ if (!isRegistered(childTxMap, resource)) {
+ return abortTransaction(tx);
+ }
+
+ ResourceConsumer oldValue = consumerTxMap.put(resource, consumer);
+ if (oldValue != null) {
return abortTransaction(tx);
}
}
@@ -117,7 +162,8 @@
tx.begin();
try {
- TransactionalMap<ResourcePath, ResourceConsumer> txMap = tx.getTransactionalMap(MAP_NAME, SERIALIZER);
+ TransactionalMap<ResourcePath, ResourceConsumer> consumerTxMap =
+ tx.getTransactionalMap(CONSUMER_MAP, SERIALIZER);
Iterator<ResourcePath> resourceIte = resources.iterator();
Iterator<ResourceConsumer> consumerIte = consumers.iterator();
@@ -127,7 +173,7 @@
// if this single release fails (because the resource is allocated to another consumer,
// the whole release fails
- if (!txMap.remove(resource, consumer)) {
+ if (!consumerTxMap.remove(resource, consumer)) {
return abortTransaction(tx);
}
}
@@ -145,7 +191,7 @@
// NOTE: getting all entries may become performance bottleneck
// TODO: revisit for better backend data structure
- return consumers.entrySet().stream()
+ return consumerMap.entrySet().stream()
.filter(x -> x.getValue().value().equals(consumer))
.map(Map.Entry::getKey)
.collect(Collectors.toList());
@@ -156,12 +202,14 @@
checkNotNull(parent);
checkNotNull(cls);
- // NOTE: getting all entries may become performance bottleneck
- // TODO: revisit for better backend data structure
- return consumers.entrySet().stream()
- .filter(x -> x.getKey().parent().isPresent() && x.getKey().parent().get().equals(parent))
- .filter(x -> x.getKey().lastComponent().getClass() == cls)
- .map(Map.Entry::getKey)
+ Versioned<List<ResourcePath>> children = childMap.get(parent);
+ if (children == null) {
+ return Collections.emptyList();
+ }
+
+ return children.value().stream()
+ .filter(x -> x.lastComponent().getClass().equals(cls))
+ .filter(consumerMap::containsKey)
.collect(Collectors.toList());
}
@@ -186,4 +234,45 @@
tx.commit();
return true;
}
+
+ /**
+ * Appends the values to the existing values associated with the specified key.
+ *
+ * @param map map holding multiple values for a key
+ * @param key key specifying values
+ * @param values values to be appended
+ * @param <K> type of the key
+ * @param <V> type of the element of the list
+ * @return true if the operation succeeds, false otherwise.
+ */
+ private <K, V> boolean appendValue(TransactionalMap<K, List<V>> map, K key, List<V> values) {
+ List<V> oldValues = map.get(key);
+ List<V> newValues;
+ if (oldValues == null) {
+ newValues = new ArrayList<>(values);
+ } else {
+ LinkedHashSet<V> newSet = new LinkedHashSet<>(oldValues);
+ newSet.addAll(values);
+ newValues = new ArrayList<>(newSet);
+ }
+
+ return map.replace(key, oldValues, newValues);
+ }
+
+ /**
+ * Checks if the specified resource is registered as a child of a resource in the map.
+ *
+ * @param map map storing parent - child relationship of resources
+ * @param resource resource to be checked
+ * @return true if the resource is registered, false otherwise.
+ */
+ private boolean isRegistered(TransactionalMap<ResourcePath, List<ResourcePath>> map, ResourcePath resource) {
+ // root is always regarded to be registered
+ if (!resource.parent().isPresent()) {
+ return true;
+ }
+
+ List<ResourcePath> value = map.get(resource.parent().get());
+ return value != null && value.contains(resource);
+ }
}