From 7569e3b1459e51c4de802e9958492265d20e0730 Mon Sep 17 00:00:00 2001 From: dmitryintel Date: Mon, 8 Jan 2018 20:46:57 -0800 Subject: [PATCH 1/3] Neither of these situation calls for removeInspectionHook (#631) --- .../securitygroup/SecurityGroupUpdateOrDeleteMetaTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osc-server/src/main/java/org/osc/core/broker/service/tasks/conformance/openstack/securitygroup/SecurityGroupUpdateOrDeleteMetaTask.java b/osc-server/src/main/java/org/osc/core/broker/service/tasks/conformance/openstack/securitygroup/SecurityGroupUpdateOrDeleteMetaTask.java index 550c09b5e..99a1b1923 100644 --- a/osc-server/src/main/java/org/osc/core/broker/service/tasks/conformance/openstack/securitygroup/SecurityGroupUpdateOrDeleteMetaTask.java +++ b/osc-server/src/main/java/org/osc/core/broker/service/tasks/conformance/openstack/securitygroup/SecurityGroupUpdateOrDeleteMetaTask.java @@ -290,7 +290,7 @@ private void buildTaskGraph(EntityManager em, boolean isDeleteTg, String domainI // are already deleted and this essentially a no op. The tasks SecurityGroupMemberVmCheckTask etc // need to make sure hooks are removed in case of delete tg. boolean shouldRemoveHooks = !this.apiFactoryService.supportsPortGroup(this.sg) - || !this.apiFactoryService.supportsNeutronSFC(this.sg); + && !this.apiFactoryService.supportsNeutronSFC(this.sg); if (shouldRemoveHooks) { tasksToSucceedToDeleteSGI.addAll(addSGMemberRemoveHooksTask(em, sgi)); } From dbc952ab658b360d0eeaadf95ecf6b5d74874b32 Mon Sep 17 00:00:00 2001 From: dmitryintel Date: Tue, 9 Jan 2018 15:44:25 -0800 Subject: [PATCH 2/3] Fixing bug #627: can delete bound SGs now. (#630) * Fixing bug #627: can delete bound SGs now. * Remove duplicate port removal calls. * Refactor deleteFromDB(). --- .../BindSecurityGroupService.java | 24 +++--- .../SecurityGroupMemberDeleteTask.java | 85 +++++++++++-------- 2 files changed, 62 insertions(+), 47 deletions(-) diff --git a/osc-server/src/main/java/org/osc/core/broker/service/securitygroup/BindSecurityGroupService.java b/osc-server/src/main/java/org/osc/core/broker/service/securitygroup/BindSecurityGroupService.java index 4c121e4dd..c0bf80f60 100644 --- a/osc-server/src/main/java/org/osc/core/broker/service/securitygroup/BindSecurityGroupService.java +++ b/osc-server/src/main/java/org/osc/core/broker/service/securitygroup/BindSecurityGroupService.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.TreeSet; @@ -122,19 +123,20 @@ public BaseJobResponse exec(BindSecurityGroupRequest request, EntityManager em) throw new VmidcBrokerValidationException( "Security group interface cannot have more than one policy for security manager not supporting multiple policy binding"); } - if (!isPolicyMappingSupported && !serviceToBindTo.getPolicyIds().isEmpty()) { - throw new VmidcBrokerValidationException( - "Security manager not supporting policy mapping cannot have one or more policies"); - } - if (isPolicyMappingSupported && serviceToBindTo.getPolicyIds().isEmpty()) { - throw new VmidcBrokerValidationException( - "Service to bind: " + serviceToBindTo + " must have a policy id."); - } - Set policies = null; - policies = PolicyEntityMgr.findPoliciesById(em, serviceToBindTo.getPolicyIds(), - vs.getDistributedAppliance().getApplianceManagerConnector()); + Set policyIds = serviceToBindTo.getPolicyIds() != null ? serviceToBindTo.getPolicyIds() : new HashSet<>(); + if (!isPolicyMappingSupported && !policyIds.isEmpty()) { + throw new VmidcBrokerValidationException( + "Security manager not supporting policy mapping cannot have one or more policies"); + } + if (isPolicyMappingSupported && policyIds.isEmpty()) { + throw new VmidcBrokerValidationException( + "Service to bind: " + serviceToBindTo + " must have a policy id."); + } + Set policies = null; + policies = PolicyEntityMgr.findPoliciesById(em, policyIds, + vs.getDistributedAppliance().getApplianceManagerConnector()); if (this.apiFactoryService.supportsFailurePolicy(this.securityGroup)) { // If failure policy is supported, failure policy is a required field if (serviceToBindTo.getFailurePolicyType() == null diff --git a/osc-server/src/main/java/org/osc/core/broker/service/tasks/conformance/openstack/securitygroup/SecurityGroupMemberDeleteTask.java b/osc-server/src/main/java/org/osc/core/broker/service/tasks/conformance/openstack/securitygroup/SecurityGroupMemberDeleteTask.java index 41319019c..6af246e61 100644 --- a/osc-server/src/main/java/org/osc/core/broker/service/tasks/conformance/openstack/securitygroup/SecurityGroupMemberDeleteTask.java +++ b/osc-server/src/main/java/org/osc/core/broker/service/tasks/conformance/openstack/securitygroup/SecurityGroupMemberDeleteTask.java @@ -16,6 +16,9 @@ *******************************************************************************/ package org.osc.core.broker.service.tasks.conformance.openstack.securitygroup; +import static java.util.stream.Collectors.toSet; + +import java.util.HashSet; import java.util.Set; import javax.persistence.EntityManager; @@ -33,9 +36,9 @@ import org.osc.core.broker.model.entities.virtualization.openstack.VMPort; import org.osc.core.broker.service.persistence.OSCEntityManager; import org.osc.core.broker.service.tasks.TransactionalTask; -import org.slf4j.LoggerFactory; import org.osgi.service.component.annotations.Component; import org.slf4j.Logger; +import org.slf4j.LoggerFactory; @Component(service=SecurityGroupMemberDeleteTask.class) public class SecurityGroupMemberDeleteTask extends TransactionalTask { @@ -60,21 +63,14 @@ public void executeTransaction(EntityManager em) throws Exception { if (this.sgm.getType() == SecurityGroupMemberType.VM) { VM vm = this.sgm.getVm(); - if (vm.getSecurityGroupMembers().size() == 1) { + vm.getSecurityGroupMembers().remove(this.sgm); + if (vm.getSecurityGroupMembers().size() == 0) { // VM has reference only to this SGM. Delete the vm too. this.log.info("No other references to VM found. Deleting VM " + vm.getName()); - int portsDeleted = 0; - int totalVmPorts = vm.getPorts().size(); - for (VMPort vmport : vm.getPorts()) { - if (vmport.getNetwork() == null) { - OSCEntityManager.delete(em, vmport, this.txBroadcastUtil); - portsDeleted++; - } - } - if (portsDeleted == totalVmPorts) { - // Only if all ports were deleted, delete the VM. - OSCEntityManager.delete(em, vm, this.txBroadcastUtil); - } + Set portsToRemove = vm.getPorts().stream().filter(p -> p.getNetwork() == null).collect(toSet()); + + // If portsToRemove equals all ports, VM itself is deleted. + deleteFromDB(em, portsToRemove); } else { vm.getSecurityGroupMembers().remove(this.sgm); } @@ -109,17 +105,7 @@ public void executeTransaction(EntityManager em) throws Exception { if (network.getSecurityGroupMembers().size() == 1) { // Network has reference only to this SGM. Delete the network too. this.log.info("No other references to Network found. Deleting Network " + network.getName()); - for (VMPort vmPort : network.getPorts()) { - // If the VM was created on behalf of this port, delete it. - VM vm = vmPort.getVm(); - OSCEntityManager.delete(em, vmPort, this.txBroadcastUtil); - if (vm != null) { - vm.removePort(vmPort); - if (vm.getSecurityGroupMembers().size() == 0 && vm.getPorts().size() <= 0) { - OSCEntityManager.delete(em, vm, this.txBroadcastUtil); - } - } - } + deleteFromDB(em, network.getPorts()); OSCEntityManager.delete(em, network, this.txBroadcastUtil); } this.log.info("Deleting Security Group member from " + this.sgm.getSecurityGroup().getName()); @@ -130,17 +116,7 @@ public void executeTransaction(EntityManager em) throws Exception { if (subnet.getSecurityGroupMembers().size() == 1) { // Subnet has reference only to this SGM. Delete the subnet too. this.log.info("No other references to Subnet found. Deleting Subnet " + subnet.getName()); - for (VMPort vmPort : subnet.getPorts()) { - // If the VM was created on behalf of this port, delete it. - VM vm = vmPort.getVm(); - OSCEntityManager.delete(em, vmPort, this.txBroadcastUtil); - if (vm != null) { - vm.removePort(vmPort); - if (vm.getSecurityGroupMembers().size() == 0 && vm.getPorts().size() <= 0) { - OSCEntityManager.delete(em, vm, this.txBroadcastUtil); - } - } - } + deleteFromDB(em, subnet.getPorts()); OSCEntityManager.delete(em, subnet, this.txBroadcastUtil); } this.log.info("Deleting Security Group member from " + this.sgm.getSecurityGroup().getName()); @@ -160,4 +136,41 @@ public Set getObjects() { return LockObjectReference.getObjectReferences(this.sgm.getSecurityGroup()); } + private void deleteFromDB(EntityManager em, Set ports) { + Set vmsToRemove = new HashSet<>(); + + // guard against ConcurrentModificationException; + Set portsClone = new HashSet<>(ports); + for (VMPort vmPort : portsClone) { + VM vm = vmPort.getVm(); + if (vm != null) { + vm.removePort(vmPort); + vmPort.setVm(null); + } + + Network network = vmPort.getNetwork(); + if (network != null) { + network.removePort(vmPort); + } + + for (DistributedApplianceInstance dai : vmPort.getDais()) { + dai.removeProtectedPort(vmPort); + vmPort.removeDai(dai); + } + + this.log.info("Removing port " + vmPort); + OSCEntityManager.delete(em, vmPort, this.txBroadcastUtil); + + if (vm != null) { + if (vm.getSecurityGroupMembers().size() == 0 && vm.getPorts().size() <= 0) { + vmsToRemove.add(vm); + } + } + } + + for (VM vm : vmsToRemove) { + this.log.info("Removing vm " + vm); + OSCEntityManager.delete(em, vm, this.txBroadcastUtil); + } + } } From 238bf0fa77df5e5883f1c4714c3cf9079144a2df Mon Sep 17 00:00:00 2001 From: dmitryintel Date: Tue, 9 Jan 2018 16:04:28 -0800 Subject: [PATCH 3/3] Rename PANMgrPlugin to Panorama (#633) --- build.xml | 4 ++-- osc-export/pom.xml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/build.xml b/build.xml index b93283b1c..ad3e475f8 100644 --- a/build.xml +++ b/build.xml @@ -79,8 +79,8 @@ - - + + diff --git a/osc-export/pom.xml b/osc-export/pom.xml index aac863f28..d67ec44e9 100644 --- a/osc-export/pom.xml +++ b/osc-export/pom.xml @@ -314,10 +314,10 @@ dir="${basedir}/../../security-mgr-sample-plugin/target/" includes="SampleMgrPlugin*.bar" /> - + + includes="Panorama*.bar" />