Skip to content

Display proper error message while deleting networks #4642

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ravening wants to merge 1 commit into from
Closed

Display proper error message while deleting networks #4642

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions ...tration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2882,8 +2882,9 @@ public boolean destroyNetwork(final long networkId, final ReservationContext con

for (final UserVmVO vm : userVms) {
if (!(vm.getState() == VirtualMachine.State.Expunging && vm.getRemoved() != null)) {
s_logger.warn("Can't delete the network, not all user vms are expunged. Vm " + vm + " is in " + vm.getState() + " state");
return false;
final String message = "Can't delete the network, not all user vms are expunged. Vm " + vm + " is in " + vm.getState() + " state";
s_logger.warn(message);
throw new InvalidParameterValueException(message);
Copy link
Copy Markdown
Member

@harikrishna-patnala harikrishna-patnala Feb 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing exceptions will cause other issues here, as already mentioned by Daan, other components are handling the boolean response either by logging error message or cleaning up some other resources upon error (AccountManagerImpl).

Copy link
Copy Markdown
Contributor

@sureshanaparti sureshanaparti Jun 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing exceptions will cause other issues here, as already mentioned by Daan, other components are handling the boolean response either by logging error message or cleaning up some other resources upon error (AccountManagerImpl).

@ravening any update?

}
}

Expand All @@ -2901,8 +2902,9 @@ public boolean destroyNetwork(final long networkId, final ReservationContext con
if (zone.getNetworkType() == NetworkType.Basic) {
final List<VMInstanceVO> systemVms = _vmDao.listNonRemovedVmsByTypeAndNetwork(network.getId(), Type.ConsoleProxy, Type.SecondaryStorageVm);
if (systemVms != null && !systemVms.isEmpty()) {
s_logger.warn("Can't delete the network, not all consoleProxy/secondaryStorage vms are expunged");
return false;
final String message = "Can't delete the network, not all consoleProxy/secondaryStorage vms are expunged";
s_logger.warn(message);
throw new InvalidParameterValueException(message);
}
}

Expand All @@ -2912,14 +2914,16 @@ public boolean destroyNetwork(final long networkId, final ReservationContext con
// get updated state for the network
network = _networksDao.findById(networkId);
if (network.getState() != Network.State.Allocated && network.getState() != Network.State.Setup && !forced) {
s_logger.debug("Network is not not in the correct state to be destroyed: " + network.getState());
return false;
final String message = "Network is not in the correct state to be destroyed: " + network.getState();
s_logger.debug(message);
throw new InvalidParameterValueException(message);
}

boolean success = true;
if (!cleanupNetworkResources(networkId, callerAccount, context.getCaller().getId())) {
s_logger.warn("Unable to delete network id=" + networkId + ": failed to cleanup network resources");
return false;
final String message = "Unable to delete network id=" + networkId + ": failed to cleanup network resources";
s_logger.warn(message);
throw new InvalidParameterValueException(message);
}

// get providers to destroy
Expand All @@ -2935,12 +2939,6 @@ public boolean destroyNetwork(final long networkId, final ReservationContext con
success = false;
s_logger.warn("Unable to complete destroy of the network: failed to destroy network element " + element.getName());
}
} catch (final ResourceUnavailableException e) {
s_logger.warn("Unable to complete destroy of the network due to element: " + element.getName(), e);
success = false;
} catch (final ConcurrentOperationException e) {
s_logger.warn("Unable to complete destroy of the network due to element: " + element.getName(), e);
success = false;
} catch (final Exception e) {
s_logger.warn("Unable to complete destroy of the network due to element: " + element.getName(), e);
success = false;
Expand Down