Yair Zaslavsky has posted comments on this change.
Change subject: core: host-deploy: Using ssh username and password to install
and update host
......................................................................
Patch Set 6: (8 inline comments)
Mostly minor comments.
Alon - what do you think about the isBlank/isEmpty issue?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
Line 316: } else if (getVdsDAO().getAllForHostname(hostName).size()
!= 0) {
Line 317: returnValue =
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST);
Line 318: } else if
(!ValidationUtils.validatePort(vds.getSshPort())) {
Line 319: returnValue =
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_PORT);
Line 320: } else if ((StringUtils.isEmpty(vds.getSshUsername())) ||
question - can we have a name containing whitespaces?
If not - I prefer isBlank.
Line 321: (vds.getSshUsername().length() >
BusinessEntitiesDefinitions.USER_USER_NAME_SIZE)) {
Line 322: returnValue =
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_USERNAME);
Line 323: } else {
Line 324: returnValue = returnValue &&
validateSingleHostAttachedToLocalStorage();
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java
Line 86: && _oldVds.getStatus() !=
VDSStatus.NonOperational
Line 87: && _oldVds.getStatus() !=
VDSStatus.InstallFailed) {
Line 88:
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_INSTALL_STATUS_ILLEGAL);
Line 89: } else if (getParameters().getInstallVds()
Line 90: &&
StringUtils.isEmpty(getParameters().getPassword())
Question - can a password contain whitespaces, if not - please use isBlank
Line 91: &&
getParameters().getVdsStaticData().getVdsType() == VDSType.VDS) {
Line 92:
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_INSTALL_EMPTY_PASSWORD);
Line 93: } else if (!getParameters().getInstallVds()
Line 94: && _oldVds.getPort() !=
getParameters().getVdsStaticData().getPort()) {
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdsOperationActionParameters.java
Line 47:
Line 48: public VdsOperationActionParameters() {
Line 49: }
Line 50:
Line 51: // Deprecated to keep old api with root password
An idea:
Can setRootPassword call setPassword, and can getRootPassword call getPassword?
This way, you will have only one field.
Line 52: public String getRootPassword() {
Line 53: return password;
Line 54: }
Line 55:
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 521: -Please remove VLAN from the interface.
Line 522: NETWORK_ALREADY_ATTACHED_TO_CLUSTER=Logical Network is already
attached to Cluster.
Line 523: ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST=Cannot ${action} ${type}.
Host with the same address already exists.
Line 524: ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_PORT=Cannot ${action}
${type}. Invalid SSH port was entered.
Line 525: ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_USERNAME=Cannot ${action}
${type}. Invalid SSH user-name was entered.
I would prefer user name, without dash.
Line 526: ACTION_TYPE_FAILED_VDS_WITH_SAME_UUID_EXIST=Cannot ${action} ${type}.
Host with the same UUID already exists.
Line 527: ACTION_TYPE_FAILED_ILLEGAL_MEMORY_SIZE=Cannot ${action} ${type}.
Illegal memory size is provided, size needs to be between ${minMemorySize} MB
and ${maxMemorySize} MB.
Line 528: ACTION_TYPE_FAILED_ILLEGAL_NUM_OF_MONITORS=Cannot ${action} ${type}.
Illegal number of monitors is provided, max allowed number of monitors is 1 for
VNC and the max number in the ValidNumOfMonitors configuration variable for
SPICE.
Line 529: ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}.
Illegal Domain name: ${Domain}. Domain name has unsupported special character
${Char}.
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
Line 1410:
Line 1411: @DefaultStringValue("Cannot ${action} ${type}. Invalid SSH port
was entered.")
Line 1412: String ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_PORT();
Line 1413:
Line 1414: @DefaultStringValue("Cannot ${action} ${type}. Invalid SSH
user-name was entered.")
same as before - without dash.
Line 1415: String ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_USERNAME();
Line 1416:
Line 1417: @DefaultStringValue("Cannot ${action} ${type}. Host with the
same UUID already exists.")
Line 1418: String ACTION_TYPE_FAILED_VDS_WITH_SAME_UUID_EXIST();
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostListModel.java
Line 859: {
Line 860: AddVdsActionParameters parameters = new
AddVdsActionParameters();
Line 861: parameters.setVdsId(host.getId());
Line 862: parameters.setvds(host);
Line 863: parameters.setPassword((String)
model.getRootPassword().getEntity());
are you going to fix all the getRootPassword and turn them to getPassword?
Line 864: parameters.setOverrideFirewall((Boolean)
model.getOverrideIpTables().getEntity());
Line 865: parameters.setRebootAfterInstallation(isVirt) ;
Line 866:
Line 867: Frontend.RunAction(VdcActionType.AddVds, parameters,
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
Line 518: -Please remove VLAN from the interface.
Line 519: NETWORK_ALREADY_ATTACHED_TO_CLUSTER=Logical Network is already
attached to Cluster.
Line 520: ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST=Cannot ${action} ${type}.
Host with the same address already exists.
Line 521: ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_PORT=Cannot ${action}
${type}. Invalid SSH port was entered.
Line 522: ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_USERNAME=Cannot ${action}
${type}. Invalid SSH user-name was entered.
please remove the dash in user-name
Line 523: ACTION_TYPE_FAILED_VDS_WITH_SAME_UUID_EXIST=Cannot ${action} ${type}.
Host with the same UUID already exists.
Line 524: ACTION_TYPE_FAILED_ILLEGAL_MEMORY_SIZE=Cannot ${action} ${type}.
Illegal memory size is provided, size needs to be between ${minMemorySize} MB
and ${maxMemorySize} MB.
Line 525: ACTION_TYPE_FAILED_ILLEGAL_NUM_OF_MONITORS=Cannot ${action} ${type}.
Illegal number of monitors is provided, max allowed number of monitors is 1 for
VNC and the max number in the ValidNumOfMonitors configuration variable for
SPICE.
Line 526: ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}.
Illegal Domain name: ${Domain}. Domain name has unsupported special character
${Char}.
....................................................
File
frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
Line 526: -Please remove VLAN from the interface.
Line 527: NETWORK_ALREADY_ATTACHED_TO_CLUSTER=Logical Network is already
attached to Cluster.
Line 528: ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST=Cannot ${action} ${type}.
Host with the same address already exists.
Line 529: ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_PORT=Cannot ${action}
${type}. Invalid SSH port was entered.
Line 530: ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_USERNAME=Cannot ${action}
${type}. Invalid SSH user-name was entered.
same - remove dash
Line 531: ACTION_TYPE_FAILED_VDS_WITH_SAME_UUID_EXIST=Cannot ${action} ${type}.
Host with the same UUID already exists.
Line 532: ACTION_TYPE_FAILED_ILLEGAL_MEMORY_SIZE=Cannot ${action} ${type}.
Illegal memory size is provided, size needs to be between ${minMemorySize} MB
and ${maxMemorySize} MB.
Line 533: ACTION_TYPE_FAILED_ILLEGAL_NUM_OF_MONITORS=Cannot ${action} ${type}.
Illegal number of monitors is provided, max allowed number of monitors is 1 for
VNC and the max number in the ValidNumOfMonitors configuration variable for
SPICE.
Line 534: ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}.
Illegal Domain name: ${Domain}. Domain name has unsupported special character
${Char}.
--
To view, visit http://gerrit.ovirt.org/16686
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d9b23771314ebebb3e10686decb4f0d5ace6d3c
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@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
