Alon Bar-Lev has posted comments on this change.
Change subject: core: host-deploy: Wrap validation of fingerprint using
EngineSSHClient
......................................................................
Patch Set 9: (6 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
Line 381: * Communicate with host by SSH session and gather vdsm-id if
exist
Line 382: *
Line 383: * @param client - already connected ssh client
Line 384: */
Line 385: private String getInstalledVdsIdIfExists(EngineSSHClient client) {
Not sure this is required.
Line 386: try {
Line 387: ByteArrayOutputStream out = new
ConstraintByteArrayOutputStream(256);
Line 388: client.executeCommand(Config.<String>
GetValue(ConfigValues.GetVdsmIdByVdsmToolCommand),
Line 389: null, out, null);
Line 422:
Line 423: return
failCanDoAction(VdcBllMessages.VDS_CANNOT_AUTHENTICATE_TO_SERVER);
Line 424: } catch (SecurityException e) {
Line 425: log.errorFormat(
Line 426: "Failed to connect due to wrong fingerprint
validation on host {0}, fingerprint: {1}",
it can be for many other reasons. I suggest keeping generic message and present
the exception text.
Also add this exception message to the text that goes to client.
Line 427: vds.getName(),
Line 428: vds.getSshKeyFingerprint(),
Line 429: e
Line 430: );
Line 460: * @param clusterId
Line 461: * ID of the cluster to which the server is being
added.
Line 462: * @return true if the server is good to be added to a gluster
cluster, else false.
Line 463: */
Line 464: private boolean isValidGlusterPeer(EngineSSHClient sshclient,
Guid clusterId) {
are you sure we cannot use SSHClient all times except of the time we set the
vds?
Line 465: if (isGlusterSupportEnabled() && clusterHasServers()) {
Line 466: try {
Line 467: // Must not allow adding a server that already is
part of another gluster cluster
Line 468: Set<String> peers =
getGlusterUtil().getPeers(sshclient);
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetServerSSHKeyFingerprintQuery.java
Line 9
Line 10
Line 11
Line 12
Line 13
can you please move this to EngineSSHClient? from all the changes, it really
does not require the dialog.
or, better, merge this query with your own query of getting fingerprint...
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetAddedGlusterServersQuery.java
Line 21
Line 22
Line 23
Line 24
Line 25
this query can use the query of get fingerprint in a loop, has no place.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/GlusterUtil.java
Line 73: * @param client
Line 74: * The already connected and authenticated SSHClient
object
Line 75: * @return Set of peers of the server
Line 76: */
Line 77: public Set<String> getPeers(EngineSSHClient client) {
I think this file can use SSHClient, it does not use any of the EngineSSHClient
functionality.
Line 78: String serversXml = executePeerStatusCommand(client);
Line 79: return extractServers(serversXml);
Line 80: }
Line 81:
--
To view, visit http://gerrit.ovirt.org/16687
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0ce892c90844bc157e9b2feaba6aeca8acad78d
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
