Port Annotation bug fix
1. Handled the case when InternalNetworkConfigListener in DeviceManager recieves an event associated with PortAnotationConfig class.
2. Added CONFIG_REMOVED event type in InternalNetworkConfigListener in DeviceManager.
3. Changed comine function in PortAnnotationOperator to take care of removing old annotations from PortDescription which are not in current
PortAnnotationConfig.
Tested using 'annotate-ports' command and 'ports' command
Change-Id: Ie4d2b529c2f559a40a296d916193318e0ccc7b93
diff --git a/apps/optical-model/src/main/java/org/onosproject/net/optical/config/OpticalPortOperator.java b/apps/optical-model/src/main/java/org/onosproject/net/optical/config/OpticalPortOperator.java
index aad6d58..756c25e 100644
--- a/apps/optical-model/src/main/java/org/onosproject/net/optical/config/OpticalPortOperator.java
+++ b/apps/optical-model/src/main/java/org/onosproject/net/optical/config/OpticalPortOperator.java
@@ -17,11 +17,13 @@
import static org.slf4j.LoggerFactory.getLogger;
+import java.util.Optional;
import java.util.Set;
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkNotNull;
+import org.onosproject.net.config.Config;
import org.onosproject.net.config.NetworkConfigService;
import org.onosproject.net.config.PortConfigOperator;
import org.onosproject.net.AnnotationKeys;
@@ -116,6 +118,12 @@
return updateDescription(number, annotations, descr);
}
+ @Override
+ public PortDescription combine(ConnectPoint cp, PortDescription descr, Optional<Config> prevConf) {
+ return combine(cp, descr);
+ }
+
+
// updates a port description whose port type has not changed.
/**
* Updates {@link PortDescription} using specified number and annotations.
diff --git a/core/api/src/main/java/org/onosproject/net/config/PortConfigOperator.java b/core/api/src/main/java/org/onosproject/net/config/PortConfigOperator.java
index fd03880..3bcae82 100644
--- a/core/api/src/main/java/org/onosproject/net/config/PortConfigOperator.java
+++ b/core/api/src/main/java/org/onosproject/net/config/PortConfigOperator.java
@@ -21,6 +21,8 @@
import com.google.common.annotations.Beta;
+import java.util.Optional;
+
/**
* {@link ConfigOperator} for Port.
* <p>
@@ -36,7 +38,6 @@
*/
void bindService(NetworkConfigService networkConfigService);
-
/**
* Generates a PortDescription containing fields from a PortDescription and
* configuration.
@@ -44,13 +45,26 @@
* @param cp {@link ConnectPoint} representing the port.
* @param descr input {@link PortDescription}
* @return Combined {@link PortDescription}
+ * @deprecated onos-2.0
*/
+ @Deprecated
PortDescription combine(ConnectPoint cp, PortDescription descr);
/**
* Generates a PortDescription containing fields from a PortDescription and
* configuration.
*
+ * @param cp {@link ConnectPoint} representing the port.
+ * @param descr input {@link PortDescription}
+ * @param prevConf previous Config {@link Config}
+ * @return Combined {@link PortDescription}
+ */
+ PortDescription combine(ConnectPoint cp, PortDescription descr, Optional<Config> prevConf);
+
+ /**
+ * Generates a PortDescription containing fields from a PortDescription and
+ * configuration.
+ *
* @param did DeviceId which the port described by {@code descr} resides.
* @param descr input {@link PortDescription}
* @return Combined {@link PortDescription}
diff --git a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
index a967c70..036ac0c 100644
--- a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
@@ -1063,13 +1063,20 @@
private class InternalNetworkConfigListener implements NetworkConfigListener {
@Override
public boolean isRelevant(NetworkConfigEvent event) {
+ DeviceId deviceId;
+ if (event.configClass().equals(PortAnnotationConfig.class)) {
+ deviceId = ((ConnectPoint) event.subject()).deviceId();
+ } else {
+ deviceId = (DeviceId) event.subject();
+ }
return (event.type() == NetworkConfigEvent.Type.CONFIG_ADDED
- || event.type() == NetworkConfigEvent.Type.CONFIG_UPDATED)
+ || event.type() == NetworkConfigEvent.Type.CONFIG_UPDATED
+ || event.type() == NetworkConfigEvent.Type.CONFIG_REMOVED)
&& (event.configClass().equals(BasicDeviceConfig.class)
|| portOpsIndex.containsKey(event.configClass())
|| event.configClass().equals(PortDescriptionsConfig.class)
|| event.configClass().equals(DeviceAnnotationConfig.class))
- && mastershipService.isLocalMaster((DeviceId) event.subject());
+ && mastershipService.isLocalMaster(deviceId);
}
@Override
@@ -1127,7 +1134,7 @@
// but cannot add new port purely from Config.
de = Optional.ofNullable(dp)
.map(provider -> store.getPortDescription(provider.id(), did, cpt.port()))
- .map(desc -> applyAllPortOps(cpt, desc))
+ .map(desc -> applyAllPortOps(cpt, desc, event.prevConfig()))
.map(desc -> store.updatePortStatus(dp.id(), did, desc))
.orElse(null);
}
@@ -1228,6 +1235,23 @@
}
/**
+ * Merges the appropriate PortConfig with the description.
+ *
+ * @param cpt ConnectPoint where the port is attached
+ * @param desc {@link PortDescription}
+ * @param prevConfig previous configuration
+ * @return merged {@link PortDescription}
+ */
+ private PortDescription applyAllPortOps(ConnectPoint cpt, PortDescription desc,
+ Optional<Config> prevConfig) {
+ PortDescription work = desc;
+ for (PortConfigOperator portOp : portOps) {
+ work = portOp.combine(cpt, work, prevConfig);
+ }
+ return portAnnotationOp.combine(cpt, work, prevConfig);
+ }
+
+ /**
* Port Enable/Disable message sent to the device's MASTER node.
*/
private class InternalPortUpDownEvent {
diff --git a/core/net/src/main/java/org/onosproject/net/device/impl/PortAnnotationOperator.java b/core/net/src/main/java/org/onosproject/net/device/impl/PortAnnotationOperator.java
index ac9ea84..700325f 100644
--- a/core/net/src/main/java/org/onosproject/net/device/impl/PortAnnotationOperator.java
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/PortAnnotationOperator.java
@@ -15,11 +15,14 @@
*/
package org.onosproject.net.device.impl;
+import java.util.HashMap;
import java.util.Map;
+import java.util.Optional;
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.DefaultAnnotations;
import org.onosproject.net.DefaultAnnotations.Builder;
+import org.onosproject.net.config.Config;
import org.onosproject.net.config.NetworkConfigService;
import org.onosproject.net.config.PortConfigOperator;
import org.onosproject.net.config.basics.PortAnnotationConfig;
@@ -72,8 +75,33 @@
builder.putAll(annotations);
return DefaultPortDescription.builder(descr)
- .annotations(builder.build())
- .build();
+ .annotations(builder.build())
+ .build();
+ }
+
+ @Override
+ public PortDescription combine(ConnectPoint cp, PortDescription descr,
+ Optional<Config> prevConfig) {
+ PortAnnotationConfig cfg = lookupConfig(cp);
+ Map<String, String> annotations = new HashMap<>();
+ if (cfg != null) {
+ annotations.putAll(cfg.annotations());
+ }
+
+ Builder builder = DefaultAnnotations.builder();
+ builder.putAll(descr.annotations());
+ if (prevConfig.isPresent()) {
+ PortAnnotationConfig prevDeviceAnnotationConfig = (PortAnnotationConfig) prevConfig.get();
+ for (String key : prevDeviceAnnotationConfig.annotations().keySet()) {
+ if (!annotations.containsKey(key)) {
+ builder.remove(key);
+ }
+ }
+ }
+ builder.putAll(annotations);
+ return DefaultPortDescription.builder(descr)
+ .annotations(builder.build())
+ .build();
}
}