Skip to content

Commit

Permalink
Fix unexpected exception on assigning labels on host networks
Browse files Browse the repository at this point in the history
Signed-off-by: Stepan Ermakov <sermakov@orionsoft.ru>

This PR fixes the following issue: an error occurs while assigning labels on host network interfaces: "Error while executing action HostSetupNetworks: Unexpected exception"

Steps to reproduce:

    1. Open "Compute->Hosts". List of hosts will be displayed.
    2. Click on any host and select "Network Interfaces" tab. Network interfaces of the selected host will be displayed.
    3. Click on "Setup Host Networks". "Setup Host Networks" modal dialog will be displayed.
    4. Click on "Labels". Labels of the host networks will be displayed.
    5. Drag-n-drop "New Label" on any network interface. Enter new label name and click "OK". New label will be added to the selected network.
    6. Click "OK" in the "Setup Host Networks" modal dialog.

Expected behavior:
    The changes applied with no errors
Current behavior:
    The error message is displayed: "Error while executing action HostSetupNetworks: Unexpected exception". The new label was not added to the host network.

Background:
    The HostSetupNetworksCommand verifies if there are changes made in the "Setup Host Networks" modal dialog (see HostSetupNetworksCommand.executeCommand, "if (noChangesDetected())" statement).
    Since there are some changes (labels were added) it makes the HostSetupNetworks call to the host VDSM but passes empty payload inside the call (because there were no real changes of networks, just labels were changed). And VDSM fails with the Unexpected Error on the empty payload.
    In this PR I introduced additional check "if (hasNetworkChanges())" that verifies whether the HostSetupNetworks call to the host VDSM is needed or not.
  • Loading branch information
sermakov-orion authored and almusil committed Nov 27, 2023
1 parent f62a440 commit 9fa9d7d
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -340,28 +340,37 @@ protected void executeCommand() {

try (EngineLock monitoringLock = acquireMonitorLock("Host setup networks")) {
int timeout = getSetupNetworksTimeout();
FutureVDSCall<VDSReturnValue> setupNetworksTask = invokeSetupNetworksCommand(timeout);

try {
VDSReturnValue retVal = setupNetworksTask.get(timeout, TimeUnit.SECONDS);
if (retVal != null) {
if (!retVal.getSucceeded() && retVal.getVdsError() == null && getParameters().rollbackOnFailure()) {
throw new EngineException(EngineError.SETUP_NETWORKS_ROLLBACK, retVal.getExceptionString());
}
boolean succeeded = true;

VdsHandler.handleVdsResult(retVal);
if (hasNetworkChanges()) {
succeeded = false;
// Update networks configuration on the host
FutureVDSCall<VDSReturnValue> setupNetworksTask = invokeSetupNetworksCommand(timeout);
VDSReturnValue retVal = setupNetworksTask.get(timeout, TimeUnit.SECONDS);
if (retVal != null) {
if (!retVal.getSucceeded() && retVal.getVdsError() == null && getParameters().rollbackOnFailure()) {
throw new EngineException(EngineError.SETUP_NETWORKS_ROLLBACK, retVal.getExceptionString());
}

if (retVal.getSucceeded()) {
VdsHandler.handleVdsResult(retVal);

VDSReturnValue returnValue =
runVdsCommand(VDSCommandType.GetCapabilities,
new VdsIdAndVdsVDSCommandParametersBase(getVds()));
VDS updatedHost = (VDS) returnValue.getReturnValue();
persistNetworkChanges(updatedHost);
succeeded = retVal.getSucceeded();
}
}

setSucceeded(true);
if (succeeded) {
// If host networks were updated successfully (or there were no host network changes, just changes
// in labels) then update networks configuration in the engine DB.
VDSReturnValue returnValue =
runVdsCommand(VDSCommandType.GetCapabilities,
new VdsIdAndVdsVDSCommandParametersBase(getVds()));
VDS updatedHost = (VDS) returnValue.getReturnValue();
persistNetworkChanges(updatedHost);
}

setSucceeded(succeeded);
} catch (TimeoutException e) {
log.debug("Host Setup networks command timed out for {} seconds", timeout);
}
Expand Down Expand Up @@ -537,6 +546,10 @@ private boolean noChangesDetected() {
return getParameters().isEmptyRequest();
}

private boolean hasNetworkChanges() {
return getParameters().hasNetworkChanges();
}

private List<VdsNetworkInterface> getRemovedBonds() {
if (removedBonds == null) {
Set<Guid> removedBondIds = getParameters().getRemovedBonds();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ public boolean isEmptyRequest() {
removedLabels.isEmpty();
}

public boolean hasNetworkChanges() {
return !networkAttachments.isEmpty() ||
!removedNetworkAttachments.isEmpty() ||
!createOrUpdateBonds.isEmpty() ||
!removedBonds.isEmpty() ||
!removedUnmanagedNetworks.isEmpty();
}

public boolean rollbackOnFailure() {
return rollbackOnFailure;
}
Expand Down

0 comments on commit 9fa9d7d

Please sign in to comment.