Sahina Bose has posted comments on this change. Change subject: gluster: bll command for start/stop of services ......................................................................
Patch Set 3: (6 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/ManageGlusterServiceCommand.java Line 87: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_INVALID_ACTION_TYPE); Line 88: } Line 89: Line 90: if (Guid.isNullOrEmpty(clusterId) && Guid.isNullOrEmpty(serverId)) { Line 91: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CLUSTERID_AND_SERVERID_BOTH_NULL); Also check if there are UP servers in cluster to perform action Line 92: } Line 93: Line 94: return true; Line 95: } Line 108: List<GlusterServerService> serviceList = null; Line 109: Line 110: List<Callable<Pair<VDS, VDSReturnValue>>> taskList = new ArrayList<Callable<Pair<VDS, VDSReturnValue>>>(); Line 111: for (final VDS upServer : servers) { Line 112: serviceList = getGlusterServerServiceDao().getByServerId(upServer.getId()); I think the serviceList should not come from each server, but from gluster_services table based on service type. Because the design is generic and may have different services for a server, we do not want to start/stop all of these services. The serviceList building can be moved outside of this method as it will be used both for cluster/server - use GlusterServiceDao Line 113: final List<String> serviceListStr = new ArrayList<String>(); Line 114: for (GlusterServerService srvc : serviceList) { Line 115: serviceListStr.add(srvc.getServiceName()); Line 116: } Line 247: */ Line 248: class ManageActionDetail { Line 249: private VdcBllMessages canDoActionMsg; Line 250: private GlusterServiceStatus status; Line 251: private VdcBllMessages statusMsg; statusMsg not used, can be removed? Line 252: private VDSCommandType vdsCmdType; Line 253: private AuditLogType actionPerformedActionLog; Line 254: private AuditLogType actionFailedActionLog; Line 255: Line 248: class ManageActionDetail { Line 249: private VdcBllMessages canDoActionMsg; Line 250: private GlusterServiceStatus status; Line 251: private VdcBllMessages statusMsg; Line 252: private VDSCommandType vdsCmdType; The VDS command type is always the same for this BLL command, you could remove from here. Line 253: private AuditLogType actionPerformedActionLog; Line 254: private AuditLogType actionFailedActionLog; Line 255: Line 256: public ManageActionDetail(VdcBllMessages canDoActionMsg, .................................................... File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/ManageGlusterServiceCommandTest.java Line 131: Line 132: params.setServiceType(ServiceType.SMB); Line 133: cmd = spy(new ManageGlusterServiceCommand(params)); Line 134: assertFalse(cmd.canDoAction()); Line 135: You can move these to separate tests. You can also verify the error message returned from canDoAction Line 136: params.setClusterId(null); Line 137: params.setServerId(null); Line 138: assertFalse(cmd.canDoAction()); Line 139: } Line 157: cmd = spy(new ManageGlusterServiceCommand(new GlusterServiceParameters(Guid.NewGuid(), Guid.NewGuid(), ServiceType.NFS, "start"))); Line 158: setUpMockUpForStart(); Line 159: cmd.executeCommand(); Line 160: assertEquals(cmd.getAuditLogTypeValue(), AuditLogType.GLUSTER_SERVICE_START_FAILED); Line 161: Again, split to different tests Line 162: cmd = spy(new ManageGlusterServiceCommand(new GlusterServiceParameters(Guid.NewGuid(), Guid.NewGuid(), ServiceType.NFS, "stop"))); Line 163: setUpMockUpForStop(); Line 164: cmd.executeCommand(); Line 165: assertEquals(cmd.getAuditLogTypeValue(), AuditLogType.GLUSTER_SERVICE_STOP_FAILED); -- To view, visit http://gerrit.ovirt.org/14831 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcab38866c49c6f5d43e3b33006c428ec9304501 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shubhendu Tripathi <shtri...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: Shubhendu Tripathi <shtri...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches