Alissa Bonas has uploaded a new change for review.
Change subject: core: check if same storage connection exists
......................................................................
core: check if same storage connection exists
Check in Add/Update storage connection whether
the same storage connection already exists.
The check was present for file domains only in AddStorageConnection
command, and now it is expanded to check for block domains as well,
and is also used by the UpdateStorageServerConnection.
Also moved the code to a parent command to allow code reuse, and added
corresponding unitests.
Change-Id: I0cfc642c49ab4fc482c5d1fabc28fad0ae79d2f4
Signed-off-by: Alissa Bonas <abo...@redhat.com>
---
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/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/UpdateStorageServerConnectionCommandTest.java
5 files changed, 153 insertions(+), 41 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/37/16037/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 0f557c3..11a6bad 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
@@ -52,7 +52,7 @@
}
protected StorageServerConnections getConnectionFromDbById(String
connectionId) {
- return getDbFacade().getStorageServerConnectionDao().get(connectionId);
+ return getStorageConnDao().get(connectionId);
}
protected Pair<Boolean, Integer> connectHostToStorage() {
@@ -63,11 +63,6 @@
protected void saveConnection(StorageServerConnections connection) {
getDbFacade().getStorageServerConnectionDao().save(connection);
- }
-
- protected boolean isConnWithSameDetailsExists() {
- String connection = getConnection().getconnection();
- return
getDbFacade().getStorageServerConnectionDao().getAllForStorage(connection).size()
!= 0;
}
@Override
@@ -94,10 +89,10 @@
}
if (checkIsConnectionFieldEmpty(paramConnection)) {
- return false;
+ return false;
}
- if (isConnWithSameDetailsExists()) {
+ if (isConnWithSameDetailsExists(getConnection())) {
return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_ALREADY_EXISTS);
}
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 d2cb6f8..9890e9b 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
@@ -11,6 +11,7 @@
import org.ovirt.engine.core.common.businessentities.StorageServerConnections;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.dao.StorageDomainDAO;
+import org.ovirt.engine.core.dao.StorageServerConnectionDAO;
public abstract class StorageServerConnectionCommandBase<T extends
StorageServerConnectionParametersBase> extends
StorageHandlingCommandBase<T> {
@@ -33,7 +34,30 @@
return getDbFacade().getStorageDomainDao();
}
+ protected StorageServerConnectionDAO getStorageConnDao() {
+ return getDbFacade().getStorageServerConnectionDao();
+ }
+
protected List<StorageDomain> getStorageDomainsByConnId(String
connectionId) {
return
getStorageDomainDao().getAllByConnectionId(Guid.createGuidFromString(connectionId));
}
+
+ protected boolean isConnWithSameDetailsExists(StorageServerConnections
connection) {
+ List<StorageServerConnections> connections = null;
+ if (connection.getstorage_type().isFileDomain()) {
+ String connectionField = connection.getconnection();
+ connections =
getStorageConnDao().getAllForStorage(connectionField);
+ }
+ else {
+ connections = getStorageConnDao().getAllForConnection(connection);
+ }
+
+ if (connections.size() > 1
+ || (connections.size() == 1 &&
!connections.get(0).getid().equalsIgnoreCase(connection.getid()))) {
+ return true;
+ }
+ else {
+ return false;
+ }
+ }
}
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 caf9aa4..363d2d0 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
@@ -30,10 +30,8 @@
import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
import org.ovirt.engine.core.compat.Guid;
-
import org.ovirt.engine.core.dao.StorageDomainDynamicDAO;
import org.ovirt.engine.core.dao.StoragePoolIsoMapDAO;
-import org.ovirt.engine.core.dao.StorageServerConnectionDAO;
import org.ovirt.engine.core.utils.transaction.TransactionMethod;
import org.ovirt.engine.core.utils.transaction.TransactionSupport;
@@ -67,7 +65,7 @@
}
if (checkIsConnectionFieldEmpty(newConnectionDetails)) {
- return false;
+ return false;
}
Guid vdsmId = getParameters().getVdsId();
@@ -88,13 +86,8 @@
return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_UNSUPPORTED_CHANGE_STORAGE_TYPE);
}
- if
(!oldConnection.getconnection().equals(newConnectionDetails.getconnection())) {
- // Check that there is no other connection with the new suggested
path
- List<StorageServerConnections> connections =
-
getStorageConnDao().getAllForStorage(newConnectionDetails.getconnection());
- if (!connections.isEmpty()) {
- return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_ALREADY_EXISTS);
- }
+ if (isConnWithSameDetailsExists(getConnection())) {
+ return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_ALREADY_EXISTS);
}
if (domains == null) {
@@ -119,7 +112,6 @@
return super.canDoAction();
}
-
protected String createDomainNamesList(List<StorageDomain> domains) {
// Build domain names list to display in the error
@@ -243,10 +235,6 @@
return newConnectionParametersForVdsm;
}
- protected StorageServerConnectionDAO getStorageConnDao() {
- return getDbFacade().getStorageServerConnectionDao();
- }
-
protected StorageDomainDynamicDAO getStorageDomainDynamicDao() {
return getDbFacade().getStorageDomainDynamicDao();
}
@@ -254,8 +242,6 @@
protected StoragePoolIsoMapDAO getStoragePoolIsoMapDao() {
return getDbFacade().getStoragePoolIsoMapDao();
}
-
-
@Override
protected Map<String, Pair<String, String>> getExclusiveLocks() {
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 bc9016b..ae1dea8 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
@@ -82,7 +82,7 @@
parameters.setStorageServerConnection(newPosixConnection);
parameters.setVdsId(Guid.Empty);
parameters.setStoragePoolId(Guid.Empty);
- doReturn(false).when(command).isConnWithSameDetailsExists();
+
doReturn(false).when(command).isConnWithSameDetailsExists(newPosixConnection);
doReturn(true).when(command).initializeVds();
CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command);
}
@@ -105,7 +105,7 @@
parameters.setVdsId(Guid.Empty);
parameters.setStoragePoolId(Guid.Empty);
doReturn(true).when(command).initializeVds();
- doReturn(false).when(command).isConnWithSameDetailsExists();
+
doReturn(false).when(command).isConnWithSameDetailsExists(newISCSIConnection);
CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command);
}
@@ -130,7 +130,7 @@
parameters.setStorageServerConnection(newPosixConnection);
parameters.setVdsId(Guid.Empty);
doReturn(true).when(command).initializeVds();
- doReturn(true).when(command).isConnWithSameDetailsExists();
+
doReturn(true).when(command).isConnWithSameDetailsExists(newPosixConnection);
CanDoActionTestUtils.runAndAssertCanDoActionFailure(command,
VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_ALREADY_EXISTS);
}
@@ -146,11 +146,12 @@
parameters.setStorageServerConnection(newPosixConnection);
parameters.setVdsId(Guid.Empty);
doReturn(true).when(command).initializeVds();
- doReturn(false).when(command).isConnWithSameDetailsExists();
+
doReturn(false).when(command).isConnWithSameDetailsExists(newPosixConnection);
Pair<Boolean, Integer> connectResult = new Pair(true, 0);
doReturn(connectResult).when(command).connectHostToStorage();
doReturn(null).when(command).getConnectionFromDbById(newPosixConnection.getid());
doNothing().when(command).saveConnection(newPosixConnection);
+
command.executeCommand();
CommandAssertUtils.checkSucceeded(command, true);
}
@@ -166,7 +167,7 @@
parameters.setStorageServerConnection(newPosixConnection);
parameters.setVdsId(Guid.Empty);
doReturn(true).when(command).initializeVds();
- doReturn(true).when(command).isConnWithSameDetailsExists();
+
doReturn(true).when(command).isConnWithSameDetailsExists(newPosixConnection);
CanDoActionTestUtils.runAndAssertCanDoActionFailure(command,
VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_ID_NOT_EMPTY);
}
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 ebe3d8f..0ef566f 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
@@ -1,5 +1,6 @@
package org.ovirt.engine.core.bll.storage;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
@@ -111,6 +112,16 @@
return connectionDetails;
}
+ private StorageServerConnections createISCSIConnection(String connection,
StorageType type, String iqn, String user, String password) {
+ Guid id = Guid.NewGuid();
+ StorageServerConnections connectionDetails =
populateBasicConnectionDetails(id, connection, type);
+ connectionDetails.setiqn(iqn);
+ connectionDetails.setuser_name(user);
+ connectionDetails.setpassword(password);
+ return connectionDetails;
+ }
+
+
private StorageServerConnections populateBasicConnectionDetails(Guid id,
String connection, StorageType type) {
StorageServerConnections connectionDetails = new
StorageServerConnections();
connectionDetails.setid(id.toString());
@@ -150,11 +161,11 @@
@Test
public void updateIScsiConnection() {
StorageServerConnections newNFSConnection = createNFSConnection(
-
"multipass.my.domain.tlv.company.com:/export/allstorage/data2",
- StorageType.ISCSI,
- NfsVersion.V4,
- 300,
- 0);
+
"multipass.my.domain.tlv.company.com:/export/allstorage/data2",
+ StorageType.ISCSI,
+ NfsVersion.V4,
+ 300,
+ 0);
parameters.setStorageServerConnection(newNFSConnection);
CanDoActionTestUtils.runAndAssertCanDoActionFailure(command,
VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_UNSUPPORTED_ACTION_FOR_STORAGE);
@@ -231,6 +242,7 @@
connections.add(conn2);
when(storageConnDao.getAllForStorage(newNFSConnection.getconnection())).thenReturn(connections);
when(storageConnDao.get(newNFSConnection.getid())).thenReturn(oldNFSConnection);
+
doReturn(true).when(command).isConnWithSameDetailsExists(newNFSConnection);
CanDoActionTestUtils.runAndAssertCanDoActionFailure(command,
VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_ALREADY_EXISTS);
}
@@ -257,6 +269,7 @@
domains.add(domain2);
when(storageConnDao.get(newNFSConnection.getid())).thenReturn(oldNFSConnection);
doReturn(domains).when(command).getStorageDomainsByConnId(newNFSConnection.getid());
+
doReturn(false).when(command).isConnWithSameDetailsExists(newNFSConnection);
List<String> messages =
CanDoActionTestUtils.runAndAssertCanDoActionFailure(command,
VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_BELONGS_TO_SEVERAL_STORAGE_DOMAINS);
@@ -279,6 +292,7 @@
parameters.setStorageServerConnection(newNFSConnection);
when(storageConnDao.get(newNFSConnection.getid())).thenReturn(oldNFSConnection);
doReturn(domains).when(command).getStorageDomainsByConnId(newNFSConnection.getid());
+
doReturn(false).when(command).isConnWithSameDetailsExists(newNFSConnection);
CanDoActionTestUtils.runAndAssertCanDoActionFailure(command,
VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_UNSUPPORTED_ACTION_FOR_STORAGE);
}
@@ -295,6 +309,7 @@
List<StorageDomain> domains = new ArrayList<StorageDomain>();
when(storageConnDao.get(newNFSConnection.getid())).thenReturn(oldNFSConnection);
doReturn(domains).when(command).getStorageDomainsByConnId(newNFSConnection.getid());
+
doReturn(false).when(command).isConnWithSameDetailsExists(newNFSConnection);
CanDoActionTestUtils.runAndAssertCanDoActionFailure(command,
VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST);
}
@@ -302,11 +317,11 @@
@Test
public void succeedCanDoActionNFS() {
StorageServerConnections newNFSConnection = createNFSConnection(
-
"multipass.my.domain.tlv.company.com:/export/allstorage/data2",
- StorageType.NFS,
- NfsVersion.V4,
- 300,
- 0);
+ "multipass.my.domain.tlv.company.com:/export/allstorage/data2",
+ StorageType.NFS,
+ NfsVersion.V4,
+ 300,
+ 0);
parameters.setStorageServerConnection(newNFSConnection);
List<StorageDomain> domains = new ArrayList<StorageDomain>();
StorageDomain domain1 = new StorageDomain();
@@ -314,13 +329,14 @@
domain1.setStatus(StorageDomainStatus.Maintenance);
domains.add(domain1);
when(storageConnDao.get(newNFSConnection.getid())).thenReturn(oldNFSConnection);
+
doReturn(false).when(command).isConnWithSameDetailsExists(newNFSConnection);
doReturn(domains).when(command).getStorageDomainsByConnId(newNFSConnection.getid());
CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command);
}
@Test
public void succeedCanDoActionPosix() {
- StorageServerConnections newPosixConnection =
createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1",
StorageType.POSIXFS, "nfs" , "timeo=30");
+ StorageServerConnections newPosixConnection =
createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1",
StorageType.POSIXFS, "nfs", "timeo=30");
parameters.setStorageServerConnection(newPosixConnection);
List<StorageDomain> domains = new ArrayList<StorageDomain>();
StorageDomain domain1 = new StorageDomain();
@@ -330,6 +346,7 @@
parameters.setStorageServerConnection(newPosixConnection);
when(storageConnDao.get(newPosixConnection.getid())).thenReturn(oldPosixConnection);
doReturn(domains).when(command).getStorageDomainsByConnId(newPosixConnection.getid());
+
doReturn(false).when(command).isConnWithSameDetailsExists(newPosixConnection);
CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command);
}
@@ -407,4 +424,93 @@
verify(storageConnDao, never()).update(newNFSConnection);
}
+ @Test
+ public void isConnWithSameDetailsExistFileDomains() {
+ StorageServerConnections newNFSConnection = createNFSConnection(
+
"multipass.my.domain.tlv.company.com:/export/allstorage/data2",
+ StorageType.NFS,
+ NfsVersion.V4,
+ 300,
+ 0);
+
+ List<StorageServerConnections> connections = new ArrayList<>();
+ StorageServerConnections connection1 = createNFSConnection(
+
"multipass.my.domain.tlv.company.com:/export/allstorage/data2",
+ StorageType.NFS,
+ NfsVersion.V4,
+ 300,
+ 0);
+ connections.add(connection1);
+ StorageServerConnections connection2 = createNFSConnection(
+
"multipass.my.domain.tlv.company.com:/export/allstorage/data2",
+ StorageType.NFS,
+ NfsVersion.V4,
+ 600,
+ 0);
+ connections.add(connection2);
+
+
when(storageConnDao.getAllForStorage(newNFSConnection.getconnection())).thenReturn(connections);
+ boolean isExists =
command.isConnWithSameDetailsExists(newNFSConnection);
+ assertTrue(isExists);
+ }
+
+ @Test
+ public void isConnWithSameDetailsExistSameConnection() {
+ StorageServerConnections newNFSConnection = createNFSConnection(
+
"multipass.my.domain.tlv.company.com:/export/allstorage/data2",
+ StorageType.NFS,
+ NfsVersion.V4,
+ 300,
+ 0);
+
+ List<StorageServerConnections> connections = new ArrayList<>();
+ StorageServerConnections connection1 = newNFSConnection;
+ connections.add(connection1);
+
+
when(storageConnDao.getAllForStorage(newNFSConnection.getconnection())).thenReturn(connections);
+ boolean isExists =
command.isConnWithSameDetailsExists(newNFSConnection);
+ assertFalse(isExists);
+ }
+
+ @Test
+ public void isConnWithSameDetailsExistNoConnections() {
+ StorageServerConnections newNFSConnection = createNFSConnection(
+
"multipass.my.domain.tlv.company.com:/export/allstorage/data2",
+ StorageType.NFS,
+ NfsVersion.V4,
+ 300,
+ 0);
+
+ List<StorageServerConnections> connections = new ArrayList<>();
+
+
when(storageConnDao.getAllForStorage(newNFSConnection.getconnection())).thenReturn(connections);
+ boolean isExists =
command.isConnWithSameDetailsExists(newNFSConnection);
+ assertFalse(isExists);
+ }
+
+ @Test
+ public void isConnWithSameDetailsExistBlockDomains() {
+ StorageServerConnections newISCSIConnection =
createISCSIConnection("1.2.3.4",
StorageType.ISCSI,"iqn.2013-04.myhat.com:aaa-target1","user1","mypassword123");
+
+ List<StorageServerConnections> connections = new ArrayList<>();
+ StorageServerConnections connection1 = createISCSIConnection("1.2.3.4",
StorageType.ISCSI,"iqn.2013-04.myhat.com:aaa-target1","user1","mypassword123");
+ connections.add(connection1);
+
+
when(storageConnDao.getAllForConnection(newISCSIConnection)).thenReturn(connections);
+ boolean isExists =
command.isConnWithSameDetailsExists(newISCSIConnection);
+ assertTrue(isExists);
+ }
+
+ @Test
+ public void isConnWithSameDetailsExistCheckSameConn() {
+ StorageServerConnections newISCSIConnection =
createISCSIConnection("1.2.3.4",
StorageType.ISCSI,"iqn.2013-04.myhat.com:aaa-target1","user1","mypassword123");
+
+ List<StorageServerConnections> connections = new ArrayList<>();
+ connections.add(newISCSIConnection);
+
+
when(storageConnDao.getAllForConnection(newISCSIConnection)).thenReturn(connections);
+ boolean isExists =
command.isConnWithSameDetailsExists(newISCSIConnection);
+ assertFalse(isExists);
+ }
+
}
--
To view, visit http://gerrit.ovirt.org/16037
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0cfc642c49ab4fc482c5d1fabc28fad0ae79d2f4
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <abo...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
