ONOS-3725 Adding ability to retain pending configs.
This allows up-load of configurations before their backing classes are registered by apps/subsystems.
Validation and delegation of network config change events is deferred until the class registration.
Change-Id: Ifc9c97fbc86e764cb03cecb1f73f7191de3e7754
diff --git a/core/api/src/main/java/org/onosproject/net/config/NetworkConfigService.java b/core/api/src/main/java/org/onosproject/net/config/NetworkConfigService.java
index f1b22c4..c3760c6 100644
--- a/core/api/src/main/java/org/onosproject/net/config/NetworkConfigService.java
+++ b/core/api/src/main/java/org/onosproject/net/config/NetworkConfigService.java
@@ -135,6 +135,28 @@
JsonNode json);
/**
+ * Applies configuration for the specified subject and configuration
+ * key using the raw JSON object. If configuration already exists, it
+ * will be updated. If the specified configuration key does not yet have
+ * a registered class associated with it, the configuration will be pending
+ * and null value will be returned. Once the backing configuration class is
+ * registered, the configuration will be validated and accepted.
+ *
+ * @param subjectClassKey subject class key
+ * @param subject configuration subject
+ * @param configKey configuration class key
+ * @param json raw JSON node containing the configuration data
+ * @param <S> type of subject
+ * @param <C> type of configuration
+ * @return configuration object or null if configuration key does not have
+ * a registered class yet
+ * @throws IllegalArgumentException if the supplied JSON node contains
+ * invalid data
+ */
+ <S, C extends Config<S>> C applyConfig(String subjectClassKey, S subject,
+ String configKey, JsonNode json);
+
+ /**
* Clears any configuration for the specified subject and configuration
* class. If one does not exist, this call has no effect.
*
@@ -145,4 +167,13 @@
*/
<S, C extends Config<S>> void removeConfig(S subject, Class<C> configClass);
+ /**
+ * Clears any configuration for the specified subject and configuration
+ * key. If one does not exist, this call has no effect.
+ *
+ * @param subject configuration subject
+ * @param configKey configuration key
+ * @param <S> type of subject
+ */
+ <S> void removeConfig(String subjectClassKey, S subject, String configKey);
}
diff --git a/core/api/src/main/java/org/onosproject/net/config/NetworkConfigStore.java b/core/api/src/main/java/org/onosproject/net/config/NetworkConfigStore.java
index 9be4b12..b9d93a2 100644
--- a/core/api/src/main/java/org/onosproject/net/config/NetworkConfigStore.java
+++ b/core/api/src/main/java/org/onosproject/net/config/NetworkConfigStore.java
@@ -129,4 +129,26 @@
*/
<S, C extends Config<S>> void clearConfig(S subject, Class<C> configClass);
+ /**
+ * Queues pending configuration for the specified subject and configuration
+ * class using the raw JSON object.
+ *
+ * @param subject configuration subject
+ * @param configKey configuration key
+ * @param json raw JSON node containing the configuration data
+ * @param <S> type of subject
+ * @throws IllegalArgumentException if the supplied JSON node contains
+ * invalid data
+ */
+ <S> void queueConfig(S subject, String configKey, JsonNode json);
+
+ /**
+ * Clears the configuration of the given class for the specified subject.
+ *
+ * @param subject configuration subject
+ * @param configKey configuration key
+ * @param <S> type of subject
+ */
+ <S> void clearQueuedConfig(S subject, String configKey);
+
}
diff --git a/core/api/src/test/java/org/onosproject/net/config/NetworkConfigServiceAdapter.java b/core/api/src/test/java/org/onosproject/net/config/NetworkConfigServiceAdapter.java
index 7307258..562fe5c 100644
--- a/core/api/src/test/java/org/onosproject/net/config/NetworkConfigServiceAdapter.java
+++ b/core/api/src/test/java/org/onosproject/net/config/NetworkConfigServiceAdapter.java
@@ -74,17 +74,24 @@
}
@Override
+ public <S, C extends Config<S>> C applyConfig(String subjectClassKey, S subject, String configKey, JsonNode json) {
+ return null;
+ }
+
+ @Override
public <S, C extends Config<S>> void removeConfig(S subject, Class<C> configClass) {
}
@Override
- public void addListener(NetworkConfigListener listener) {
+ public <S> void removeConfig(String subjectClassKey, S subject, String configKey) {
+ }
+ @Override
+ public void addListener(NetworkConfigListener listener) {
}
@Override
public void removeListener(NetworkConfigListener listener) {
-
}
}
diff --git a/core/net/src/main/java/org/onosproject/net/config/impl/NetworkConfigManager.java b/core/net/src/main/java/org/onosproject/net/config/impl/NetworkConfigManager.java
index db484ee..b2a17a4 100644
--- a/core/net/src/main/java/org/onosproject/net/config/impl/NetworkConfigManager.java
+++ b/core/net/src/main/java/org/onosproject/net/config/impl/NetworkConfigManager.java
@@ -56,8 +56,11 @@
private static final String NULL_FACTORY_MSG = "Factory cannot be null";
private static final String NULL_SCLASS_MSG = "Subject class cannot be null";
+ private static final String NULL_SKEY_MSG = "Subject key cannot be null";
private static final String NULL_CCLASS_MSG = "Config class cannot be null";
+ private static final String NULL_CKEY_MSG = "Config key cannot be null";
private static final String NULL_SUBJECT_MSG = "Subject cannot be null";
+ private static final String NULL_JSON_MSG = "JSON cannot be null";
// Inventory of configuration factories
private final Map<ConfigKey, ConfigFactory> factories = Maps.newConcurrentMap();
@@ -156,6 +159,8 @@
@Override
public Class<? extends Config> getConfigClass(String subjectClassKey, String configKey) {
+ checkNotNull(subjectClassKey, NULL_SKEY_MSG);
+ checkNotNull(configKey, NULL_CKEY_MSG);
return configClasses.get(new ConfigIdentifier(subjectClassKey, configKey));
}
@@ -182,7 +187,7 @@
}
@Override
- public <S, T extends Config<S>> T getConfig(S subject, Class<T> configClass) {
+ public <S, C extends Config<S>> C getConfig(S subject, Class<C> configClass) {
checkNotNull(subject, NULL_SUBJECT_MSG);
checkNotNull(configClass, NULL_CCLASS_MSG);
return store.getConfig(subject, configClass);
@@ -200,16 +205,47 @@
public <S, C extends Config<S>> C applyConfig(S subject, Class<C> configClass, JsonNode json) {
checkNotNull(subject, NULL_SUBJECT_MSG);
checkNotNull(configClass, NULL_CCLASS_MSG);
+ checkNotNull(subject, NULL_JSON_MSG);
return store.applyConfig(subject, configClass, json);
}
@Override
+ @SuppressWarnings("unchecked")
+ public <S, C extends Config<S>> C applyConfig(String subjectClassKey, S subject,
+ String configKey, JsonNode json) {
+ checkNotNull(subjectClassKey, NULL_SKEY_MSG);
+ checkNotNull(subject, NULL_SUBJECT_MSG);
+ checkNotNull(configKey, NULL_CKEY_MSG);
+ checkNotNull(subject, NULL_JSON_MSG);
+ Class<? extends Config> configClass = configClasses.get(new ConfigIdentifier(subjectClassKey, configKey));
+ if (configClass != null) {
+ return store.applyConfig(subject, (Class<C>) configClass, json);
+ } else {
+ store.queueConfig(subject, configKey, json);
+ return null;
+ }
+ }
+
+ @Override
public <S, C extends Config<S>> void removeConfig(S subject, Class<C> configClass) {
checkNotNull(subject, NULL_SUBJECT_MSG);
checkNotNull(configClass, NULL_CCLASS_MSG);
store.clearConfig(subject, configClass);
}
+ @Override
+ public <S> void removeConfig(String subjectClassKey, S subject, String configKey) {
+ checkNotNull(subjectClassKey, NULL_SKEY_MSG);
+ checkNotNull(subject, NULL_SUBJECT_MSG);
+ checkNotNull(configKey, NULL_CCLASS_MSG);
+ Class<? extends Config> configClass = configClasses.get(new ConfigIdentifier(subjectClassKey, configKey));
+ if (configClass != null) {
+ store.clearConfig(subject, configClass);
+ } else {
+ store.clearQueuedConfig(subject, configKey);
+ }
+ }
+
// Auxiliary store delegate to receive notification about changes in
// the network configuration store state - by the store itself.
private class InternalStoreDelegate implements NetworkConfigStoreDelegate {
@@ -259,17 +295,17 @@
}
static final class ConfigIdentifier {
- final String subjectKey;
+ final String subjectClassKey;
final String configKey;
- protected ConfigIdentifier(String subjectKey, String configKey) {
- this.subjectKey = subjectKey;
+ protected ConfigIdentifier(String subjectClassKey, String configKey) {
+ this.subjectClassKey = subjectClassKey;
this.configKey = configKey;
}
@Override
public int hashCode() {
- return Objects.hash(subjectKey, configKey);
+ return Objects.hash(subjectClassKey, configKey);
}
@Override
@@ -279,10 +315,11 @@
}
if (obj instanceof ConfigIdentifier) {
final ConfigIdentifier other = (ConfigIdentifier) obj;
- return Objects.equals(this.subjectKey, other.subjectKey)
+ return Objects.equals(this.subjectClassKey, other.subjectClassKey)
&& Objects.equals(this.configKey, other.configKey);
}
return false;
}
}
+
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/config/impl/DistributedNetworkConfigStore.java b/core/store/dist/src/main/java/org/onosproject/store/config/impl/DistributedNetworkConfigStore.java
index 27c7912..d0a9920 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/config/impl/DistributedNetworkConfigStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/config/impl/DistributedNetworkConfigStore.java
@@ -29,7 +29,7 @@
import com.fasterxml.jackson.databind.node.TextNode;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
-
+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;
@@ -117,10 +117,35 @@
@Override
public void addConfigFactory(ConfigFactory configFactory) {
factoriesByConfig.put(configFactory.configClass().getName(), configFactory);
+ processPendingConfigs(configFactory);
notifyDelegate(new NetworkConfigEvent(CONFIG_REGISTERED, configFactory.configKey(),
configFactory.configClass()));
}
+ // Sweep through any pending configurations, validate them and then prune them.
+ private void processPendingConfigs(ConfigFactory configFactory) {
+ Set<ConfigKey> toBePruned = Sets.newHashSet();
+ configs.keySet().forEach(k -> {
+ if (Objects.equals(k.configKey, configFactory.configKey())) {
+ validateConfig(k, configFactory, configs.get(k).value());
+ toBePruned.add(k); // Prune whether valid or not
+ }
+ });
+ toBePruned.forEach(configs::remove);
+ }
+
+ @SuppressWarnings("unchecked")
+ private void validateConfig(ConfigKey key, ConfigFactory configFactory, JsonNode json) {
+ Config config = createConfig(key.subject, configFactory.configClass(), json);
+ try {
+ checkArgument(config.isValid(), INVALID_CONFIG_JSON);
+ configs.putAndGet(key(key.subject, configFactory.configClass()), json);
+ } catch (Exception e) {
+ log.warn("Failed to validate pending {} configuration for {}: {}",
+ key.configKey, configFactory.subjectFactory().subjectKey(key.subject), json);
+ }
+ }
+
@Override
public void removeConfigFactory(ConfigFactory configFactory) {
factoriesByConfig.remove(configFactory.configClass().getName());
@@ -152,7 +177,7 @@
ImmutableSet.Builder<S> builder = ImmutableSet.builder();
String cName = configClass.getName();
configs.keySet().forEach(k -> {
- if (subjectClass.isInstance(k.subject) && cName.equals(k.configClass)) {
+ if (subjectClass.isInstance(k.subject) && Objects.equals(cName, k.configClass)) {
builder.add((S) k.subject);
}
});
@@ -164,7 +189,7 @@
public <S> Set<Class<? extends Config<S>>> getConfigClasses(S subject) {
ImmutableSet.Builder<Class<? extends Config<S>>> builder = ImmutableSet.builder();
configs.keySet().forEach(k -> {
- if (Objects.equals(subject, k.subject) && delegate != null) {
+ if (Objects.equals(subject, k.subject) && k.configClass != null && delegate != null) {
builder.add(factoriesByConfig.get(k.configClass).configClass());
}
});
@@ -201,8 +226,12 @@
// Re-create the config if for some reason what we attempted to put
// was supplanted by someone else already.
- return versioned.value() == json ? config :
- createConfig(subject, configClass, versioned.value());
+ return versioned.value() == json ? config : createConfig(subject, configClass, versioned.value());
+ }
+
+ @Override
+ public <S> void queueConfig(S subject, String configKey, JsonNode json) {
+ configs.put(key(subject, configKey), json);
}
@Override
@@ -210,6 +239,11 @@
configs.remove(key(subject, configClass));
}
+ @Override
+ public <S> void clearQueuedConfig(S subject, String configKey) {
+ configs.remove(key(subject, configKey));
+ }
+
/**
* Produces a config from the specified subject, config class and raw JSON.
*
@@ -248,19 +282,35 @@
return new ConfigKey(subject, configClass);
}
+ // Produces a key for uniquely tracking a subject config.
+ private static ConfigKey key(Object subject, String configKey) {
+ return new ConfigKey(subject, configKey);
+ }
+
// Auxiliary key to track subject configurations.
+ // Keys with non-null configKey are pending configurations.
private static final class ConfigKey {
final Object subject;
+ final String configKey;
final String configClass;
+ // Create a key for pending configuration class
+ private ConfigKey(Object subject, String configKey) {
+ this.subject = subject;
+ this.configKey = configKey;
+ this.configClass = null;
+ }
+
+ // Create a key for registered class configuration
private ConfigKey(Object subject, Class<?> configClass) {
this.subject = subject;
+ this.configKey = null;
this.configClass = configClass.getName();
}
@Override
public int hashCode() {
- return Objects.hash(subject, configClass);
+ return Objects.hash(subject, configKey, configClass);
}
@Override
@@ -271,6 +321,7 @@
if (obj instanceof ConfigKey) {
final ConfigKey other = (ConfigKey) obj;
return Objects.equals(this.subject, other.subject)
+ && Objects.equals(this.configKey, other.configKey)
&& Objects.equals(this.configClass, other.configClass);
}
return false;
@@ -280,6 +331,11 @@
private class InternalMapListener implements MapEventListener<ConfigKey, JsonNode> {
@Override
public void event(MapEvent<ConfigKey, JsonNode> event) {
+ // Do not delegate pending configs.
+ if (event.key().configClass == null) {
+ return;
+ }
+
NetworkConfigEvent.Type type;
switch (event.type()) {
case INSERT:
diff --git a/core/store/dist/src/test/java/org/onosproject/store/config/impl/DistributedNetworkConfigStoreTest.java b/core/store/dist/src/test/java/org/onosproject/store/config/impl/DistributedNetworkConfigStoreTest.java
index 06fe7b3..4a399b5 100644
--- a/core/store/dist/src/test/java/org/onosproject/store/config/impl/DistributedNetworkConfigStoreTest.java
+++ b/core/store/dist/src/test/java/org/onosproject/store/config/impl/DistributedNetworkConfigStoreTest.java
@@ -62,9 +62,8 @@
* Config factory class for testing.
*/
public class MockConfigFactory extends ConfigFactory<String, BasicConfig> {
- protected MockConfigFactory(SubjectFactory<String> subjectFactory,
- Class<BasicConfig> configClass, String configKey) {
- super(subjectFactory, configClass, configKey);
+ protected MockConfigFactory(Class<BasicConfig> configClass, String configKey) {
+ super(new MockSubjectFactory("strings"), configClass, configKey);
}
@Override
public BasicConfig createConfig() {
@@ -73,11 +72,25 @@
}
/**
+ * Subject factory class for testing.
+ */
+ public class MockSubjectFactory extends SubjectFactory<String> {
+ protected MockSubjectFactory(String subjectClassKey) {
+ super(String.class, subjectClassKey);
+ }
+
+ @Override
+ public String createSubject(String subjectKey) {
+ return subjectKey;
+ }
+ }
+
+ /**
* Tests creation, query and removal of a config.
*/
@Test
public void testCreateConfig() {
- configStore.addConfigFactory(new MockConfigFactory(null, BasicConfig.class, "config1"));
+ configStore.addConfigFactory(new MockConfigFactory(BasicConfig.class, "config1"));
configStore.createConfig("config1", BasicConfig.class);
assertThat(configStore.getConfigClasses("config1"), hasSize(1));
@@ -101,7 +114,7 @@
*/
@Test
public void testCreateFactory() {
- MockConfigFactory mockFactory = new MockConfigFactory(null, BasicConfig.class, "config1");
+ MockConfigFactory mockFactory = new MockConfigFactory(BasicConfig.class, "config1");
assertThat(configStore.getConfigFactory(BasicConfig.class), nullValue());
@@ -117,7 +130,7 @@
*/
@Test
public void testApplyConfig() {
- configStore.addConfigFactory(new MockConfigFactory(null, BasicConfig.class, "config1"));
+ configStore.addConfigFactory(new MockConfigFactory(BasicConfig.class, "config1"));
configStore.applyConfig("config1", BasicConfig.class, new ObjectMapper().createObjectNode());
assertThat(configStore.getConfigClasses("config1"), hasSize(1));
diff --git a/web/api/src/main/java/org/onosproject/rest/resources/NetworkConfigWebResource.java b/web/api/src/main/java/org/onosproject/rest/resources/NetworkConfigWebResource.java
index 9e474fd..9e301f5 100644
--- a/web/api/src/main/java/org/onosproject/rest/resources/NetworkConfigWebResource.java
+++ b/web/api/src/main/java/org/onosproject/rest/resources/NetworkConfigWebResource.java
@@ -264,8 +264,9 @@
InputStream request) throws IOException {
NetworkConfigService service = get(NetworkConfigService.class);
ObjectNode root = (ObjectNode) mapper().readTree(request);
- service.applyConfig(service.getSubjectFactory(subjectClassKey).createSubject(subjectKey),
- service.getConfigClass(subjectClassKey, configKey), root);
+ service.applyConfig(subjectClassKey,
+ service.getSubjectFactory(subjectClassKey).createSubject(subjectKey),
+ configKey, root);
return Response.ok().build();
}
@@ -279,13 +280,14 @@
private void consumeSubjectJson(NetworkConfigService service,
ObjectNode subjectNode, Object subject,
- String subjectKey) {
- subjectNode.fieldNames().forEachRemaining(c ->
- service.applyConfig(subject, service.getConfigClass(subjectKey, c),
- subjectNode.path(c)));
+ String subjectClassKey) {
+ subjectNode.fieldNames().forEachRemaining(configKey ->
+ service.applyConfig(subjectClassKey, subject, configKey, subjectNode.path(configKey)));
}
+ // FIXME: Refactor to allow queued configs to be removed
+
/**
* Clear entire network configuration base.
*
diff --git a/web/api/src/test/java/org/onosproject/rest/resources/NetworkConfigWebResourceTest.java b/web/api/src/test/java/org/onosproject/rest/resources/NetworkConfigWebResourceTest.java
index 40470d9..a983fc2 100644
--- a/web/api/src/test/java/org/onosproject/rest/resources/NetworkConfigWebResourceTest.java
+++ b/web/api/src/test/java/org/onosproject/rest/resources/NetworkConfigWebResourceTest.java
@@ -56,6 +56,7 @@
*/
public class NetworkConfigWebResourceTest extends ResourceTest {
+
MockNetworkConfigService mockNetworkConfigService;
public class MockDeviceConfig extends Config<Device> {
@@ -201,7 +202,7 @@
mockNetworkConfigService = new MockNetworkConfigService();
ServiceDirectory testDirectory =
new TestServiceDirectory()
- .add(NetworkConfigService.class, mockNetworkConfigService);
+ .add(NetworkConfigService.class, mockNetworkConfigService);
BaseResource.setServiceDirectory(testDirectory);
}