Fix: remove security group rules when receiving SG remove event
Change-Id: I99c6755d9a07ce6f39028f16314522bfbfebb62d
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/DistributedSecurityGroupStore.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/DistributedSecurityGroupStore.java
index 0e1a880..405479b 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/DistributedSecurityGroupStore.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/DistributedSecurityGroupStore.java
@@ -173,7 +173,7 @@
event.newValue().value()));
break;
case REMOVE:
- log.debug("OpenStack security group removed {}", event.newValue());
+ log.debug("OpenStack security group removed {}", event.oldValue());
eventExecutor.execute(() ->
notifyDelegate(new OpenstackSecurityGroupEvent(
OPENSTACK_SECURITY_GROUP_REMOVED,
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingFloatingIpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingFloatingIpHandler.java
index 8a59d36..beaa1f1 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingFloatingIpHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingFloatingIpHandler.java
@@ -27,6 +27,7 @@
import org.onlab.packet.IpAddress;
import org.onlab.packet.MacAddress;
import org.onlab.packet.VlanId;
+import org.onosproject.cfg.ComponentConfigService;
import org.onosproject.cluster.ClusterService;
import org.onosproject.cluster.LeadershipService;
import org.onosproject.cluster.NodeId;
@@ -85,6 +86,7 @@
import static org.onosproject.openstacknetworking.api.OpenstackNetworkEvent.Type.OPENSTACK_PORT_PRE_REMOVE;
import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.associatedFloatingIp;
import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getGwByComputeDevId;
+import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getPropertyValueAsBoolean;
import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.isAssociatedWithVM;
import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.swapStaleLocation;
import static org.onosproject.openstacknetworking.util.RulePopulatorUtil.buildExtension;
@@ -101,6 +103,8 @@
private static final String ERR_FLOW = "Failed set flows for floating IP %s: ";
private static final String ERR_UNSUPPORTED_NET_TYPE = "Unsupported network type %s";
+ private static final String USE_SECURITY_GROUP = "useSecurityGroup";
+
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected CoreService coreService;
@@ -114,6 +118,9 @@
protected ClusterService clusterService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+ protected ComponentConfigService componentConfigService;
+
+ @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected OpenstackNodeService osNodeService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
@@ -879,14 +886,22 @@
NetFloatingIP fip =
associatedFloatingIp(instPort, osRouterAdminService.floatingIps());
+ boolean sgFlag = getPropertyValueAsBoolean(
+ componentConfigService.getProperties(
+ OpenstackSecurityGroupHandler.class.getName()),
+ USE_SECURITY_GROUP);
+
if (fip != null) {
-
- instancePortService.updateInstancePort(instPort.updateState(REMOVE_PENDING));
-
+ instancePortService.updateInstancePort(
+ instPort.updateState(REMOVE_PENDING));
eventExecutor.execute(() ->
updateFipStore(instancePortService.instancePort(event.port().getId())));
} else {
- instancePortService.removeInstancePort(instPort.portId());
+ // FIXME: we have dependency with security group, need to
+ // find a better way to remove this dependency
+ if (!sgFlag) {
+ instancePortService.removeInstancePort(instPort.portId());
+ }
}
break;
default:
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java
index 55f9c45..d3bc4d4 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java
@@ -48,9 +48,9 @@
import org.onosproject.net.flow.TrafficTreatment;
import org.onosproject.net.flow.criteria.ExtensionSelector;
import org.onosproject.openstacknetworking.api.InstancePort;
+import org.onosproject.openstacknetworking.api.InstancePortAdminService;
import org.onosproject.openstacknetworking.api.InstancePortEvent;
import org.onosproject.openstacknetworking.api.InstancePortListener;
-import org.onosproject.openstacknetworking.api.InstancePortService;
import org.onosproject.openstacknetworking.api.OpenstackFlowRuleService;
import org.onosproject.openstacknetworking.api.OpenstackNetworkEvent;
import org.onosproject.openstacknetworking.api.OpenstackNetworkListener;
@@ -58,6 +58,7 @@
import org.onosproject.openstacknetworking.api.OpenstackSecurityGroupEvent;
import org.onosproject.openstacknetworking.api.OpenstackSecurityGroupListener;
import org.onosproject.openstacknetworking.api.OpenstackSecurityGroupService;
+import org.onosproject.openstacknetworking.api.PreCommitPortService;
import org.onosproject.openstacknetworking.util.RulePopulatorUtil;
import org.onosproject.openstacknode.api.OpenstackNode;
import org.onosproject.openstacknode.api.OpenstackNodeEvent;
@@ -81,14 +82,16 @@
import static java.util.concurrent.Executors.newSingleThreadExecutor;
import static org.onlab.util.Tools.groupedThreads;
import static org.onosproject.openstacknetworking.api.Constants.ACL_TABLE;
-import static org.onosproject.openstacknetworking.api.Constants.JUMP_TABLE;
import static org.onosproject.openstacknetworking.api.Constants.CT_TABLE;
import static org.onosproject.openstacknetworking.api.Constants.ERROR_TABLE;
+import static org.onosproject.openstacknetworking.api.Constants.JUMP_TABLE;
import static org.onosproject.openstacknetworking.api.Constants.OPENSTACK_NETWORKING_APP_ID;
import static org.onosproject.openstacknetworking.api.Constants.PRIORITY_ACL_RULE;
import static org.onosproject.openstacknetworking.api.Constants.PRIORITY_CT_DROP_RULE;
import static org.onosproject.openstacknetworking.api.Constants.PRIORITY_CT_HOOK_RULE;
import static org.onosproject.openstacknetworking.api.Constants.PRIORITY_CT_RULE;
+import static org.onosproject.openstacknetworking.api.InstancePort.State.REMOVE_PENDING;
+import static org.onosproject.openstacknetworking.api.OpenstackNetworkEvent.Type.OPENSTACK_PORT_PRE_REMOVE;
import static org.onosproject.openstacknetworking.util.RulePopulatorUtil.computeCtMaskFlag;
import static org.onosproject.openstacknetworking.util.RulePopulatorUtil.computeCtStateFlag;
import static org.onosproject.openstacknetworking.util.RulePopulatorUtil.niciraConnTrackTreatmentBuilder;
@@ -113,7 +116,7 @@
protected CoreService coreService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
- protected InstancePortService instancePortService;
+ protected InstancePortAdminService instancePortService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected MastershipService mastershipService;
@@ -142,6 +145,9 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected ClusterService clusterService;
+ @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+ protected PreCommitPortService preCommitPortService;
+
private final InstancePortListener instancePortListener =
new InternalInstancePortListener();
private final OpenstackNetworkListener osNetworkListener =
@@ -256,6 +262,16 @@
final String action = install ? "Installed " : "Removed ";
log.debug(action + "security group rule ID : " + sgId);
});
+
+ if (install) {
+ preCommitPortService.subscribePreCommit(instPort.portId(),
+ OPENSTACK_PORT_PRE_REMOVE, this.getClass().getName());
+ log.info("Subscribed the port {} on listening pre-remove event", instPort.portId());
+ } else {
+ preCommitPortService.unsubscribePreCommit(instPort.portId(),
+ OPENSTACK_PORT_PRE_REMOVE, instancePortService, this.getClass().getName());
+ log.info("Unsubscribed the port {} on listening pre-remove event", instPort.portId());
+ }
}
private void updateSecurityGroupRule(InstancePort instPort, Port port,
@@ -716,9 +732,11 @@
switch (event.type()) {
case OPENSTACK_PORT_PRE_REMOVE:
- eventExecutor.execute(() -> {
- setSecurityGroupRules(instPort, osPort, false);
- });
+ instancePortService.updateInstancePort(
+ instPort.updateState(REMOVE_PENDING));
+ eventExecutor.execute(() ->
+ setSecurityGroupRules(instPort, osPort, false)
+ );
break;
default:
// do nothing for the other events
@@ -804,8 +822,8 @@
securityGroupRuleToRemove.getId());
});
break;
- case OPENSTACK_SECURITY_GROUP_CREATED:
case OPENSTACK_SECURITY_GROUP_REMOVED:
+ case OPENSTACK_SECURITY_GROUP_CREATED:
default:
// do nothing
break;
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/util/OpenstackNetworkingUtil.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/util/OpenstackNetworkingUtil.java
index 4f4e570..b755017 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/util/OpenstackNetworkingUtil.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/util/OpenstackNetworkingUtil.java
@@ -452,6 +452,20 @@
}
/**
+ * Obtains the boolean property value with specified property key name.
+ *
+ * @param properties a collection of properties
+ * @param name key name
+ * @return mapping value
+ */
+ public static boolean getPropertyValueAsBoolean(Set<ConfigProperty> properties, String name) {
+ Optional<ConfigProperty> property =
+ properties.stream().filter(p -> p.name().equals(name)).findFirst();
+
+ return property.map(ConfigProperty::asBoolean).orElse(false);
+ }
+
+ /**
* Prints out the JSON string in pretty format.
*
* @param mapper Object mapper