From 4f2d9047689a7814346b39e1501534367751d1de Mon Sep 17 00:00:00 2001 From: Gabriel Brascher Date: Fri, 19 Feb 2021 16:29:05 -0300 Subject: [PATCH 1/3] Cleanup unnecessary code and enhance a few log messages at IpAddressManagerImpl --- .../dao/NetworkOfferingServiceMapDaoImpl.java | 8 +------ .../cloud/network/IpAddressManagerImpl.java | 23 +++++++++++-------- .../com/cloud/network/NetworkModelImpl.java | 1 + 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/offerings/dao/NetworkOfferingServiceMapDaoImpl.java b/engine/schema/src/main/java/com/cloud/offerings/dao/NetworkOfferingServiceMapDaoImpl.java index 7868be2ad69a..67b341a93618 100644 --- a/engine/schema/src/main/java/com/cloud/offerings/dao/NetworkOfferingServiceMapDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/offerings/dao/NetworkOfferingServiceMapDaoImpl.java @@ -129,7 +129,6 @@ public void deleteByOfferingId(long networkOfferingId) { @Override public List listProvidersForServiceForNetworkOffering(long networkOfferingId, Service service) { SearchCriteria sc = ProvidersSearch.create(); - ; sc.setParameters("networkOfferingId", networkOfferingId); sc.setParameters("service", service.getName()); @@ -140,21 +139,16 @@ public List listProvidersForServiceForNetworkOffering(long networkOfferi @Override public boolean isProviderForNetworkOffering(long networkOfferingId, Provider provider) { SearchCriteria sc = AllFieldsSearch.create(); - ; sc.setParameters("networkOfferingId", networkOfferingId); sc.setParameters("provider", provider.getName()); - if (findOneBy(sc) != null) { - return true; - } - return false; + return findOneBy(sc) != null; } @Override public List listServicesForNetworkOffering(long networkOfferingId) { SearchCriteria sc = ServicesSearch.create(); - ; sc.setParameters("networkOfferingId", networkOfferingId); return customSearch(sc, null); } diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index 51af56a887a2..aca3f63ae838 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -824,11 +824,11 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient if (vlanDbIds == null || vlanDbIds.contains(nonDedicatedVlan.getId())) nonDedicatedVlanDbIds.add(nonDedicatedVlan.getId()); } - if (dedicatedVlanDbIds != null && !dedicatedVlanDbIds.isEmpty()) { + if (!dedicatedVlanDbIds.isEmpty()) { fetchFromDedicatedRange = true; sc.setParameters("vlanId", dedicatedVlanDbIds.toArray()); errorMessage.append(", vlanId id=" + Arrays.toString(dedicatedVlanDbIds.toArray())); - } else if (nonDedicatedVlanDbIds != null && !nonDedicatedVlanDbIds.isEmpty()) { + } else if (!nonDedicatedVlanDbIds.isEmpty()) { sc.setParameters("vlanId", nonDedicatedVlanDbIds.toArray()); errorMessage.append(", vlanId id=" + Arrays.toString(nonDedicatedVlanDbIds.toArray())); } else { @@ -877,7 +877,7 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient if (addrs.size() == 0 && fetchFromDedicatedRange) { // Verify if account is allowed to acquire IPs from the system boolean useSystemIps = UseSystemPublicIps.valueIn(owner.getId()); - if (useSystemIps && nonDedicatedVlanDbIds != null && !nonDedicatedVlanDbIds.isEmpty()) { + if (useSystemIps && !nonDedicatedVlanDbIds.isEmpty()) { fetchFromDedicatedRange = false; sc.setParameters("vlanId", nonDedicatedVlanDbIds.toArray()); errorMessage.append(", vlanId id=" + Arrays.toString(nonDedicatedVlanDbIds.toArray())); @@ -1106,6 +1106,10 @@ public boolean applyIpAssociations(Network network, boolean postApplyRules, bool return success; } + private String generateErrorMessageForOperationOnDisabledZone(String operation, DataCenter zone) { + return String.format("Cannot %s, Zone [id: %d, uuid: %s, name: %s] is currently disabled.", operation, zone.getId(), zone.getUuid(), zone.getName()); + } + @DB @Override public AcquirePodIpCmdResponse allocatePodIp(String zoneId, String podId) throws ConcurrentOperationException, ResourceAllocationException { @@ -1113,8 +1117,7 @@ public AcquirePodIpCmdResponse allocatePodIp(String zoneId, String podId) throws DataCenter zone = _entityMgr.findByUuid(DataCenter.class, zoneId); Account caller = CallContext.current().getCallingAccount(); if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { - ResourceAllocationException ex = new ResourceAllocationException("Cannot perform this operation, " + "Zone is currently disabled" + "zoneId=" + zone.getUuid(), - ResourceType.network); + ResourceAllocationException ex = new ResourceAllocationException(generateErrorMessageForOperationOnDisabledZone("allocate Pod IP addresses", zone), ResourceType.network); throw ex; } @@ -1124,7 +1127,7 @@ public AcquirePodIpCmdResponse allocatePodIp(String zoneId, String podId) throws HostPodVO podvo = null; podvo = _hpDao.findByUuid(podId); if (podvo == null) - throw new ResourceAllocationException("No sush pod exists", ResourceType.network); + throw new ResourceAllocationException("No such pod exists", ResourceType.network); vo = _privateIPAddressDao.takeIpAddress(zone.getId(), podvo.getId(), 0, caller.getId() + "", false); if(vo == null) @@ -1163,7 +1166,7 @@ public void releasePodIp(Long id) throws CloudRuntimeException { DataCenter zone = _entityMgr.findById(DataCenter.class, ipVO.getDataCenterId()); Account caller = CallContext.current().getCallingAccount(); if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { - throw new CloudRuntimeException("Cannot perform this operation, " + "Zone is currently disabled" + "zoneId=" + ipVO.getDataCenterId()); + throw new CloudRuntimeException(generateErrorMessageForOperationOnDisabledZone("release Pod IP", zone)); } try { _privateIPAddressDao.releasePodIpAddress(id); @@ -1183,7 +1186,7 @@ public IpAddress allocateIp(final Account ipOwner, final boolean isSystem, Accou if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { // zone is of type DataCenter. See DataCenterVO.java. - PermissionDeniedException ex = new PermissionDeniedException("Cannot perform this operation, " + "Zone is currently disabled"); + PermissionDeniedException ex = new PermissionDeniedException(generateErrorMessageForOperationOnDisabledZone("allocate IP addresses", zone)); ex.addProxyObject(zone.getUuid(), "zoneId"); throw ex; } @@ -1367,7 +1370,7 @@ public IPAddressVO associateIPToGuestNetwork(long ipId, long networkId, boolean } if (ipToAssoc.getAssociatedWithNetworkId() != null) { - s_logger.debug("IP " + ipToAssoc + " is already assocaited with network id" + networkId); + s_logger.debug("IP " + ipToAssoc + " is already associated with network id" + networkId); return ipToAssoc; } @@ -1445,7 +1448,7 @@ public IPAddressVO associateIPToGuestNetwork(long ipId, long networkId, boolean s_logger.warn("Failed to associate ip address, so releasing ip from the database " + ip); _ipAddressDao.markAsUnavailable(ip.getId()); if (!applyIpAssociations(network, true)) { - // if fail to apply ip assciations again, unassign ip address without updating resource + // if fail to apply ip associations again, unassign ip address without updating resource // count and generating usage event as there is no need to keep it in the db _ipAddressDao.unassignIpAddress(ip.getId()); } diff --git a/server/src/main/java/com/cloud/network/NetworkModelImpl.java b/server/src/main/java/com/cloud/network/NetworkModelImpl.java index 4322478d93e1..572576412af5 100644 --- a/server/src/main/java/com/cloud/network/NetworkModelImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkModelImpl.java @@ -1554,6 +1554,7 @@ public boolean areServicesEnabledInZone(long zoneId, NetworkOffering offering, L if (!checkedProvider.contains(providerName)) { result = result && isProviderEnabledInPhysicalNetwork(physicalNtwkId, providerName); } + checkedProvider.add(providerName); } } From b9b47c7004e383e2c513a9925586e0e6b7ad470f Mon Sep 17 00:00:00 2001 From: Gabriel Brascher Date: Fri, 7 May 2021 10:33:57 -0300 Subject: [PATCH 2/3] Add toString method for DataCenterVO --- engine/schema/src/main/java/com/cloud/dc/DataCenterVO.java | 5 +++++ .../main/java/com/cloud/network/IpAddressManagerImpl.java | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/dc/DataCenterVO.java b/engine/schema/src/main/java/com/cloud/dc/DataCenterVO.java index d71f34cdc7e7..c107e6c74d86 100644 --- a/engine/schema/src/main/java/com/cloud/dc/DataCenterVO.java +++ b/engine/schema/src/main/java/com/cloud/dc/DataCenterVO.java @@ -471,4 +471,9 @@ public void setIp6Dns2(String ip6Dns2) { public PartitionType partitionType() { return PartitionType.Zone; } + + @Override + public String toString() { + return String.format("Zone [{id: \"%s\", name: \"%s\", uuid: \"%s\"}]", id, name, uuid); + } } diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index 1c84ff4986b5..83237a31db40 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -838,7 +838,7 @@ public List doInTransaction(TransactionStatus status) throws Insuff } if (vlanUse == VlanType.VirtualNetwork) { - if !dedicatedVlanDbIds.isEmpty()) { + if (!dedicatedVlanDbIds.isEmpty()) { fetchFromDedicatedRange = true; sc.setParameters("vlanId", dedicatedVlanDbIds.toArray()); errorMessage.append(", vlanId id=" + Arrays.toString(dedicatedVlanDbIds.toArray())); @@ -1125,7 +1125,7 @@ public boolean applyIpAssociations(Network network, boolean postApplyRules, bool } private String generateErrorMessageForOperationOnDisabledZone(String operation, DataCenter zone) { - return String.format("Cannot %s, Zone [id: %d, uuid: %s, name: %s] is currently disabled.", operation, zone.getId(), zone.getUuid(), zone.getName()); + return String.format("Cannot %s, %s is currently disabled.", operation, zone); } @DB From 0f4f57a0e60fc501addfeb9270b449e6bd15d8f3 Mon Sep 17 00:00:00 2001 From: Gabriel Brascher Date: Fri, 25 Jun 2021 16:40:00 -0300 Subject: [PATCH 3/3] line too long --- .../src/main/java/com/cloud/network/IpAddressManagerImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index 83237a31db40..949feee1d4e4 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -1135,7 +1135,8 @@ public AcquirePodIpCmdResponse allocatePodIp(String zoneId, String podId) throws DataCenter zone = _entityMgr.findByUuid(DataCenter.class, zoneId); Account caller = CallContext.current().getCallingAccount(); if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { - ResourceAllocationException ex = new ResourceAllocationException(generateErrorMessageForOperationOnDisabledZone("allocate Pod IP addresses", zone), ResourceType.network); + ResourceAllocationException ex = new ResourceAllocationException( + generateErrorMessageForOperationOnDisabledZone("allocate Pod IP addresses", zone), ResourceType.network); throw ex; }