Copilot commented on code in PR #12446:
URL: https://github.com/apache/cloudstack/pull/12446#discussion_r2697278184
##########
server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java:
##########
@@ -2037,35 +2016,31 @@ public Boolean doInTransaction(TransactionStatus
status) {
}
// remove LB-Vm mappings that were state to revoke
- List<LoadBalancerVMMapVO> lbVmMaps =
_lb2VmMapDao.listByLoadBalancerId(lb.getId(), true);
- List<Long> instanceIds = new ArrayList<Long>();
-
- for (LoadBalancerVMMapVO lbVmMap : lbVmMaps) {
- instanceIds.add(lbVmMap.getInstanceId());
- _lb2VmMapDao.remove(lb.getId(),
lbVmMap.getInstanceId(), lbVmMap.getInstanceIp(), null);
+ List<LoadBalancerVMMapVO> lbVmMapsToRevoke =
_lb2VmMapDao.listByLoadBalancerId(lb.getId(), true);
+ for (LoadBalancerVMMapVO lbVmMapToRevoke :
lbVmMapsToRevoke) {
+ _lb2VmMapDao.remove(lb.getId(),
lbVmMapToRevoke.getInstanceId(), lbVmMapToRevoke.getInstanceIp(), null);
logger.debug("Load balancer rule {} is removed for
Instance {} and IP {}",
- lb, lbVmMap.getInstanceId(),
lbVmMap.getInstanceIp());;
+ lb, lbVmMapToRevoke.getInstanceId(),
lbVmMapToRevoke.getInstanceIp());;
Review Comment:
Double semicolon at the end of the statement. Remove one semicolon.
```suggestion
lb, lbVmMapToRevoke.getInstanceId(),
lbVmMapToRevoke.getInstanceIp());
```
##########
server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java:
##########
@@ -2015,9 +2015,9 @@ private boolean assignLBruleToNewVm(long vmId,
AutoScaleVmGroupVO asGroup) {
}
}
}
- lstVmId.add(new Long(vmId));
+ vmIds.add(new Long(vmId));
Review Comment:
The explicit Long constructor call is unnecessary and deprecated. Use
Long.valueOf(vmId) or simply add vmId directly since autoboxing will handle the
conversion.
```suggestion
vmIds.add(vmId);
```
##########
server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java:
##########
@@ -1448,7 +1428,7 @@ private boolean removeFromLoadBalancerInternal(long
loadBalancerId, List<Long> i
}
map.setRevoke(true);
_lb2VmMapDao.persist(map);
- logger.debug("Set load balancer rule for revoke: rule
{}, vmId {}, vmip {}", loadBalancer::toString, () ->
_vmDao.findById(instanceId), vmIp::toString);
+ logger.debug("Set load balancer rule for revoke: rule
{}, vmId {}, vmIp {}", loadBalancer::toString, () ->
_vmDao.findById(instanceId), vmIp::toString);
Review Comment:
Redundant call to 'toString' on a String object.
```suggestion
logger.debug("Set load balancer rule for revoke:
rule {}, vmId {}, vmIp {}", loadBalancer::toString, () ->
_vmDao.findById(instanceId), vmIp);
```
##########
server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java:
##########
@@ -1130,31 +1119,28 @@ public boolean assignToLoadBalancer(long
loadBalancerId, List<Long> instanceIds,
}
List<String> vmIpsList = vmIdIpMap.get(instanceId);
- String vmLbIp = null;
-
if (vmIpsList != null) {
-
//check if the ips belongs to nic secondary ip
for (String ip: vmIpsList) {
- // skip the primary ip from vm secondary ip comparisions
- if (ip.equals(priIp)) {
+ // skip the primary ip from vm secondary ip comparison
+ if (ip.equals(primaryIp)) {
continue;
}
-
if(_nicSecondaryIpDao.findByIp4AddressAndNicId(ip,nicInSameNetwork.getId()) ==
null) {
+ if
(_nicSecondaryIpDao.findByIp4AddressAndNicId(ip,nicInSameNetwork.getId()) ==
null) {
Review Comment:
Missing space after comma in method call arguments. Add space after the
comma for better readability.
```suggestion
if (_nicSecondaryIpDao.findByIp4AddressAndNicId(ip,
nicInSameNetwork.getId()) == null) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
