Shireesh Anjal has uploaded a new change for review. Change subject: gluster: Use brick server for advanced details ......................................................................
gluster: Use brick server for advanced details Currently, there is a bug in glusterfs, because of which the command 'gluster volume status' for a particular brick works properly only if it is executed on the server where the brick resides. While that bug gets fixed in glusterfs, I am changing the code here to make sure that we always execute the command on the brick's server (provided it is UP). Another refactoring done in this patch is to expect volume and brick IDs instead of names in GlusterVolumeAdvancedDetailsParameters Change-Id: Id4acb0bbe84bacab714926d69b639bbda970df9a Bug-Url: https://bugzilla.redhat.com/877961 Signed-off-by: Shireesh Anjal <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQuery.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterQueriesCommandBase.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQueryTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/gluster/GlusterVolumeAdvancedDetailsParameters.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeBrickListModel.java 6 files changed, 110 insertions(+), 45 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/79/12979/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQuery.java index eefa99b..31f073d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQuery.java @@ -3,7 +3,9 @@ import java.util.Arrays; import java.util.List; -import org.apache.commons.lang.StringUtils; +import org.ovirt.engine.core.common.businessentities.VDS; +import org.ovirt.engine.core.common.businessentities.VDSStatus; +import org.ovirt.engine.core.common.businessentities.gluster.GlusterBrickEntity; import org.ovirt.engine.core.common.businessentities.gluster.GlusterStatus; import org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeAdvancedDetails; import org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeEntity; @@ -24,6 +26,10 @@ private static final List<GlusterVolumeType> replicateVolumeTypes = Arrays.asList(new GlusterVolumeType[] { GlusterVolumeType.REPLICATE, GlusterVolumeType.DISTRIBUTED_REPLICATE }); + private GlusterBrickEntity brick = null; + private Guid clusterId = null; + private Guid volumeId = null; + private boolean detailRequired = false; public GetGlusterVolumeAdvancedDetailsQuery(P params) { super(params); @@ -31,39 +37,45 @@ @Override protected void executeQueryCommand() { - String volumeName = getParameters().getVolumeName(); - if (StringUtils.isNotEmpty(volumeName)) { - getQueryReturnValue().setReturnValue(fetchAdvancedDetails(volumeName)); + clusterId = getParameters().getClusterId(); + detailRequired = getParameters().isDetailRequired(); + volumeId = getParameters().getVolumeId(); + if (volumeId != null) { + brick = getBrick(getParameters().getBrickId()); + getQueryReturnValue().setReturnValue(fetchAdvancedDetails(getGlusterVolumeDao().getById(volumeId).getName())); } else { getQueryReturnValue().setReturnValue(getServiceInfo()); } } - /* + private GlusterBrickEntity getBrick(Guid brickId) { + return (brickId == null) ? null : getGlusterBrickDao().getById(brickId); + } + + /** * To get the service info, the UI will not pass the volume name, in that case engine will fetch the volume name in * the database. * * NFS volume name should be passed to get nfs service details, similarly REPLICATE/DISTRIBUTED_REPLICATE volume * name should be passed as an argument to get the SHD details. * - * So to get volume name from database engine will do the following steps. - * 1. First fetch NFS + REPLICATE/DISTRIBUTED_REPLICATE volume name - * 2. If not found then fetch the nfs volume name and then fetch - * REPLICATE/DISTRIBUTED_REPLICATE volume name - * 3. The VDS query will be called twice, one with nfs volume name and - * another with replicate volume name, finally combine the service details. + * So to get volume name from database engine will do the following steps.<br> + * 1. First fetch NFS + REPLICATE/DISTRIBUTED_REPLICATE volume name<br> + * 2. If not found then fetch the nfs volume name and then fetch REPLICATE/DISTRIBUTED_REPLICATE volume name<br> + * 3. The VDS query will be called twice, one with nfs volume name and another with replicate volume name, finally + * combine the service details. */ private GlusterVolumeAdvancedDetails getServiceInfo() { // Get Nfs + Replicated/Distributed Replicate volume. - GlusterVolumeEntity nfsReplicateVolume = getNfsReplicateVolume(getParameters().getClusterId()); + GlusterVolumeEntity nfsReplicateVolume = getNfsReplicateVolume(clusterId); if (nfsReplicateVolume != null) { return fetchAdvancedDetails(nfsReplicateVolume.getName()); } // Get Nfs enabled volume - GlusterVolumeEntity nfsVolume = getNfsVolume(getParameters().getClusterId()); + GlusterVolumeEntity nfsVolume = getNfsVolume(clusterId); // Get Replicated volume - GlusterVolumeEntity replicateVolume = getReplicateVolume(getParameters().getClusterId()); + GlusterVolumeEntity replicateVolume = getReplicateVolume(clusterId); // If there is no volume present in the cluster, then return empty Volume Advanced Details if (nfsVolume == null && replicateVolume == null) { @@ -91,14 +103,34 @@ return nfsServiceInfo; } + /** + * Returns the server id on which the brick resides if <br> + * a) brick id passed is not null <br> + * b) the brick server is UP <br> + * Otherwise returns a random up server from the cluster + */ + protected Guid getUpServerId() { + if (brick == null) { + return super.getUpServerId(clusterId); + } + + VDS brickServer = getVdsDao().get(brick.getServerId()); + if (brickServer.getStatus() == VDSStatus.Up) { + return brickServer.getId(); + } + + // brick server is down + return super.getUpServerId(clusterId); + } + private GlusterVolumeAdvancedDetails fetchAdvancedDetails(String volumeName) { VDSReturnValue returnValue = runVdsCommand(VDSCommandType.GetGlusterVolumeAdvancedDetails, - new GlusterVolumeAdvancedDetailsVDSParameters(getUpServerId(getParameters().getClusterId()), - getParameters().getClusterId(), + new GlusterVolumeAdvancedDetailsVDSParameters(getUpServerId(), + clusterId, volumeName, - getParameters().getBrickName(), - getParameters().isDetailRequired())); + brick.getQualifiedName(), + detailRequired)); return (GlusterVolumeAdvancedDetails) returnValue.getReturnValue(); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterQueriesCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterQueriesCommandBase.java index f2e1228..d885a4a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterQueriesCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterQueriesCommandBase.java @@ -12,6 +12,7 @@ import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dao.VdsDAO; import org.ovirt.engine.core.dao.gluster.GlusterBrickDao; import org.ovirt.engine.core.dao.gluster.GlusterHooksDao; import org.ovirt.engine.core.dao.gluster.GlusterVolumeDao; @@ -21,6 +22,10 @@ super(parameters); } + protected VdsDAO getVdsDao() { + return DbFacade.getInstance().getVdsDao(); + } + protected GlusterVolumeDao getGlusterVolumeDao() { return DbFacade.getInstance() .getGlusterVolumeDao(); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQueryTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQueryTest.java index 509c934..5016885 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQueryTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQueryTest.java @@ -19,32 +19,36 @@ import org.ovirt.engine.core.common.businessentities.VDSStatus; import org.ovirt.engine.core.common.businessentities.gluster.BrickDetails; import org.ovirt.engine.core.common.businessentities.gluster.BrickProperties; +import org.ovirt.engine.core.common.businessentities.gluster.GlusterBrickEntity; import org.ovirt.engine.core.common.businessentities.gluster.GlusterClientInfo; import org.ovirt.engine.core.common.businessentities.gluster.GlusterStatus; import org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeAdvancedDetails; +import org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeEntity; import org.ovirt.engine.core.common.businessentities.gluster.MallInfo; import org.ovirt.engine.core.common.businessentities.gluster.MemoryStatus; import org.ovirt.engine.core.common.businessentities.gluster.Mempool; -import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend; import org.ovirt.engine.core.common.queries.gluster.GlusterVolumeAdvancedDetailsParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSParametersBase; import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dao.VdsDAO; +import org.ovirt.engine.core.dao.gluster.GlusterBrickDao; +import org.ovirt.engine.core.dao.gluster.GlusterVolumeDao; public class GetGlusterVolumeAdvancedDetailsQueryTest extends AbstractQueryTest<GlusterVolumeAdvancedDetailsParameters, GetGlusterVolumeAdvancedDetailsQuery<GlusterVolumeAdvancedDetailsParameters>> { private static final Guid CLUSTER_ID = new Guid("b399944a-81ab-4ec5-8266-e19ba7c3c9d1"); - private static final String VOLUME_NAME = "test-volume1"; - private static final String BRICK_NAME = "test-server1:/tmp/b1"; + private static final Guid VOLUME_ID = Guid.NewGuid(); + private static final Guid BRICK_ID = Guid.NewGuid(); + private static final Guid SERVER_ID = Guid.NewGuid(); private static final String SERVER_NAME = "server1"; private GlusterVolumeAdvancedDetails expectedVolumeAdvancedDetails; private ClusterUtils clusterUtils; - private VDSBrokerFrontend vdsBrokerFrontend; private VdsDAO vdsDao; - private GlusterVolumeAdvancedDetailsParameters parameters; + private GlusterVolumeDao volumeDao; + private GlusterBrickDao brickDao; @Before @Override @@ -55,10 +59,22 @@ } private void setupExpectedVolume() { - parameters = new GlusterVolumeAdvancedDetailsParameters(CLUSTER_ID, VOLUME_NAME, BRICK_NAME, false); expectedVolumeAdvancedDetails = new GlusterVolumeAdvancedDetails(); - expectedVolumeAdvancedDetails.setVolumeId(Guid.NewGuid()); + expectedVolumeAdvancedDetails.setVolumeId(VOLUME_ID); expectedVolumeAdvancedDetails.setBrickDetails(getBrickDetails()); + } + + private GlusterVolumeEntity getVolume() { + GlusterVolumeEntity volume = new GlusterVolumeEntity(); + volume.setId(VOLUME_ID); + return volume; + } + + private GlusterBrickEntity getBrick() { + GlusterBrickEntity brick = new GlusterBrickEntity(); + brick.setId(BRICK_ID); + brick.setServerId(SERVER_ID); + return brick; } private List<BrickDetails> getBrickDetails() { @@ -131,15 +147,27 @@ private void setupMock() { clusterUtils = mock(ClusterUtils.class); vdsDao = mock(VdsDAO.class); + volumeDao = mock(GlusterVolumeDao.class); + brickDao = mock(GlusterBrickDao.class); + doReturn(clusterUtils).when(getQuery()).getClusterUtils(); doReturn(getVds(VDSStatus.Up)).when(clusterUtils).getUpServer(CLUSTER_ID); + doReturn(vdsDao).when(clusterUtils).getVdsDao(); + doReturn(vdsDao).when(getQuery()).getVdsDao(); when(vdsDao.getAllForVdsGroupWithStatus(CLUSTER_ID, VDSStatus.Up)).thenReturn(Collections.singletonList(getVds(VDSStatus.Up))); + when(vdsDao.get(SERVER_ID)).thenReturn(getVds(VDSStatus.Up)); + + doReturn(volumeDao).when(getQuery()).getGlusterVolumeDao(); + when(volumeDao.getById(VOLUME_ID)).thenReturn(getVolume()); + + doReturn(brickDao).when(getQuery()).getGlusterBrickDao(); + when(brickDao.getById(BRICK_ID)).thenReturn(getBrick()); // Mock the query's parameters doReturn(CLUSTER_ID).when(getQueryParameters()).getClusterId(); - doReturn(VOLUME_NAME).when(getQueryParameters()).getVolumeName(); - doReturn(BRICK_NAME).when(getQueryParameters()).getBrickName(); + doReturn(VOLUME_ID).when(getQueryParameters()).getVolumeId(); + doReturn(BRICK_ID).when(getQueryParameters()).getBrickId(); doReturn(true).when(getQueryParameters()).isDetailRequired(); VDSReturnValue returnValue = new VDSReturnValue(); diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/gluster/GlusterVolumeAdvancedDetailsParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/gluster/GlusterVolumeAdvancedDetailsParameters.java index 873e625..e014546 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/gluster/GlusterVolumeAdvancedDetailsParameters.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/gluster/GlusterVolumeAdvancedDetailsParameters.java @@ -9,34 +9,34 @@ public class GlusterVolumeAdvancedDetailsParameters extends GlusterParameters { private static final long serialVersionUID = -1224829720081853632L; - private String volumeName; - private String brickName; + private Guid volumeId; + private Guid brickId; private boolean detailRequired; public GlusterVolumeAdvancedDetailsParameters(Guid clusterId, - String volumeName, - String brickName, + Guid volumeId, + Guid brickId, boolean detailRequired) { super(clusterId); - setVolumeName(volumeName); - setBrickName(brickName); + setVolumeId(volumeId); + setBrickId(brickId); setDetailRequired(detailRequired); } - public String getVolumeName() { - return volumeName; + public Guid getVolumeId() { + return volumeId; } - public void setVolumeName(String volumeName) { - this.volumeName = volumeName; + public void setVolumeId(Guid volumeId) { + this.volumeId = volumeId; } - public String getBrickName() { - return brickName; + public Guid getBrickId() { + return brickId; } - public void setBrickName(String brickName) { - this.brickName = brickName; + public void setBrickId(Guid brickId) { + this.brickId = brickId; } public boolean isDetailRequired() { diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java index 0ad85c2..d469c44 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java @@ -1109,15 +1109,15 @@ }; // Passing empty values for Volume and Brick to get the services of all the volumes/hosts in the cluster GlusterVolumeAdvancedDetailsParameters parameters = - new GlusterVolumeAdvancedDetailsParameters(clusterId, "", "", false); //$NON-NLS-1$ //$NON-NLS-2$ + new GlusterVolumeAdvancedDetailsParameters(clusterId, null, null, false); //$NON-NLS-1$ //$NON-NLS-2$ Frontend.RunQuery(VdcQueryType.GetGlusterVolumeAdvancedDetails, parameters, aQuery); } - public static void GetGlusterVolumeBrickDetails(AsyncQuery aQuery, Guid clusterId, String volume, String brick) { + public static void GetGlusterVolumeBrickDetails(AsyncQuery aQuery, Guid clusterId, Guid volumeId, Guid brickId) { GlusterVolumeAdvancedDetailsParameters parameters = - new GlusterVolumeAdvancedDetailsParameters(clusterId, volume, brick, true); + new GlusterVolumeAdvancedDetailsParameters(clusterId, volumeId, brickId, true); Frontend.RunQuery(VdcQueryType.GetGlusterVolumeAdvancedDetails, parameters, aQuery); diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeBrickListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeBrickListModel.java index fe4781b..73278ab 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeBrickListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeBrickListModel.java @@ -851,7 +851,7 @@ brickModel.getMemoryPools().setItems(memoryPools); } } - },true), volumeEntity.getClusterId(), volumeEntity.getName(), brickEntity.getQualifiedName()); + },true), volumeEntity.getClusterId(), volumeEntity.getId(), brickEntity.getId()); UICommand command = new UICommand("Cancel", this); //$NON-NLS-1$ command.setTitle(ConstantsManager.getInstance().getConstants().close()); -- To view, visit http://gerrit.ovirt.org/12979 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id4acb0bbe84bacab714926d69b639bbda970df9a Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shireesh Anjal <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
