Moti Asayag has posted comments on this change. Change subject: core: set cluster for host registration if null ......................................................................
Patch Set 4: (11 comments) https://gerrit.ovirt.org/#/c/37842/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RegisterVdsQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RegisterVdsQuery.java: Line 182: Line 183: logable.addCustomValue("VdsName1", getParameters().getVdsName()); Line 184: Line 185: Guid vdsGroupId = getClusterId(); Line 186: if (vdsGroupId == null || Guid.Empty.equals(vdsGroupId)) { please replace with: Guid.isNullOrEmpty() Line 187: reportClusterError(); Line 188: getQueryReturnValue().setSucceeded(false); Line 189: return; Line 190: } Line 184: Line 185: Guid vdsGroupId = getClusterId(); Line 186: if (vdsGroupId == null || Guid.Empty.equals(vdsGroupId)) { Line 187: reportClusterError(); Line 188: getQueryReturnValue().setSucceeded(false); false should be the default value of the return value - no need to set it again. Line 189: return; Line 190: } Line 191: if (provisionedVds != null) { Line 192: // In provision don't set host on pending - isPending = false Line 205: } Line 206: } Line 207: Line 208: private void reportClusterError() { Line 209: log.error("AddVdsCommand: No default or valid cluster was found, host registration failed."); no need for 'AddVdsCommand' in the error message: 1. this is not the right class command name 2. the class name is already printed as part of slf4j configuration. Line 210: AuditLogableBase logableBase = new AuditLogableBase(); Line 211: AuditLogDirector.log(logableBase, AuditLogType.HOST_FAILED_REGISTRATION_INVALID_CLUSTER); Line 212: } Line 213: Line 211: AuditLogDirector.log(logableBase, AuditLogType.HOST_FAILED_REGISTRATION_INVALID_CLUSTER); Line 212: } Line 213: Line 214: private Guid getClusterId() { Line 215: Guid vdsGroupId = getParameters().getVdsGroupId(); s/vdsGroupId/clusterId Line 216: if (Guid.Empty.equals(getParameters().getVdsGroupId())) { Line 217: vdsGroupId = Guid.createGuidFromStringDefaultEmpty( Line 218: Config.<String> getValue(ConfigValues.AutoRegistrationDefaultVdsGroupID)); Line 219: log.debug( Line 216: if (Guid.Empty.equals(getParameters().getVdsGroupId())) { Line 217: vdsGroupId = Guid.createGuidFromStringDefaultEmpty( Line 218: Config.<String> getValue(ConfigValues.AutoRegistrationDefaultVdsGroupID)); Line 219: log.debug( Line 220: "RegisterVdsQuery::ExecuteCommand - VdsGroupId received as -1, using AutoRegistrationDefaultVdsGroupID: '{}'", please remove "RegisterVdsQuery::ExecuteCommand - " from the message. Use: "Cluster id was not provided for registering the host {}, the cluster id {} is taken from config value {}" where the first {} is host name the second is the value for the cluster id the third {} stands for the config value name: ConfigValues.AutoRegistrationDefaultVdsGroupID.name() so we shouldn't care of any future refactor of this config value. Line 221: vdsGroupId); Line 222: } Line 223: if (vdsGroupId == null || Guid.Empty.equals(vdsGroupId)) { Line 224: // try to get the default cluster id Line 218: Config.<String> getValue(ConfigValues.AutoRegistrationDefaultVdsGroupID)); Line 219: log.debug( Line 220: "RegisterVdsQuery::ExecuteCommand - VdsGroupId received as -1, using AutoRegistrationDefaultVdsGroupID: '{}'", Line 221: vdsGroupId); Line 222: } please add a space line to separate the blocks Line 223: if (vdsGroupId == null || Guid.Empty.equals(vdsGroupId)) { Line 224: // try to get the default cluster id Line 225: VdsGroupDAO dao = DbFacade.getInstance().getVdsGroupDao(); Line 226: VDSGroup cluster = dao.getByName("Default"); Line 219: log.debug( Line 220: "RegisterVdsQuery::ExecuteCommand - VdsGroupId received as -1, using AutoRegistrationDefaultVdsGroupID: '{}'", Line 221: vdsGroupId); Line 222: } Line 223: if (vdsGroupId == null || Guid.Empty.equals(vdsGroupId)) { please replace with: Guid.isNullOrEmpty() Line 224: // try to get the default cluster id Line 225: VdsGroupDAO dao = DbFacade.getInstance().getVdsGroupDao(); Line 226: VDSGroup cluster = dao.getByName("Default"); Line 227: if (cluster != null) { Line 221: vdsGroupId); Line 222: } Line 223: if (vdsGroupId == null || Guid.Empty.equals(vdsGroupId)) { Line 224: // try to get the default cluster id Line 225: VdsGroupDAO dao = DbFacade.getInstance().getVdsGroupDao(); s/DbFacade.getInstance()/getDbFacade() s/dao/clusterDao Line 226: VDSGroup cluster = dao.getByName("Default"); Line 227: if (cluster != null) { Line 228: vdsGroupId = cluster.getId(); Line 229: } else { Line 228: vdsGroupId = cluster.getId(); Line 229: } else { Line 230: // this may occur when the default cluster is removed Line 231: List<VDSGroup> clusters = dao.getAll(); Line 232: if (clusters.size() > 0) { use if (!clusters.isEmpty()) Line 233: vdsGroupId = clusters.get(0).getId(); Line 234: } Line 235: } Line 236: } https://gerrit.ovirt.org/#/c/37842/4/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java: Line 130: HOST_BOND_SLAVE_STATE_UP(611, AuditLogSeverity.NORMAL), Line 131: HOST_BOND_SLAVE_STATE_DOWN(612, AuditLogSeverity.WARNING), Line 132: Line 133: // Host Registration Line 134: HOST_FAILED_REGISTRATION_INVALID_CLUSTER(618), maybe HOST_REGISTRATION_FAILED_INVALID_CLUSTER ? Line 135: Line 136: // Disk alignment audit logs Line 137: DISK_ALIGNMENT_SCAN_START(700), Line 138: DISK_ALIGNMENT_SCAN_FAILURE(701, AuditLogSeverity.WARNING), https://gerrit.ovirt.org/#/c/37842/4/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties File backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties: Line 236: HOST_INTERFACE_STATE_UP=Interface ${InterfaceName} on host ${VdsName}, changed state to up Line 237: HOST_INTERFACE_STATE_DOWN=Interface ${InterfaceName} on host ${VdsName}, changed state to down Line 238: HOST_BOND_SLAVE_STATE_UP=Slave ${SlaveName} of bond ${BondName} on host ${VdsName}, changed state to up Line 239: HOST_BOND_SLAVE_STATE_DOWN=Slave ${SlaveName} of bond ${BondName} on host ${VdsName}, changed state to down Line 240: HOST_FAILED_REGISTRATION_INVALID_CLUSTER=No default or valid cluster was found, Host registration failed this message can contain more information to note the exact host which its registration failed due to this error. Please include host name or any other identifier of the host Line 241: VDS_DETECTED=Status of host ${VdsName} was set to ${HostStatus}. Line 242: VDS_FAILURE=Host ${VdsName} is non responsive. Line 243: VDS_MAINTENANCE=Host ${VdsName} was switched to Maintenance Mode. Line 244: VDS_MAINTENANCE_MANUAL_HA=Host ${VdsName} was switched to Maintenance mode, but Hosted Engine HA maintenance could not be enabled. Please enable it manually. -- To view, visit https://gerrit.ovirt.org/37842 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5eda0fe04de8338a49000d62338717584518b153 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
