This is an automated email from the ASF dual-hosted git repository. rohit pushed a commit to branch 4.19 in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push: new 6b4955affe9 Fix message publish in transaction (#8980) 6b4955affe9 is described below commit 6b4955affe9f9658eb8128b3b7d18f08a1f05752 Author: Vishesh <vishes...@gmail.com> AuthorDate: Tue May 7 13:27:31 2024 +0530 Fix message publish in transaction (#8980) * Fix message publish in transaction * Resolve comments --- .../cloud/configuration/ConfigurationManager.java | 3 ++- .../engine/orchestration/NetworkOrchestrator.java | 30 +++++++++++++++++----- .../configuration/ConfigurationManagerImpl.java | 17 ++++++++---- .../cloud/vpc/MockConfigurationManagerImpl.java | 5 ++-- 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java index 0232d07b8be..728511ba8d5 100644 --- a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java +++ b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import com.cloud.dc.VlanVO; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.impl.ConfigurationSubGroupVO; @@ -189,7 +190,7 @@ public interface ConfigurationManager { * @param caller TODO * @return success/failure */ - boolean deleteVlanAndPublicIpRange(long userId, long vlanDbId, Account caller); + VlanVO deleteVlanAndPublicIpRange(long userId, long vlanDbId, Account caller); void checkZoneAccess(Account caller, DataCenter zone); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index c49a0fd09b1..09500051df6 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -255,6 +255,8 @@ import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; import com.googlecode.ipv6.IPv6Address; +import static com.cloud.configuration.ConfigurationManager.MESSAGE_DELETE_VLAN_IP_RANGE_EVENT; + /** * NetworkManagerImpl implements NetworkManager. */ @@ -3298,16 +3300,16 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra final NetworkVO networkFinal = network; try { - Transaction.execute(new TransactionCallbackNoReturn() { + final List<VlanVO> deletedVlanRangeToPublish = Transaction.execute(new TransactionCallback<List<VlanVO>>() { @Override - public void doInTransactionWithoutResult(final TransactionStatus status) { + public List<VlanVO> doInTransaction(TransactionStatus status) { final NetworkGuru guru = AdapterBase.getAdapterByName(networkGurus, networkFinal.getGuruName()); if (!guru.trash(networkFinal, _networkOfferingDao.findById(networkFinal.getNetworkOfferingId()))) { throw new CloudRuntimeException("Failed to trash network."); } - - if (!deleteVlansInNetwork(networkFinal, context.getCaller().getId(), callerAccount)) { + Pair<Boolean, List<VlanVO>> deletedVlans = deleteVlansInNetwork(networkFinal, context.getCaller().getId(), callerAccount); + if (!deletedVlans.first()) { s_logger.warn("Failed to delete network " + networkFinal + "; was unable to cleanup corresponding ip ranges"); throw new CloudRuntimeException("Failed to delete network " + networkFinal + "; was unable to cleanup corresponding ip ranges"); } else { @@ -3341,8 +3343,10 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra _resourceLimitMgr.decrementResourceCount(networkFinal.getAccountId(), ResourceType.network, networkFinal.getDisplayNetwork()); } } + return deletedVlans.second(); } }); + publishDeletedVlanRanges(deletedVlanRangeToPublish); if (_networksDao.findById(network.getId()) == null) { // remove its related ACL permission final Pair<Class<?>, Long> networkMsg = new Pair<Class<?>, Long>(Network.class, networkFinal.getId()); @@ -3360,6 +3364,14 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra return success; } + private void publishDeletedVlanRanges(List<VlanVO> deletedVlanRangeToPublish) { + if (CollectionUtils.isNotEmpty(deletedVlanRangeToPublish)) { + for (VlanVO vlan : deletedVlanRangeToPublish) { + _messageBus.publish(_name, MESSAGE_DELETE_VLAN_IP_RANGE_EVENT, PublishScope.LOCAL, vlan); + } + } + } + @Override public boolean resourceCountNeedsUpdate(final NetworkOffering ntwkOff, final ACLType aclType) { //Update resource count only for Isolated account specific non-system networks @@ -3367,15 +3379,19 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra return updateResourceCount; } - protected boolean deleteVlansInNetwork(final NetworkVO network, final long userId, final Account callerAccount) { + protected Pair<Boolean, List<VlanVO>> deleteVlansInNetwork(final NetworkVO network, final long userId, final Account callerAccount) { final long networkId = network.getId(); //cleanup Public vlans final List<VlanVO> publicVlans = _vlanDao.listVlansByNetworkId(networkId); + List<VlanVO> deletedPublicVlanRange = new ArrayList<>(); boolean result = true; for (final VlanVO vlan : publicVlans) { - if (!_configMgr.deleteVlanAndPublicIpRange(userId, vlan.getId(), callerAccount)) { + VlanVO vlanRange = _configMgr.deleteVlanAndPublicIpRange(userId, vlan.getId(), callerAccount); + if (vlanRange == null) { s_logger.warn("Failed to delete vlan " + vlan.getId() + ");"); result = false; + } else { + deletedPublicVlanRange.add(vlanRange); } } @@ -3395,7 +3411,7 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra _dcDao.releaseVnet(BroadcastDomainType.getValue(network.getBroadcastUri()), network.getDataCenterId(), network.getPhysicalNetworkId(), network.getAccountId(), network.getReservationId()); } - return result; + return new Pair<>(result, deletedPublicVlanRange); } public class NetworkGarbageCollector extends ManagedContextRunnable { diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 18355a47fe1..b59c8d018ee 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -5348,7 +5348,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati @Override @DB - public boolean deleteVlanAndPublicIpRange(final long userId, final long vlanDbId, final Account caller) { + public VlanVO deleteVlanAndPublicIpRange(final long userId, final long vlanDbId, final Account caller) { VlanVO vlanRange = _vlanDao.findById(vlanDbId); if (vlanRange == null) { throw new InvalidParameterValueException("Please specify a valid IP range id."); @@ -5454,9 +5454,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati } }); - messageBus.publish(_name, MESSAGE_DELETE_VLAN_IP_RANGE_EVENT, PublishScope.LOCAL, vlanRange); - - return true; + return vlanRange; } @Override @@ -5962,7 +5960,16 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati throw new InvalidParameterValueException("Please specify a valid IP range id."); } - return deleteVlanAndPublicIpRange(CallContext.current().getCallingUserId(), vlanDbId, CallContext.current().getCallingAccount()); + return deleteAndPublishVlanAndPublicIpRange(CallContext.current().getCallingUserId(), vlanDbId, CallContext.current().getCallingAccount()); + } + + private boolean deleteAndPublishVlanAndPublicIpRange(final long userId, final long vlanDbId, final Account caller) { + VlanVO deletedVlan = deleteVlanAndPublicIpRange(userId, vlanDbId, caller); + if (deletedVlan != null) { + messageBus.publish(_name, MESSAGE_DELETE_VLAN_IP_RANGE_EVENT, PublishScope.LOCAL, deletedVlan); + return true; + } + return false; } @Override diff --git a/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java index 8c6e73fcf70..eb314b7cb5d 100644 --- a/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java @@ -26,6 +26,7 @@ import com.cloud.dc.DataCenterVO; import com.cloud.dc.HostPodVO; import com.cloud.dc.Pod; import com.cloud.dc.Vlan; +import com.cloud.dc.VlanVO; import com.cloud.domain.Domain; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InsufficientCapacityException; @@ -515,9 +516,9 @@ public class MockConfigurationManagerImpl extends ManagerBase implements Configu * @see com.cloud.configuration.ConfigurationManager#deleteVlanAndPublicIpRange(long, long, com.cloud.user.Account) */ @Override - public boolean deleteVlanAndPublicIpRange(long userId, long vlanDbId, Account caller) { + public VlanVO deleteVlanAndPublicIpRange(long userId, long vlanDbId, Account caller) { // TODO Auto-generated method stub - return false; + return null; } /* (non-Javadoc)