Martin Mucha has uploaded a new change for review.
Change subject: core: validations from LabelNicCommand refactored out to
HostInterfaceValidator
......................................................................
core: validations from LabelNicCommand refactored out to HostInterfaceValidator
Change-Id: I3039fb4a92627036582f03147526522b19c1a2e5
Signed-off-by: Martin Mucha <mmu...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/NetworkConfigurator.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/LabelNicCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostInterfaceValidator.java
4 files changed, 137 insertions(+), 83 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/77/35977/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
index 486dac2..4890bfc 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
@@ -1967,6 +1967,7 @@
*/
protected boolean validate(ValidationResult validationResult) {
if (!validationResult.isValid()) {
+ //TODO MM: duplicity with fail can do action.
addCanDoActionMessage(validationResult.getMessage());
for (String variableReplacement :
validationResult.getVariableReplacements()) {
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/NetworkConfigurator.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/NetworkConfigurator.java
index 52ab592..21bb0b5 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/NetworkConfigurator.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/NetworkConfigurator.java
@@ -185,9 +185,13 @@
return nicVlanId == mgmtVlanId;
}
+ /**
+ * filters out bonds with less than two slaves.
+ * @return all non-bonds and bonds with two or more slaves.
+ */
public List<VdsNetworkInterface>
filterBondsWithoutSlaves(List<VdsNetworkInterface> interfaces) {
- List<VdsNetworkInterface> filteredList = new
ArrayList<VdsNetworkInterface>();
- Map<String, Integer> bonds = new HashMap<String, Integer>();
+ List<VdsNetworkInterface> filteredList = new ArrayList<>();
+ Map<String, Integer> bonds = new HashMap<>();
for (VdsNetworkInterface iface : interfaces) {
if (Boolean.TRUE.equals(iface.getBonded())) {
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/LabelNicCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/LabelNicCommand.java
index 184956e..425d4d4 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/LabelNicCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/LabelNicCommand.java
@@ -1,26 +1,22 @@
package org.ovirt.engine.core.bll.network.host;
-import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
-import java.util.Map;
-import org.apache.commons.lang.StringUtils;
import org.ovirt.engine.core.bll.CommandBase;
import org.ovirt.engine.core.bll.network.AddNetworksByLabelParametersBuilder;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
+import org.ovirt.engine.core.bll.validator.HostInterfaceValidator;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.VdcObjectType;
import org.ovirt.engine.core.common.action.LabelNicParameters;
+import org.ovirt.engine.core.common.action.PersistentSetupNetworksParameters;
import org.ovirt.engine.core.common.action.VdcActionType;
import org.ovirt.engine.core.common.action.VdcReturnValueBase;
-import org.ovirt.engine.core.common.businessentities.Entities;
import org.ovirt.engine.core.common.businessentities.network.Network;
import
org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface;
import org.ovirt.engine.core.common.errors.VdcBllMessages;
-import org.ovirt.engine.core.common.utils.ValidationUtils;
import org.ovirt.engine.core.compat.Guid;
-import org.ovirt.engine.core.utils.NetworkUtils;
public class LabelNicCommand<T extends LabelNicParameters> extends
CommandBase<T> {
@@ -35,11 +31,10 @@
@Override
protected void executeCommand() {
- VdcReturnValueBase result =
- runInternalAction(VdcActionType.PersistentSetupNetworks,
- new
AddNetworksByLabelParametersBuilder(getContext()).buildParameters(getNic(),
- getLabel(),
- getClusterNetworksByLabel()),
cloneContextAndDetachFromParent());
+ VdcReturnValueBase result =
runInternalAction(VdcActionType.PersistentSetupNetworks,
+ createPersistentSetupNetworksParameters(),
+ cloneContextAndDetachFromParent());
+
if (result.getSucceeded()) {
getReturnValue().setActionReturnValue(getLabel());
} else {
@@ -47,6 +42,11 @@
}
setSucceeded(result.getSucceeded());
+ }
+
+ private PersistentSetupNetworksParameters
createPersistentSetupNetworksParameters() {
+ return new AddNetworksByLabelParametersBuilder(getContext())
+ .buildParameters(getNic(), getLabel(),
getClusterNetworksWithLabel(getLabel()));
}
@Override
@@ -57,48 +57,15 @@
@Override
protected boolean canDoAction() {
- if (getNic() == null) {
- return
failCanDoAction(VdcBllMessages.HOST_NETWORK_INTERFACE_NOT_EXIST);
- }
+ HostInterfaceValidator validator = new
HostInterfaceValidator(getNic());
- if (NetworkUtils.isLabeled(getNic()) &&
getNic().getLabels().contains(getLabel())) {
- return failCanDoAction(VdcBllMessages.INTERFACE_ALREADY_LABELED);
- }
-
- if (!ValidationUtils.validateInputs(getValidationGroups(),
getNic()).isEmpty()) {
- return
failCanDoAction(VdcBllMessages.IMPROPER_INTERFACE_IS_LABELED);
- }
-
- if (Boolean.TRUE.equals(getNic().getBonded())) {
- int slavesCount = 0;
- for (VdsNetworkInterface nic : getHostInterfaces()) {
- if (StringUtils.equals(getNic().getName(), nic.getBondName()))
{
- slavesCount++;
- if (slavesCount == 2) {
- break;
- }
- }
- }
-
- if (slavesCount < 2) {
- return
failCanDoAction(VdcBllMessages.IMPROPER_BOND_IS_LABELED);
- }
- }
-
- for (VdsNetworkInterface nic : getHostInterfaces()) {
- if (!StringUtils.equals(nic.getName(), getNicName()) &&
NetworkUtils.isLabeled(nic)
- && nic.getLabels().contains(getLabel())) {
- return
failCanDoAction(VdcBllMessages.OTHER_INTERFACE_ALREADY_LABELED, "$LabeledNic "
+ nic.getName());
- }
- }
-
- List<String> assignedNetworks =
validateNetworksNotAssignedToIncorrectNics();
- if (!assignedNetworks.isEmpty()) {
- return
failCanDoAction(VdcBllMessages.LABELED_NETWORK_ATTACHED_TO_WRONG_INTERFACE,
"$AssignedNetworks "
- + StringUtils.join(assignedNetworks, ", "));
- }
-
- return true;
+ return
+ validate(validator.interfaceExists()) &&
+ validate(validator.interfaceAlreadyLabeledWith(getLabel())) &&
+ validate(validator.labeledValidBond(getHostInterfaces())) &&
+ validate(validator.addLabelToNicAndValidate(getLabel(),
getValidationGroups())) &&
+
validate(validator.anotherInterfaceAlreadyLabeledWithThisLabel(getLabel(),
getHostInterfaces())) &&
+
validate(validator.labeledNetworkAttachedToThisInterface(getHostInterfaces(),
getClusterNetworksWithLabel(getLabel())));
}
private List<VdsNetworkInterface> getHostInterfaces() {
@@ -109,24 +76,9 @@
return hostNics;
}
- public List<String> validateNetworksNotAssignedToIncorrectNics() {
- Map<String, VdsNetworkInterface> nicsByNetworkName =
Entities.hostInterfacesByNetworkName(getHostInterfaces());
- List<String> badlyAssignedNetworks = new ArrayList<>();
- for (Network network : getClusterNetworksByLabel()) {
- if (nicsByNetworkName.containsKey(network.getName())) {
- VdsNetworkInterface assignedNic =
nicsByNetworkName.get(network.getName());
- if (!StringUtils.equals(getNicName(),
NetworkUtils.stripVlan(assignedNic))) {
- badlyAssignedNetworks.add(network.getName());
- }
- }
- }
-
- return badlyAssignedNetworks;
- }
-
- private List<Network> getClusterNetworksByLabel() {
+ private List<Network> getClusterNetworksWithLabel(String label) {
if (labeledNetworks == null) {
- labeledNetworks =
getNetworkDAO().getAllByLabelForCluster(getLabel(), getVds().getVdsGroupId());
+ labeledNetworks = getNetworkDAO().getAllByLabelForCluster(label,
getVds().getVdsGroupId());
}
return labeledNetworks;
@@ -143,10 +95,6 @@
}
return nic;
- }
-
- public String getNicName() {
- return getNic().getName();
}
public String getLabel() {
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostInterfaceValidator.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostInterfaceValidator.java
index 7db7454..d584729 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostInterfaceValidator.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostInterfaceValidator.java
@@ -1,11 +1,16 @@
package org.ovirt.engine.core.bll.validator;
+import java.util.ArrayList;
import java.util.List;
+import java.util.Map;
import org.apache.commons.lang.StringUtils;
import org.ovirt.engine.core.bll.ValidationResult;
+import org.ovirt.engine.core.common.businessentities.Entities;
+import org.ovirt.engine.core.common.businessentities.network.Network;
import
org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface;
import org.ovirt.engine.core.common.errors.VdcBllMessages;
+import org.ovirt.engine.core.common.utils.ValidationUtils;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.utils.NetworkUtils;
@@ -22,8 +27,37 @@
return
ValidationResult.failWith(VdcBllMessages.HOST_NETWORK_INTERFACE_NOT_EXIST).when(iface
== null);
}
+ public ValidationResult interfaceAlreadyLabeledWith(String label) {
+ return
ValidationResult.failWith(VdcBllMessages.INTERFACE_ALREADY_LABELED)
+ .when(NetworkUtils.isLabeled(iface) &&
iface.getLabels().contains(label));
+ }
+
public ValidationResult interfaceInHost(Guid hostId) {
return
ValidationResult.failWith(VdcBllMessages.NIC_NOT_EXISTS_ON_HOST).when(!iface.getVdsId().equals(hostId));
+ }
+
+ public ValidationResult labeledValidBond(List<VdsNetworkInterface> nics) {
+ if (!Boolean.TRUE.equals(iface.getBonded())) {
+ return ValidationResult.VALID;
+ }
+
+ return
ValidationResult.failWith(VdcBllMessages.IMPROPER_BOND_IS_LABELED).when(getSlaveCount(nics,
2) < 2);
+ }
+
+ /**
+ * This method expects, that NIC is valid prior to calling this method
thus potential validation fail can be only
+ * caused by added label.
+ *
+ * requires to be called after it's verified that getNic() is at least 2
slaves bond.
+ *
+ * @param label label to add to NIC.
+ * @param commandValidationGroups validationGroups of calling command.
+ * @return
+ */
+ public ValidationResult addLabelToNicAndValidate(String label,
List<Class<?>> commandValidationGroups) {
+ iface.getLabels().add(label);
+ List<String> validationResult =
ValidationUtils.validateInputs(commandValidationGroups, iface);
+ return
ValidationResult.failWith(VdcBllMessages.IMPROPER_INTERFACE_IS_LABELED).when(!validationResult.isEmpty());
}
public ValidationResult validBond(List<VdsNetworkInterface> nics) {
@@ -31,18 +65,85 @@
return ValidationResult.VALID;
}
- int slavesCount = 0;
- for (VdsNetworkInterface nic : nics) {
- if (StringUtils.equals(nic.getName(), iface.getBondName())) {
- slavesCount++;
- if (slavesCount == 2) {
- break;
+ int slavesCount = getSlaveCount(nics);
+ return
ValidationResult.failWith(VdcBllMessages.NETWORK_BONDS_INVALID_SLAVE_COUNT,
+ "$NETWORK_BONDS_INVALID_SLAVE_COUNT_LIST " +
slavesCount).when(slavesCount < 2);
+ }
+
+ public ValidationResult anotherInterfaceAlreadyLabeledWithThisLabel(String
label,
+ List<VdsNetworkInterface> interfacesToCheck) {
+
+ for (VdsNetworkInterface nic : interfacesToCheck) {
+ //do not compare with self.
+ boolean notTheSameNic = !StringUtils.equals(nic.getName(),
iface.getName());
+
+ if (notTheSameNic) {
+
ValidationResult.failWith(VdcBllMessages.OTHER_INTERFACE_ALREADY_LABELED,
+ "$LabeledNic " + nic.getName())
+ .when(NetworkUtils.isLabeled(nic) &&
nic.getLabels().contains(label));
+ }
+ }
+
+ return ValidationResult.VALID;
+ }
+
+ /**
+ * verifies if given (labeled) networks are assigned to any other
interface (taken from hostInterfaces),
+ * than <code>this.iface</code>. If there's such interface, it's
considered as an error.
+ */
+ public ValidationResult
labeledNetworkAttachedToThisInterface(List<VdsNetworkInterface> hostInterfaces,
+ List<Network> clusterNetworksWithLabel) {
+
+ List<String> assignedNetworks =
validateNetworksNotAssignedToIncorrectNics(hostInterfaces,
+ clusterNetworksWithLabel);
+
+ return
ValidationResult.failWith(VdcBllMessages.LABELED_NETWORK_ATTACHED_TO_WRONG_INTERFACE,
+ "$AssignedNetworks " + StringUtils.join(assignedNetworks, ",
"))
+ .when(!assignedNetworks.isEmpty());
+ }
+
+// there's no network with given label assigned to another nic
+
+ /**
+ * @param hostInterfaces all interfaces for VDS
+ * @param networks networks on given cluster with given label.
+ * @return list of networks names, which are assigned to one of given
<code>hostInterfaces</code> and such interface
+ * is not related to <code>this.iface</code>. Neither NIC assigned to
network nor it's base interface is
+ * <code>this.iface</code>.
+ */
+ private List<String>
validateNetworksNotAssignedToIncorrectNics(List<VdsNetworkInterface>
hostInterfaces,
+ List<Network> networks) {
+
+ Map<String, VdsNetworkInterface> networkNameToNicMap =
Entities.hostInterfacesByNetworkName(hostInterfaces);
+ List<String> badlyAssignedNetworks = new ArrayList<>();
+
+ for (Network network : networks) {
+ boolean networkIsAssignedToHostInterface =
networkNameToNicMap.containsKey(network.getName());
+ if (networkIsAssignedToHostInterface) {
+ VdsNetworkInterface assignedHostInterface =
networkNameToNicMap.get(network.getName());
+ if (!StringUtils.equals(iface.getName(),
NetworkUtils.stripVlan(assignedHostInterface))) {
+ badlyAssignedNetworks.add(network.getName());
}
}
}
- return
ValidationResult.failWith(VdcBllMessages.NETWORK_BONDS_INVALID_SLAVE_COUNT,
- "$NETWORK_BONDS_INVALID_SLAVE_COUNT_LIST " +
slavesCount).when(slavesCount < 2);
+ return badlyAssignedNetworks;
+ }
+
+ private int getSlaveCount(List<VdsNetworkInterface> nics) {
+ return getSlaveCount(nics, Integer.MAX_VALUE);
+ }
+
+ private int getSlaveCount(List<VdsNetworkInterface> nics, int
maxSlavesCount) {
+ int slavesCount = 0;
+
+ for (int i = 0; i < nics.size() && slavesCount < maxSlavesCount; i++) {
+ VdsNetworkInterface nic = nics.get(i);
+ if (StringUtils.equals(iface.getName(), nic.getBondName())) {
+ slavesCount++;
+ }
+ }
+ return slavesCount;
}
public ValidationResult networkCanBeAttached() {
--
To view, visit http://gerrit.ovirt.org/35977
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3039fb4a92627036582f03147526522b19c1a2e5
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <mmu...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
