Gilad Chaplik has uploaded a new change for review.
Change subject: core: RunVmCommand.canDoAction cleanup (1)
......................................................................
core: RunVmCommand.canDoAction cleanup (1)
Inital commit includes adding a new class 'RunVmValidator';
Eventually, run vm validations will be
extracted to this class, and VmRunHandler will be removed.
This patch includes moving custom proerprties validation to
the validator and matching tests.
Note: RunVmValidator will have a deprecated static method
compatible to the old one, to avoid regressions in an
exsisting VmPoolCommandBase static reference.
Change-Id: I60bbdb54d150878123968893c23169de253e1ba2
Signed-off-by: Gilad Chaplik <gchap...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
A
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
A
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
M
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/vmproperties/VmPropertiesUtils.java
10 files changed, 305 insertions(+), 164 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/97/13397/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
index 7ce1273..ff7b593 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
@@ -161,7 +161,7 @@
if (returnValue) {
List<ValidationError> validationErrors =
validateCustomProperties(vmStaticFromParams);
if (!validationErrors.isEmpty()) {
- VmHandler.handleCustomPropertiesError(validationErrors,
reasons);
+ new VmHandler().handleCustomPropertiesError(validationErrors,
reasons);
returnValue = false;
}
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
index 3a576d4..affe6b7 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
@@ -21,6 +21,7 @@
import org.ovirt.engine.core.bll.quota.QuotaVdsGroupConsumptionParameter;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.bll.utils.VmDeviceUtils;
+import org.ovirt.engine.core.bll.validator.RunVmValidator;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.FeatureSupported;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -67,7 +68,6 @@
import org.ovirt.engine.core.utils.linq.Predicate;
import org.ovirt.engine.core.utils.log.Log;
import org.ovirt.engine.core.utils.log.LogFactory;
-import org.ovirt.engine.core.utils.vmproperties.VmPropertiesUtils;
@LockIdNameAttribute
@@ -75,6 +75,7 @@
public class RunVmCommand<T extends RunVmParams> extends RunVmCommandBase<T>
implements QuotaVdsDependent {
+ private final RunVmValidator runVmValidator = new RunVmValidator();
private static final long serialVersionUID = 3317745769686161108L;
private String _cdImagePath = "";
private String _floppyImagePath = "";
@@ -673,7 +674,14 @@
return true;
}
else {
- boolean canDoAction = canRunVm(vm) && validateNetworkInterfaces();
+ List<String> messages = getReturnValue().getCanDoActionMessages();
+ boolean canDoAction =
+ getRunVmValidator().validateVmProperties(vm, messages) &&
+ canRunVm(vm) &&
+ validateNetworkInterfaces();
+ if (!canDoAction) {
+ return false;
+ }
// check for Vm Payload
if (canDoAction && getParameters().getVmPayload() != null) {
@@ -694,16 +702,16 @@
}
}
+ protected RunVmValidator getRunVmValidator() {
+ return runVmValidator;
+ }
+
protected boolean canRunVm(VM vm) {
return getVmRunHandler().canRunVm(vm,
getReturnValue().getCanDoActionMessages(),
getParameters(),
getVdsSelector(),
- getSnapshotsValidator(), getVmPropertiesUtils());
- }
-
- protected VmPropertiesUtils getVmPropertiesUtils() {
- return VmPropertiesUtils.getInstance();
+ getSnapshotsValidator());
}
@Override
@@ -788,7 +796,7 @@
/**
* @return true if all VM network interfaces are valid
*/
- private boolean validateNetworkInterfaces() {
+ protected boolean validateNetworkInterfaces() {
Map<String, VmNetworkInterface> interfaceNetworkMap =
Entities.interfacesByNetworkName(getVm().getInterfaces());
Set<String> interfaceNetworkNames = interfaceNetworkMap.keySet();
List<Network> clusterNetworks =
getNetworkDAO().getAllForCluster(getVm().getVdsGroupId());
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
index 7ed8c4f..274f592 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
@@ -194,7 +194,7 @@
List<ValidationError> validationErrors =
validateCustomProperties(vmFromParams.getStaticData());
if (!validationErrors.isEmpty()) {
- VmHandler.handleCustomPropertiesError(validationErrors,
getReturnValue().getCanDoActionMessages());
+ new VmHandler().handleCustomPropertiesError(validationErrors,
getReturnValue().getCanDoActionMessages());
return false;
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
index d9d0d72..f2774b9 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
@@ -1,6 +1,5 @@
package org.ovirt.engine.core.bll;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
@@ -453,7 +452,7 @@
return retVal;
}
- protected static void handleCustomPropertiesError(List<ValidationError>
validationErrors, ArrayList<String> message) {
+ public static void handleCustomPropertiesError(List<ValidationError>
validationErrors, List<String> message) {
String invalidSyntaxMsg =
VdcBllMessages.ACTION_TYPE_FAILED_INVALID_CUSTOM_VM_PROPERTIES_INVALID_SYNTAX.name();
List<String> errorMessages =
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
index 9220603..fe0b1dd 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
@@ -6,6 +6,7 @@
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
+import org.ovirt.engine.core.bll.validator.RunVmValidator;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -28,7 +29,6 @@
import org.ovirt.engine.core.dal.dbbroker.auditloghandling.CustomLogField;
import org.ovirt.engine.core.dal.dbbroker.auditloghandling.CustomLogFields;
import org.ovirt.engine.core.dao.VmPoolDAO;
-import org.ovirt.engine.core.utils.vmproperties.VmPropertiesUtils;
@CustomLogFields({ @CustomLogField("VmPoolName") })
public abstract class VmPoolCommandBase<T extends VmPoolParametersBase>
extends CommandBase<T> {
@@ -266,16 +266,16 @@
true,
new VdsFreeMemoryChecker(new NonWaitingDelayer()));
- return VmRunHandler.getInstance().canRunVm(vm,
- messages,
- runVmParams,
- vdsSelector,
- new SnapshotsValidator(),
- getVmPropertiesUtils());
+ return getRunVmValidator().canRunVm(vm, messages) &&
+ VmRunHandler.getInstance().canRunVm(vm,
+ messages,
+ runVmParams,
+ vdsSelector,
+ new SnapshotsValidator());
}
- protected static VmPropertiesUtils getVmPropertiesUtils() {
- return VmPropertiesUtils.getInstance();
+ private static RunVmValidator getRunVmValidator() {
+ return new RunVmValidator();
}
@Override
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
index 5462cdc..faf2caf 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
@@ -42,7 +42,6 @@
import org.ovirt.engine.core.dao.DiskDao;
import org.ovirt.engine.core.dao.StorageDomainDAO;
import org.ovirt.engine.core.dao.StoragePoolDAO;
-import org.ovirt.engine.core.utils.vmproperties.VmPropertiesUtils;
/** A utility class for verifying running a vm*/
public class VmRunHandler {
@@ -64,135 +63,126 @@
* @return true if the given VM can run with the given properties, false
otherwise
*/
public boolean canRunVm(VM vm, ArrayList<String> message, RunVmParams
runParams,
- VdsSelector vdsSelector, SnapshotsValidator snapshotsValidator,
VmPropertiesUtils vmPropsUtils) {
+ VdsSelector vdsSelector, SnapshotsValidator snapshotsValidator) {
boolean retValue = true;
- List<VmPropertiesUtils.ValidationError> validationErrors =
vmPropsUtils.validateVMProperties(
- vm.getVdsGroupCompatibilityVersion(),
- vm.getStaticData());
+ BootSequence boot_sequence = (runParams.getBootSequence() != null) ?
+ runParams.getBootSequence() : vm.getDefaultBootSequence();
+ Guid storagePoolId = vm.getStoragePoolId();
+ // Block from running a VM with no HDD when its first boot device is
+ // HD and no other boot devices are configured
+ List<Disk> vmDisks = getDiskDao().getAllForVm(vm.getId(), true);
+ if (boot_sequence == BootSequence.C && vmDisks.size() == 0) {
+ String messageStr = !vmDisks.isEmpty() ?
+
VdcBllMessages.VM_CANNOT_RUN_FROM_DISK_WITHOUT_PLUGGED_DISK.toString() :
+
VdcBllMessages.VM_CANNOT_RUN_FROM_DISK_WITHOUT_DISK.toString();
- if (!validationErrors.isEmpty()) {
- VmHandler.handleCustomPropertiesError(validationErrors, message);
+ message.add(messageStr);
retValue = false;
} else {
- BootSequence boot_sequence = (runParams.getBootSequence() != null)
?
- runParams.getBootSequence() : vm.getDefaultBootSequence();
- Guid storagePoolId = vm.getStoragePoolId();
- // Block from running a VM with no HDD when its first boot device
is
- // HD and no other boot devices are configured
- List<Disk> vmDisks = getDiskDao().getAllForVm(vm.getId(), true);
- if (boot_sequence == BootSequence.C && vmDisks.size() == 0) {
- String messageStr = !vmDisks.isEmpty() ?
-
VdcBllMessages.VM_CANNOT_RUN_FROM_DISK_WITHOUT_PLUGGED_DISK.toString() :
-
VdcBllMessages.VM_CANNOT_RUN_FROM_DISK_WITHOUT_DISK.toString();
+ // If CD appears as first and there is no ISO in storage
+ // pool/ISO inactive -
+ // you cannot run this VM
- message.add(messageStr);
+ if (boot_sequence == BootSequence.CD &&
findActiveISODomain(storagePoolId) == null) {
+
message.add(VdcBllMessages.VM_CANNOT_RUN_FROM_CD_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO.toString());
retValue = false;
} else {
- // If CD appears as first and there is no ISO in storage
- // pool/ISO inactive -
- // you cannot run this VM
-
- if (boot_sequence == BootSequence.CD &&
findActiveISODomain(storagePoolId) == null) {
-
message.add(VdcBllMessages.VM_CANNOT_RUN_FROM_CD_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO.toString());
+ // if there is network in the boot sequence, check that the
+ // vm has network,
+ // otherwise the vm cannot be run in vdsm
+ if (boot_sequence == BootSequence.N
+ &&
DbFacade.getInstance().getVmNetworkInterfaceDao().getAllForVm(vm.getId()).size()
== 0) {
+
message.add(VdcBllMessages.VM_CANNOT_RUN_FROM_NETWORK_WITHOUT_NETWORK.toString());
retValue = false;
- } else {
- // if there is network in the boot sequence, check that the
- // vm has network,
- // otherwise the vm cannot be run in vdsm
- if (boot_sequence == BootSequence.N
- &&
DbFacade.getInstance().getVmNetworkInterfaceDao().getAllForVm(vm.getId()).size()
== 0) {
-
message.add(VdcBllMessages.VM_CANNOT_RUN_FROM_NETWORK_WITHOUT_NETWORK.toString());
+ }
+
+ if (retValue) {
+ ValidationResult vmNotLockedResult = new
VmValidator(vm).vmNotLocked();
+ if (!vmNotLockedResult.isValid()) {
+ message.add(vmNotLockedResult.getMessage().name());
+ retValue = false;
+ }
+ }
+
+ if (retValue) {
+ ValidationResult vmDuringSnapshotResult =
+ snapshotsValidator.vmNotDuringSnapshot(vm.getId());
+ if (!vmDuringSnapshotResult.isValid()) {
+
message.add(vmDuringSnapshotResult.getMessage().name());
+ retValue = false;
+ }
+ }
+
+ List<DiskImage> vmImages =
ImagesHandler.filterImageDisks(vmDisks, true, false);
+ if (retValue && !vmImages.isEmpty()) {
+ storage_pool sp =
getStoragePoolDAO().get(vm.getStoragePoolId());
+ ValidationResult spUpResult = new
StoragePoolValidator(sp).isUp();
+ if (!spUpResult.isValid()) {
+ message.add(spUpResult.getMessage().name());
retValue = false;
}
if (retValue) {
- ValidationResult vmNotLockedResult = new
VmValidator(vm).vmNotLocked();
- if (!vmNotLockedResult.isValid()) {
- message.add(vmNotLockedResult.getMessage().name());
- retValue = false;
- }
+ retValue = performImageChecksForRunningVm(vm, message,
runParams, vmImages);
}
- if (retValue) {
- ValidationResult vmDuringSnapshotResult =
-
snapshotsValidator.vmNotDuringSnapshot(vm.getId());
- if (!vmDuringSnapshotResult.isValid()) {
-
message.add(vmDuringSnapshotResult.getMessage().name());
- retValue = false;
- }
- }
-
- List<DiskImage> vmImages =
ImagesHandler.filterImageDisks(vmDisks, true, false);
- if (retValue && !vmImages.isEmpty()) {
- storage_pool sp =
getStoragePoolDAO().get(vm.getStoragePoolId());
- ValidationResult spUpResult = new
StoragePoolValidator(sp).isUp();
- if (!spUpResult.isValid()) {
- message.add(spUpResult.getMessage().name());
- retValue = false;
- }
-
- if (retValue) {
- retValue = performImageChecksForRunningVm(vm,
message, runParams, vmImages);
- }
-
- // Check if iso and floppy path exists
- if (retValue && !vm.isAutoStartup()
- &&
!validateIsoPath(findActiveISODomain(vm.getStoragePoolId()),
- runParams,
- message)) {
- retValue = false;
- } else if (retValue) {
- boolean isVmDuringInit = ((Boolean) getBackend()
- .getResourceManager()
-
.RunVdsCommand(VDSCommandType.IsVmDuringInitiating,
- new
IsVmDuringInitiatingVDSCommandParameters(vm.getId()))
- .getReturnValue()).booleanValue();
- if (vm.isRunning() || (vm.getStatus() ==
VMStatus.NotResponding) || isVmDuringInit) {
- retValue = false;
-
message.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_RUNNING.toString());
- } else if (vm.getStatus() == VMStatus.Paused &&
vm.getRunOnVds() != null) {
- VDS vds =
DbFacade.getInstance().getVdsDao().get(
- new Guid(vm.getRunOnVds().toString()));
- if (vds.getStatus() != VDSStatus.Up) {
- retValue = false;
-
message.add(VdcBllMessages.VAR__HOST_STATUS__UP.toString());
-
message.add(VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL.toString());
- }
- }
-
- boolean isStatelessVm =
shouldVmRunAsStateless(runParams, vm);
-
- if (retValue && isStatelessVm &&
!snapshotsValidator.vmNotInPreview(vm.getId()).isValid()) {
- retValue = false;
-
message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_WHILE_IN_PREVIEW.toString());
- }
-
- // if the VM itself is stateless or run once as
stateless
- if (retValue && isStatelessVm &&
vm.isAutoStartup()) {
- retValue = false;
-
message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_HA.toString());
- }
-
- if (retValue && isStatelessVm &&
!hasSpaceForSnapshots(vm, message)) {
- return false;
- }
- }
- }
-
- retValue = retValue == false ? retValue :
vdsSelector.canFindVdsToRunOn(message, false);
-
- /**
- * only if can do action ok then check with actions matrix
that status is valid for this
- * action
- */
- if (retValue
- && !VdcActionUtils.CanExecute(Arrays.asList(vm),
VM.class,
- VdcActionType.RunVm)) {
+ // Check if iso and floppy path exists
+ if (retValue && !vm.isAutoStartup()
+ &&
!validateIsoPath(findActiveISODomain(vm.getStoragePoolId()),
+ runParams,
+ message)) {
retValue = false;
-
message.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL.toString());
+ } else if (retValue) {
+ boolean isVmDuringInit = ((Boolean) getBackend()
+ .getResourceManager()
+
.RunVdsCommand(VDSCommandType.IsVmDuringInitiating,
+ new
IsVmDuringInitiatingVDSCommandParameters(vm.getId()))
+ .getReturnValue()).booleanValue();
+ if (vm.isRunning() || (vm.getStatus() ==
VMStatus.NotResponding) || isVmDuringInit) {
+ retValue = false;
+
message.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_RUNNING.toString());
+ } else if (vm.getStatus() == VMStatus.Paused &&
vm.getRunOnVds() != null) {
+ VDS vds = DbFacade.getInstance().getVdsDao().get(
+ new Guid(vm.getRunOnVds().toString()));
+ if (vds.getStatus() != VDSStatus.Up) {
+ retValue = false;
+
message.add(VdcBllMessages.VAR__HOST_STATUS__UP.toString());
+
message.add(VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL.toString());
+ }
+ }
+
+ boolean isStatelessVm =
shouldVmRunAsStateless(runParams, vm);
+
+ if (retValue && isStatelessVm &&
!snapshotsValidator.vmNotInPreview(vm.getId()).isValid()) {
+ retValue = false;
+
message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_WHILE_IN_PREVIEW.toString());
+ }
+
+ // if the VM itself is stateless or run once as
stateless
+ if (retValue && isStatelessVm && vm.isAutoStartup()) {
+ retValue = false;
+
message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_HA.toString());
+ }
+
+ if (retValue && isStatelessVm &&
!hasSpaceForSnapshots(vm, message)) {
+ return false;
+ }
}
}
+
+ retValue = retValue == false ? retValue :
vdsSelector.canFindVdsToRunOn(message, false);
+
+ /**
+ * only if can do action ok then check with actions matrix
that status is valid for this
+ * action
+ */
+ if (retValue
+ && !VdcActionUtils.CanExecute(Arrays.asList(vm),
VM.class,
+ VdcActionType.RunVm)) {
+ retValue = false;
+
message.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL.toString());
+ }
}
}
return retValue;
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
new file mode 100644
index 0000000..96e707d
--- /dev/null
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
@@ -0,0 +1,35 @@
+package org.ovirt.engine.core.bll.validator;
+
+import java.util.List;
+
+import org.ovirt.engine.core.bll.VmHandler;
+import org.ovirt.engine.core.common.businessentities.VM;
+import org.ovirt.engine.core.utils.vmproperties.VmPropertiesUtils;
+
+public class RunVmValidator {
+
+ public boolean validateVmProperties(VM vm, List<String> messages) {
+ List<VmPropertiesUtils.ValidationError> validationErrors =
+ getVmPropertiesUtils().validateVMProperties(
+ vm.getVdsGroupCompatibilityVersion(),
+ vm.getStaticData());
+
+ if (!validationErrors.isEmpty()) {
+ VmHandler.handleCustomPropertiesError(validationErrors, messages);
+ return false;
+ }
+
+ return true;
+ }
+
+ public VmPropertiesUtils getVmPropertiesUtils() {
+ return VmPropertiesUtils.getInstance();
+ }
+
+ // Compatibility method for static VmPoolCommandBase.canRunPoolVm
+ // who uses the same validation as runVmCommand
+ public boolean canRunVm(VM vm, List<String> messages) {
+ return validateVmProperties(vm, messages);
+ }
+
+}
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
index ed373cc..bb835d5 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
@@ -18,13 +18,13 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
-import java.util.HashSet;
import java.util.List;
import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.Matchers;
import org.mockito.Mock;
import org.mockito.Spy;
import org.mockito.invocation.InvocationOnMock;
@@ -32,6 +32,7 @@
import org.mockito.stubbing.Answer;
import org.ovirt.engine.core.bll.interfaces.BackendInternal;
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
+import org.ovirt.engine.core.bll.validator.RunVmValidator;
import org.ovirt.engine.core.common.action.RunVmParams;
import org.ovirt.engine.core.common.businessentities.Disk;
import org.ovirt.engine.core.common.businessentities.DiskImage;
@@ -40,7 +41,6 @@
import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
-import org.ovirt.engine.core.common.businessentities.VmStatic;
import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.common.config.ConfigValues;
import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend;
@@ -50,14 +50,12 @@
import org.ovirt.engine.core.common.vdscommands.VdsAndVmIDVDSParametersBase;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.compat.NGuid;
-import org.ovirt.engine.core.compat.Version;
import org.ovirt.engine.core.dal.VdcBllMessages;
import org.ovirt.engine.core.dao.DiskDao;
import org.ovirt.engine.core.dao.StorageDomainDAO;
import org.ovirt.engine.core.dao.StoragePoolDAO;
import org.ovirt.engine.core.dao.VmDAO;
import org.ovirt.engine.core.utils.MockConfigRule;
-import org.ovirt.engine.core.utils.vmproperties.VmPropertiesUtils;
@RunWith(MockitoJUnitRunner.class)
public class RunVmCommandTest {
@@ -85,6 +83,9 @@
@Spy
private final VmRunHandler vmRunHandler = VmRunHandler.getInstance();
+
+ @Mock
+ private RunVmValidator runVmValidator;
@Mock
private BackendInternal backend;
@@ -309,8 +310,8 @@
command = spy(new RunVmCommand<RunVmParams>(param));
mockVmRunHandler();
+ mockSuccessfulRunVmValidator();
mockSuccessfulSnapshotValidator();
- mockVmPropertiesUtils();
mockBackend();
}
@@ -326,6 +327,24 @@
anyListOf(String.class),
any(RunVmParams.class),
anyListOf(DiskImage.class));
+ }
+
+ @Test
+ public void testCanDoAction() {
+ final ArrayList<Disk> disks = new ArrayList<Disk>();
+ final DiskImage diskImage = createImage();
+ disks.add(diskImage);
+ initDAOMocks(disks);
+ final VM vm = new VM();
+ vm.setStatus(VMStatus.Down);
+ vm.setStoragePoolId(Guid.NewGuid());
+ doReturn(new VdsSelector(vm, new NGuid(), true, new
VdsFreeMemoryChecker(command))).when(command)
+ .getVdsSelector();
+ doReturn(vm).when(command).getVm();
+ doReturn(true).when(command).canRunVm(vm);
+ doReturn(true).when(command).validateNetworkInterfaces();
+ assertTrue(command.canDoAction());
+
assertTrue(command.getReturnValue().getCanDoActionMessages().isEmpty());
}
@Test
@@ -404,20 +423,6 @@
assertEquals(shouldPass,
!messages.contains("VM_CANNOT_RUN_STATELESS_HA"));
}
- private VmPropertiesUtils mockVmPropertiesUtils() {
- // Mocks vm properties utils (mocks a successful validation)
- VmPropertiesUtils utils = spy(new VmPropertiesUtils());
- doReturn(Collections.singletonMap("agent",
"true")).when(utils).getPredefinedProperties(any(Version.class),
- any(VmStatic.class));
- doReturn(Collections.singletonMap("buff",
"123")).when(utils).getUserDefinedProperties(any(Version.class),
- any(VmStatic.class));
- doReturn(new HashSet<Version>(Arrays.asList(Version.v3_0,
Version.v3_1))).when(utils)
- .getSupportedClusterLevels();
-
doReturn(Collections.emptyList()).when(utils).validateVMProperties(any(Version.class),
any(VmStatic.class));
- doReturn(utils).when(command).getVmPropertiesUtils();
- return utils;
- }
-
@Test
public void canRunVmFailStatelessWhenVmHA() {
canRunStatelessVmTest(true, false, Boolean.TRUE, false);
@@ -472,4 +477,11 @@
doReturn(snapshotsValidator).when(command).getSnapshotsValidator();
return snapshotsValidator;
}
+
+ private RunVmValidator mockSuccessfulRunVmValidator() {
+ RunVmValidator runVmValidator = mock(RunVmValidator.class);
+ when(runVmValidator.validateVmProperties(any(VM.class),
Matchers.anyListOf(String.class))).thenReturn(true);
+ doReturn(runVmValidator).when(command).getRunVmValidator();
+ return runVmValidator;
+ }
}
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
new file mode 100644
index 0000000..4360983
--- /dev/null
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
@@ -0,0 +1,98 @@
+package org.ovirt.engine.core.bll.validator;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.ovirt.engine.core.common.businessentities.VM;
+import org.ovirt.engine.core.compat.Version;
+import org.ovirt.engine.core.utils.exceptions.InitializationException;
+import org.ovirt.engine.core.utils.vmproperties.VmPropertiesUtils;
+
+public class RunVmValidatorTest {
+
+ private RunVmValidator runVmValidator = new RunVmValidator() {
+ @Override
+ public VmPropertiesUtils getVmPropertiesUtils() {
+ return mockVmPropertiesUtils();
+ };
+ };
+
+ @Before
+ public void setup() {
+ }
+
+ @Test
+ public void testValidEmptyCustomProerties() {
+ VM vm = new VM();
+ vm.setCustomProperties("");
+ vm.setVdsGroupCompatibilityVersion(Version.v3_3);
+ List<String> messages = new ArrayList<String>();
+ assertTrue(runVmValidator.validateVmProperties(vm, messages));
+ assertTrue(messages.isEmpty());
+ }
+
+ @Test
+ public void testWrongFormatCustomProerties() {
+ VM vm = new VM();
+ vm.setCustomProperties("sap_agent;"); // missing '= true'
+ vm.setVdsGroupCompatibilityVersion(Version.v3_3);
+ List<String> messages = new ArrayList<String>();
+ assertFalse(runVmValidator.validateVmProperties(vm, messages));
+ assertFalse(messages.isEmpty());
+ }
+
+ @Test
+ public void testNotValidCustomProerties() {
+ VM vm = new VM();
+ vm.setCustomProperties("property=value;");
+ vm.setVdsGroupCompatibilityVersion(Version.v3_3);
+ List<String> messages = new ArrayList<String>();
+ assertFalse(runVmValidator.validateVmProperties(vm, messages));
+ assertFalse(messages.isEmpty());
+ }
+
+ @Test
+ public void testValidCustomProerties() {
+ VM vm = new VM();
+ vm.setCustomProperties("sap_agent=true;");
+ vm.setVdsGroupCompatibilityVersion(Version.v3_3);
+ List<String> messages = new ArrayList<String>();
+ assertTrue(runVmValidator.validateVmProperties(vm, messages));
+ assertTrue(messages.isEmpty());
+ }
+
+ private VmPropertiesUtils mockVmPropertiesUtils() {
+ VmPropertiesUtils utils = new VmPropertiesUtils() {
+ @Override
+ protected String getPredefinedVMProperties(Version version) {
+ return
"sap_agent=^(true|false)$;sndbuf=^[0-9]+$;vhost=^(([a-zA-Z0-9_]*):(true|false))(,(([a-zA-Z0-9_]*):(true|false)))*$;viodiskcache=^(none|writeback|writethrough)$";
+ }
+
+ @Override
+ protected String getUserdefinedVMProperties(Version version) {
+ return "";
+ }
+
+ @Override
+ protected Set<Version> getSupportedClusterLevels() {
+ return new HashSet<Version>(Arrays.asList(Version.v3_2,
Version.v3_3));
+ }
+
+ };
+ try {
+ utils.init();
+ } catch (InitializationException e) {
+ e.printStackTrace();
+ }
+ return utils;
+ }
+
+}
diff --git
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/vmproperties/VmPropertiesUtils.java
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/vmproperties/VmPropertiesUtils.java
index 6f326e0..1fc2247 100644
---
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/vmproperties/VmPropertiesUtils.java
+++
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/vmproperties/VmPropertiesUtils.java
@@ -27,7 +27,15 @@
*/
public class VmPropertiesUtils {
- private static VmPropertiesUtils vmPropertiesUtils;
+ private static VmPropertiesUtils vmPropertiesUtils = null;
+
+ public static VmPropertiesUtils getInstance() {
+ if (vmPropertiesUtils == null) {
+ vmPropertiesUtils = new VmPropertiesUtils();
+ }
+ return vmPropertiesUtils;
+ }
+
private final Pattern SEMICOLON_PATTERN = Pattern.compile(";");
private final String PROPERTIES_DELIMETER = ";";
private final String KEY_VALUE_DELIMETER = "=";
@@ -55,11 +63,6 @@
private Map<Version, Map<String, Pattern>> predefinedProperties;
private Map<Version, Map<String, Pattern>> userdefinedProperties;
private Map<Version, String> allVmProperties;
-
- static {
- vmPropertiesUtils = new VmPropertiesUtils();
-
- }
// Defines why validation failed
public enum ValidationFailureReason {
@@ -111,10 +114,6 @@
}
}
- public static VmPropertiesUtils getInstance() {
- return vmPropertiesUtils;
- }
-
public void init() throws InitializationException {
try {
predefinedProperties = new HashMap<Version, Map<String,
Pattern>>();
@@ -144,7 +143,7 @@
}
}
- private String getUserdefinedVMProperties(Version version) {
+ protected String getUserdefinedVMProperties(Version version) {
return Config.<String> GetValue(ConfigValues.UserDefinedVMProperties,
version.getValue());
}
@@ -152,7 +151,7 @@
return Config.<String> GetValue(ConfigValues.PredefinedVMProperties,
version.getValue());
}
- public Set<Version> getSupportedClusterLevels() {
+ protected Set<Version> getSupportedClusterLevels() {
Set<Version> versions = Config.<java.util.HashSet<Version>>
GetValue(ConfigValues.SupportedClusterLevels);
return versions;
}
--
To view, visit http://gerrit.ovirt.org/13397
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I60bbdb54d150878123968893c23169de253e1ba2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
