Allon Mureinik has uploaded a new change for review. Change subject: engine: StorageConnections ops won't assume VDS ......................................................................
engine: StorageConnections ops won't assume VDS All the storage connection operations should be able to work without an active host, and not validate the connection information. (If a host is passed, connection information is validated). This patch decouples storage connection commands from storage domain commands, and stops assuming the presence of a host and/or a storage pool. Change-Id: I628bf2a63f64d3acaa3520ad144e1eef744f1204 Signed-off-by: Allon Mureinik <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectStorageToVdsCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DisconnectStorageServerConnectionCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageServerConnectionCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/StorageServerConnectionParametersBase.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java 11 files changed, 143 insertions(+), 146 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/88/17288/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java index 5ecf7d1..bb0a4c1 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java @@ -31,23 +31,31 @@ @Override protected void executeCommand() { + boolean success = true; StorageServerConnections connection = getConnection(); - boolean isValidConnection = true; - Pair<Boolean, Integer> result = connectHostToStorage(); - isValidConnection = result.getFirst(); - // Add storage connection to the database. - if (isValidConnection) { + // Attempt to connect only if a host is given. + // If not, just save the connection to the database + if (!Guid.isNullOrEmpty(getVdsId())) { + Pair<Boolean, Integer> result = connectHostToStorage(); + boolean isValidConnection = result.getFirst(); + + // Process failure + if (!isValidConnection) { + VdcFault fault = new VdcFault(); + fault.setError(VdcBllErrors.forValue(result.getSecond())); + getReturnValue().setFault(fault); + success = false; + } + } + + setSucceeded(success); + + if (success) { connection.setid(Guid.newGuid().toString()); saveConnection(connection); getReturnValue().setActionReturnValue(getConnection().getid()); setSucceeded(true); - } - else { - VdcFault fault = new VdcFault(); - fault.setError(VdcBllErrors.forValue(result.getSecond())); - getReturnValue().setFault(fault); - setSucceeded(false); } } @@ -96,14 +104,13 @@ return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_ALREADY_EXISTS); } - if (getParameters().getVdsId().equals(Guid.Empty)) { - if (!initializeVds()) { - return false; + if (!Guid.isNullOrEmpty(getParameters().getVdsId())) { + if (getVds() == null) { + return failCanDoAction(VdcBllMessages.VDS_INVALID_SERVER_ID); } - } else if (getVds() == null) { - return failCanDoAction(VdcBllMessages.VDS_INVALID_SERVER_ID); - } else if (getVds().getStatus() != VDSStatus.Up) { - return failCanDoAction(VdcBllMessages.VDS_ADD_STORAGE_SERVER_STATUS_MUST_BE_UP); + if (getVds().getStatus() != VDSStatus.Up) { + return failCanDoAction(VdcBllMessages.VDS_ADD_STORAGE_SERVER_STATUS_MUST_BE_UP); + } } return true; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectStorageToVdsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectStorageToVdsCommand.java index 5bf5f8a..3c7cba8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectStorageToVdsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectStorageToVdsCommand.java @@ -5,9 +5,7 @@ import org.ovirt.engine.core.bll.Backend; import org.ovirt.engine.core.bll.InternalCommandAttribute; import org.ovirt.engine.core.common.action.StorageServerConnectionParametersBase; -import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.businessentities.StorageServerConnections; -import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.errors.VdcFault; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.StorageServerConnectionManagementVDSParameters; @@ -19,20 +17,6 @@ StorageServerConnectionCommandBase<T> { public ConnectStorageToVdsCommand(T parameters) { super(parameters); - } - - @Override - protected boolean canDoAction() { - Guid id = getParameters().getStoragePoolId(); - if (id != null && !id.equals(Guid.Empty)) { - StoragePool dc = getStoragePoolDAO().get(id); - if (dc == null) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NOT_EXIST); - return false; - } - } - return true; - } @Override @@ -62,7 +46,7 @@ .getResourceManager() .RunVdsCommand( VDSCommandType.ConnectStorageServer, - new StorageServerConnectionManagementVDSParameters(vdsId, getParameters().getStoragePoolId(), + new StorageServerConnectionManagementVDSParameters(vdsId, Guid.Empty, getParameters().getStorageServerConnection().getstorage_type(), new java.util.ArrayList<StorageServerConnections>(java.util.Arrays .asList(new StorageServerConnections[] { getConnection() })))) diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DisconnectStorageServerConnectionCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DisconnectStorageServerConnectionCommand.java index a259533..883ddd8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DisconnectStorageServerConnectionCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DisconnectStorageServerConnectionCommand.java @@ -2,12 +2,14 @@ import java.util.ArrayList; import java.util.Arrays; + import org.ovirt.engine.core.bll.Backend; import org.ovirt.engine.core.bll.InternalCommandAttribute; import org.ovirt.engine.core.common.action.StorageServerConnectionParametersBase; import org.ovirt.engine.core.common.businessentities.StorageServerConnections; import org.ovirt.engine.core.common.vdscommands.StorageServerConnectionManagementVDSParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; +import org.ovirt.engine.core.compat.Guid; @InternalCommandAttribute public class DisconnectStorageServerConnectionCommand<T extends StorageServerConnectionParametersBase> extends @@ -26,10 +28,10 @@ .getResourceManager() .RunVdsCommand( VDSCommandType.DisconnectStorageServer, - new StorageServerConnectionManagementVDSParameters(getParameters().getVdsId(), getParameters() - .getStoragePoolId(), getParameters().getStorageServerConnection().getstorage_type(), - new ArrayList<>(Arrays - .asList(new StorageServerConnections[] { getConnection() })))).getSucceeded() ; + new StorageServerConnectionManagementVDSParameters(getParameters().getVdsId(), Guid.Empty, + getParameters().getStorageServerConnection().getstorage_type(), + new ArrayList<>(Arrays + .asList(new StorageServerConnections[]{getConnection()})))).getSucceeded(); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java index 6072f00..618372e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java @@ -8,7 +8,6 @@ import java.util.Map; import java.util.Set; -import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.CommandBase; import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.bll.utils.PermissionSubject; @@ -21,8 +20,6 @@ import org.ovirt.engine.core.common.businessentities.StorageFormatType; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; -import org.ovirt.engine.core.common.businessentities.StorageServerConnections; -import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VDSStatus; import org.ovirt.engine.core.common.config.Config; @@ -70,7 +67,8 @@ executeInScope(TransactionScopeOption.Suppress, new TransactionMethod<Void>() { @Override public Void runInTransaction() { - int master_domain_version = getStoragePoolDAO().increaseStoragePoolMasterVersion(getStoragePool().getId()); + int master_domain_version = + getStoragePoolDAO().increaseStoragePoolMasterVersion(getStoragePool().getId()); getStoragePool().setmaster_domain_version(master_domain_version); return null; } @@ -174,8 +172,7 @@ } /** - * Check that we are not trying to attach more than one ISO or export - * domain to the same data center. + * Check that we are not trying to attach more than one ISO or export domain to the same data center. */ protected boolean checkStorageDomainType(final StorageDomain storageDomain) { // Nothing to check if the storage domain is not an ISO or export: @@ -285,6 +282,7 @@ * The following method should check if the format of the storage domain allows to it to be attached to the storage * pool. At case of failure the false value will be return and appropriate error message will be added to * canDoActionMessages + * * @param storageDomain * -the domain object * @param storagePool @@ -381,29 +379,5 @@ protected void executeInScope(TransactionScopeOption scope, TransactionMethod<?> code) { TransactionSupport.executeInScope(scope, code); - } - - protected boolean checkIsConnectionFieldEmpty(StorageServerConnections connection) { - if (StringUtils.isEmpty(connection.getconnection())) { - String fieldName = getFieldName(connection); - addCanDoActionMessage(String.format("$fieldName %1$s", fieldName)); - addCanDoActionMessage(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION); - return true; - } - return false; - } - - private String getFieldName(StorageServerConnections paramConnection) { - String fieldName = null; - if (paramConnection.getstorage_type().equals(StorageType.ISCSI)) { - fieldName = "address"; - } - else if (paramConnection.getstorage_type().isFileDomain()) { - fieldName = "path"; - } - else { - fieldName = "connection"; - } - return fieldName; } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java index 017f6b3..0e76abd 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java @@ -3,21 +3,26 @@ import java.util.Collections; import java.util.List; +import org.apache.commons.lang.StringUtils; +import org.ovirt.engine.core.bll.CommandBase; import org.ovirt.engine.core.bll.MultiLevelAdministrationHandler; import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.StorageServerConnectionParametersBase; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageServerConnections; +import org.ovirt.engine.core.common.businessentities.StorageType; +import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dao.LunDAO; import org.ovirt.engine.core.dao.StorageDomainDAO; import org.ovirt.engine.core.dao.StorageServerConnectionDAO; public abstract class StorageServerConnectionCommandBase<T extends StorageServerConnectionParametersBase> extends - StorageHandlingCommandBase<T> { + CommandBase<T> { public StorageServerConnectionCommandBase(T parameters) { super(parameters); + setVdsId(parameters.getVdsId()); } protected StorageServerConnections getConnection() { @@ -62,4 +67,26 @@ || (connections.size() == 1 && !connections.get(0).getid().equalsIgnoreCase(connection.getid()))); return isDuplicateConnExists; } + + protected boolean checkIsConnectionFieldEmpty(StorageServerConnections connection) { + if (StringUtils.isEmpty(connection.getconnection())) { + String fieldName = getFieldName(connection); + addCanDoActionMessage(String.format("$fieldName %1$s", fieldName)); + addCanDoActionMessage(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION); + return true; + } + return false; + } + + private static String getFieldName(StorageServerConnections paramConnection) { + String fieldName; + if (paramConnection.getstorage_type().equals(StorageType.ISCSI)) { + fieldName = "address"; + } else if (paramConnection.getstorage_type().isFileDomain()) { + fieldName = "path"; + } else { + fieldName = "connection"; + } + return fieldName; + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java index 50782cf..bc3ec04 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -17,7 +18,6 @@ import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainType; import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMap; -import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId; import org.ovirt.engine.core.common.businessentities.StorageServerConnections; import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VM; @@ -68,11 +68,6 @@ if (checkIsConnectionFieldEmpty(newConnectionDetails)) { return false; - } - - Guid vdsmId = getParameters().getVdsId(); - if (vdsmId == null || vdsmId.equals(Guid.Empty)) { - return failCanDoAction(VdcBllMessages.VDS_EMPTY_NAME_OR_ID); } // Check if connection exists by id, otherwise there's nothing to update @@ -194,8 +189,8 @@ @Override protected void executeCommand() { - boolean isDomainUpdateRequired = doDomainsUseConnection(getConnection()); - StoragePoolIsoMap map = null; + boolean isDomainUpdatePossible = !Guid.isNullOrEmpty(getVdsId()); + boolean isDomainUpdateRequired = isDomainUpdatePossible && doDomainsUseConnection(getConnection()); List<StorageDomain> updatedDomains = new ArrayList<>(); boolean hasConnectStorageSucceeded = false; if (isDomainUpdateRequired) { @@ -220,8 +215,9 @@ getStorageConnDao().update(getParameters().getStorageServerConnection()); if (isDomainUpdateRequired) { for (StorageDomain domain : domains) { - map = getStoragePoolIsoMap(domain); - restoreStateAfterUpdate(map); + for (StoragePoolIsoMap map : getStoragePoolIsoMap(domain)) { + restoreStateAfterUpdate(map); + } } if (hasConnectStorageSucceeded) { disconnectFromStorage(); @@ -245,22 +241,22 @@ return !getLuns().isEmpty(); } - protected StoragePoolIsoMap getStoragePoolIsoMap(StorageDomain storageDomain) { - StoragePoolIsoMapId mapId = new StoragePoolIsoMapId(storageDomain.getId(), getParameters().getStoragePoolId()); - return getStoragePoolIsoMapDao().get(mapId); + protected Collection<StoragePoolIsoMap> getStoragePoolIsoMap(StorageDomain storageDomain) { + return getStoragePoolIsoMapDao().getAllForStorage(storageDomain.getId()); } protected void changeStorageDomainStatusInTransaction(final StorageDomainStatus status) { executeInNewTransaction(new TransactionMethod<Void>() { @Override public Void runInTransaction() { + CompensationContext context = getCompensationContext(); for (StorageDomain domain : domains) { - StoragePoolIsoMap map = getStoragePoolIsoMap(domain); - CompensationContext context = getCompensationContext(); - context.snapshotEntityStatus(map, map.getstatus()); - updateStatus(map, status); - getCompensationContext().stateChanged(); + for (StoragePoolIsoMap map : getStoragePoolIsoMap(domain)) { + context.snapshotEntityStatus(map, map.getstatus()); + updateStatus(map, status); + } } + getCompensationContext().stateChanged(); return null; } }); @@ -293,7 +289,7 @@ protected boolean connectToStorage() { StorageServerConnectionManagementVDSParameters newConnectionParametersForVdsm = createParametersForVdsm(getParameters().getVdsId(), - getParameters().getStoragePoolId(), + Guid.Empty, getParameters().getStorageServerConnection().getstorage_type(), getParameters().getStorageServerConnection()); return runVdsCommand(VDSCommandType.ConnectStorageServer, newConnectionParametersForVdsm).getSucceeded(); @@ -302,7 +298,7 @@ protected void disconnectFromStorage() { StorageServerConnectionManagementVDSParameters connectionParametersForVdsm = createParametersForVdsm(getParameters().getVdsId(), - getParameters().getStoragePoolId(), + Guid.Empty, getConnection().getstorage_type(), getConnection()); boolean isDisconnectSucceeded = diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java index 45be23c..24806ab 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java @@ -6,6 +6,9 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; +import java.util.ArrayList; +import java.util.List; + import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; @@ -23,9 +26,6 @@ import org.ovirt.engine.core.dao.StorageServerConnectionDAO; import org.ovirt.engine.core.utils.MockEJBStrategyRule; -import java.util.ArrayList; -import java.util.List; - @RunWith(MockitoJUnitRunner.class) public class AddStorageServerConnectionCommandTest { @ClassRule @@ -42,20 +42,25 @@ public void prepareParams() { parameters = new StorageServerConnectionParametersBase(); parameters.setVdsId(Guid.newGuid()); - parameters.setStoragePoolId(Guid.newGuid()); command = spy(new AddStorageServerConnectionCommand<StorageServerConnectionParametersBase>(parameters)); doReturn(storageConnDao).when(command).getStorageConnDao(); } - - private StorageServerConnections createPosixConnection(String connection, StorageType type, String vfsType, String mountOptions) { + private StorageServerConnections createPosixConnection(String connection, + StorageType type, + String vfsType, + String mountOptions) { StorageServerConnections connectionDetails = populateBasicConnectionDetails(connection, type); connectionDetails.setVfsType(vfsType); connectionDetails.setMountOptions(mountOptions); return connectionDetails; } - private StorageServerConnections createISCSIConnection(String connection, StorageType type, String iqn, String user, String password) { + private StorageServerConnections createISCSIConnection(String connection, + StorageType type, + String iqn, + String user, + String password) { StorageServerConnections connectionDetails = populateBasicConnectionDetails(connection, type); connectionDetails.setiqn(iqn); connectionDetails.setuser_name(user); @@ -78,7 +83,6 @@ null, "timeo=30"); parameters.setStorageServerConnection(newPosixConnection); - parameters.setStoragePoolId(Guid.Empty); CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE); } @@ -92,44 +96,42 @@ "timeo=30"); parameters.setStorageServerConnection(newPosixConnection); parameters.setVdsId(Guid.Empty); - parameters.setStoragePoolId(Guid.Empty); doReturn(false).when(command).isConnWithSameDetailsExists(newPosixConnection); - doReturn(true).when(command).initializeVds(); CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command); } - @Test - public void addISCSIEmptyIqn() { - StorageServerConnections newISCSIConnection = createISCSIConnection("10.35.16.25", StorageType.ISCSI,"","user1","mypassword123"); + @Test + public void addISCSIEmptyIqn() { + StorageServerConnections newISCSIConnection = + createISCSIConnection("10.35.16.25", StorageType.ISCSI, "", "user1", "mypassword123"); parameters.setStorageServerConnection(newISCSIConnection); parameters.setVdsId(Guid.Empty); - parameters.setStoragePoolId(Guid.Empty); - doReturn(true).when(command).initializeVds(); CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.VALIDATION_STORAGE_CONNECTION_EMPTY_IQN); } - @Test - public void addISCSINonEmptyIqn() { - StorageServerConnections newISCSIConnection = createISCSIConnection("10.35.16.25", StorageType.ISCSI,"iqn.2013-04.myhat.com:aaa-target1","user1","mypassword123"); + @Test + public void addISCSINonEmptyIqn() { + StorageServerConnections newISCSIConnection = + createISCSIConnection("10.35.16.25", + StorageType.ISCSI, + "iqn.2013-04.myhat.com:aaa-target1", + "user1", + "mypassword123"); parameters.setStorageServerConnection(newISCSIConnection); parameters.setVdsId(Guid.Empty); - parameters.setStoragePoolId(Guid.Empty); - doReturn(true).when(command).initializeVds(); doReturn(false).when(command).isConnWithSameDetailsExists(newISCSIConnection); CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command); } - @Test - public void addNFSEmptyConn() { - StorageServerConnections newPosixConnection = createPosixConnection("",StorageType.POSIXFS, "nfs" , "timeo=30"); + @Test + public void addNFSEmptyConn() { + StorageServerConnections newPosixConnection = createPosixConnection("", StorageType.POSIXFS, "nfs", "timeo=30"); parameters.setStorageServerConnection(newPosixConnection); parameters.setVdsId(Guid.Empty); - parameters.setStoragePoolId(Guid.Empty); - doReturn(true).when(command).initializeVds(); CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION); - } + } @Test public void addExistingConnection() { @@ -140,7 +142,6 @@ "timeo=30"); parameters.setStorageServerConnection(newPosixConnection); parameters.setVdsId(Guid.Empty); - doReturn(true).when(command).initializeVds(); doReturn(true).when(command).isConnWithSameDetailsExists(newPosixConnection); CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_ALREADY_EXISTS); @@ -156,7 +157,6 @@ newPosixConnection.setid(""); parameters.setStorageServerConnection(newPosixConnection); parameters.setVdsId(Guid.Empty); - doReturn(true).when(command).initializeVds(); doReturn(false).when(command).isConnWithSameDetailsExists(newPosixConnection); Pair<Boolean, Integer> connectResult = new Pair(true, 0); doReturn(connectResult).when(command).connectHostToStorage(); @@ -177,22 +177,24 @@ newPosixConnection.setid(Guid.newGuid().toString()); parameters.setStorageServerConnection(newPosixConnection); parameters.setVdsId(Guid.Empty); - doReturn(true).when(command).initializeVds(); doReturn(true).when(command).isConnWithSameDetailsExists(newPosixConnection); CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_ID_NOT_EMPTY); } @Test - public void addISCSIEmptyConn() { - StorageServerConnections newISCSIConnection = createISCSIConnection("", StorageType.ISCSI,"iqn.2013-04.myhat.com:aaa-target1","user1","mypassword123"); + public void addISCSIEmptyConn() { + StorageServerConnections newISCSIConnection = + createISCSIConnection("", + StorageType.ISCSI, + "iqn.2013-04.myhat.com:aaa-target1", + "user1", + "mypassword123"); parameters.setStorageServerConnection(newISCSIConnection); parameters.setVdsId(Guid.Empty); - parameters.setStoragePoolId(Guid.Empty); - doReturn(true).when(command).initializeVds(); CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION); - } + } @Test public void isConnWithSameDetailsExist() { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageServerConnectionCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageServerConnectionCommandTest.java index 324d598..9777d06 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageServerConnectionCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageServerConnectionCommandTest.java @@ -21,8 +21,8 @@ import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StorageServerConnections; import org.ovirt.engine.core.common.businessentities.StorageType; -import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dao.LunDAO; import org.ovirt.engine.core.dao.StorageServerConnectionDAO; import org.ovirt.engine.core.utils.MockEJBStrategyRule; @@ -71,7 +71,6 @@ private void prepareCommand() { parameters = new StorageServerConnectionParametersBase(); parameters.setVdsId(Guid.newGuid()); - parameters.setStoragePoolId(Guid.newGuid()); command = spy(new RemoveStorageServerConnectionCommand(parameters)); doReturn(lunDAO).when(command).getLunDao(); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java index f4deba1..be8c975 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java @@ -10,6 +10,7 @@ import static org.mockito.Mockito.when; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -99,7 +100,6 @@ private void prepareCommand() { parameters = new StorageServerConnectionParametersBase(); parameters.setVdsId(Guid.newGuid()); - parameters.setStoragePoolId(Guid.newGuid()); command = spy(new UpdateStorageServerConnectionCommand<StorageServerConnectionParametersBase>(parameters)); doReturn(storageConnDao).when(command).getStorageConnDao(); @@ -147,7 +147,6 @@ return connectionDetails; } - private StorageServerConnections populateBasicConnectionDetails(Guid id, String connection, StorageType type) { StorageServerConnections connectionDetails = new StorageServerConnections(); connectionDetails.setid(id.toString()); @@ -174,7 +173,7 @@ parameters.setVdsId(null); parameters.setStorageServerConnection(newNFSConnection); when(storageConnDao.get(newNFSConnection.getid())).thenReturn(oldNFSConnection); - CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.VDS_EMPTY_NAME_OR_ID); + CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command); } @Test @@ -187,7 +186,8 @@ 0); parameters.setStorageServerConnection(newNFSConnection); parameters.setVdsId(Guid.Empty); - CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.VDS_EMPTY_NAME_OR_ID); + when(storageConnDao.get(newNFSConnection.getid())).thenReturn(oldNFSConnection); + CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command); } @Test @@ -526,7 +526,7 @@ returnValueConnectSuccess.setSucceeded(true); StorageDomainDynamic domainDynamic = new StorageDomainDynamic(); StorageDomain domain = createDomain(domainDynamic); - doReturn(map).when(command).getStoragePoolIsoMap(domain); + doReturn(Collections.singletonList(map)).when(command).getStoragePoolIsoMap(domain); returnValueConnectSuccess.setReturnValue(domain); doReturn(returnValueConnectSuccess).when(command).getStatsForDomain(domain); doReturn(true).when(command).connectToStorage(); @@ -559,9 +559,9 @@ doNothing().when(storageConnDao).update(newNFSConnection); command.executeCommand(); CommandAssertUtils.checkSucceeded(command, true); - verify(command,never()).connectToStorage(); - verify(command,never()).disconnectFromStorage(); - verify(command,never()).changeStorageDomainStatusInTransaction(StorageDomainStatus.Locked); + verify(command, never()).connectToStorage(); + verify(command, never()).disconnectFromStorage(); + verify(command, never()).changeStorageDomainStatusInTransaction(StorageDomainStatus.Locked); } @@ -582,7 +582,7 @@ doReturn(domains).when(command).getStorageDomainsByConnId(newNFSConnection.getid()); StorageDomainDynamic domainDynamic = new StorageDomainDynamic(); StoragePoolIsoMap map = new StoragePoolIsoMap(); - doReturn(map).when(command).getStoragePoolIsoMap(domain); + doReturn(Collections.singletonList(map)).when(command).getStoragePoolIsoMap(domain); doReturn(returnValueUpdate).when(command).getStatsForDomain(domain); doReturn(true).when(command).connectToStorage(); doNothing().when(command).changeStorageDomainStatusInTransaction(StorageDomainStatus.Locked); @@ -611,7 +611,7 @@ domains.add(domain); doReturn(domains).when(command).getStorageDomainsByConnId(newNFSConnection.getid()); StoragePoolIsoMap map = new StoragePoolIsoMap(); - doReturn(map).when(command).getStoragePoolIsoMap(domain); + doReturn(Collections.singletonList(map)).when(command).getStoragePoolIsoMap(domain); doReturn(returnValueUpdate).when(command).getStatsForDomain(domain); doNothing().when(command).changeStorageDomainStatusInTransaction(StorageDomainStatus.Locked); doNothing().when(command).changeStorageDomainStatusInTransaction(StorageDomainStatus.Maintenance); diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/StorageServerConnectionParametersBase.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/StorageServerConnectionParametersBase.java index caab264..f1d9ce1 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/StorageServerConnectionParametersBase.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/StorageServerConnectionParametersBase.java @@ -5,11 +5,21 @@ import org.ovirt.engine.core.common.businessentities.StorageServerConnections; import org.ovirt.engine.core.compat.Guid; -public class StorageServerConnectionParametersBase extends StoragePoolParametersBase { - private static final long serialVersionUID = 1516671099713952453L; +public class StorageServerConnectionParametersBase extends VdcActionParametersBase { + private static final long serialVersionUID = 6389650711081394484L; @Valid private StorageServerConnections privateStorageServerConnection; + + private Guid vdsId; + + public StorageServerConnectionParametersBase(StorageServerConnections connection, Guid vdsId) { + setStorageServerConnection(connection); + setVdsId(vdsId); + } + + public StorageServerConnectionParametersBase() { + } public StorageServerConnections getStorageServerConnection() { return privateStorageServerConnection; @@ -19,12 +29,11 @@ privateStorageServerConnection = value; } - public StorageServerConnectionParametersBase(StorageServerConnections connection, Guid vdsId) { - super(Guid.Empty); - setStorageServerConnection(connection); - setVdsId(vdsId); + public Guid getVdsId() { + return vdsId; } - public StorageServerConnectionParametersBase() { + public void setVdsId(Guid vdsId) { + this.vdsId = vdsId; } } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java index fc620de..0a71091 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java @@ -1513,10 +1513,8 @@ VDS host = (VDS) model.getHost().getSelectedItem(); Guid hostId = Guid.Empty; - Guid storagePoolId = Guid.Empty; if (host != null) { hostId = host.getId(); - storagePoolId = host.getStoragePoolId(); } IStorageModel storageModel = model.getSelectedItem(); connection = new StorageServerConnections(); @@ -1533,7 +1531,6 @@ StorageServerConnectionParametersBase parameters = new StorageServerConnectionParametersBase(connection, hostId); - parameters.setStoragePoolId(storagePoolId); Frontend.RunAction(VdcActionType.UpdateStorageServerConnection, parameters, new IFrontendActionAsyncCallback() { @Override -- To view, visit http://gerrit.ovirt.org/17288 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I628bf2a63f64d3acaa3520ad144e1eef744f1204 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
