Martin Mucha has uploaded a new change for review. Change subject: webadmin: using hostNetworkingAPI from gui. ......................................................................
webadmin: using hostNetworkingAPI from gui. Change-Id: Ib9d99c4963a0b0013797d1e6ed1612edaf19f0b0 Signed-off-by: Martin Mucha <[email protected]> --- M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/NetworkAttachment.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostSetupNetworksModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/SetupNetworksJoinBondsModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/BondNetworkInterfaceModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkCommand.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperation.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationCommandTarget.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java A frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/ExecuteNetworkCommandInNetworkOperationTest.java 9 files changed, 962 insertions(+), 66 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/34/38234/1 diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/NetworkAttachment.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/NetworkAttachment.java index 5eed3ef..4442bb3 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/NetworkAttachment.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/NetworkAttachment.java @@ -1,5 +1,7 @@ package org.ovirt.engine.core.common.businessentities.network; +import java.util.HashMap; +import java.util.List; import java.util.Map; import javax.validation.constraints.NotNull; @@ -32,6 +34,15 @@ private boolean overrideConfiguration; private ReportedConfigurations reportedConfigurations; + ////TODO MM: move to better place. + public static Map<Guid, NetworkAttachment> mapByNetworkId(List<NetworkAttachment> networkAttachments) { + Map<Guid, NetworkAttachment> result = new HashMap<>(networkAttachments.size()); + for (NetworkAttachment networkAttachment : networkAttachments) { + result.put(networkAttachment.getNetworkId(), networkAttachment); + } + return result; + } + public Guid getId() { return id; } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostSetupNetworksModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostSetupNetworksModel.java index 6396042..dbf062d 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostSetupNetworksModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostSetupNetworksModel.java @@ -13,6 +13,7 @@ import java.util.SortedSet; import java.util.TreeSet; +import org.ovirt.engine.core.common.action.HostSetupNetworksParameters; import org.ovirt.engine.core.common.action.SetupNetworksParameters; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdcReturnValueBase; @@ -22,6 +23,7 @@ import org.ovirt.engine.core.common.businessentities.network.Bond; import org.ovirt.engine.core.common.businessentities.network.HostNetworkQos; import org.ovirt.engine.core.common.businessentities.network.Network; +import org.ovirt.engine.core.common.businessentities.network.NetworkAttachment; import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; import org.ovirt.engine.core.common.queries.ConfigurationValues; import org.ovirt.engine.core.common.queries.IdQueryParameters; @@ -66,6 +68,8 @@ public class HostSetupNetworksModel extends EntityModel<VDS> { private EntityModel<Boolean> checkConnectivity; + private final HostSetupNetworksParameters hostSetupNetworksParameters; + private List<NetworkAttachment> allNetworkAttachments; public EntityModel<Boolean> getCheckConnectivity() { @@ -146,6 +150,7 @@ public HostSetupNetworksModel(SearchableListModel listModel, VDS host) { this.sourceListModel = listModel; setEntity(host); + hostSetupNetworksParameters = new HostSetupNetworksParameters(getEntity().getId()); setTitle(ConstantsManager.getInstance().getMessages().setupHostNetworksTitle(host.getName())); setHelpTag(HelpTag.host_setup_networks); @@ -209,7 +214,7 @@ NetworkOperation candidate = NetworkOperationFactory.operationFor(op1, op2, true); if (drop) { - onOperation(candidate, candidate.getCommand(op1, op2, allNics)); + onOperation(candidate, candidate.getCommand(op1, op2, allNics, allNetworkAttachments, hostSetupNetworksParameters)); } // raise the candidate event only if it was changed @@ -223,7 +228,7 @@ } public Map<NetworkOperation, List<NetworkCommand>> commandsFor(NetworkItemModel<?> item) { - return operationFactory.commandsFor(item, allNics); + return operationFactory.commandsFor(item, allNics, allNetworkAttachments, hostSetupNetworksParameters); } public List<VdsNetworkInterface> getAllNics() { @@ -283,6 +288,9 @@ mockDst.setItems(new ArrayList<>(potentialNetworks)); boolean valid = !NetworkOperationFactory.operationFor(mockSrc, mockDst).isNullOperation(); + //TODO MM: note that if you place breakpoint below and overwrite valid to false when editing NIC, new bond + // creation dialog will be displayed. Too weird behavior for validation functionality. Namely it does not + // block dialog from being closed. This renders returing valid useless. if (!valid) { candidateOperation(mockSrc, mockDst, true); // trick to get a red-highlighted error status } @@ -295,8 +303,13 @@ labelModel.commit(iface); NetworkInterfaceModel ifaceModel = nicMap.get(iface.getName()); - NetworkOperation.clearNetworks(ifaceModel, allNics); - NetworkOperation.attachNetworks(ifaceModel, new ArrayList<>(potentialNetworks), allNics); + NetworkOperation.clearNetworks(ifaceModel, allNics, allNetworkAttachments, hostSetupNetworksParameters); + NetworkOperation.attachNetworks(ifaceModel, + new ArrayList<>(potentialNetworks), + allNics, + allNetworkAttachments, + hostSetupNetworksParameters); + } public void onEdit(NetworkItemModel<?> item) { @@ -317,14 +330,25 @@ if (!bondDialogModel.validate()) { return; } - sourceListModel.setConfirmWindow(null); + + String bondName = entity.getName(); + + sourceListModel.setConfirmWindow(null); //TODO MM: why closing prior to validating changes? Collection<LogicalNetworkModel> potentialNetworks = - computeLabelChanges(bondDialogModel.getLabelsModel(), nicMap.get(entity.getName()) + computeLabelChanges(bondDialogModel.getLabelsModel(), nicMap.get(bondName) .getItems()); if (validateLabelChanges(potentialNetworks)) { setBondOptions(entity, bondDialogModel); commitLabelChanges(bondDialogModel.getLabelsModel(), entity, potentialNetworks); redraw(); + + Map<String, Bond> bondNameToBondMap = NetworkOperation.byName(hostSetupNetworksParameters.getBonds()); + + //delete old bond if present. + hostSetupNetworksParameters.getBonds().remove(bondNameToBondMap.get(bondName)); + hostSetupNetworksParameters.getBonds().add((Bond) entity); + } else { + //TODO MM: where's else? (Why) We're not setting bonding mode if labels are not valid? And if they're not valid, why it's ok to close dialog? } } }; @@ -343,13 +367,15 @@ if (!interfacePopupModel.validate()) { return; } - sourceListModel.setConfirmWindow(null); + sourceListModel.setConfirmWindow(null); //TODO MM: why closing prior to validating changes? Collection<LogicalNetworkModel> potentialNetworks = computeLabelChanges(interfacePopupModel.getLabelsModel(), nicMap.get(entity.getName()) .getItems()); if (validateLabelChanges(potentialNetworks)) { commitLabelChanges(interfacePopupModel.getLabelsModel(), entity, potentialNetworks); redraw(); + } else { + //TODO MM: where's else? } } }; @@ -476,6 +502,24 @@ } else { networksToSync.remove(logicalNetwork.getName()); } + + Network updatedNetwork = logicalNetwork.getEntity(); + Guid updatedNetworkId = updatedNetwork.getId(); + + Map<Guid, NetworkAttachment> networkIdToPreexistingNetworkAttachment = + NetworkAttachment.mapByNetworkId(allNetworkAttachments); + Map<Guid, NetworkAttachment> networkIdToNewOrUpdatedNetworkAttachments = + NetworkAttachment.mapByNetworkId(hostSetupNetworksParameters.getNetworkAttachments()); + + + Guid networkAttachmentId = networkIdToPreexistingNetworkAttachment.get(updatedNetworkId).getId(); + + NetworkAttachment previousUpdate = networkIdToNewOrUpdatedNetworkAttachments.get(updatedNetworkId); + hostSetupNetworksParameters.getNetworkAttachments().remove(previousUpdate); + + NetworkAttachment updatedNetworkAttachment = + NetworkOperation.newNetworkAttachment(updatedNetwork, entity, networkAttachmentId); + hostSetupNetworksParameters.getNetworkAttachments().add(updatedNetworkAttachment); sourceListModel.setConfirmWindow(null); } @@ -814,7 +858,7 @@ bondedModel.setBonded(true); bondedModels.add(bondedModel); } - nicModel = new BondNetworkInterfaceModel(nic, nicNetworks, nicLabels, bondedModels, this); + nicModel = new BondNetworkInterfaceModel((Bond)nic, nicNetworks, nicLabels, bondedModels, this); } else { nicModel = new NetworkInterfaceModel(nic, nicNetworks, nicLabels, this); } @@ -863,9 +907,7 @@ @Override public void onSuccess(Object model, Object returnValue) { - List<VdsNetworkInterface> bonds = - ((VdcQueryReturnValue) returnValue).getReturnValue(); - allBonds = bonds; + allBonds = ((VdcQueryReturnValue) returnValue).getReturnValue(); // chain the DC labels query queryLabels(); @@ -873,7 +915,9 @@ }; VDS vds = getEntity(); - Frontend.getInstance().runQuery(VdcQueryType.GetVdsFreeBondsByVdsId, new IdQueryParameters(vds.getId()), asyncQuery); + Frontend.getInstance().runQuery(VdcQueryType.GetVdsFreeBondsByVdsId, + new IdQueryParameters(vds.getId()), + asyncQuery); } private void queryInterfaces() { @@ -886,8 +930,29 @@ { VdcQueryReturnValue returnValue = (VdcQueryReturnValue) returnValueObj; Object returnValue2 = returnValue.getReturnValue(); - List<VdsNetworkInterface> allNics = (List<VdsNetworkInterface>) returnValue2; - HostSetupNetworksModel.this.allNics = allNics; + HostSetupNetworksModel.this.allNics = (List<VdsNetworkInterface>) returnValue2; + + // chain the network attachments query + queryNetworkAttachments(); + } + }; + + VDS vds = getEntity(); + IdQueryParameters params = new IdQueryParameters(vds.getId()); + params.setRefresh(false); + Frontend.getInstance().runQuery(VdcQueryType.GetVdsInterfacesByVdsId, params, asyncQuery); + } + + private void queryNetworkAttachments() { + // query for network attachments + AsyncQuery asyncQuery = new AsyncQuery(); + asyncQuery.setModel(this); + asyncQuery.asyncCallback = new INewAsyncCallback() { + @Override + public void onSuccess(Object model, Object returnValueObj) { + VdcQueryReturnValue returnValue = (VdcQueryReturnValue) returnValueObj; + Object returnValue2 = returnValue.getReturnValue(); + HostSetupNetworksModel.this.allNetworkAttachments = (List<NetworkAttachment>) returnValue2; // chain the free bonds query queryFreeBonds(); @@ -897,7 +962,7 @@ VDS vds = getEntity(); IdQueryParameters params = new IdQueryParameters(vds.getId()); params.setRefresh(false); - Frontend.getInstance().runQuery(VdcQueryType.GetVdsInterfacesByVdsId, params, asyncQuery); + Frontend.getInstance().runQuery(VdcQueryType.GetNetworkAttachmentsByHostId, params, asyncQuery); } private void queryNetworks() { @@ -908,8 +973,7 @@ @Override public void onSuccess(Object model, Object returnValue) { - List<Network> networks = (List<Network>) returnValue; - allNetworks = networks; + allNetworks = (List<Network>) returnValue; initNetworkModels(); initDcNetworkParams(); @@ -1000,7 +1064,8 @@ params.setNetworksToSync(model.getNetworksToSync()); model.startProgress(null); - Frontend.getInstance().runAction(VdcActionType.SetupNetworks, params, new IFrontendActionAsyncCallback() { +// Frontend.getInstance().runAction(VdcActionType.SetupNetworks, params, new IFrontendActionAsyncCallback() { + Frontend.getInstance().runAction(VdcActionType.HostSetupNetworks, hostSetupNetworksParameters, new IFrontendActionAsyncCallback() { @Override public void executed(FrontendActionAsyncResult result) { diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/SetupNetworksJoinBondsModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/SetupNetworksJoinBondsModel.java index b8939ab..27ceddf 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/SetupNetworksJoinBondsModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/SetupNetworksJoinBondsModel.java @@ -10,6 +10,7 @@ import java.util.Map.Entry; import java.util.Set; +import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; import org.ovirt.engine.core.compat.KeyValuePairCompat; import org.ovirt.engine.ui.uicommonweb.models.EntityModel; import org.ovirt.engine.ui.uicommonweb.models.hosts.network.BondNetworkInterfaceModel; @@ -45,7 +46,7 @@ getBondingOptions().setItems(bondOptions); getBondingOptions().setSelectedItem(pairForBondOption.get(target.getBondOptions())); - setLabelsModel(new NicLabelModel(Arrays.asList(source.getEntity(), target.getEntity()), suggestedLabels, labelToIface)); + setLabelsModel(new NicLabelModel(Arrays.<VdsNetworkInterface>asList(source.getEntity(), target.getEntity()), suggestedLabels, labelToIface)); } private void addBondOptionIfMissing(String candidateOption) { diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/BondNetworkInterfaceModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/BondNetworkInterfaceModel.java index 7fab412..fefcfcc 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/BondNetworkInterfaceModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/BondNetworkInterfaceModel.java @@ -3,7 +3,7 @@ import java.util.Collection; import java.util.List; -import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; +import org.ovirt.engine.core.common.businessentities.network.Bond; import org.ovirt.engine.ui.uicommonweb.models.hosts.HostSetupNetworksModel; /** @@ -13,14 +13,16 @@ private final List<NetworkInterfaceModel> bonded; - public BondNetworkInterfaceModel(VdsNetworkInterface bondNic, + public BondNetworkInterfaceModel(Bond bondNic, Collection<LogicalNetworkModel> nicNetworks, Collection<NetworkLabelModel> nicLabels, List<NetworkInterfaceModel> bonded, HostSetupNetworksModel setupModel) { super(bondNic, nicNetworks, nicLabels, setupModel); this.bonded = bonded; - for (NetworkInterfaceModel bondedNic : bonded) { - bondedNic.setBond(this); + Bond bond = (Bond) getEntity(); + for (NetworkInterfaceModel bondedNetworkInterfaceModel : bonded) { + bondedNetworkInterfaceModel.setBond(this); + bond.getSlaves().add(bondedNetworkInterfaceModel.getName()); } } @@ -41,4 +43,9 @@ public String getBondOptions() { return getEntity().getBondOptions(); } + + @Override + public Bond getEntity() { + return (Bond) super.getEntity(); + } } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkCommand.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkCommand.java index 2e42d0a..04eab64 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkCommand.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkCommand.java @@ -2,6 +2,8 @@ import java.util.List; +import org.ovirt.engine.core.common.action.HostSetupNetworksParameters; +import org.ovirt.engine.core.common.businessentities.network.NetworkAttachment; import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; import org.ovirt.engine.ui.uicommonweb.ICommandTarget; import org.ovirt.engine.ui.uicommonweb.UICommand; @@ -14,22 +16,36 @@ private final NetworkItemModel<?> op1; private final NetworkItemModel<?> op2; private final List<VdsNetworkInterface> allNics; + private final List<NetworkAttachment> allNetworkAttachments; + private final HostSetupNetworksParameters hostSetupNetworksParameters; public NetworkCommand(String name, ICommandTarget target, NetworkItemModel<?> op1, NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics) { + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters) { super(name, target); this.op1 = op1; this.op2 = op2; this.allNics = allNics; + this.allNetworkAttachments = allNetworkAttachments; + this.hostSetupNetworksParameters = hostSetupNetworksParameters; } public List<VdsNetworkInterface> getAllNics() { return allNics; } + public List<NetworkAttachment> getAllNetworkAttachments() { + return allNetworkAttachments; + } + + public HostSetupNetworksParameters getHostSetupNetworksParameters() { + return hostSetupNetworksParameters; + } + public NetworkItemModel<?> getOp1() { return op1; } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperation.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperation.java index e3fdeaa..6dbf570 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperation.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperation.java @@ -1,12 +1,21 @@ package org.ovirt.engine.ui.uicommonweb.models.hosts.network; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; +import org.ovirt.engine.core.common.action.HostSetupNetworksParameters; +import org.ovirt.engine.core.common.businessentities.network.Bond; +import org.ovirt.engine.core.common.businessentities.network.IpConfiguration; +import org.ovirt.engine.core.common.businessentities.network.Network; +import org.ovirt.engine.core.common.businessentities.network.NetworkAttachment; import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; +import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.ui.uicompat.ConstantsManager; /** @@ -17,6 +26,8 @@ * Some Operations have required parameters * */ +//TODO MM: NetworkOperation contains duplicates; getVerb, isNullOperation to constructor +//TODO MM: fix naming: model/entity etc. (.*?)Model should be named as \1Model and \1Model.getEntity should be called \1 and not entity. Etc. public enum NetworkOperation { BREAK_BOND { @@ -37,7 +48,10 @@ @Override protected void executeNetworkCommand(NetworkItemModel<?> op1, NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics, Object... params) { + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters, + Object... params) { assert op1 instanceof BondNetworkInterfaceModel; assert op2 == null; List<VdsNetworkInterface> bridgesToRemove = new ArrayList<>(); @@ -50,10 +64,24 @@ networksToDetach.add(bondNetwork); } for (LogicalNetworkModel networkToDetach : networksToDetach) { - DETACH_NETWORK.getCommand(networkToDetach, null, allNics).execute(); + NetworkCommand command = DETACH_NETWORK.getCommand(networkToDetach, + null, + allNics, + allNetworkAttachments, + hostSetupNetworksParameters); + command.execute(); } + Map<String, Bond> bondsMap = byName(hostSetupNetworksParameters.getBonds()); + Bond bond = bondModel.getEntity(); + boolean bondActuallyExisted = bond.getId() != null; String bondName = bondModel.getName(); + + hostSetupNetworksParameters.getBonds().remove(bondsMap.get(bondName)); + if (bondActuallyExisted) { + hostSetupNetworksParameters.getRemovedBonds().add(bond); + } + // delete bonds for (VdsNetworkInterface iface : allNics) { if (iface.getName().startsWith(bondName)) { @@ -90,12 +118,18 @@ @Override protected void executeNetworkCommand(NetworkItemModel<?> op1, NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics, Object... params) { + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters, + Object... params) { assert op1 instanceof LogicalNetworkModel; LogicalNetworkModel networkToDetach = (LogicalNetworkModel) op1; assert networkToDetach.isAttached(); - // remove vlan bridges - detachNetwork(allNics, networkToDetach); + + detachNetworkAndUpdateHostSetupNetworksParameters(allNics, + networkToDetach, + allNetworkAttachments, + hostSetupNetworksParameters); } }; } @@ -121,7 +155,10 @@ @Override protected void executeNetworkCommand(NetworkItemModel<?> op1, NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics, Object... params) { + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters, + Object... params) { assert op1 instanceof LogicalNetworkModel; assert op2 instanceof NetworkInterfaceModel; LogicalNetworkModel networkModelToAttach = (LogicalNetworkModel) op1; @@ -129,8 +166,34 @@ // is network already attached? if (networkModelToAttach.isAttached()) { // detach first - DETACH_NETWORK.getCommand(op1, null, allNics).execute(); + NetworkCommand command = DETACH_NETWORK.getCommand(op1, + null, + allNics, + allNetworkAttachments, + hostSetupNetworksParameters); + command.execute(); } + + Network networkToAttach = networkModelToAttach.getEntity(); + Map<Guid, NetworkAttachment> allNetworkAttachmentMap = NetworkAttachment.mapByNetworkId( allNetworkAttachments); + boolean networkUsedInPreexistingAttachment = allNetworkAttachmentMap.containsKey(networkToAttach.getId()); + Map<Guid, NetworkAttachment> removedNetworkAttachmentByIdMap = NetworkAttachment.mapByNetworkId( + hostSetupNetworksParameters.getRemovedNetworkAttachments()); + + VdsNetworkInterface targetNic = targetNicModel.getEntity(); + + boolean previouslyDetachedNetwork = removedNetworkAttachmentByIdMap.containsKey(networkToAttach.getId()); + if (previouslyDetachedNetwork) { + hostSetupNetworksParameters.getRemovedNetworkAttachments().remove( removedNetworkAttachmentByIdMap.get(networkToAttach.getId())); + } + + if (networkUsedInPreexistingAttachment) { + Guid oldNetworkAttachmentId = allNetworkAttachmentMap.get(networkToAttach.getId()).getId(); + hostSetupNetworksParameters.getNetworkAttachments().add(newNetworkAttachment(networkToAttach, targetNic, oldNetworkAttachmentId)); + } else { + hostSetupNetworksParameters.getNetworkAttachments().add(newNetworkAttachment(networkToAttach, targetNic)); + } + VdsNetworkInterface vlanBridge = networkModelToAttach.attach(targetNicModel, true); if (vlanBridge != null) { Iterator<VdsNetworkInterface> i = allNics.iterator(); @@ -172,7 +235,10 @@ @Override protected void executeNetworkCommand(NetworkItemModel<?> op1, NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics, Object... params) { + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters, + Object... params) { assert op1 instanceof NetworkInterfaceModel && !(op1 instanceof BondNetworkInterfaceModel); assert op2 instanceof NetworkInterfaceModel && !(op2 instanceof BondNetworkInterfaceModel); assert params.length == 1 : "incorrect params length"; //$NON-NLS-1$ @@ -180,18 +246,30 @@ NetworkInterfaceModel nic2Model = (NetworkInterfaceModel) op2; // detach possible networks from both nics - clearNetworks(nic1Model, allNics); - clearNetworks(nic2Model, allNics); + clearNetworks(nic1Model, allNics, allNetworkAttachments, hostSetupNetworksParameters); + clearNetworks(nic2Model, allNics, allNetworkAttachments, hostSetupNetworksParameters); // param - VdsNetworkInterface bond = (VdsNetworkInterface) params[0]; + Bond bond = (Bond) params[0]; String bondName = bond.getName(); // add to nic list allNics.add(bond); - nic1Model.getEntity().setBondName(bondName); - nic2Model.getEntity().setBondName(bondName); + VdsNetworkInterface nic1 = nic1Model.getEntity(); + VdsNetworkInterface nic2 = nic2Model.getEntity(); + nic1.setBondName(bondName); + nic2.setBondName(bondName); + bond.getSlaves().add(nic1.getName()); + bond.getSlaves().add(nic2.getName()); + + Map<String, Bond> removedBondsMap = byName(hostSetupNetworksParameters.getRemovedBonds()); + boolean previouslyRemovedBond = removedBondsMap.containsKey(bond.getName()); + if (previouslyRemovedBond) { + hostSetupNetworksParameters.getRemovedBonds().remove(removedBondsMap.get(bond.getName())); + } + hostSetupNetworksParameters.getBonds().add(bond); + } }; } @@ -214,27 +292,41 @@ @Override protected void executeNetworkCommand(NetworkItemModel<?> op1, NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics, Object... params) { + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters, + Object... params) { assert op1 instanceof BondNetworkInterfaceModel; assert op2 instanceof BondNetworkInterfaceModel; assert params.length == 1 : "incorrect params length"; //$NON-NLS-1$ - Set<NetworkInterfaceModel> slaveModels = new HashSet<>(); + Set<NetworkInterfaceModel> slaveModels = new HashSet<>(); ////TODO MM: why set? There shouldn't be overlap in slaves and if there is, this cover-ups problem. slaveModels.addAll(((BondNetworkInterfaceModel) op1).getBonded()); slaveModels.addAll(((BondNetworkInterfaceModel) op2).getBonded()); // break both bonds - BREAK_BOND.getCommand(op1, null, allNics).execute(); - BREAK_BOND.getCommand(op2, null, allNics).execute(); + BREAK_BOND.getCommand(op1, null, allNics, allNetworkAttachments, hostSetupNetworksParameters).execute(); + BREAK_BOND.getCommand(op2, null, allNics, allNetworkAttachments, hostSetupNetworksParameters).execute(); // param - VdsNetworkInterface bond = (VdsNetworkInterface) params[0]; + Bond bond = (Bond) params[0]; String bondName = bond.getName(); // add to nic list allNics.add(bond); for (NetworkInterfaceModel slaveModel : slaveModels) { - slaveModel.getEntity().setBondName(bondName); + VdsNetworkInterface slave = slaveModel.getEntity(); + slave.setBondName(bondName); + bond.getSlaves().add(slave.getName()); + } + + Map<String, Bond> removedBondsMap = byName(hostSetupNetworksParameters.getRemovedBonds()); + boolean previouslyRemovedBond = removedBondsMap.containsKey(bond.getName()); + if (previouslyRemovedBond) { + hostSetupNetworksParameters.getRemovedBonds().remove(removedBondsMap.get(bond.getName())); + hostSetupNetworksParameters.getBonds().add(bond); + } else { + hostSetupNetworksParameters.getBonds().add(bond); } } }; @@ -259,7 +351,10 @@ @Override protected void executeNetworkCommand(NetworkItemModel<?> op1, NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics, Object... params) { + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters, + Object... params) { assert op1 instanceof NetworkInterfaceModel; assert op2 instanceof BondNetworkInterfaceModel; @@ -272,12 +367,26 @@ : new ArrayList<LogicalNetworkModel>(); // Detach possible networks from the nic - clearNetworks(nic, allNics); + clearNetworks(nic, allNics, allNetworkAttachments, hostSetupNetworksParameters); // Attach previous nic networks to bond - attachNetworks(bond, networksToReatach, allNics); + attachNetworks(bond, networksToReatach, allNics, allNetworkAttachments, hostSetupNetworksParameters); - nic.getEntity().setBondName(bond.getEntity().getName()); + Bond bondEntity = bond.getEntity(); + String bondName = bondEntity.getName(); + nic.getEntity().setBondName(bondName); + String slaveName = nic.getEntity().getName(); + + Map<String, Bond> bondsMap = byName(hostSetupNetworksParameters.getBonds()); + + //TODO MM: removing and adding back a slave will end up in bond update even if there's no need for that. + boolean bondIsAlreadyBeingUpdated = bondsMap.containsKey(bondName); + if (bondIsAlreadyBeingUpdated) { + bondsMap.get(bond.getName()).getSlaves().add(slaveName); + } else { + bondEntity.getSlaves().add(slaveName); + hostSetupNetworksParameters.getBonds().add(bondEntity); + } } }; } @@ -301,8 +410,11 @@ @Override protected void executeNetworkCommand(NetworkItemModel<?> op1, NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics, Object... params) { - ADD_TO_BOND.getCommand(op2, op1, allNics).execute(); + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters, + Object... params) { + ADD_TO_BOND.getCommand(op2, op1, allNics, allNetworkAttachments, hostSetupNetworksParameters).execute(); } }; } @@ -331,15 +443,33 @@ @Override protected void executeNetworkCommand(NetworkItemModel<?> op1, NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics, Object... params) { + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters, + Object... params) { assert op1 instanceof NetworkInterfaceModel; + assert op2 == null; NetworkInterfaceModel nicModel = (NetworkInterfaceModel) op1; VdsNetworkInterface slaveNic = nicModel.getEntity(); slaveNic.setBondName(null); // is there are only two nics, break the bond BondNetworkInterfaceModel bondModel = nicModel.getBond(); + Bond bond = bondModel.getEntity(); + String bondName = bond.getName(); + if (bondModel.getBonded().size() == 2) { - BREAK_BOND.getCommand(bondModel, null, allNics).execute(); + BREAK_BOND.getCommand(bondModel, null, allNics, allNetworkAttachments, hostSetupNetworksParameters).execute(); + } else { + Map<String, Bond> bondsMap = byName(hostSetupNetworksParameters.getBonds()); + boolean bondWasAlreadyUpdated = bondsMap.containsKey(bondName); + if (bondWasAlreadyUpdated) { + Bond formerlyUpdatedBond = bondsMap.get(bondName); + formerlyUpdatedBond.getSlaves().remove(slaveNic.getName()); + } else { + bond.getSlaves().remove(slaveNic.getName()); + hostSetupNetworksParameters.getBonds().add(bond); + } + } } }; @@ -373,8 +503,19 @@ @Override protected void executeNetworkCommand(NetworkItemModel<?> op1, NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics, Object... params) { - DETACH_NETWORK.getCommand(op1, op2, allNics).execute(); + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters, + Object... params) { + + assert op1 instanceof LogicalNetworkModel; + LogicalNetworkModel networkToDetach = (LogicalNetworkModel) op1; + assert networkToDetach.isAttached(); + Guid networkId = networkToDetach.getEntity().getId(); + assert networkId == null; + + detachNetworkWithoutUpdatingHostSetupNetworksParameters(allNics, networkToDetach); + hostSetupNetworksParameters.getRemovedUnmanagedNetworks().add(networkToDetach.getEntity().getName()); } }; } @@ -542,22 +683,54 @@ }; - public static void clearNetworks(NetworkInterfaceModel nic, List<VdsNetworkInterface> allNics) { + public static void clearNetworks(NetworkInterfaceModel nic, + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters) { List<LogicalNetworkModel> attachedNetworks = nic.getItems(); if (attachedNetworks.size() > 0) { for (LogicalNetworkModel networkModel : new ArrayList<>(attachedNetworks)) { - DETACH_NETWORK.getCommand(networkModel, null, allNics).execute(); + DETACH_NETWORK.getCommand(networkModel, null, allNics, allNetworkAttachments, hostSetupNetworksParameters).execute(); } } } - public static void attachNetworks(NetworkInterfaceModel nic, List<LogicalNetworkModel> networks, List<VdsNetworkInterface> allNics) { + public static void attachNetworks(NetworkInterfaceModel nic, + List<LogicalNetworkModel> networks, + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters) { for (LogicalNetworkModel networkModel : networks) { - ATTACH_NETWORK.getCommand(networkModel, nic, allNics).execute(); + ATTACH_NETWORK.getCommand(networkModel, nic, allNics, allNetworkAttachments, hostSetupNetworksParameters).execute(); } } - private static void detachNetwork(List<VdsNetworkInterface> allNics, + private static void detachNetworkAndUpdateHostSetupNetworksParameters(List<VdsNetworkInterface> allNics, + LogicalNetworkModel networkToDetach, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters) { + + detachNetworkWithoutUpdatingHostSetupNetworksParameters(allNics, networkToDetach); + + Guid networkId = networkToDetach.getEntity().getId(); + Map<Guid, NetworkAttachment> allNetworkAttachmentMap = NetworkAttachment.mapByNetworkId(allNetworkAttachments); + boolean detachingPreexistingNetworkAttachment = allNetworkAttachmentMap.containsKey(networkId); + + if (detachingPreexistingNetworkAttachment) { + hostSetupNetworksParameters.getRemovedNetworkAttachments().add(allNetworkAttachmentMap.get(networkId)); + } + + //if network attachment was issued to be updated, remove it from such request + for (Iterator<NetworkAttachment> iterator = hostSetupNetworksParameters.getNetworkAttachments().iterator(); iterator.hasNext(); ) { + NetworkAttachment networkAttachment = iterator.next(); + if (networkAttachment.getNetworkId().equals(networkId)) { + iterator.remove(); + } + + } + } + + private static void detachNetworkWithoutUpdatingHostSetupNetworksParameters(List<VdsNetworkInterface> allNics, LogicalNetworkModel networkToDetach) { // remove the vlan device if (networkToDetach.hasVlan()) { @@ -580,8 +753,18 @@ */ public NetworkCommand getCommand(final NetworkItemModel<?> op1, final NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics) { - return new NetworkCommand(getMenuTitle(op1, op2), getTarget(), op1, op2, allNics); + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters) { + + + return new NetworkCommand(getMenuTitle(op1, op2), + getTarget(), + op1, + op2, + allNics, + allNetworkAttachments, + hostSetupNetworksParameters); } /** @@ -640,6 +823,8 @@ protected void executeNetworkCommand(NetworkItemModel<?> op1, NetworkItemModel<?> op2, List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters, Object... params) { // NOOP @@ -685,4 +870,33 @@ private static boolean isDisplayNetwork(LogicalNetworkModel logicalNetworkModel) { return logicalNetworkModel.getEntity().getCluster().isDisplay(); } + + public static NetworkAttachment newNetworkAttachment(Network network, VdsNetworkInterface targetNic) { + return newNetworkAttachment(network, targetNic, null); + } + + public static NetworkAttachment newNetworkAttachment(Network network, VdsNetworkInterface targetNic, Guid networkAttachmentId) { + NetworkAttachment networkAttachment = new NetworkAttachment(); + networkAttachment.setId(networkAttachmentId); + networkAttachment.setNetworkId(network.getId()); + networkAttachment.setNicId(targetNic.getId()); + networkAttachment.setOverrideConfiguration(true); + IpConfiguration ipConfiguration = new IpConfiguration(); + networkAttachment.setIpConfiguration(ipConfiguration); + ipConfiguration.setGateway(network.getGateway()); + ipConfiguration.setNetmask(network.getSubnet()); + ipConfiguration.setAddress(network.getAddr()); + ipConfiguration.setBootProtocol(targetNic.getBootProtocol()); + + return networkAttachment; + } + + //TODO MM: rename & move to better place. + public static <E extends VdsNetworkInterface> Map<String, E> byName(Collection<E> collection) { + Map<String, E> map = new HashMap<>(); + for (E e : collection) { + map.put(e.getName(), e); + } + return map; + } } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationCommandTarget.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationCommandTarget.java index b07be46..b73f23f 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationCommandTarget.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationCommandTarget.java @@ -2,6 +2,8 @@ import java.util.List; +import org.ovirt.engine.core.common.action.HostSetupNetworksParameters; +import org.ovirt.engine.core.common.businessentities.network.NetworkAttachment; import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; import org.ovirt.engine.ui.uicommonweb.ICommandTarget; import org.ovirt.engine.ui.uicommonweb.UICommand; @@ -22,11 +24,16 @@ NetworkItemModel<?> op1 = command.getOp1(); NetworkItemModel<?> op2 = command.getOp2(); List<VdsNetworkInterface> allNics = command.getAllNics(); - executeNetworkCommand(op1, op2, allNics, params); + List<NetworkAttachment> allNetworkAttachments = command.getAllNetworkAttachments(); + HostSetupNetworksParameters hostSetupNetworksParameters = command.getHostSetupNetworksParameters(); + executeNetworkCommand(op1, op2, allNics, allNetworkAttachments, hostSetupNetworksParameters, params); } protected abstract void executeNetworkCommand(NetworkItemModel<?> op1, NetworkItemModel<?> op2, - List<VdsNetworkInterface> allNics, Object... params); + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters, + Object... params); } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java index de7fd96..3982d42 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java @@ -7,6 +7,8 @@ import java.util.Map; import java.util.Set; +import org.ovirt.engine.core.common.action.HostSetupNetworksParameters; +import org.ovirt.engine.core.common.businessentities.network.NetworkAttachment; import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; /** @@ -238,14 +240,22 @@ * @param allNics * @return */ - public Map<NetworkOperation, List<NetworkCommand>> commandsFor(NetworkItemModel<?> item, List<VdsNetworkInterface> allNics) { + public Map<NetworkOperation, List<NetworkCommand>> commandsFor(NetworkItemModel<?> item, + List<VdsNetworkInterface> allNics, + List<NetworkAttachment> allNetworkAttachments, + HostSetupNetworksParameters hostSetupNetworksParameters) { Map<NetworkOperation, List<NetworkCommand>> operations = new HashMap<>(); // with nics for (NetworkInterfaceModel nic : nics) { NetworkOperation operation = operationFor(item, nic); if (!operation.isNullOperation()) { assertBinary(item, nic, operation); - addToOperationMultiMap(operations, operation, operation.getCommand(item, nic, allNics)); + NetworkCommand command = operation.getCommand(item, + nic, + allNics, + allNetworkAttachments, + hostSetupNetworksParameters); + addToOperationMultiMap(operations, operation, command); } } // with networks @@ -253,7 +263,12 @@ NetworkOperation operation = operationFor(item, network); if (!operation.isNullOperation()) { assertBinary(item, network, operation); - addToOperationMultiMap(operations, operation, operation.getCommand(item, network, allNics)); + NetworkCommand command = operation.getCommand(item, + network, + allNics, + allNetworkAttachments, + hostSetupNetworksParameters); + addToOperationMultiMap(operations, operation, command); } } @@ -262,7 +277,12 @@ if (!operation.isNullOperation()) { assert operation.isUnary() : "Operation " + operation.name() //$NON-NLS-1$ + " is Binary, while a Uniary Operation is expected for " + item.getName(); //$NON-NLS-1$ - addToOperationMultiMap(operations, operation, operation.getCommand(item, null, allNics)); + NetworkCommand command = operation.getCommand(item, + null, + allNics, + allNetworkAttachments, + hostSetupNetworksParameters); + addToOperationMultiMap(operations, operation, command); } return operations; diff --git a/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/ExecuteNetworkCommandInNetworkOperationTest.java b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/ExecuteNetworkCommandInNetworkOperationTest.java new file mode 100644 index 0000000..ce1b9db --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/ExecuteNetworkCommandInNetworkOperationTest.java @@ -0,0 +1,555 @@ +package org.ovirt.engine.ui.uicommonweb.models.hosts.network; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.junit.Assert.assertThat; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.hamcrest.Matcher; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; +import org.ovirt.engine.core.common.action.HostSetupNetworksParameters; +import org.ovirt.engine.core.common.businessentities.network.Bond; +import org.ovirt.engine.core.common.businessentities.network.Network; +import org.ovirt.engine.core.common.businessentities.network.NetworkAttachment; +import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.ui.uicommonweb.TypeResolver; +import org.ovirt.engine.ui.uicompat.ConstantsManager; +import org.ovirt.engine.ui.uicompat.UIMessages; + +@RunWith(MockitoJUnitRunner.class) +public class ExecuteNetworkCommandInNetworkOperationTest { + + private HostSetupNetworksParameters hsnp = new HostSetupNetworksParameters(Guid.newGuid()); + + @Mock + private LogicalNetworkModel logicalNetworkModelOfNetworkA; + + @Mock + private LogicalNetworkModel logicalNetworkModelOfNetworkC; + + @Mock + private BondNetworkInterfaceModel bondNetworkInterfaceModelA; + + @Mock + private BondNetworkInterfaceModel bondNetworkInterfaceModelB; + + private Network networkA = createNetwork("networkA"); //$NON-NLS-1$ + private Network networkC = createNetwork("networkC"); //$NON-NLS-1$ + + @Mock + private NetworkInterfaceModel networkInterfaceModelOfNicA; + + @Mock + private NetworkInterfaceModel networkInterfaceModelOfNicB; + + @Mock + private NetworkInterfaceModel networkInterfaceModelOfNicC; + + @Mock + private NetworkInterfaceModel networkInterfaceModelOfNicD; + + private VdsNetworkInterface nicA = createNic("nicA"); //$NON-NLS-1$ + private VdsNetworkInterface nicB = createNic("nicB"); //$NON-NLS-1$ + private VdsNetworkInterface nicC = createNic("nicC"); //$NON-NLS-1$ + private VdsNetworkInterface nicD = createNic("nicD"); //$NON-NLS-1$ + + private final Guid existingBondId = Guid.newGuid(); + private final String existingBondName = "existingBond"; //$NON-NLS-1$ + private Bond existingBond = createBond(existingBondId, existingBondName, Arrays.asList(nicA, nicB)); + private Bond newlyCreatedBond = createBond(null, "newlyCreatedBond", Collections.<VdsNetworkInterface>emptyList()); //$NON-NLS-1$ + + private List<VdsNetworkInterface> allNics = new ArrayList<>(); + + private List<NetworkAttachment> allNetworkAttachments = new ArrayList<>(); + + @Before + public void setUp() throws Exception { + when(logicalNetworkModelOfNetworkA.getEntity()).thenReturn(networkA); + when(logicalNetworkModelOfNetworkC.getEntity()).thenReturn(networkC); + when(networkInterfaceModelOfNicA.getEntity()).thenReturn(nicA); + when(networkInterfaceModelOfNicB.getEntity()).thenReturn(nicB); + when(networkInterfaceModelOfNicC.getEntity()).thenReturn(nicC); + when(networkInterfaceModelOfNicD.getEntity()).thenReturn(nicD); + + //mock manager/resolver so it's possible to delegate from one NetworkOperation to another. + ConstantsManager constantsManagerMock = Mockito.mock(ConstantsManager.class); + UIMessages uiMessagesMock = Mockito.mock(UIMessages.class); + when(constantsManagerMock.getMessages()).thenReturn(uiMessagesMock); + when(uiMessagesMock.detachNetwork(anyString())).thenReturn("doh"); //$NON-NLS-1$ + ConstantsManager.setInstance(constantsManagerMock); + TypeResolver typeResolverMock = Mockito.mock(TypeResolver.class); + TypeResolver.setInstance(typeResolverMock); + } + + /* + * At the beginning there was a void, then NetworkAttachment was created attaching given network and nic. + * */ + @Test + public void testCreatingBrandNewNetworkAttachment() throws Exception { + when(logicalNetworkModelOfNetworkA.isAttached()).thenReturn(false); + + NetworkOperation.ATTACH_NETWORK.getTarget().executeNetworkCommand( + logicalNetworkModelOfNetworkA, + networkInterfaceModelOfNicA, + allNics, + allNetworkAttachments, + hsnp + ); + + assertThat(hsnp.getNetworkAttachments().size(), is(1)); + NetworkAttachment networkAttachment = hsnp.getNetworkAttachments().iterator().next(); + assertNetworkAttachment(networkAttachment, null, networkA.getId(), nicA.getId()); + + assertThat(hsnp.getRemovedNetworkAttachments().isEmpty(), is(true)); + } + + /* + * At the beginning there was a NetworkAttachment. Suddenly network was detached from the nic, but in the end, + * network was back attached to the nic unchanged. + * */ + @Test + public void testReattachingPreexistingNetworkAfterItsBeingDetached() throws Exception { + when(logicalNetworkModelOfNetworkA.isAttached()).thenReturn(false); + + Guid networkAttachmentId = Guid.newGuid(); + NetworkAttachment networkAttachment = NetworkOperation.newNetworkAttachment(networkA, nicA, networkAttachmentId); + allNetworkAttachments.add(networkAttachment); + hsnp.getRemovedNetworkAttachments().add(networkAttachment); + + NetworkOperation.ATTACH_NETWORK.getTarget().executeNetworkCommand( + logicalNetworkModelOfNetworkA, + networkInterfaceModelOfNicA, + allNics, + allNetworkAttachments, + hsnp + ); + + assertThat(hsnp.getNetworkAttachments().size(), is(1)); + + NetworkAttachment updatedNetworkAttachment = hsnp.getNetworkAttachments().iterator().next(); + assertNetworkAttachment(updatedNetworkAttachment, networkAttachmentId, networkA.getId(), nicA.getId()); + + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(0)); + } + + /* + * At the beginning there was a NetworkAttachment. Suddenly network was detached from the nic, and gets firmly + * attached to another nic. + * */ + @Test + public void testReattachingPreexistingNetworkToDifferentNicAfterItsBeingDetached() throws Exception { + when(logicalNetworkModelOfNetworkA.isAttached()).thenReturn(false); + when(networkInterfaceModelOfNicA.getEntity()).thenReturn(nicB); + + Guid networkAttachmentId = Guid.newGuid(); + NetworkAttachment formerAttachment = NetworkOperation.newNetworkAttachment(networkA, nicA, networkAttachmentId); + allNetworkAttachments.add(formerAttachment); + hsnp.getRemovedNetworkAttachments().add(formerAttachment); + + NetworkOperation.ATTACH_NETWORK.getTarget().executeNetworkCommand( + logicalNetworkModelOfNetworkA, + networkInterfaceModelOfNicA, + allNics, + allNetworkAttachments, + hsnp + ); + + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(0)); + assertThat(hsnp.getNetworkAttachments().size(), is(1)); + assertNetworkAttachment(hsnp.getNetworkAttachments().iterator().next(), + networkAttachmentId, + networkA.getId(), + nicB.getId()); + } + + /* + * At the beginning there was a NetworkAttachment, and network gets detached from the nic. + * */ + @Test + public void testDetachingPreexistingNetworkAttachment() throws Exception { + Guid networkAttachmentId = Guid.newGuid(); + NetworkAttachment networkAttachment = NetworkOperation.newNetworkAttachment(networkA, nicA, networkAttachmentId); + allNetworkAttachments.add(networkAttachment); + when(logicalNetworkModelOfNetworkA.hasVlan()).thenReturn(false); + when(logicalNetworkModelOfNetworkA.isAttached()).thenReturn(true); + + NetworkOperation.DETACH_NETWORK.getTarget().executeNetworkCommand( + logicalNetworkModelOfNetworkA, + null, + allNics, + allNetworkAttachments, + hsnp + ); + + assertThat(hsnp.getNetworkAttachments().size(), is(0)); + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(1)); + NetworkAttachment removedNetworkAttachment = hsnp.getRemovedNetworkAttachments().iterator().next(); + assertNetworkAttachment(removedNetworkAttachment, networkAttachmentId, networkA.getId(), nicA.getId()); + } + + /* + * At the beginning there was a void, then NetworkAttachment was created attaching given network with nic, + * and then her was immediately detached from him. + * */ + @Test + public void testDetachingPreviouslyAddedNetworkAttachment() throws Exception { + NetworkAttachment networkAttachment = NetworkOperation.newNetworkAttachment(networkA, nicA); + hsnp.getNetworkAttachments().add(networkAttachment); + when(logicalNetworkModelOfNetworkA.hasVlan()).thenReturn(false); + when(logicalNetworkModelOfNetworkA.isAttached()).thenReturn(true); + + NetworkOperation.DETACH_NETWORK.getTarget().executeNetworkCommand( + logicalNetworkModelOfNetworkA, + null, + allNics, + allNetworkAttachments, + hsnp + ); + + assertThat(hsnp.getNetworkAttachments().size(), is(0)); + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(0)); + } + + /* + * At the beginning there was a bond, which was then broken. + * */ + @Test + public void testBreakingExistingBond() throws Exception { + when(bondNetworkInterfaceModelA.getItems()).thenReturn(Collections.<LogicalNetworkModel> emptyList()); + when(bondNetworkInterfaceModelA.getEntity()).thenReturn(existingBond); + + NetworkOperation.BREAK_BOND.getTarget().executeNetworkCommand( + bondNetworkInterfaceModelA, + null, + allNics, + allNetworkAttachments, + hsnp + ); + + assertThat(hsnp.getNetworkAttachments().size(), is(0)); + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(0)); + + assertThat(hsnp.getBonds().size(), is(0)); + assertThat(hsnp.getRemovedBonds().size(), is(1)); + assertBond(hsnp.getRemovedBonds().iterator().next(), existingBond.getId(), Arrays.asList(nicA, nicB)); + } + + /* + * At the beginning there was a void, then bond was created and was immediately broken. + * */ + @Test + public void testBreakingNewlyCreatedBond() throws Exception { + when(bondNetworkInterfaceModelA.getItems()).thenReturn(Collections.<LogicalNetworkModel> emptyList()); + when(bondNetworkInterfaceModelA.getEntity()).thenReturn(newlyCreatedBond); + + NetworkOperation.BREAK_BOND.getTarget().executeNetworkCommand( + bondNetworkInterfaceModelA, + null, + allNics, + allNetworkAttachments, + hsnp + ); + + assertThat(hsnp.getNetworkAttachments().size(), is(0)); + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(0)); + + assertThat(hsnp.getBonds().size(), is(0)); + assertThat(hsnp.getRemovedBonds().size(), is(0)); + } + + /* + * At the beginning there was a bond, which was then broken. + * */ + @Test + public void testBreakingExistingBondWithNetworkAttached() throws Exception { + NetworkAttachment networkAttachment = NetworkOperation.newNetworkAttachment(networkA, existingBond); + + allNetworkAttachments.add(networkAttachment); + when(logicalNetworkModelOfNetworkA.hasVlan()).thenReturn(false); + when(logicalNetworkModelOfNetworkA.isAttached()).thenReturn(true); + + when(bondNetworkInterfaceModelA.getItems()).thenReturn(Collections.singletonList(logicalNetworkModelOfNetworkA)); + when(bondNetworkInterfaceModelA.getEntity()).thenReturn(existingBond); + + NetworkOperation.BREAK_BOND.getTarget().executeNetworkCommand( + bondNetworkInterfaceModelA, + null, + allNics, + allNetworkAttachments, + hsnp + ); + + assertThat(hsnp.getNetworkAttachments().size(), is(0)); + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(1)); + assertNetworkAttachment(hsnp.getRemovedNetworkAttachments().iterator().next(), networkAttachment.getId(), networkA.getId(), existingBond.getId()); + + assertThat(hsnp.getBonds().size(), is(0)); + assertThat(hsnp.getRemovedBonds().size(), is(1)); + assertBond(hsnp.getRemovedBonds().iterator().next(), existingBond.getId(), Arrays.asList(nicA, nicB)); + } + + /* + * At the beginning there was two nics (one with NetworkAttachment), which, after being introduced to each other, + * formed a firm bond adopting NetworkAttachment as their own. + * */ + @Test + public void testBondingTwoNicsWithReattachingNetworkAttachmentOnNewlyCreatedBond() throws Exception { + Guid networkAttachmentId = Guid.newGuid(); + NetworkAttachment networkAttachment = NetworkOperation.newNetworkAttachment(networkA, nicA, networkAttachmentId); + + allNetworkAttachments.add(networkAttachment); + when(logicalNetworkModelOfNetworkA.isAttached()).thenReturn(true); + + when(networkInterfaceModelOfNicA.getItems()).thenReturn(Collections.singletonList(logicalNetworkModelOfNetworkA)); + + NetworkOperation.BOND_WITH.getTarget().executeNetworkCommand( + networkInterfaceModelOfNicA, + networkInterfaceModelOfNicB, + allNics, + allNetworkAttachments, + hsnp, + newlyCreatedBond + ); + + when(logicalNetworkModelOfNetworkA.isAttached()).thenReturn(false); + + //this is not part of BOND_WITH command, it's simply called after it. BOND_WITH is actually: "detach networks and create bond". + //in production code, probably due to some problems with listeners, this is actually called three times, luckily each time overwriting previous call. + NetworkOperation.attachNetworks(networkInterfaceModelOfNicA, + Collections.singletonList(logicalNetworkModelOfNetworkA), + allNics, + allNetworkAttachments, + hsnp); + + //related network attachment will be updated, not removed and created new one. + assertThat(hsnp.getNetworkAttachments().size(), is(1)); + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(0)); + assertNetworkAttachment(hsnp.getNetworkAttachments().iterator().next(), networkAttachmentId, networkA.getId(), nicA.getId()); + + assertThat(hsnp.getBonds().size(), is(1)); + assertBond(hsnp.getBonds().iterator().next(), null, Arrays.asList(nicA, nicB)); + assertThat(hsnp.getRemovedBonds().size(), is(0)); + } + + /* + * At the beginning there was bond. It was broken into two nics, but they get back together in the end. + * */ + @Test + public void testReBondingTwoNicsWithReattachingNetworkAttachmentOnNewlyCreatedBond() throws Exception { + + when(logicalNetworkModelOfNetworkA.isAttached()).thenReturn(false); + + hsnp.getRemovedBonds().add(existingBond); + + NetworkOperation.BOND_WITH.getTarget().executeNetworkCommand( + networkInterfaceModelOfNicA, + networkInterfaceModelOfNicB, + allNics, + allNetworkAttachments, + hsnp, + createBond(existingBondId, existingBondName, Collections.<VdsNetworkInterface> emptyList()) + ); + + //related network attachment will be updated, not removed and created new one. + assertThat(hsnp.getNetworkAttachments().size(), is(0)); + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(0)); + + assertThat(hsnp.getBonds().size(), is(1)); + assertBond(hsnp.getBonds().iterator().next(), existingBondId, Arrays.asList(nicA, nicB)); + assertThat(hsnp.getRemovedBonds().size(), is(0)); + } + + /* + * At the beginning there was bond without any network attachment. Then another nic showed up joining the family + * bringing his own network attachment along. + * */ + @Test + public void testAddingNewNicWithNetworkAttachmentToExistingBondWithoutAnyAttachment() throws Exception { + Guid networkAttachmentId = Guid.newGuid(); + NetworkAttachment networkAttachment = + NetworkOperation.newNetworkAttachment(networkC, nicC, networkAttachmentId); + + allNetworkAttachments.add(networkAttachment); + + //this can be confusing. network *is* attached but it gets detached as a part of ADD_TO_BOND, so consulting this method, it will be detached. + when(logicalNetworkModelOfNetworkC.isAttached()).thenReturn(true, false); + + when(networkInterfaceModelOfNicC.getItems()).thenReturn(Collections.singletonList(logicalNetworkModelOfNetworkC)); + + when(bondNetworkInterfaceModelA.getItems()).thenReturn(Collections.<LogicalNetworkModel> emptyList()); + when(bondNetworkInterfaceModelA.getEntity()).thenReturn(existingBond); + + + NetworkOperation.ADD_TO_BOND.getTarget().executeNetworkCommand( + networkInterfaceModelOfNicC, + bondNetworkInterfaceModelA, + allNics, + allNetworkAttachments, + hsnp + ); + + //related network attachment will be updated, not removed and created new one. + assertThat(hsnp.getNetworkAttachments().size(), is(1)); + assertNetworkAttachment(hsnp.getNetworkAttachments().iterator().next(), + networkAttachmentId, + networkC.getId(), + existingBondId); + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(0)); + + assertThat(hsnp.getBonds().size(), is(1)); + assertBond(hsnp.getBonds().iterator().next(), existingBondId, Arrays.asList(nicA, nicB, nicC)); + assertThat(hsnp.getRemovedBonds().size(), is(0)); + } + + /* + * At the beginning there was a bond with three slaves, the one of them left others. + * */ + @Test + public void testRemoveSlaveFromBond() throws Exception { + + Bond bond = createBond(existingBondId, existingBondName, Arrays.asList(nicA, nicB, nicC)); + + + Guid networkAttachmentId = Guid.newGuid(); + NetworkAttachment networkAttachment = + NetworkOperation.newNetworkAttachment(networkA, bond, networkAttachmentId); + + allNetworkAttachments.add(networkAttachment); + + when(bondNetworkInterfaceModelA.getEntity()).thenReturn(bond); + + when(networkInterfaceModelOfNicA.getBond()).thenReturn(bondNetworkInterfaceModelA); + + when(bondNetworkInterfaceModelA.getEntity()).thenReturn(bond); + + + NetworkOperation.REMOVE_FROM_BOND.getTarget().executeNetworkCommand( + networkInterfaceModelOfNicA, + null, + allNics, + allNetworkAttachments, + hsnp + ); + + //related network attachment will be updated, not removed and created new one. + assertThat(hsnp.getNetworkAttachments().size(), is(0)); + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(0)); + + assertThat(hsnp.getBonds().size(), is(1)); + assertBond(hsnp.getBonds().iterator().next(), existingBondId, Arrays.asList(nicB, nicC)); + assertThat(hsnp.getRemovedBonds().size(), is(0)); + } + + /* + * At the beginning there was two nics (one with NetworkAttachment), which, after being introduced to each other, + * formed a firm bond adopting NetworkAttachment as their own. + * */ + @Test + public void testJoiningBonds() throws Exception { + Guid networkAttachmentId = Guid.newGuid(); + NetworkAttachment networkAttachment = NetworkOperation.newNetworkAttachment(networkA, + existingBond, + networkAttachmentId); + + Guid bondBId = Guid.newGuid(); + Bond bondB = createBond(bondBId, "bondB", Arrays.asList(nicC, nicD)); //$NON-NLS-1$ + + allNetworkAttachments.add(networkAttachment); + when(logicalNetworkModelOfNetworkA.isAttached()).thenReturn(true); + + when(bondNetworkInterfaceModelA.getItems()).thenReturn(Collections.singletonList(logicalNetworkModelOfNetworkA)); + when(bondNetworkInterfaceModelA.getEntity()).thenReturn(existingBond); + when(bondNetworkInterfaceModelA.getBonded()).thenReturn(Arrays.asList(networkInterfaceModelOfNicA, networkInterfaceModelOfNicB)); + when(bondNetworkInterfaceModelB.getItems()).thenReturn(Collections.<LogicalNetworkModel>emptyList()); + when(bondNetworkInterfaceModelB.getEntity()).thenReturn(bondB); + when(bondNetworkInterfaceModelB.getBonded()).thenReturn(Arrays.asList(networkInterfaceModelOfNicC, networkInterfaceModelOfNicD)); + + NetworkOperation.JOIN_BONDS.getTarget().executeNetworkCommand( + bondNetworkInterfaceModelA, + bondNetworkInterfaceModelB, + allNics, + allNetworkAttachments, + hsnp, + newlyCreatedBond + ); + + assertThat(hsnp.getNetworkAttachments().size(), is(0)); + assertThat(hsnp.getRemovedNetworkAttachments().size(), is(1)); + assertNetworkAttachment(hsnp.getRemovedNetworkAttachments().iterator().next(), networkAttachmentId, networkA.getId(), existingBondId); + + assertThat(hsnp.getBonds().size(), is(1)); + assertBond(hsnp.getBonds().iterator().next(), null, Arrays.asList(nicA, nicB, nicC, nicD)); + assertThat(hsnp.getRemovedBonds().size(), is(2)); + + Bond firstRemovedBond = hsnp.getRemovedBonds().get(0); + assertBond(firstRemovedBond, existingBondId, Arrays.asList(nicA, nicB)); + Bond secondRemovedBond = hsnp.getRemovedBonds().get(1); + assertBond(secondRemovedBond, bondBId, Arrays.asList(nicC, nicD)); + } + + private void assertBond(Bond bond, Guid bondId, List<VdsNetworkInterface> slaves) { + Matcher attachmentIdMatcher = bondId == null ? nullValue() : is(bondId); + assertThat("id mismatch", bond.getId(), attachmentIdMatcher); //$NON-NLS-1$ + + if (slaves != null) { + for (VdsNetworkInterface slave : slaves) { + assertThat("missing slave", bond.getSlaves().contains(slave.getName()), is(true)); //$NON-NLS-1$ + } + } + assertThat("invalid slaves count", bond.getSlaves().size(), is(slaves.size())); //$NON-NLS-1$ + } + + private void assertNetworkAttachment(NetworkAttachment networkAttachment, + Guid attachmentId, + Guid networkId, + Guid nicId) { + Matcher attachmentIdMatcher = attachmentId == null ? nullValue() : is(attachmentId); + assertThat("id mismatch", networkAttachment.getId(), attachmentIdMatcher); //$NON-NLS-1$ + assertThat("network id mismatch", networkAttachment.getNetworkId(), equalTo(networkId)); //$NON-NLS-1$ + assertThat("nicId mismatch", networkAttachment.getNicId(), equalTo(nicId)); //$NON-NLS-1$ + } + + private Network createNetwork(String networkA) { + Network result = new Network(); + + result.setId(Guid.newGuid()); + result.setName(networkA); + + return result; + } + + private VdsNetworkInterface createNic(String nicName) { + VdsNetworkInterface result = new VdsNetworkInterface(); + + result.setId(Guid.newGuid()); + result.setName(nicName); + + return result; + } + + private Bond createBond(Guid id, String bondName, List<VdsNetworkInterface> slaves) { + Bond result = new Bond(); + + result.setId(id); + result.setName(bondName); + + for (VdsNetworkInterface slave : slaves) { + result.getSlaves().add(slave.getName()); + } + + return result; + } +} -- To view, visit https://gerrit.ovirt.org/38234 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib9d99c4963a0b0013797d1e6ed1612edaf19f0b0 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
