Alon Bar-Lev has uploaded a new change for review. Change subject: utils: cleanup: SSHClient: add properties get methods ......................................................................
utils: cleanup: SSHClient: add properties get methods This is required to allow proper messaging and probably other uses. Came up again at discussion[1]. Does not effect stability, no reason not to provide. [1] http://gerrit.ovirt.org/#/c/7584/ Change-Id: I236d6b71402b840b9c338c255b16e298cd0716fa Signed-off-by: Alon Bar-Lev <[email protected]> --- M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/VdsInstallerSSH.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHClient.java A backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ssh/PropertiesTest.java 3 files changed, 149 insertions(+), 53 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/09/8009/1 diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/VdsInstallerSSH.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/VdsInstallerSSH.java index 42b09a8..d03a742 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/VdsInstallerSSH.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/VdsInstallerSSH.java @@ -30,8 +30,7 @@ private SSHClient client; private IVdsInstallerCallback callback; - private String host; - private int port = 22; + private int port = 22; // until api supprt port /* for testing */ void setPort(int port) { @@ -113,13 +112,12 @@ ); boolean ret = false; - this.host = server; try { this.client = new SSHClient(); this.client.setHardTimeout(hardTimeout); this.client.setSoftTimeout(softTimeout); - this.client.setHost(this.host, this.port); + this.client.setHost(server, this.port); // port until API supports port this.client.setUser(user); if (keyStore == null) { @@ -206,7 +204,7 @@ _callbackAddMessage( String.format( "<BSTRAP component='RHEV_INSTALL' status='OK' message='Connected to Host %1$s with SSH key fingerprint: %2$s'/>", - this.host, + this.client.getDisplayHost(), fingerprint ) ); @@ -219,7 +217,7 @@ catch (Exception e) { String m = String.format( "Could not connect to server %1$s", - this.host + this.client.getDisplayHost() ); log.error(m, e); _callbackFailed(m); @@ -394,7 +392,7 @@ } } - log.info(String.format("SSH execute %1$s '%2$s'", this.host, command)); + log.info(String.format("SSH execute %1$s '%2$s'", this.client.getDisplayHost(), command)); boolean ret = false; @@ -423,7 +421,7 @@ log.error( String.format( "SSH error running command %1$s:'%2$s'", - this.host, + this.client.getDisplayHost(), command ), e @@ -431,7 +429,7 @@ _callbackFailed( String.format( "SSH command failed while executing at host '%1$s', refer to logs for further information", - this.host + this.client.getDisplayHost() ) ); } @@ -446,7 +444,7 @@ public final boolean receiveFile(String source, String destination) { - log.info(String.format("SSH receive %1$s:'%2$s' '%3$s')", this.host, source, destination)); + log.info(String.format("SSH receive %1$s:'%2$s' '%3$s')", this.client.getDisplayHost(), source, destination)); boolean ret = false; @@ -458,7 +456,7 @@ catch (FileNotFoundException e) { String m = String.format( "SSH could not receive file %1$s:'%2$s': '%3$s'", - this.host, + this.client.getDisplayHost(), source, destination ); @@ -468,7 +466,7 @@ catch (Exception e) { String m = String.format( "SSH could not receive file %1$s:'%2$s': '%3$s'", - this.host, + this.client.getDisplayHost(), source, destination ); @@ -482,7 +480,7 @@ public final boolean sendFile(String source, String destination) { - log.debug(String.format("SSH send %1$s:'%2$s' '%3$s'", this.host, source, destination)); + log.debug(String.format("SSH send %1$s:'%2$s' '%3$s'", this.client.getDisplayHost(), source, destination)); boolean ret = false; @@ -495,7 +493,7 @@ log.error ( String.format( "SSH could not send file %2$s %1$s:%3$s", - this.host, + this.client.getDisplayHost(), source, destination ), @@ -506,7 +504,7 @@ catch (Exception e) { String m = String.format( "SSH could not send file %2$s %1$s:%3$s", - this.host, + this.client.getDisplayHost(), source, destination ); diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHClient.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHClient.java index 2b328b4..cfaf698 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHClient.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHClient.java @@ -38,6 +38,7 @@ private static final int STREAM_BUFFER_SIZE = 8192; private static final int CONSTRAINT_BUFFER_SIZE = 1024; private static final int THREAD_JOIN_WAIT_TIME = 2000; + private static final int DEFAULT_SSH_PORT = 22; private static Log log = LogFactory.getLog(SSHClient.class); @@ -49,7 +50,7 @@ private String password; private KeyPair keyPair; private String host; - private int port; + private int port = DEFAULT_SSH_PORT; private PublicKey serverKey; /** @@ -172,6 +173,73 @@ } /** + * Set host. + * @param host host. + */ + public void setHost(String host) { + setHost(host, DEFAULT_SSH_PORT); + } + + /** + * Get host. + * @return host as set by setHost() + */ + public String getHost() { + return this.host; + } + + /** + * Get port. + * @return port. + */ + public int getPort() { + return this.port; + } + + /** + * Get hard timeout. + * @return timeout. + */ + public long getHardTimeout() { + return this.hardTimeout; + } + + /** + * Get soft timeout. + * @return timeout. + */ + public long getSoftTimeout() { + return this.softTimeout; + } + + /** + * Get user. + * @return user. + */ + public String getUser() { + return this.user; + } + + public String getDisplayHost() { + StringBuffer ret = new StringBuffer(100); + if (this.host == null) { + ret.append("N/A"); + } + else { + if (this.user != null) { + ret.append(this.user); + ret.append("@"); + } + ret.append(this.host); + if (this.port != DEFAULT_SSH_PORT) { + ret.append(":"); + ret.append(this.port); + } + } + return ret.toString(); + } + + /** * Get server key * @return server key. */ @@ -184,7 +252,7 @@ */ public void connect() throws Exception { - log.debug(String.format("Connecting: '%1$s:%2$s", this.host, this.port)); + log.debug(String.format("Connecting: '%1$s'", this.getDisplayHost())); try { this.client = _createSshClient(); @@ -209,9 +277,8 @@ if (!cfuture.await(this.softTimeout)) { throw new TimeLimitExceededException( String.format( - "SSH connection timed out connecting to '%1$s:%2$d'", - this.host, - this.port + "SSH connection timed out connecting to '%1$s'", + this.getDisplayHost() ) ); } @@ -233,18 +300,16 @@ if ((stat & ClientSession.CLOSED) != 0) { throw new IOException( String.format( - "SSH session closed during connection '%1$s:%2$d'", - this.host, - this.port + "SSH session closed during connection '%1$s'", + this.getDisplayHost() ) ); } if ((stat & ClientSession.TIMEOUT) != 0) { throw new TimeLimitExceededException( String.format( - "SSH timed out waiting for authentication request '%1$s:%2$d'", - this.host, - this.port + "SSH timed out waiting for authentication request '%1$s'", + this.getDisplayHost() ) ); } @@ -254,7 +319,7 @@ throw e; } - log.debug(String.format("Connected: '%1$s:%2$s", this.host, this.port)); + log.debug(String.format("Connected: '%1$s'", this.getDisplayHost())); } /** @@ -262,7 +327,7 @@ */ public void authenticate() throws Exception { - log.debug(String.format("Authenticating: '%1$s:%2$s", this.host, this.port)); + log.debug(String.format("Authenticating: '%1$s'", this.getDisplayHost())); try { AuthFuture afuture; @@ -275,27 +340,24 @@ else { throw new AuthenticationException( String.format( - "SSH authentication failure '%1$s:%2$s', no password or key", - this.host, - this.port + "SSH authentication failure '%1$s', no password or key", + this.getDisplayHost() ) ); } if (!afuture.await(this.softTimeout)) { throw new TimeLimitExceededException( String.format( - "SSH authentication timed out connecting to '%1$s:%2$d'", - this.host, - this.port + "SSH authentication timed out connecting to '%1$s'", + this.getDisplayHost() ) ); } if (!afuture.isSuccess()) { throw new AuthenticationException( String.format( - "SSH authentication to '%1$s:%2$d' failed%3$s", - this.host, - this.port, + "SSH authentication to '%1$s' failed%2$s", + this.getDisplayHost(), ( this.keyPair == null ? " make sure host is configured for password authentication" : @@ -310,7 +372,7 @@ throw e; } - log.debug(String.format("Authenticated: '%1$s:%2$s", this.host, this.port)); + log.debug(String.format("Authenticated: '%1$s'", this.getDisplayHost())); } /** @@ -401,9 +463,8 @@ if (hardTimeout) { throw new TimeLimitExceededException( String.format( - "SSH session hard timeout host '%1$s:%2$d'", - this.host, - this.port + "SSH session hard timeout host '%1$s'", + this.getDisplayHost() ) ); } @@ -411,9 +472,8 @@ if ((stat & ClientChannel.TIMEOUT) != 0) { throw new TimeLimitExceededException( String.format( - "SSH session timeout host '%1$s:%2$d'", - this.host, - this.port + "SSH session timeout host '%1$s'", + this.getDisplayHost() ) ); } @@ -431,9 +491,8 @@ if ((stat & ClientChannel.EXIT_SIGNAL) != 0) { throw new IOException( String.format( - "Signal received during SSH session host '%1$s:%2$d'", - this.host, - this.port + "Signal received during SSH session host '%1$s'", + this.getDisplayHost() ) ); } @@ -441,9 +500,8 @@ if ((stat & ClientChannel.EXIT_STATUS) != 0 && channel.getExitStatus() != 0) { throw new IOException( String.format( - "Command returned failure code %3$d during SSH session %1$s:%2$d' '%4$s'", - this.host, - this.port, + "Command returned failure code %2$d during SSH session '%1$s' '%3$s'", + this.getDisplayHost(), channel.getExitStatus(), command ) @@ -453,9 +511,8 @@ if ((stat & ClientChannel.TIMEOUT) != 0) { throw new TimeLimitExceededException( String.format( - "SSH session timeout waiting for status host %1$s:%2$d", - this.host, - this.port + "SSH session timeout waiting for status host '%1$s'", + this.getDisplayHost() ) ); } diff --git a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ssh/PropertiesTest.java b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ssh/PropertiesTest.java new file mode 100644 index 0000000..df1669e --- /dev/null +++ b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ssh/PropertiesTest.java @@ -0,0 +1,41 @@ +package org.ovirt.engine.core.utils.ssh; + +import org.junit.Test; +import static org.junit.Assert.*; + +/** + * Basic tests. + * + * Authentication and sanity. + */ +public class PropertiesTest { + @Test + public void testProperties() { + SSHClient ssh = null; + try { + ssh = new SSHClient(); + + assertEquals(ssh.getHost(), null); + assertEquals(ssh.getPort(), 22); + assertEquals(ssh.getUser(), null); + assertEquals(ssh.getDisplayHost(), "N/A"); + + ssh.setHost("host1"); + assertEquals(ssh.getHost(), "host1"); + assertEquals(ssh.getDisplayHost(), "host1"); + ssh.setHost("host1", 1234); + assertEquals(ssh.getPort(), 1234); + assertEquals(ssh.getDisplayHost(), "host1:1234"); + ssh.setUser("user1"); + assertEquals(ssh.getUser(), "user1"); + assertEquals(ssh.getDisplayHost(), "user1@host1:1234"); + ssh.setHost("host2"); + assertEquals(ssh.getDisplayHost(), "user1@host2"); + } + finally { + if (ssh != null) { + ssh.disconnect(); + } + } + } +} -- To view, visit http://gerrit.ovirt.org/8009 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I236d6b71402b840b9c338c255b16e298cd0716fa Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
