CLOUDSTACK-6245: the security group rule is lagging behind the rules in DB, due to there is a worker thread launched inside a transaction Reviewed-by: Alex
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/d4fdc184 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/d4fdc184 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/d4fdc184 Branch: refs/heads/4.3 Commit: d4fdc184fe9c6717d2ed4e4fe4c39d9759a90608 Parents: e5c391f Author: Edison Su <[email protected]> Authored: Mon Mar 17 14:34:53 2014 -0700 Committer: Edison Su <[email protected]> Committed: Mon Mar 17 14:37:49 2014 -0700 ---------------------------------------------------------------------- .../security/SecurityGroupManagerImpl.java | 33 ++++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d4fdc184/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java index e48bb34..51c93b7 100755 --- a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java +++ b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java @@ -713,7 +713,7 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro final Integer startPortOrTypeFinal = startPortOrType; final Integer endPortOrCodeFinal = endPortOrCode; final String protocolFinal = protocol; - return Transaction.execute(new TransactionCallback<List<SecurityGroupRuleVO>>() { + List<SecurityGroupRuleVO> newRules = Transaction.execute(new TransactionCallback<List<SecurityGroupRuleVO>>() { @Override public List<SecurityGroupRuleVO> doInTransaction(TransactionStatus status) { // Prevents other threads/management servers from creating duplicate security rules @@ -756,9 +756,6 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro if (s_logger.isDebugEnabled()) { s_logger.debug("Added " + newRules.size() + " rules to security group " + securityGroup.getName()); } - final ArrayList<Long> affectedVms = new ArrayList<Long>(); - affectedVms.addAll(_securityGroupVMMapDao.listVmIdsBySecurityGroup(securityGroup.getId())); - scheduleRulesetUpdateToHosts(affectedVms, true, null); return newRules; } catch (Exception e) { s_logger.warn("Exception caught when adding security group rules ", e); @@ -771,6 +768,15 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro } }); + try { + final ArrayList<Long> affectedVms = new ArrayList<Long>(); + affectedVms.addAll(_securityGroupVMMapDao.listVmIdsBySecurityGroup(securityGroup.getId())); + scheduleRulesetUpdateToHosts(affectedVms, true, null); + } catch (Exception e) { + s_logger.debug("can't update rules on host, ignore", e); + } + + return newRules; } @Override @@ -810,7 +816,8 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro SecurityGroup securityGroup = _securityGroupDao.findById(rule.getSecurityGroupId()); _accountMgr.checkAccess(caller, null, true, securityGroup); - return Transaction.execute(new TransactionCallback<Boolean>() { + long securityGroupId = rule.getSecurityGroupId(); + Boolean result = Transaction.execute(new TransactionCallback<Boolean>() { @Override public Boolean doInTransaction(TransactionStatus status) { SecurityGroupVO groupHandle = null; @@ -825,11 +832,7 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro _securityGroupRuleDao.remove(id); s_logger.debug("revokeSecurityGroupRule succeeded for security rule id: " + id); - - final ArrayList<Long> affectedVms = new ArrayList<Long>(); - affectedVms.addAll(_securityGroupVMMapDao.listVmIdsBySecurityGroup(groupHandle.getId())); - scheduleRulesetUpdateToHosts(affectedVms, true, null); - + return true; } catch (Exception e) { s_logger.warn("Exception caught when deleting security rules ", e); @@ -841,6 +844,16 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro } } }); + + try { + final ArrayList<Long> affectedVms = new ArrayList<Long>(); + affectedVms.addAll(_securityGroupVMMapDao.listVmIdsBySecurityGroup(securityGroupId)); + scheduleRulesetUpdateToHosts(affectedVms, true, null); + } catch (Exception e) { + s_logger.debug("Can't update rules for host, ignore", e); + } + + return result; } @Override
