[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Hello a...@phdata.io, Philip Zeyliger, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10580 to look at the new patch set (#3). Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides value passed in via -i / --impalad .. IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides value passed in via -i / --impalad After additional testing around IMPALA-2782, it was discovered that impala-shell starts the session displaying the expected hostname (as passed -i flag) on the prompt. This gives the impression that the load balancer was bypassed, however the actual TSSLSocket is still created with the hostname passed in via the -b or --kerberos_host_fqdn flag. This change ensures that the hostname used to create the TSSLSocket will always be the one passed in via the -i flag on impala-shell. This change is required by IMPALA-2782. Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a --- M shell/impala_client.py 1 file changed, 7 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/10580/3 -- To view, visit http://gerrit.cloudera.org:8080/10580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a Gerrit-Change-Number: 10580 Gerrit-PatchSet: 3 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: a...@phdata.io
[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/10580 )
Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag
overrides value passed in via -i / --impalad
..
Patch Set 3:
(5 comments)
I think this is close. I could be swayed on the test.
http://gerrit.cloudera.org:8080/#/c/10580/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10580/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides
subjects shouldn't be wrapped, in theory.
http://gerrit.cloudera.org:8080/#/c/10580/3//COMMIT_MSG@10
PS3, Line 10: After additional testing around IMPALA-2782, it was discovered
Let's find a way to add a test for this scenario here.
We can't fully test it because we don't have a kerberos environment, but we can
at least figure out if the right socket is opened.
Use Python or netcat to start listening on two ports, A (the load balancer) and
B (the actual impalad).
With one set of impala-shell options, it should try to connect to A and with
another set it should try to connect to B. We can see if A and B have anything
to read.
Let me know if you think that's unreasonable effort-wise.
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py
File shell/impala_client.py:
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py@66
PS3, Line 66: self.impalad = impalad
It would be reasonable to introduce:
self.impalad_host = impalad[0].encode(...)
self.impalad_port = int(impalad[1])
here
I don't insist.
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py@281
PS3, Line 281: # principal. So in the presence of a load balancer, the its
hostname is expected by
"the its hostname" -> this doesn't parse to me. Can you clear it up?
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py@284
PS3, Line 284: if self.kerberos_host_fqdn is not None:
: host, port =
(self.kerberos_host_fqdn.split(':')[0].encode('ascii', 'ignore'),
: int(self.impalad[1]))
: else:
: host, port = self.impalad[0].encode('ascii', 'ignore'),
int(self.impalad[1])
Please rename this to "sasl_host", since the only use is line 306.
Please move the "port = int(self.impalad[1])" part of this to line 292 and call
it "sock_port". I had to think a little bit too hard about why you didn't need
a port number for the load balancer, and, in part, I think that helped obscure
the bug.
--
To view, visit http://gerrit.cloudera.org:8080/10580
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a
Gerrit-Change-Number: 10580
Gerrit-PatchSet: 3
Gerrit-Owner: Vincent Tran
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: a...@phdata.io
Gerrit-Comment-Date: Fri, 15 Jun 2018 15:55:10 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Hello a...@phdata.io, Philip Zeyliger, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10580 to look at the new patch set (#4). Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides value passed in via -i / --impalad .. IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides value passed in via -i / --impalad After additional testing around IMPALA-2782, it was discovered that impala-shell starts the session displaying the expected hostname (as passed -i flag) on the prompt. This gives the impression that the load balancer was bypassed, however the actual TSSLSocket is still created with the hostname passed in via the -b or --kerberos_host_fqdn flag. This change ensures that the hostname used to create the TSSLSocket will always be the one passed in via the -i flag on impala-shell. This change is required by IMPALA-2782. Testing: Using netcat, we verified that the impala daemon host[:port] value passed into the -i/--impalad option is indeed the one impala-shell tries to connect to in both cases (with and without -b) Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a --- M shell/impala_client.py M tests/shell/test_shell_commandline.py 2 files changed, 48 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/10580/4 -- To view, visit http://gerrit.cloudera.org:8080/10580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a Gerrit-Change-Number: 10580 Gerrit-PatchSet: 4 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: a...@phdata.io
[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Vincent Tran has posted comments on this change. (
http://gerrit.cloudera.org:8080/10580 )
Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag
overrides value passed in via -i / --impalad
..
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10580/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10580/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides
value passed in via -i / --impalad
> subjects shouldn't be wrapped, in theory.
Done
http://gerrit.cloudera.org:8080/#/c/10580/3//COMMIT_MSG@10
PS3, Line 10: that impala-shell starts the session displaying the expected
> Let's find a way to add a test for this scenario here.
Done
I think that to verify this scenario we must prove that the impala daemon
target passed into -i/--impalad is always the one that the actual socket is
created against.
I don't know if it really makes sense create a load balancer socket and listen
on it in this scenario. The value that used in the -b option is just the
hostname of the load balancer. The port doesn't really matter.
Checkout my test in the latest patchset. Let me know if it makes sense to you.
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py
File shell/impala_client.py:
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py@66
PS3, Line 66: self.impalad_host = impalad[0].encode('ascii', 'ignore')
> It would be reasonable to introduce:
Yeah. This does make sense and adds more readability.
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py@281
PS3, Line 281: # the user. impala-shell checks to ensure this host matches
the host in the kerberos
> "the its hostname" -> this doesn't parse to me. Can you clear it up?
Done
http://gerrit.cloudera.org:8080/#/c/10580/3/shell/impala_client.py@284
PS3, Line 284: # connect directly to an impalad.
: if self.kerberos_host_fqdn is not None:
: sasl_host =
self.kerberos_host_fqdn.split(':')[0].encode('ascii', 'ignore')
: else:
: sasl_host = self.impalad_host
> Please rename this to "sasl_host", since the only use is line 306.
Done
--
To view, visit http://gerrit.cloudera.org:8080/10580
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a
Gerrit-Change-Number: 10580
Gerrit-PatchSet: 4
Gerrit-Owner: Vincent Tran
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Vincent Tran
Gerrit-Reviewer: a...@phdata.io
Gerrit-Comment-Date: Sat, 16 Jun 2018 23:28:29 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10580 ) Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides value passed in via -i / --impalad .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a Gerrit-Change-Number: 10580 Gerrit-PatchSet: 5 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vincent Tran Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Sun, 17 Jun 2018 03:24:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10580 ) Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides value passed in via -i / --impalad .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2689/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a Gerrit-Change-Number: 10580 Gerrit-PatchSet: 5 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vincent Tran Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Sun, 17 Jun 2018 03:24:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10580 ) Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides value passed in via -i / --impalad .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a Gerrit-Change-Number: 10580 Gerrit-PatchSet: 4 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vincent Tran Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Sun, 17 Jun 2018 03:23:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10580 ) Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides value passed in via -i / --impalad .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a Gerrit-Change-Number: 10580 Gerrit-PatchSet: 5 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vincent Tran Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Sun, 17 Jun 2018 06:45:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10580 ) Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides value passed in via -i / --impalad .. IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides value passed in via -i / --impalad After additional testing around IMPALA-2782, it was discovered that impala-shell starts the session displaying the expected hostname (as passed -i flag) on the prompt. This gives the impression that the load balancer was bypassed, however the actual TSSLSocket is still created with the hostname passed in via the -b or --kerberos_host_fqdn flag. This change ensures that the hostname used to create the TSSLSocket will always be the one passed in via the -i flag on impala-shell. This change is required by IMPALA-2782. Testing: Using netcat, we verified that the impala daemon host[:port] value passed into the -i/--impalad option is indeed the one impala-shell tries to connect to in both cases (with and without -b) Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a Reviewed-on: http://gerrit.cloudera.org:8080/10580 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M shell/impala_client.py M tests/shell/test_shell_commandline.py 2 files changed, 48 insertions(+), 10 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/10580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a Gerrit-Change-Number: 10580 Gerrit-PatchSet: 6 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vincent Tran Gerrit-Reviewer: a...@phdata.io
[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/10580 )
Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag
overrides value passed in via -i / --impalad
..
Patch Set 6:
(2 comments)
My mistake--I somehow missed the new test being added when I went to re-review
this. Some comments. I know you're already working on the fixed port number.
http://gerrit.cloudera.org:8080/#/c/10580/6/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/10580/6/tests/shell/test_shell_commandline.py@628
PS6, Line 628: impalad_sock = Popen(shlex.split("nc -lp %d -w %d" %
(impala_daemon_port, ncat_timeout,)),
"nc" isn't covered in bin/bootstrap-system.sh, so it could be missing. In my
case, builds that use "test-with-docker.py" failed. Easiest answer is to just
add it in there.
http://gerrit.cloudera.org:8080/#/c/10580/6/tests/shell/test_shell_commandline.py@640
PS6, Line 640: impala_shell = Popen(shlex.split("%s %s %s" % (shell_cmd,
args1, args2, )))
I don't see this used. Does this process get cleaned up?
--
To view, visit http://gerrit.cloudera.org:8080/10580
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a
Gerrit-Change-Number: 10580
Gerrit-PatchSet: 6
Gerrit-Owner: Vincent Tran
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Vincent Tran
Gerrit-Reviewer: a...@phdata.io
Gerrit-Comment-Date: Mon, 18 Jun 2018 16:35:27 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7130: impala-shell -b / --kerberos host fqdn flag overrides value passed in via -i / --impalad
Vincent Tran has posted comments on this change. ( http://gerrit.cloudera.org:8080/10580 ) Change subject: IMPALA-7130: impala-shell -b / --kerberos_host_fqdn flag overrides value passed in via -i / --impalad .. Patch Set 6: Addressed all of these in https://gerrit.cloudera.org/#/c/10747/ No longer using netcat Cleanup added -- To view, visit http://gerrit.cloudera.org:8080/10580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a Gerrit-Change-Number: 10580 Gerrit-PatchSet: 6 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vincent Tran Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Mon, 18 Jun 2018 17:51:23 + Gerrit-HasComments: No
