nvazquez commented on a change in pull request #5985:
URL: https://github.com/apache/cloudstack/pull/5985#discussion_r804896865
##########
File path:
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1622,6 +1625,69 @@ private ExecutionResult
prepareNetworkElementCommand(IpAssocCommand cmd) {
return new ExecutionResult(true, null);
}
+ private ExecutionResult cleanupNetworkElementCommand(IpAssocCommand cmd) {
+ VmwareContext context = getServiceContext();
+ try {
+ VmwareHypervisorHost hyperHost = getHyperHost(context);
+ IpAddressTO[] ips = cmd.getIpAddresses();
+ String routerName =
cmd.getAccessDetail(NetworkElementCommand.ROUTER_NAME);
+
+ VirtualMachineMO vmMo = hyperHost.findVmOnHyperHost(routerName);
+ // command may sometimes be redirect to a wrong host, we relax
+ // the check and will try to find it within cluster
+ if (vmMo == null) {
+ if (hyperHost instanceof HostMO) {
+ ClusterMO clusterMo = new
ClusterMO(hyperHost.getContext(), ((HostMO) hyperHost).getParentMor());
Review comment:
What about datacenter instead of cluster search?
##########
File path:
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1622,6 +1625,69 @@ private ExecutionResult
prepareNetworkElementCommand(IpAssocCommand cmd) {
return new ExecutionResult(true, null);
}
+ private ExecutionResult cleanupNetworkElementCommand(IpAssocCommand cmd) {
+ VmwareContext context = getServiceContext();
+ try {
+ VmwareHypervisorHost hyperHost = getHyperHost(context);
+ IpAddressTO[] ips = cmd.getIpAddresses();
+ String routerName =
cmd.getAccessDetail(NetworkElementCommand.ROUTER_NAME);
+
+ VirtualMachineMO vmMo = hyperHost.findVmOnHyperHost(routerName);
+ // command may sometimes be redirect to a wrong host, we relax
+ // the check and will try to find it within cluster
+ if (vmMo == null) {
+ if (hyperHost instanceof HostMO) {
+ ClusterMO clusterMo = new
ClusterMO(hyperHost.getContext(), ((HostMO) hyperHost).getParentMor());
+ vmMo = clusterMo.findVmOnHyperHost(routerName);
+ }
+ }
+
+ if (vmMo == null) {
+ String msg = String.format("Router %s no longer exists to
execute IPAssoc command ", routerName);
+ s_logger.error(msg);
+ throw new Exception(msg);
+ }
+
+ for (IpAddressTO ip : ips) {
Review comment:
I think this for loop is not needed since we ensure the logic is
executed only when it has one item as is not add. Could be refactored to
pre-checks and logic to reduce indentation
--
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]
