Martin Peřina has posted comments on this change. Change subject: restapi: Host Power-Management Refactor (#977674) - WIP ......................................................................
Patch Set 23: (55 comments) http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddFenceAgentCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddFenceAgentCommand.java: Line 26: } Line 27: Line 28: @Override Line 29: protected boolean canDoAction() { Line 30: if (getParameters() == null || getParameters().getAgent() == null || getParameters().getAgent().getIp() == null Could you please reformat to one condition per line to make it more readable Line 31: || getParameters().getAgent().getOrder() == null || getParameters().getAgent().getHostId() == null Line 32: || getParameters().getAgent().getPassword() == null || getParameters().getAgent().getType() == null Line 33: || getParameters().getAgent().getUser() == null) { Line 34: return failCanDoAction(VdcBllMessages.VDS_ADD_FENCE_AGENT_MANDATORY_PARAMETERS_MISSING); http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java: Line 358: && StringUtils.isEmpty(getParameters().getPassword())) { Line 359: // We block vds installations if it's not a RHEV-H and password is empty Line 360: // Note that this may override local host SSH policy. See BZ#688718. Line 361: returnValue = failCanDoAction(VdcBllMessages.VDS_CANNOT_INSTALL_EMPTY_PASSWORD); Line 362: } else if (!isPowerManagementLegal(getParameters().getVdsStaticData().isPmEnabled(), Please reformat: } else if (!isPowerManagementLegal(getParameters().getVdsStaticData().isPmEnabled(), getParameters().getFenceAgents(), getVdsGroup().getcompatibility_version().toString())) { returnValue = false; Line 363: getParameters().getFenceAgents(), Line 364: getVdsGroup() Line 365: .getcompatibility_version().toString())) { Line 366: returnValue = false; http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceVdsBaseCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceVdsBaseCommand.java: Line 48: Line 49: public abstract class FenceVdsBaseCommand<T extends FenceVdsActionParameters> extends VdsCommand<T> { Line 50: private static final int SLEEP_BEFORE_FIRST_ATTEMPT = 5000; Line 51: private static final String INTERNAL_FENCE_USER = "Engine"; Line 52: protected boolean skippedDueToFencingPolicy = false; Please initialize non static attributes inside constructors only Line 53: Line 54: /** Line 55: * Constructor for command creation when compensation is applied on startup Line 56: * Line 95: event = AuditLogType.USER_VDS_START.name(); Line 96: } Line 97: if (getVds().getpm_enabled() && isPowerManagementLegal()) { Line 98: // && isPowerManagementLegal(getVds().getStaticData(), getVdsGroup().getcompatibility_version().toString())) Line 99: // { If the code is not needed, please remove it Line 100: // check if we are in the interval of X seconds from startup Line 101: // if yes , system is still initializing , ignore fence operations Line 102: Date waitTo = Line 103: Backend.getInstance() Line 106: Date now = new Date(); Line 107: if (waitTo.before(now) || waitTo.equals(now)) { Line 108: // Check Quiet time between PM operations, this is done only if command is not internal and parent Line 109: // command is not <Restart> Line 110: int secondsLeftToNextPmOp = Please reformat: int secondsLeftToNextPmOp = (isInternalExecution() || (getParameters().getParentCommand() == VdcActionType.RestartVds)) ? 0 : DbFacade.getInstance().getAuditLogDao().getTimeToWaitForNextPmOp( getVds().getName(), event); Line 111: (isInternalExecution() || (getParameters().getParentCommand() == VdcActionType.RestartVds)) Line 112: ? Line 113: 0 Line 114: : Line 181: AlertIfPowerManagementOperationFailed(); Line 182: } Line 183: } Line 184: // Successful fencing with reboot or shutdown op. Clear the power management policy flag Line 185: else if ((getParameters().getAction() == FenceActionType.Restart Please reformat: else if ((getParameters().getAction() == FenceActionType.Restart || getParameters().getAction() == FenceActionType.Stop) && getParameters().getKeepPolicyPMEnabled() == false) { Line 186: || getParameters().getAction() == FenceActionType.Stop) Line 187: && getParameters().getKeepPolicyPMEnabled() == false) { Line 188: getVds().setPowerManagementControlledByPolicy(false); Line 189: getDbFacade().getVdsDynamicDao().updateVdsDynamicPowerManagementPolicyFlag( http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNewVdsFenceStatusQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNewVdsFenceStatusQuery.java: Line 29: Line 30: private VDS getHost() { Line 31: Guid id = getParameters().getVdsId(); Line 32: VDS vds = new VDS(); Line 33: vds.setId((Guid) ((id != null) ? id : Guid.Empty)); Please simplify: vds.setId(id != null ? id : Guid.Empty); Line 34: vds.setStoragePoolId(getParameters().getStoragePoolId()); Line 35: vds.getFenceAgents().add(getParameters().getAgent()); Line 36: vds.setPmProxyPreferences(getParameters().getPmProxyPreferences()); Line 37: return vds; http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java: Line 166: } Line 167: return true; Line 168: } Line 169: Line 170: private void processFence() { I would change condition to: if (getVds().getpm_enabled()) { FenceExecutor executor = new FenceExecutor(getVds()); vdsProxyFound = new ProxyHostLocator(getVds()).isProxyHostAvailable(); if (vdsProxyFound) { VDSFenceReturnValue returnValue = executor.checkStatus(); fenceSucceeded = returnValue.isSucceeded(); //potential bug here - if no proxy host found, this variable is still 'true' fenceStatusReturnValue = (FenceStatusReturnValue) returnValue.getFenceResult().getReturnValue(); } } Using above we won't be executing findProxyHost for no reason Line 171: FenceExecutor executor = new FenceExecutor(getVds()); Line 172: vdsProxyFound = new ProxyHostLocator(getVds()).isProxyHostAvailable(); Line 173: if (getVds().getpm_enabled() && vdsProxyFound) { Line 174: VDSFenceReturnValue returnValue = executor.checkStatus(); http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProxyHostLocator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProxyHostLocator.java: Line 27: public class ProxyHostLocator { Line 28: Line 29: private static final Logger log = LoggerFactory.getLogger(ProxyHostLocator.class); Line 30: Line 31: private final VDS _vds; Please don't use underscores Line 32: private FencingPolicy fencingPolicy; Line 33: Line 34: public ProxyHostLocator(VDS _vds) { Line 35: super(); Line 31: private final VDS _vds; Line 32: private FencingPolicy fencingPolicy; Line 33: Line 34: public ProxyHostLocator(VDS _vds) { Line 35: super(); Please remove, it's redundant as parent class is Object Line 36: this._vds = _vds; Line 37: } Line 38: Line 39: public ProxyHostLocator(VDS vds, FencingPolicy fencingPolicy) { Line 32: private FencingPolicy fencingPolicy; Line 33: Line 34: public ProxyHostLocator(VDS _vds) { Line 35: super(); Line 36: this._vds = _vds; I would check vds for null and throw IllegalArgumentException as needed Line 37: } Line 38: Line 39: public ProxyHostLocator(VDS vds, FencingPolicy fencingPolicy) { Line 40: this(vds); Line 54: } Line 55: Line 56: public VDS findProxyHost(boolean withRetries, Guid excludedHostId) { Line 57: PMProxyOptions proxyOption = null; Line 58: final Guid NO_VDS = Guid.Empty; Please use Guid.Empty directly Line 59: int count; Line 60: // make sure that loop is executed at least once , no matter what is the Line 61: // value in config Line 62: int retries = Math.max(Config.<Integer> getValue(ConfigValues.FindFenceProxyRetries), 1); Line 62: int retries = Math.max(Config.<Integer> getValue(ConfigValues.FindFenceProxyRetries), 1); Line 63: int delayInMs = 1000 * Config.<Integer> getValue(ConfigValues.FindFenceProxyDelayBetweenRetriesInSec); Line 64: VDS proxyHost = null; Line 65: // get PM Proxy preferences or use defaults if not defined Line 66: String pmProxyPreferences = (StringUtils.isEmpty(_vds.getPmProxyPreferences())) Please reformat: String pmProxyPreferences = StringUtils.isEmpty(_vds.getPmProxyPreferences()) ? Config.<String> getValue(ConfigValues.FenceProxyDefaultPreferences) : _vds.getPmProxyPreferences(); Line 67: ? Line 68: Config.<String> getValue(ConfigValues.FenceProxyDefaultPreferences) Line 69: : _vds.getPmProxyPreferences(); Line 70: String[] pmProxyOptions = pmProxyPreferences.split(","); Line 76: continue; Line 77: } Line 78: // check if this is a new host, no need to retry , only status is Line 79: // available on new host. Line 80: if (_vds.getId().equals(NO_VDS)) { I think it's safer to use: if (Guid.Empty.equals(vds.getId()) { Line 81: // try first to find a Host in UP status Line 82: proxyHost = getFenceProxy(true, false, proxyOption, excludedHostId); Line 83: // trying other Hosts that are not in UP since they can be a proxy for fence operations Line 84: if (proxyHost == null) { Line 132: } Line 133: Line 134: private boolean isHostNetworkUnreacable(VDS vds) { Line 135: VdsDynamic vdsDynamic = vds.getDynamicData(); Line 136: return (vdsDynamic.getStatus() == VDSStatus.Down Please reformat: return vdsDynamic.getStatus() == VDSStatus.Down || vdsDynamic.getStatus() == VDSStatus.Reboot || vdsDynamic.getStatus() == VDSStatus.Kdumping || vdsDynamic.getStatus() == VDSStatus.NonResponsive || (vdsDynamic.getStatus() == VDSStatus.NonOperational && vdsDynamic.getNonOperationalReason() == NonOperationalReason.NETWORK_UNREACHABLE); Line 137: || vdsDynamic.getStatus() == VDSStatus.Reboot Line 138: || vdsDynamic.getStatus() == VDSStatus.Kdumping Line 139: || vdsDynamic.getStatus() == VDSStatus.NonResponsive Line 140: || (vdsDynamic.getStatus() == VDSStatus.NonOperational Line 144: private VDS getFenceProxy(final boolean onlyUpHost, Line 145: final boolean filterSelf, Line 146: final PMProxyOptions proxyOptions, Line 147: Guid excludedProxyHostId) { Line 148: Please remove empty line Line 149: List<VDS> hosts = DbFacade.getInstance().getVdsDao().getAll(); Line 150: if (excludedProxyHostId != null) { Line 151: removeExcludedHost(excludedProxyHostId, hosts); Line 152: } Line 169: private VDS getFenceProxy(List<VDS> hosts, Line 170: final boolean onlyUpHost, Line 171: final boolean filterSelf, Line 172: final PMProxyOptions proxyOptions) { Line 173: Please remove empty line Line 174: VDS proxyHost = LinqUtils.firstOrNull(hosts, new Predicate<VDS>() { Line 175: @Override Line 176: public boolean eval(VDS vds) { Line 177: if (!areAgentsSupported(vds)) { Line 176: public boolean eval(VDS vds) { Line 177: if (!areAgentsSupported(vds)) { Line 178: return false; Line 179: } Line 180: if (proxyOptions == PMProxyOptions.CLUSTER) { When you are touching this part of code, it would be nice to reformat using Java coding conventions: if (proxyOptions == PMProxyOptions.CLUSTER) { if (!vds.getVdsGroupId().equals(_vds.getVdsGroupId())) { return false; } if (onlyUpHost) { if (filterSelf) { return !vds.getId().equals(_vds.getId()) && vds.getStatus() == VDSStatus.Up; } else { return vds.getStatus() == VDSStatus.Up; } } else { if (filterSelf) { return !isHostNetworkUnreacable(vds) && !vds.getId().equals(_vds.getId()); } else { return !isHostNetworkUnreacable(vds); } } } else if (proxyOptions == PMProxyOptions.DC) { if (!vds.getStoragePoolId().equals(_vds.getStoragePoolId())) { return false; } if (onlyUpHost) { if (filterSelf) { return !vds.getId().equals(_vds.getId()) && vds.getStatus() == VDSStatus.Up; } else { return vds.getStatus() == VDSStatus.Up; } } else { if (filterSelf) { return !isHostNetworkUnreacable(vds) && !vds.getId().equals(_vds.getId()); } else { return !isHostNetworkUnreacable(vds); } } } else if (proxyOptions == PMProxyOptions.OTHER_DC) { if (vds.getStoragePoolId().equals(_vds.getStoragePoolId())) { return false; } if (onlyUpHost) { if (filterSelf) { return !vds.getId().equals(_vds.getId()) && vds.getStatus() == VDSStatus.Up; } else { return vds.getStatus() == VDSStatus.Up; } } else { if (filterSelf) { return !isHostNetworkUnreacable(vds) && !vds.getId().equals(_vds.getId()); } else { return !isHostNetworkUnreacable(vds); } } } Line 181: if (!vds.getVdsGroupId().equals(_vds.getVdsGroupId())) { Line 182: return false; Line 183: } Line 184: if (onlyUpHost) { Line 261: if (fencingPolicy != null) { Line 262: supported = Line 263: supported Line 264: && vds.getSupportedClusterVersionsSet() Line 265: .contains(FencingPolicyHelper.getMinimalSupportedVersion(fencingPolicy)); Please cache minimal supported version for fencing policy same way as it was done in FenceExecutor Line 266: } Line 267: return supported; Line 268: } Line 269: }); http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveFenceAgentCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveFenceAgentCommand.java: Line 9: public class RemoveFenceAgentCommand<T extends FenceAgentCommandParameterBase> extends FenceAgentCommandBase { Line 10: Line 11: @Override Line 12: protected boolean canDoAction() { Line 13: if (getParameters() == null || getParameters().getAgent() == null || getParameters().getAgent().getId() == null) { Please reformat to make it more readable, one condition per line Line 14: return failCanDoAction(VdcBllMessages.VDS_REMOVE_FENCE_AGENT_ID_REQUIRED); Line 15: } Line 16: return super.canDoAction(); Line 17: } http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestartVdsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestartVdsCommand.java: Line 85: skippedDueToFencingPolicy = true; Line 86: runVdsCommand(VDSCommandType.SetVdsStatus, new SetVdsStatusVDSCommandParameters(vdsId, Line 87: VDSStatus.NonResponsive)); Line 88: return; Line 89: } else if (returnValue.getSucceeded()) { Why did you add else here? IMHO it just makes code harder to read Line 90: executeFenceVdsManuallyAction(vdsId, sessionId); Line 91: Line 92: // execute StartVds action Line 93: returnValue = executeVdsFenceAction(vdsId, sessionId, FenceActionType.Start, VdcActionType.StartVds); http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/pm/PmHealthCheckManager.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/pm/PmHealthCheckManager.java: Line 33: import org.slf4j.LoggerFactory; Line 34: Line 35: import java.util.HashMap; Line 36: import java.util.Map; Line 37: import java.util.Map.Entry; Please correct import order Line 38: Line 39: /** Line 40: * Responsible for checking PM enabled hosts by sending a status command to each host configured PM agent cards and Line 41: * raise alerts for failed operations. Line 42: */ Line 43: public class PmHealthCheckManager { Line 44: Line 45: private static final Logger log = LoggerFactory.getLogger(PmHealthCheckManager.class); Line 46: // private static final String AGENT_ID = "AgentId"; What is this comment for? Line 47: private static final PmHealthCheckManager instance = new PmHealthCheckManager(); Line 48: private boolean active = false; Line 49: Line 50: private PmHealthCheckManager() { Line 158: addAlert(hostId, AuditLogType.VDS_ALERT_PM_HEALTH_CHECK_STOP_MIGHT_FAIL); Line 159: } Line 160: } Line 161: Line 162: private void removeAlert(Guid hostId, AuditLogType auditMessage) { Please remove this method and use AlertDirector inline Line 163: AlertDirector.RemoveVdsAlert(hostId, auditMessage); Line 164: } Line 165: Line 166: private void addAlert(Guid hostId, AuditLogType auditMessage) { Line 162: private void removeAlert(Guid hostId, AuditLogType auditMessage) { Line 163: AlertDirector.RemoveVdsAlert(hostId, auditMessage); Line 164: } Line 165: Line 166: private void addAlert(Guid hostId, AuditLogType auditMessage) { Please remove this method and use AlertDirector inline Line 167: AlertDirector.AddVdsAlert(hostId, auditMessage); Line 168: } Line 169: Line 170: /** http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/pm/PowerManagementHelper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/pm/PowerManagementHelper.java: Line 6: import java.util.List; Line 7: Line 8: import org.ovirt.engine.core.common.businessentities.FenceAgent; Line 9: Line 10: public class PowerManagementHelper { Just a thought: wouldn't it be better to return existing fence agents from db as Map<Integer,List<FenceAgent>> (order would be the key) and work with this map instead of returning just List<FenceAgent> and then use this complicated approach? Line 11: Line 12: /** Line 13: * Get an iterator over the fencing-agents of this host. Agents are returned sorted by their 'order' attribute - Line 14: * lowest order first, highest order last. http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/FenceAgent.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/FenceAgent.java: Line 11: import org.ovirt.engine.core.common.validation.annotation.HostnameOrIp; Line 12: import org.ovirt.engine.core.common.validation.group.PowerManagementCheck; Line 13: import org.ovirt.engine.core.compat.Guid; Line 14: Line 15: public class FenceAgent implements BusinessEntity<Guid> { Please define proper equals() and hashcode() methods Line 16: Line 17: private static final long serialVersionUID = -5910560758520427911L; Line 18: private Guid id; Line 19: private Guid hostId; Line 16: Line 17: private static final long serialVersionUID = -5910560758520427911L; Line 18: private Guid id; Line 19: private Guid hostId; Line 20: private Integer order; Please use primitive int Line 21: Line 22: @EditableField Line 23: private HashMap<String, String> optionsMap; Line 24: Line 42: @EditableField Line 43: @Range(min = BusinessEntitiesDefinitions.NETWORK_MIN_LEGAL_PORT, Line 44: max = BusinessEntitiesDefinitions.NETWORK_MAX_LEGAL_PORT, Line 45: message = "VALIDATION.VDS.PORT.RANGE") Line 46: private Integer port; Please use primitive int Line 47: Line 48: @EditableField Line 49: @Size(max = BusinessEntitiesDefinitions.GENERAL_MAX_SIZE) Line 50: private String options; Line 136: * Line 137: * @param map Line 138: * @return Line 139: */ Line 140: public static String optionsMapToString(HashMap<String, String> map) { Please move this method to some helper class Line 141: String result = ""; Line 142: String seperator = ""; Line 143: Iterator<Map.Entry<String, String>> it = map.entrySet().iterator(); Line 144: while (it.hasNext()) { Line 161: * @param pmOptions Line 162: * String representation of the map Line 163: * @return A parsed map Line 164: */ Line 165: public static HashMap<String, String> pmOptionsStringToMap(String pmOptions) { Please move this method to some helper class Line 166: HashMap<String, String> map = new HashMap<String, String>(); Line 167: if (pmOptions == null || pmOptions.equals("")) { Line 168: return map; Line 169: } Line 181: } Line 182: return map; Line 183: } Line 184: Line 185: public static class FenceAgentOrderComparator implements Comparator<FenceAgent> { Please move this class into its own file Line 186: Line 187: @Override Line 188: public int compare(FenceAgent agent1, FenceAgent agent2) { Line 189: return agent1.getOrder().compareTo(agent2.getOrder()); http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetNewVdsFenceStatusParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetNewVdsFenceStatusParameters.java: Line 5: Line 6: public class GetNewVdsFenceStatusParameters extends VdcQueryParametersBase { Line 7: private static final long serialVersionUID = -3663389765505476776L; Line 8: Line 9: private Guid _vds_id; Please don't use underscores: private Guid vdsId; Line 10: private FenceAgent agent; Line 11: private String pmProxyPreferences; Line 12: Line 13: public GetNewVdsFenceStatusParameters() { Line 46: Line 47: public void setPmProxyPreferences(String pmProxyPreferences) { Line 48: this.pmProxyPreferences = pmProxyPreferences; Line 49: } Line 50: Please remove the line http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 927: VDS_REGISTER_UNIQUE_ID_AMBIGUOUS=Cannot add host which has the same unique ID as one of the hosts: ${HostNameList}.\nThis might occur in case this host was already added to the system. Line 928: VDS_CANNOT_CONNECT_TO_SERVER=Cannot ${action} ${type}. Connecting to host via SSH has failed, verify that the host is reachable (IP address, routable address etc.) You may refer to the engine.log file for further details. Line 929: VDS_CANNOT_AUTHENTICATE_TO_SERVER=Cannot ${action} ${type}. SSH authentication failed, verify authentication parameters are correct (Username/Password, public-key etc.) You may refer to the engine.log file for further details. Line 930: VDS_SECURITY_CONNECTION_ERROR=Cannot ${action} ${type}. SSH connection failed, ${ErrorMsg}. Line 931: VDS_REMOVE_FENCE_AGENT_ID_REQUIRED=Cannot Remove Fence-Agent. Agent ID not provided. Please rephrase: Cannot remove fence agent, agent ID was not provided. Line 932: VDS_REMOVE_FENCE_AGENTS_VDS_ID_REQUIRED=Cannot Remove Fence-Agents. Host ID not provided. Line 933: VDS_ADD_FENCE_AGENT_MANDATORY_PARAMETERS_MISSING=Cannot Add Fence-Agent. One or more mandatory parameters were not provided. Mandatory parameters are: type, order, ip, username, address, password. Line 934: AUTO_MIGRATE_DISABLED=Cannot migrate - check relevant configuration options. Line 935: AUTO_MIGRATE_VDS_NOT_FOUND=Cannot migrate - Host not found. Line 928: VDS_CANNOT_CONNECT_TO_SERVER=Cannot ${action} ${type}. Connecting to host via SSH has failed, verify that the host is reachable (IP address, routable address etc.) You may refer to the engine.log file for further details. Line 929: VDS_CANNOT_AUTHENTICATE_TO_SERVER=Cannot ${action} ${type}. SSH authentication failed, verify authentication parameters are correct (Username/Password, public-key etc.) You may refer to the engine.log file for further details. Line 930: VDS_SECURITY_CONNECTION_ERROR=Cannot ${action} ${type}. SSH connection failed, ${ErrorMsg}. Line 931: VDS_REMOVE_FENCE_AGENT_ID_REQUIRED=Cannot Remove Fence-Agent. Agent ID not provided. Line 932: VDS_REMOVE_FENCE_AGENTS_VDS_ID_REQUIRED=Cannot Remove Fence-Agents. Host ID not provided. Please rephrase: Cannot remove fence agent, host ID was not provided. Line 933: VDS_ADD_FENCE_AGENT_MANDATORY_PARAMETERS_MISSING=Cannot Add Fence-Agent. One or more mandatory parameters were not provided. Mandatory parameters are: type, order, ip, username, address, password. Line 934: AUTO_MIGRATE_DISABLED=Cannot migrate - check relevant configuration options. Line 935: AUTO_MIGRATE_VDS_NOT_FOUND=Cannot migrate - Host not found. Line 936: AUTO_MIGRATE_ALREADY_RUNNING_ON_VDS=Cannot migrate - VM already running on Host. Line 929: VDS_CANNOT_AUTHENTICATE_TO_SERVER=Cannot ${action} ${type}. SSH authentication failed, verify authentication parameters are correct (Username/Password, public-key etc.) You may refer to the engine.log file for further details. Line 930: VDS_SECURITY_CONNECTION_ERROR=Cannot ${action} ${type}. SSH connection failed, ${ErrorMsg}. Line 931: VDS_REMOVE_FENCE_AGENT_ID_REQUIRED=Cannot Remove Fence-Agent. Agent ID not provided. Line 932: VDS_REMOVE_FENCE_AGENTS_VDS_ID_REQUIRED=Cannot Remove Fence-Agents. Host ID not provided. Line 933: VDS_ADD_FENCE_AGENT_MANDATORY_PARAMETERS_MISSING=Cannot Add Fence-Agent. One or more mandatory parameters were not provided. Mandatory parameters are: type, order, ip, username, address, password. Please rephrase: Cannot add fence agent, one or more mandatory parameters (type, order, ip address, port, username, password) were not provided. Line 934: AUTO_MIGRATE_DISABLED=Cannot migrate - check relevant configuration options. Line 935: AUTO_MIGRATE_VDS_NOT_FOUND=Cannot migrate - Host not found. Line 936: AUTO_MIGRATE_ALREADY_RUNNING_ON_VDS=Cannot migrate - VM already running on Host. Line 937: AUTO_MIGRATE_UNSUCCESSFUL=Cannot migrate - Previous migration was unsuccessful. http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties File backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties: Line 619: VDS_ALERT_FENCE_OPERATION_SKIPPED=Host ${VdsName} became non responsive. It has no power management configured. Please check the host status, manually reboot it, and click "Confirm Host Has Been Rebooted" Line 620: VDS_ALERT_FENCE_NO_PROXY_HOST=There is no other host in the data center that can be used to test the power management settings. Line 621: VDS_ALERT_FENCE_STATUS_VERIFICATION_FAILED=Failed to verify Host ${Host} ${Status} status, Please ${Status} Host ${Host} manually. Line 622: VDS_ALERT_SECONDARY_AGENT_USED_FOR_FENCE_OPERATION=Secondary fence agent was used to ${Operation} Host ${VdsName} Line 623: VDS_ALERT_PM_HEALTH_CHECK_FENCE_AGENT_NON_RESPONSIVE=Health check on Host ${VdsName} indicates that Fence-Agent ${AgentId} is non-responsive. Please rephrase: Health check on host ${VdsName} indicates that fence agent ${AgentId} is non-responsive. Line 624: VDS_ALERT_PM_HEALTH_CHECK_START_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Start this host using Power-Management are expected to fail. Line 625: VDS_ALERT_PM_HEALTH_CHECK_STOP_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Stop this host using Power-Management are expected to fail. Line 626: VDS_ALERT_PM_HEALTH_CHECK_RESTART_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Restart this host using Power-Management are expected to fail. Line 627: VDS_ALERT_FENCE_OPERATION_SKIPPED_BROKEN_CONNECTIVITY=Host ${VdsName} became non responsive and was not restarted due to Fencing Policy: ${Percents} percents of the Hosts in the Cluster have connectivity issues. Line 620: VDS_ALERT_FENCE_NO_PROXY_HOST=There is no other host in the data center that can be used to test the power management settings. Line 621: VDS_ALERT_FENCE_STATUS_VERIFICATION_FAILED=Failed to verify Host ${Host} ${Status} status, Please ${Status} Host ${Host} manually. Line 622: VDS_ALERT_SECONDARY_AGENT_USED_FOR_FENCE_OPERATION=Secondary fence agent was used to ${Operation} Host ${VdsName} Line 623: VDS_ALERT_PM_HEALTH_CHECK_FENCE_AGENT_NON_RESPONSIVE=Health check on Host ${VdsName} indicates that Fence-Agent ${AgentId} is non-responsive. Line 624: VDS_ALERT_PM_HEALTH_CHECK_START_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Start this host using Power-Management are expected to fail. Please rephrase: Health check on host ${VdsName} indicates that future attempts to start this host using power management are expected to fail. Line 625: VDS_ALERT_PM_HEALTH_CHECK_STOP_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Stop this host using Power-Management are expected to fail. Line 626: VDS_ALERT_PM_HEALTH_CHECK_RESTART_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Restart this host using Power-Management are expected to fail. Line 627: VDS_ALERT_FENCE_OPERATION_SKIPPED_BROKEN_CONNECTIVITY=Host ${VdsName} became non responsive and was not restarted due to Fencing Policy: ${Percents} percents of the Hosts in the Cluster have connectivity issues. Line 628: VDS_ALERT_NOT_RESTARTED_DUE_TO_POLICY=Host ${VdsName} became non responsive and was not restarted due to the Cluster Fencing Policy. Line 621: VDS_ALERT_FENCE_STATUS_VERIFICATION_FAILED=Failed to verify Host ${Host} ${Status} status, Please ${Status} Host ${Host} manually. Line 622: VDS_ALERT_SECONDARY_AGENT_USED_FOR_FENCE_OPERATION=Secondary fence agent was used to ${Operation} Host ${VdsName} Line 623: VDS_ALERT_PM_HEALTH_CHECK_FENCE_AGENT_NON_RESPONSIVE=Health check on Host ${VdsName} indicates that Fence-Agent ${AgentId} is non-responsive. Line 624: VDS_ALERT_PM_HEALTH_CHECK_START_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Start this host using Power-Management are expected to fail. Line 625: VDS_ALERT_PM_HEALTH_CHECK_STOP_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Stop this host using Power-Management are expected to fail. Please rephrase: Health check on host ${VdsName} indicates that future attempts to stop this host using power management are expected to fail. Line 626: VDS_ALERT_PM_HEALTH_CHECK_RESTART_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Restart this host using Power-Management are expected to fail. Line 627: VDS_ALERT_FENCE_OPERATION_SKIPPED_BROKEN_CONNECTIVITY=Host ${VdsName} became non responsive and was not restarted due to Fencing Policy: ${Percents} percents of the Hosts in the Cluster have connectivity issues. Line 628: VDS_ALERT_NOT_RESTARTED_DUE_TO_POLICY=Host ${VdsName} became non responsive and was not restarted due to the Cluster Fencing Policy. Line 629: VDS_ALERT_FENCE_DISABLED_BY_CLUSTER_POLICY=Host ${VdsName} became Non Responsive and was not restarted due to disabled fencing in the Cluster Fencing Policy. Line 622: VDS_ALERT_SECONDARY_AGENT_USED_FOR_FENCE_OPERATION=Secondary fence agent was used to ${Operation} Host ${VdsName} Line 623: VDS_ALERT_PM_HEALTH_CHECK_FENCE_AGENT_NON_RESPONSIVE=Health check on Host ${VdsName} indicates that Fence-Agent ${AgentId} is non-responsive. Line 624: VDS_ALERT_PM_HEALTH_CHECK_START_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Start this host using Power-Management are expected to fail. Line 625: VDS_ALERT_PM_HEALTH_CHECK_STOP_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Stop this host using Power-Management are expected to fail. Line 626: VDS_ALERT_PM_HEALTH_CHECK_RESTART_MIGHT_FAIL=Health check on Host ${VdsName} indicates that future attempts to Restart this host using Power-Management are expected to fail. Please rephrase: Health check on host ${VdsName} indicates that future attempts to restart this host using power management are expected to fail. Line 627: VDS_ALERT_FENCE_OPERATION_SKIPPED_BROKEN_CONNECTIVITY=Host ${VdsName} became non responsive and was not restarted due to Fencing Policy: ${Percents} percents of the Hosts in the Cluster have connectivity issues. Line 628: VDS_ALERT_NOT_RESTARTED_DUE_TO_POLICY=Host ${VdsName} became non responsive and was not restarted due to the Cluster Fencing Policy. Line 629: VDS_ALERT_FENCE_DISABLED_BY_CLUSTER_POLICY=Host ${VdsName} became Non Responsive and was not restarted due to disabled fencing in the Cluster Fencing Policy. Line 630: VDS_HOST_NOT_RESPONDING_CONNECTING=Host ${VdsName} is not responding. It will stay in Connecting state for a grace period of ${Seconds} seconds and after that an attempt to fence the host will be issued. http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml File backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml: Please remove trailing spaces Line 1: Line 2: Line 3: Line 4: !!org.ovirt.engine.api.rsdl.MetaData Line 3276: headers: Line 3277: Content-Type: {value: application/xml|json, required: true} Line 3278: Expect: {value: 201-created, required: false} Line 3279: Correlation-Id: {value: 'any string', required: false} Line 3280: - name: /hosts/{host:id}/fencingagents|rel=get Shouldn't it be: /hosts/{host:id}/fenceagents|rel=get or even better /hosts/{host:id}/fence_agents|rel=get Line 3281: description: get all fencing agents defined for this host Line 3282: request: Line 3283: body: Line 3284: parameterType: null http://gerrit.ovirt.org/#/c/27578/23/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/DeprecatedPowerManagementMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/DeprecatedPowerManagementMapper.java: Line 308: * Set the deprecated 'concurrent' field's value on the API 'Agent' objects. Line 309: */ Line 310: private static void setConcurrent(PowerManagement pm, List<FenceAgent> fenceAgents) { Line 311: assert fenceAgents.size() >= 2; Line 312: boolean concurrent = fenceAgents.get(0).getOrder() == fenceAgents.get(1).getOrder(); Please use primitive int for order or use equals for value comparison Line 313: // When a second agent exists, 'concurrent' field is relevant for both agents, so here we Line 314: // set it retroactively in the first agent. Line 315: pm.getAgents().getAgents().get(0).setConcurrent(concurrent); Line 316: pm.getAgents().getAgents().get(1).setConcurrent(concurrent); http://gerrit.ovirt.org/#/c/27578/23/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml File frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml: Line 264: <include name="common/businessentities/ImageOperation.java" /> Line 265: <include name="common/businessentities/ImageDbOperationScope.java" /> Line 266: <include name="common/businessentities/VdcOption.java" /> Line 267: <include name="common/errors/VdcFault.java" /> Line 268: <include name="common/errors/VdcBllErrors.java" /> Please remove trailing spaces Line 269: <include name="common/errors/SqlInjectionException.java" /> Line 270: <include name="common/errors/SearchEngineIllegalCharacterException.java" /> Line 271: <include name="common/businessentities/VdsMovedToNonOperationalType.java" /> Line 272: http://gerrit.ovirt.org/#/c/27578/23/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Cloner.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Cloner.java: Line 258: Line 259: return obj; Line 260: } Line 261: Line 262: private static List<FenceAgent> cloneAgents(List<FenceAgent> agents) { Why are you cloning empty list to null? IMHO the method should contain: List<FenceAgent> clonedAgents = null; if (agents != null) { clonedAgents = new LinkedList<FenceAgent>(); for (FenceAgent agent : agents) { clonedAgents.add(cloneAgent(agent)); } } return clonedAgents; Line 263: if (agents == null || agents.isEmpty()) { Line 264: return null; Line 265: } else { Line 266: List<FenceAgent> clonedAgents = new LinkedList<FenceAgent>(); Line 277: clonedAgent.setHostId(agent.getHostId()); Line 278: clonedAgent.setIp(agent.getIp()); Line 279: clonedAgent.setOptions(agent.getOptions()); Line 280: clonedAgent.setOptionsMap(agent.getOptionsMap()); Line 281: clonedAgent.setOrder(agent.getOrder().intValue()); Integer class is immutable so you can use: clonedAgent.setOrder(agent.getOrder()); But I would prefer to use primitive int for order Line 282: clonedAgent.setPassword(agent.getPassword()); Line 283: clonedAgent.setPort(agent.getPort().intValue()); Line 284: clonedAgent.setType(agent.getType()); Line 285: clonedAgent.setUser(agent.getUser()); Line 279: clonedAgent.setOptions(agent.getOptions()); Line 280: clonedAgent.setOptionsMap(agent.getOptionsMap()); Line 281: clonedAgent.setOrder(agent.getOrder().intValue()); Line 282: clonedAgent.setPassword(agent.getPassword()); Line 283: clonedAgent.setPort(agent.getPort().intValue()); Same as for order Line 284: clonedAgent.setType(agent.getType()); Line 285: clonedAgent.setUser(agent.getUser()); Line 286: return clonedAgent; Line 287: } http://gerrit.ovirt.org/#/c/27578/23/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostModel.java: Line 1807: if (secondaryAgent.getPort() != null) { Line 1808: getPmSecondaryPort().setEntity(secondaryAgent.getPort().toString()); Line 1809: } Line 1810: setPmSecondaryOptionsMap(secondaryAgent.getOptionsMap()); Line 1811: getPmSecondaryConcurrent().setEntity(secondaryAgent.getOrder() == primaryAgent.getOrder()); Please use primitive int for order or use equals for value comparison Line 1812: } Line 1813: } Line 1814: getDisableAutomaticPowerManagement().setEntity(vds.isDisablePowerManagementPolicy()); Line 1815: getPmKdumpDetection().setEntity(vds.isPmKdumpDetection()); http://gerrit.ovirt.org/#/c/27578/23/packaging/dbscripts/fence_agents_sp.sql File packaging/dbscripts/fence_agents_sp.sql: Line 1: Create or replace FUNCTION GetFenceAgentsByVdsId(v_vds_guid UUID) RETURNS SETOF fence_agents STABLE Please use v_vds_id Line 2: AS $procedure$ Line 3: BEGIN Line 4: RETURN QUERY SELECT fence_agents.* Line 5: FROM fence_agents Line 6: WHERE vds_id = v_vds_guid; Line 7: END; $procedure$ Line 8: LANGUAGE plpgsql; Line 9: Line 10: Create or replace FUNCTION GetFenceAgentById(v_guid UUID) RETURNS SETOF fence_agents STABLE Please use v_id Line 11: AS $procedure$ Line 12: BEGIN Line 13: RETURN QUERY SELECT fence_agents.* Line 14: FROM fence_agents Line 15: WHERE id = v_guid; Line 16: END; $procedure$ Line 17: LANGUAGE plpgsql; Line 18: Line 19: Create or replace FUNCTION DeleteFenceAgent(v_guid UUID) RETURNS VOID Please use v_id Line 20: AS $procedure$ Line 21: BEGIN Line 22: DELETE FROM fence_agents Line 23: WHERE id = v_guid; Line 23: WHERE id = v_guid; Line 24: END; $procedure$ Line 25: LANGUAGE plpgsql; Line 26: Line 27: Create or replace FUNCTION DeleteFenceAgentsByVdsId(v_vds_guid UUID) RETURNS VOID Please use v_vds_id Line 28: AS $procedure$ Line 29: BEGIN Line 30: DELETE FROM fence_agents Line 31: WHERE vds_id = v_vds_guid; Line 31: WHERE vds_id = v_vds_guid; Line 32: END; $procedure$ Line 33: LANGUAGE plpgsql; Line 34: Line 35: Create or replace FUNCTION UpdateFenceAgent(v_guid UUID , Please use v_id Line 36: v_vds_id UUID , Line 37: v_agent_order INTEGER , Line 38: v_ip VARCHAR(255) , Line 39: v_type VARCHAR(255) , http://gerrit.ovirt.org/#/c/27578/23/packaging/dbscripts/upgrade/03_06_0425_create_fence_agents_table.sql File packaging/dbscripts/upgrade/03_06_0425_create_fence_agents_table.sql: Line 7: ip VARCHAR(255) NOT NULL, Line 8: type VARCHAR(255) NOT NULL, Line 9: agent_user VARCHAR(255) NOT NULL, Line 10: agent_password text NOT NULL, Line 11: port INTEGER, Is there any fencing agent with defined IP address but undefined port? Line 12: options VARCHAR(255) NOT NULL DEFAULT '' Line 13: ); Line 14: Line 15: -- create index for vds id -- To view, visit http://gerrit.ovirt.org/27578 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id0b384347a52db8b0aa6ae684fad40a5491dd2f7 Gerrit-PatchSet: 23 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ori Liel <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Ori Liel <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[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
