Return-Path: X-Original-To: apmail-aurora-reviews-archive@minotaur.apache.org Delivered-To: apmail-aurora-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id EF1B517E4F for ; Sat, 12 Sep 2015 00:29:58 +0000 (UTC) Received: (qmail 7456 invoked by uid 500); 12 Sep 2015 00:29:58 -0000 Delivered-To: apmail-aurora-reviews-archive@aurora.apache.org Received: (qmail 7407 invoked by uid 500); 12 Sep 2015 00:29:58 -0000 Mailing-List: contact reviews-help@aurora.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@aurora.apache.org Delivered-To: mailing list reviews@aurora.apache.org Received: (qmail 7384 invoked by uid 99); 12 Sep 2015 00:29:58 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 12 Sep 2015 00:29:58 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 955B9272F7D; Sat, 12 Sep 2015 00:29:57 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============2750326411649648889==" MIME-Version: 1.0 Subject: Re: Review Request 38326: Adding ssh options into "aurora task" commands. From: "Bill Farner" To: "Bill Farner" , "Zameer Manji" Cc: "Maxim Khutornenko" , "Aurora" Date: Sat, 12 Sep 2015 00:29:57 -0000 Message-ID: <20150912002957.1695.54307@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Bill Farner" X-ReviewGroup: Aurora X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/38326/ X-Sender: "Bill Farner" References: <20150911233611.12750.47201@reviews.apache.org> In-Reply-To: <20150911233611.12750.47201@reviews.apache.org> Reply-To: "Bill Farner" X-ReviewRequest-Repository: aurora --===============2750326411649648889== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Sept. 11, 2015, 4:36 p.m., Bill Farner wrote: > > How would you feel about an env var instead of command line arg? This seems like something people might put in their bash profile. > > Maxim Khutornenko wrote: > I don't really like relying on env variables in places where direct input is accepted. This is too brittle as users may not realize we are reading system envs. It's also harder to reproduce as simple copy/paste will no longer work across envrionments. > > Bill Farner wrote: > ``` > users may not realize we are reading system envs > ``` > > That's what docs and help text are for, right? > > The real downside is that if a user _needs_ an SSH option set all the time, they have to include it all the time rather than once in their shell profile. > > Curious what others think. > > Maxim Khutornenko wrote: > We don't rely on any envrionment variables anywhere in our client cli, do we? I remember there was a long discussion about relying on the implicit profile settings when we were brainstorming client v2 design and the outcome was: it's too confusing and error prone. Users don't read docs every time or always remember how their bash profile looks. We do for specifying how to view job diffs: ``` $ grep -R os.environ src/main/python/apache/aurora/client src/main/python/apache/aurora/client/cli/jobs.py: diff_program = os.environ.get("DIFF_VIEWER", "diff") ``` Implicit is bad when it's risky or has consequences, i don't think this is either. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38326/#review98708 ----------------------------------------------------------- On Sept. 11, 2015, 4:31 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38326/ > ----------------------------------------------------------- > > (Updated Sept. 11, 2015, 4:31 p.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Bugs: AURORA-1491 > https://issues.apache.org/jira/browse/AURORA-1491 > > > Repository: aurora > > > Description > ------- > > Adding ssh options into "aurora task" commands. > > > Diffs > ----- > > src/main/python/apache/aurora/client/api/command_runner.py c7238e274e53138187c2fe6fe5f14b7ae5f43ba2 > src/main/python/apache/aurora/client/cli/options.py 41b13d6f0e1ed355ea8b958b875c20f065349465 > src/main/python/apache/aurora/client/cli/task.py d1f2568ac0afdd95c65523fde41f0dd16670a7a8 > src/test/python/apache/aurora/client/cli/test_task.py 3ad0b70a7d918055ffee34f593d108a28de6e9f9 > > Diff: https://reviews.apache.org/r/38326/diff/ > > > Testing > ------- > > In vagrant: > ``` > vagrant@aurora:~$ aurora task run -v --ssh-user=vagrant --ssh-options='-v -k' --executor-sandbox devcluster/www-data/prod/hello 'ls' > DEBUG] Command=(['task', 'run', '-v', '--ssh-user=vagrant', '--ssh-options=-v -k', '--executor-sandbox', 'devcluster/www-data/prod/hello', 'ls']) > DEBUG] Using auth module: > DEBUG] Running command: ['ssh', '-n', '-q', '-v', '-k', 'vagrant@192.168.33.7', u'cd /var/lib/mesos/slaves/*/frameworks/*/executors/thermos-1442013544190-www-data-prod-hello-0-fe7fa2f1-e21b-4cc3-9107-0777da35537e/runs/latest;ls'] > 192.168.33.7: OpenSSH_6.6.1, OpenSSL 1.0.1f 6 Jan 2014 > 192.168.33.7: debug1: Reading configuration data /etc/ssh/ssh_config > 192.168.33.7: debug1: /etc/ssh/ssh_config line 19: Applying options for * > 192.168.33.7: debug1: /etc/ssh/ssh_config line 57: Applying options for * > 192.168.33.7: debug1: Connecting to 192.168.33.7 [192.168.33.7] port 22. > 192.168.33.7: debug1: Connection established. > 192.168.33.7: debug1: identity file /home/vagrant/.ssh/id_rsa type 1 > 192.168.33.7: debug1: identity file /home/vagrant/.ssh/id_rsa-cert type -1 > 192.168.33.7: debug1: identity file /home/vagrant/.ssh/id_dsa type -1 > 192.168.33.7: debug1: identity file /home/vagrant/.ssh/id_dsa-cert type -1 > 192.168.33.7: debug1: identity file /home/vagrant/.ssh/id_ecdsa type -1 > 192.168.33.7: debug1: identity file /home/vagrant/.ssh/id_ecdsa-cert type -1 > 192.168.33.7: debug1: identity file /home/vagrant/.ssh/id_ed25519 type -1 > 192.168.33.7: debug1: identity file /home/vagrant/.ssh/id_ed25519-cert type -1 > 192.168.33.7: debug1: Enabling compatibility mode for protocol 2.0 > 192.168.33.7: debug1: Local version string SSH-2.0-OpenSSH_6.6.1p1 Ubuntu-2ubuntu2 > 192.168.33.7: debug1: Remote protocol version 2.0, remote software version OpenSSH_6.6.1p1 Ubuntu-2ubuntu2 > 192.168.33.7: debug1: match: OpenSSH_6.6.1p1 Ubuntu-2ubuntu2 pat OpenSSH_6.6.1* compat 0x04000000 > 192.168.33.7: debug1: SSH2_MSG_KEXINIT sent > 192.168.33.7: debug1: SSH2_MSG_KEXINIT received > 192.168.33.7: debug1: kex: server->client aes128-ctr hmac-md5-etm@openssh.com none > 192.168.33.7: debug1: kex: client->server aes128-ctr hmac-md5-etm@openssh.com none > 192.168.33.7: debug1: sending SSH2_MSG_KEX_ECDH_INIT > 192.168.33.7: debug1: expecting SSH2_MSG_KEX_ECDH_REPLY > 192.168.33.7: debug1: Server host key: ECDSA 46:15:09:58:38:39:76:02:68:e1:c2:43:1f:70:bf:6e > 192.168.33.7: Warning: Permanently added '192.168.33.7' (ECDSA) to the list of known hosts. > 192.168.33.7: debug1: ssh_ecdsa_verify: signature correct > 192.168.33.7: debug1: SSH2_MSG_NEWKEYS sent > 192.168.33.7: debug1: expecting SSH2_MSG_NEWKEYS > 192.168.33.7: debug1: SSH2_MSG_NEWKEYS received > 192.168.33.7: debug1: Roaming not allowed by server > 192.168.33.7: debug1: SSH2_MSG_SERVICE_REQUEST sent > 192.168.33.7: debug1: SSH2_MSG_SERVICE_ACCEPT received > 192.168.33.7: debug1: Authentications that can continue: publickey,password > 192.168.33.7: debug1: Next authentication method: publickey > 192.168.33.7: debug1: Offering RSA public key: /home/vagrant/.ssh/id_rsa > 192.168.33.7: debug1: Server accepts key: pkalg ssh-rsa blen 279 > 192.168.33.7: debug1: key_parse_private2: missing begin marker > 192.168.33.7: debug1: read PEM private key done: type RSA > 192.168.33.7: debug1: Authentication succeeded (publickey). > 192.168.33.7: Authenticated to 192.168.33.7 ([192.168.33.7]:22). > 192.168.33.7: debug1: channel 0: new [client-session] > 192.168.33.7: debug1: Requesting no-more-sessions@openssh.com > 192.168.33.7: debug1: Entering interactive session. > 192.168.33.7: debug1: Sending environment. > 192.168.33.7: debug1: Sending env LANG = en_US.UTF-8 > 192.168.33.7: debug1: Sending env LC_CTYPE = en_US.UTF-8 > 192.168.33.7: debug1: Sending command: cd /var/lib/mesos/slaves/*/frameworks/*/executors/thermos-1442013544190-www-data-prod-hello-0-fe7fa2f1-e21b-4cc3-9107-0777da35537e/runs/latest;ls > 192.168.33.7: debug1: client_input_channel_req: channel 0 rtype exit-status reply 0 > 192.168.33.7: checkpoints > 192.168.33.7: __main__.log > 192.168.33.7: sandbox > 192.168.33.7: stderr > 192.168.33.7: stdout > 192.168.33.7: task.json > 192.168.33.7: thermos_executor.pex > 192.168.33.7: thermos_runner.aurora.root.log.DEBUG.20150911-231905.16314 > 192.168.33.7: thermos_runner.aurora.root.log.ERROR.20150911-231905.16314 > 192.168.33.7: thermos_runner.aurora.root.log.FATAL.20150911-231905.16314 > 192.168.33.7: thermos_runner.aurora.root.log.INFO.20150911-231905.16314 > 192.168.33.7: thermos_runner.aurora.root.log.WARNING.20150911-231905.16314 > 192.168.33.7: thermos_runner.DEBUG > 192.168.33.7: thermos_runner.ERROR > 192.168.33.7: thermos_runner.FATAL > 192.168.33.7: thermos_runner.INFO > 192.168.33.7: thermos_runner.pex > 192.168.33.7: thermos_runner.WARNING > 192.168.33.7: debug1: channel 0: free: client-session, nchannels 1 > 192.168.33.7: debug1: fd 1 clearing O_NONBLOCK > 192.168.33.7: Transferred: sent 3568, received 2996 bytes, in 0.2 seconds > 192.168.33.7: Bytes per second: sent 22735.7, received 19090.8 > 192.168.33.7: debug1: Exit status 0 > DEBUG] Command terminated successfully > > ``` > > > Thanks, > > Maxim Khutornenko > > --===============2750326411649648889==--