Kanagaraj M has uploaded a new change for review. Change subject: engine: Fix brick validation during Add Bricks ......................................................................
engine: Fix brick validation during Add Bricks While adding a new brick(s) to a volume, server_id and brick_directory combination will be validated against the entries in the gluster_volume_bricks table. If there are any existing entries found for the server:brick_dir, an error message will be thrown to the user. Introduced a new procedure to get a brick entity based on server_id and brick directory. Change-Id: I83ec15a183d81a274388c313006208d439809548 Bug-Url: https://bugzilla.redhat.com/856109 Signed-off-by: Kanagaraj M <[email protected]> --- M backend/manager/dbscripts/gluster_volumes_sp.sql M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommandTest.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDao.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDaoDbFacadeImpl.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDaoTest.java M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java 8 files changed, 57 insertions(+), 37 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/12/12612/1 diff --git a/backend/manager/dbscripts/gluster_volumes_sp.sql b/backend/manager/dbscripts/gluster_volumes_sp.sql index 7c211b6..b927470 100644 --- a/backend/manager/dbscripts/gluster_volumes_sp.sql +++ b/backend/manager/dbscripts/gluster_volumes_sp.sql @@ -186,6 +186,17 @@ END; $procedure$ LANGUAGE plpgsql; +Create or replace FUNCTION GetBrickByServerIdAndDirectory(v_server_id UUID, v_brick_dir VARCHAR(4096)) + RETURNS SETOF gluster_volume_bricks + AS $procedure$ +BEGIN + RETURN QUERY SELECT * + FROM gluster_volume_bricks + WHERE server_id = v_server_id + AND brick_dir = v_brick_dir; +END; $procedure$ +LANGUAGE plpgsql; + Create or replace FUNCTION GetGlusterOptionById(v_id UUID) RETURNS SETOF gluster_volume_options AS $procedure$ diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommand.java index 22ba098..a735f18 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommand.java @@ -12,7 +12,6 @@ import org.ovirt.engine.core.common.businessentities.gluster.GlusterStatus; import org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeEntity; import org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeType; -import org.ovirt.engine.core.common.utils.ObjectUtils; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; import org.ovirt.engine.core.common.vdscommands.gluster.GlusterVolumeBricksActionVDSParameters; @@ -187,25 +186,17 @@ private boolean validateBricks(List<GlusterBrickEntity> newBricks) { for (GlusterBrickEntity brick : newBricks) { - if (brickExists(brick)) { + GlusterBrickEntity existingBrick = + getGlusterBrickDao().getBrickByServerIdAndDirectory(brick.getServerId(), brick.getBrickDirectory()); + if (existingBrick != null) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_BRICK_ALREADY_EXISTS_IN_VOLUME); addCanDoActionMessage(String.format("$brick %1$s", brick.getQualifiedName())); - addCanDoActionMessage(String.format("$volumeName %1$s", getGlusterVolumeDao().getById(brick.getVolumeId()).getName())); + addCanDoActionMessage(String.format("$volumeName %1$s", + getGlusterVolumeDao().getById(existingBrick.getVolumeId()).getName())); return false; } } return true; - } - - public boolean brickExists(GlusterBrickEntity newBrick) { - for (GlusterBrickEntity brick : getGlusterBrickDao().getGlusterVolumeBricksByServerId(newBrick.getServerId())) { - if (ObjectUtils.objectsEqual(newBrick.getVolumeId(), brick.getVolumeId()) - && ObjectUtils.objectsEqual(newBrick.getServerId(), brick.getServerId()) - && ObjectUtils.objectsEqual(newBrick.getBrickDirectory(), brick.getBrickDirectory())) { - return true; - } - } - return false; } @Override diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommandTest.java index bfa0c03..4f1bafa 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommandTest.java @@ -56,6 +56,8 @@ private Guid volumeId2 = new Guid("b2cb2f73-fab3-4a42-93f0-d5e4c069a43e"); + private static final String BRICK_DIRECTORY = "/tmp/s1"; + /** * The command under test. */ @@ -93,27 +95,23 @@ return bricks; } - private List<GlusterBrickEntity> getBricks(Guid glusterServerId) { - List<GlusterBrickEntity> bricks = new ArrayList<GlusterBrickEntity>(); - GlusterBrickEntity brick; - for (Integer i = 0; i < 5; i++) { - brick = - new GlusterBrickEntity(volumeId1, - new VdsStatic(serverName, - "127.0.0.1", - "0934390834", - 20, - new Guid(), - glusterServerId, - serverName, - true, - VDSType.oVirtNode), - "/tmp/s" + i.toString(), - GlusterStatus.UP); - bricks.add(brick); - } - return bricks; + private GlusterBrickEntity getBrick(Guid volumeId) { + GlusterBrickEntity brick = + new GlusterBrickEntity(volumeId, + new VdsStatic(serverName, + "127.0.0.1", + "0934390834", + 20, + new Guid(), + serverId, + serverName, + true, + VDSType.oVirtNode), + BRICK_DIRECTORY, + GlusterStatus.UP); + return brick; } + private VDS getVds(VDSStatus status) { VDS vds = new VDS(); vds.setId(Guid.NewGuid()); @@ -130,7 +128,7 @@ doReturn(getVds(VDSStatus.Up)).when(command).getUpServer(); doReturn(getSingleBrickVolume(volumeId1)).when(volumeDao).getById(volumeId1); doReturn(getMultiBrickVolume(volumeId2)).when(volumeDao).getById(volumeId2); - doReturn(getBricks(serverId)).when(brickDao).getGlusterVolumeBricksByServerId(serverId); + doReturn(getBrick(volumeId1)).when(brickDao).getBrickByServerIdAndDirectory(serverId, BRICK_DIRECTORY); doReturn(null).when(volumeDao).getById(null); doReturn(getVdsStatic()).when(vdsStaticDao).get(serverId); doReturn(getVDsGroup()).when(command).getVdsGroup(); diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDao.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDao.java index 50ef365..1258690 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDao.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDao.java @@ -29,4 +29,6 @@ public void updateBrickOrder(Guid brickId, int brickOrder); public List<GlusterBrickEntity> getGlusterVolumeBricksByServerId(Guid serverId); + + public GlusterBrickEntity getBrickByServerIdAndDirectory(Guid serverId, String brickDirectory); } diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDaoDbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDaoDbFacadeImpl.java index b2849cb..52b2cc4 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDaoDbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDaoDbFacadeImpl.java @@ -137,6 +137,15 @@ } @Override + public GlusterBrickEntity getBrickByServerIdAndDirectory(Guid serverId, String brickDirectory) { + return getCallsHandler().executeRead( + "GetBrickByServerIdAndDirectory", brickRowMapper, + getCustomMapSqlParameterSource() + .addValue("server_id", serverId) + .addValue("brick_dir", brickDirectory)); + } + + @Override protected MapSqlParameterSource createFullParametersMapper(GlusterBrickEntity brick) { return createBrickParams(brick); } diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index aca3b52..0b34673 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -823,7 +823,7 @@ ACTION_TYPE_FAILED_CAN_NOT_INCREASE_STRIPE_COUNT_MORE_THAN_ONE=Cannot ${action} ${type}. Stripe count cannot be increased by more than one. ACTION_TYPE_FAILED_CAN_NOT_REDUCE_STRIPE_COUNT=Cannot ${action} ${type}. Stripe count can not be reduced. ACTION_TYPE_FAILED_NO_UP_SERVER_FOUND=Cannot ${action} ${type}. No up server found in ${clusterName}. -ACTION_TYPE_FAILED_BRICK_ALREADY_EXISTS_IN_VOLUME=Cannot ${action} ${type}. Brick ${brick} already used by the volume ${volumeName}. +ACTION_TYPE_FAILED_BRICK_ALREADY_EXISTS_IN_VOLUME=Cannot ${action} ${type}. Brick ${brick} is already used by the volume ${volumeName}. ERROR_GET_STORAGE_DOMAIN_LIST=Cannot get Storage Domains list. VDS_CANNOT_REMOVE_HOST_HAVING_GLUSTER_VOLUME=Cannot remove gluster server. Server having Gluster volume(s). ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_AND_MIGRATABLE=Migratable VM's cannot be pinned to CPU. diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDaoTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDaoTest.java index 7e5469a..8c2b3a9 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDaoTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/gluster/GlusterBrickDaoTest.java @@ -20,6 +20,9 @@ private static final Guid EXISTING_VOL_ID = new Guid("0c3f45f6-3fe9-4b35-a30c-be0d1a835ea8"); private static final Guid EXISTING_BRICK_ID = new Guid("6ccdc294-d77b-4929-809d-8afe7634b47d"); + private static final Guid BRICK_SERVER_ID = new Guid("23f6d691-5dfb-472b-86dc-9e1d2d3c18f3"); + private static final String BRICK_DIRECTORY = "/export/test-vol-distribute-1/dir1"; + private GlusterBrickDao dao; private VdsStatic server; @@ -52,6 +55,12 @@ } @Test + public void testGetBrickByServerIdAndDirectory() { + GlusterBrickEntity brick = dao.getBrickByServerIdAndDirectory(BRICK_SERVER_ID, BRICK_DIRECTORY); + assertNotNull(brick); + } + + @Test public void testRemove() { GlusterBrickEntity existingBrick = dao.getById(EXISTING_BRICK_ID); assertNotNull(existingBrick); diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index 6c89e5c..4ecd81b 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -2182,7 +2182,7 @@ @DefaultStringValue("Cannot ${action} ${type}. Stripe count can not be reduced.") String ACTION_TYPE_FAILED_CAN_NOT_REDUCE_STRIPE_COUNT(); - @DefaultStringValue("Cannot ${action} ${type}. Brick ${brick} already used by the volume ${volumeName}.") + @DefaultStringValue("Cannot ${action} ${type}. Brick ${brick} is already used by the volume ${volumeName}.") String ACTION_TYPE_FAILED_BRICK_ALREADY_EXISTS_IN_VOLUME(); @DefaultStringValue("Cannot ${action} ${type}. No up server found in ${clusterName}.") -- To view, visit http://gerrit.ovirt.org/12612 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I83ec15a183d81a274388c313006208d439809548 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Kanagaraj M <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
