On Fri, Jan 07, 2011 at 01:42:51PM +0100, Michael Hanselmann wrote:
> With this patch OpConnectConsole will no longer just return a command
> with arguments, but rather a detailed description about how to connect
> to an instance's console. Unittests for some parts are included. Another
> patch will follow to adjust the hypervisor abstractions for the new
> model.
>
> +def _DoConsole(console, show_command, cluster_name, feedback_fn=ToStdout,
> + _runcmd_fn=utils.RunCmd):
> + """Acts based on the result of L{opcodes.OpConnectConsole}.
> +
> + @type console: L{objects.InstanceConsole}
> + @param console: Console object
> + @type show_command: bool
> + @param show_command: Whether to just display commands
> + @type cluster_name: string
> + @param cluster_name: Cluster name as retrieved from master daemon
> +
> + """
> + assert console.Validate()
> +
> + if console.kind == constants.CONS_MESSAGE:
> + feedback_fn(console.message)
> + elif console.kind == constants.CONS_VNC:
> + feedback_fn("Instance %s has VNC listening on %s:%s (display %s),"
> + " URL <vnc://%s:%s/>",
> + console.instance, console.host, console.port,
> + console.display, console.host, console.port)
> + elif console.kind == constants.CONS_SSH:
> + # Convert to string if not already one
> + if isinstance(console.command, basestring):
> + cmd = console.command
> + else:
> + cmd = utils.ShellQuoteArgs(console.command)
> +
> + srun = ssh.SshRunner(cluster_name=cluster_name)
> + ssh_cmd = srun.BuildCmd(console.host, console.user, cmd,
> + batch=True, quiet=False, tty=True)
> +
> + if show_command:
> + feedback_fn(utils.ShellQuoteArgs(ssh_cmd))
> + else:
> + result = _runcmd_fn(ssh_cmd, interactive=True)
> + if result.failed:
> + raise errors.OpExecError("Running \"%s\" as %...@%s failed: %s" %
> + (cmd, console.user, console.host,
> + result.fail_reason))
I still think showing the SSH command to the user is not good.
Rationale:
- if the user is non-privileged, this could leak information and it
doesn't help anyway (they can't run it manually)
- if the user is privileged, they can extract it from the logs
So I would just log it (to commands.log), and say "SSH to the primary
node of instance %s failed, please check cluster configuration".
Rest LGTM.
iustin
