On Mon, Feb 1, 2010 at 8:54 PM, Lucas Meneghel Rodrigues <[email protected]> wrote: > On Tue, Feb 2, 2010 at 2:45 AM, Lucas Meneghel Rodrigues <[email protected]> > wrote: >> Implementing the approach suggested by Jongki Suwandi to >> resolve the multiple messages like: >> >> [stderr] Warning: Permanently added '192.168.122.99' (RSA) to the list >> of known hosts. >> >> By redirecting them to a filedescriptor of a temporary >> file, once an abstract ssh instance finishes, the file >> is unlinked, so we get rid of all fingerprint registers. >> >> This way we're able to drop the -q flag to the base ssh >> command, and we have only one message like the above per >> autoserv execution. > > Just for the record, we can implement this in such a way that each > instance of abstract_ssh gets its own unique temporary file, so it is > safer when considering multiple autoserv executions. > However, this is a first pass implementation, if you guys think it's a > better idea going with unique temporary files per autoserv execution, > let me know! >
It generally looks good to me. I do wonder if the possible proliferation of file descriptors (and temp file space) could be a problem; in practice it's not really a big deal since you generally don't create all that many host objects in a single autoserv. Maybe it's best to just leave that kind of optimization for when we really need it. -- John >> Signed-off-by: Lucas Meneghel Rodrigues <[email protected]> >> --- >> server/hosts/abstract_ssh.py | 24 +++++++++++++++--------- >> server/hosts/ssh_host.py | 9 +++++---- >> 2 files changed, 20 insertions(+), 13 deletions(-) >> >> diff --git a/server/hosts/abstract_ssh.py b/server/hosts/abstract_ssh.py >> index e3bfe61..e386b53 100644 >> --- a/server/hosts/abstract_ssh.py >> +++ b/server/hosts/abstract_ssh.py >> @@ -10,15 +10,16 @@ enable_master_ssh = >> global_config.get_config_value('AUTOSERV', >> type=bool, default=False) >> >> >> -def make_ssh_command(user="root", port=22, opts='', connect_timeout=30, >> - alive_interval=300): >> - base_command = ("/usr/bin/ssh -a -q -x %s -o StrictHostKeyChecking=no " >> - "-o UserKnownHostsFile=/dev/null -o BatchMode=yes " >> +def make_ssh_command(user="root", port=22, opts='', hosts_file='/dev/null', >> + connect_timeout=30, alive_interval=300): >> + base_command = ("/usr/bin/ssh -a -x %s -o StrictHostKeyChecking=no " >> + "-o UserKnownHostsFile=%s -o BatchMode=yes " >> "-o ConnectTimeout=%d -o ServerAliveInterval=%d " >> "-l %s -p %d") >> assert isinstance(connect_timeout, (int, long)) >> assert connect_timeout > 0 # can't disable the timeout >> - return base_command % (opts, connect_timeout, alive_interval, user, >> port) >> + return base_command % (opts, hosts_file, connect_timeout, >> + alive_interval, user, port) >> >> >> # import site specific Host class >> @@ -44,6 +45,9 @@ class AbstractSSHHost(SiteHost): >> self.port = port >> self.password = password >> self._use_rsync = None >> + self.known_hosts_file = os.tmpfile() >> + known_hosts_fd = self.known_hosts_file.fileno() >> + self.known_hosts_fd = '/dev/fd/%s' % known_hosts_fd >> >> """ >> Master SSH connection background job, socket temp directory and >> socket >> @@ -94,8 +98,9 @@ class AbstractSSHHost(SiteHost): >> appropriate rsync command for copying them. Remote paths must be >> pre-encoded. >> """ >> - ssh_cmd = make_ssh_command(self.user, self.port, >> - self.master_ssh_option) >> + ssh_cmd = make_ssh_command(user=self.user, port=self.port, >> + opts=self.master_ssh_option, >> + hosts_file=self.known_hosts_fd) >> if delete_dest: >> delete_flag = "--delete" >> else: >> @@ -116,8 +121,8 @@ class AbstractSSHHost(SiteHost): >> pre-encoded. >> """ >> command = ("scp -rq %s -o StrictHostKeyChecking=no " >> - "-o UserKnownHostsFile=/dev/null -P %d %s '%s'") >> - return command % (self.master_ssh_option, >> + "-o UserKnownHostsFile=%s -P %d %s '%s'") >> + return command % (self.master_ssh_option, self.known_hosts_fd, >> self.port, " ".join(sources), dest) >> >> >> @@ -512,6 +517,7 @@ class AbstractSSHHost(SiteHost): >> def close(self): >> super(AbstractSSHHost, self).close() >> self._cleanup_master_ssh() >> + self.known_hosts_file.close() >> >> >> def _cleanup_master_ssh(self): >> diff --git a/server/hosts/ssh_host.py b/server/hosts/ssh_host.py >> index 32d42ba..2f5a080 100644 >> --- a/server/hosts/ssh_host.py >> +++ b/server/hosts/ssh_host.py >> @@ -54,10 +54,11 @@ class SSHHost(abstract_ssh.AbstractSSHHost): >> Construct an ssh command with proper args for this host. >> """ >> options = "%s %s" % (options, self.master_ssh_option) >> - base_cmd = abstract_ssh.make_ssh_command(self.user, self.port, >> - options, >> - connect_timeout, >> - alive_interval) >> + base_cmd = abstract_ssh.make_ssh_command(user=self.user, >> port=self.port, >> + opts=options, >> + >> hosts_file=self.known_hosts_fd, >> + >> connect_timeout=connect_timeout, >> + >> alive_interval=alive_interval) >> return "%s %s" % (base_cmd, self.hostname) >> >> >> -- >> 1.6.6 >> >> _______________________________________________ >> Autotest mailing list >> [email protected] >> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest >> > > > > -- > Lucas > _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
