You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by yolandagao <gi...@git.apache.org> on 2015/10/25 21:24:42 UTC

[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

GitHub user yolandagao opened a pull request:

    https://github.com/apache/spark/pull/9272

    [SPARK-10181][SQL] Do kerberos login for credentials during hive client initialization

    On driver process start up, UserGroupInformation.loginUserFromKeytab is called with the principal and keytab passed in, and therefore static var UserGroupInfomation,loginUser is set to that principal with kerberos credentials saved in its private credential set, and all threads within the driver process are supposed to see and use this login credentials to authenticate with Hive and Hadoop. However, because of IsolatedClientLoader, UserGroupInformation class is not shared for hive metastore clients, and instead it is loaded separately and of course not able to see the prepared kerberos login credentials in the main thread.
    
    The first proposed fix would cause other classloader conflict errors, and is not an appropriate solution. This new change does kerberos login during hive client initialization, which will make credentials ready for the particular hive client instance.
    
     @yhuai Please take a look and let me know. If you are not the right person to talk to, could you point me to someone responsible for this?

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/yolandagao/spark master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/9272.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #9272
    
----
commit 59253b3bc731338df87301ed3f2a2a8432af8a8b
Author: Yu Gao <yg...@us.ibm.com>
Date:   2015-10-21T01:23:24Z

    SPARK-10181 UserGroupInformation class needs to be shared for login credentials

commit 3e79e9476ad8f922fe7ff0dc5c72767754fd0ff2
Author: Yu Gao <yg...@us.ibm.com>
Date:   2015-10-22T00:05:36Z

    Merge remote branch 'upstream/master'

commit 9efa819b8ef1b7d0072de1119da2a0329484c16b
Author: Yu Gao <yg...@us.ibm.com>
Date:   2015-10-24T02:17:19Z

    Revert "SPARK-10181 UserGroupInformation class needs to be shared for login credentials"
    
    This reverts commit 59253b3bc731338df87301ed3f2a2a8432af8a8b.

commit 793458590e5b1fe18e34d793ea31cce2b0576c4b
Author: Yu Gao <yg...@us.ibm.com>
Date:   2015-10-25T01:27:44Z

    Merge remote branch 'upstream/master'

commit 30a7d0529ded31c29b5c60e62cb80f7648606e0c
Author: Yu Gao <yg...@us.ibm.com>
Date:   2015-10-25T19:54:01Z

    SPARK-10181 Do kerberos login for credentials in hive client initialization

commit 836bb8bada29bf7366d6d04135bfc88cdecd96c8
Author: Yu Gao <yg...@us.ibm.com>
Date:   2015-10-25T19:56:11Z

    Merge remote branch 'upstream/master'

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-156784632
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/45944/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-156746093
  
    @yolandagao Thank you for the update. Overall looks good. Left two comments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r43707087
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -520,6 +520,8 @@ object SparkSubmit {
           }
           if (args.principal != null) {
             require(args.keytab != null, "Keytab must be specified when the keytab is specified")
    +        sysProps.put("spark.yarn.keytab", args.keytab)
    +        sysProps.put("spark.yarn.principal", args.principal)
    --- End diff --
    
    We might want to look at yarn Client.scala setupCredentials() since its doing something pretty similar it looks like.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-155152432
  
    Merged build started.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-154594150
  
    ok to test


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-155152397
  
     Merged build triggered.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-155155001
  
    **[Test build #45392 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45392/consoleFull)** for PR 9272 at commit [`7d09f5d`](https://github.com/apache/spark/commit/7d09f5df98a2b2d9941e416a6495add2b8620867).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-154858798
  
     Merged build triggered.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-154876596
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yolandagao <gi...@git.apache.org>.
Github user yolandagao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r43592751
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -520,6 +520,8 @@ object SparkSubmit {
           }
           if (args.principal != null) {
             require(args.keytab != null, "Keytab must be specified when the keytab is specified")
    +        sysProps.put("spark.yarn.keytab", args.keytab)
    +        sysProps.put("spark.yarn.principal", args.principal)
    --- End diff --
    
    @harishreedharan Hi Hari, could you let us know the preferred way to pass principal and keytab  parameters from spark submit to spark sql? waiting for your response to proceed. Thank you!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-156784614
  
    **[Test build #45944 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45944/consoleFull)** for PR 9272 at commit [`caf51a7`](https://github.com/apache/spark/commit/caf51a723591b0820d9c48a0e07fec50f511296b).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r44344327
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/ClientWrapper.scala ---
    @@ -150,6 +152,21 @@ private[hive] class ClientWrapper(
         val original = Thread.currentThread().getContextClassLoader
         // Switch to the initClassLoader.
         Thread.currentThread().setContextClassLoader(initClassLoader)
    +
    +    val sparkConf = new SparkConf
    --- End diff --
    
    Instead of creating a new Spark Conf, can we use `SparkEnv.get.conf` to get the spark conf associated with the current spark context?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r44863636
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/ClientWrapper.scala ---
    @@ -149,6 +151,26 @@ private[hive] class ClientWrapper(
         val original = Thread.currentThread().getContextClassLoader
         // Switch to the initClassLoader.
         Thread.currentThread().setContextClassLoader(initClassLoader)
    +
    +    // Set up kerberos credentials for UserGroupInformation.loginUser within
    +    // current class loader
    +    // Instead of using the spark conf of the current spark context, a new instance of
    +    // SparkConf is needed for the original value of spark.yarn.keytab specified by user,
    +    // as yarn.Client resets it for the link name in distributed cache
    +    val sparkConf = new SparkConf
    +    if (sparkConf.contains("spark.yarn.principal") && sparkConf.contains("spark.yarn.keytab")) {
    --- End diff --
    
    Let's make it clear that we set these two settings in SparkSubmit.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r43705844
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -520,6 +520,8 @@ object SparkSubmit {
           }
           if (args.principal != null) {
             require(args.keytab != null, "Keytab must be specified when the keytab is specified")
    --- End diff --
    
    I know this line wasn't changed in this PR but since we are in this file anyways could we maybe cleanup this error message (I think it is meant to say  "Keytab must be specified when the kerberos principal is specified"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r43040951
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -520,6 +520,8 @@ object SparkSubmit {
           }
           if (args.principal != null) {
             require(args.keytab != null, "Keytab must be specified when the keytab is specified")
    +        sysProps.put("spark.yarn.keytab", args.keytab)
    +        sysProps.put("spark.yarn.principal", args.principal)
    --- End diff --
    
    @harishreedharan I see you changed this part of code last time. If we want to pass these two arguments to Spark SQL, what is the recommended way?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/9272


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-154859065
  
    **[Test build #45308 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45308/consoleFull)** for PR 9272 at commit [`7d09f5d`](https://github.com/apache/spark/commit/7d09f5df98a2b2d9941e416a6495add2b8620867).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-152893639
  
    @yolandagao Is https://github.com/apache/spark/commit/40d3c6797a3dfd037eb69b2bcd336d8544deddf5 for the same issue (for a similar issue)? @steveloughran Can you take a look at this one? I am not sure if it is the same issue that you addressed in SPARK-11265.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yolandagao <gi...@git.apache.org>.
Github user yolandagao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r43724262
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -520,6 +520,8 @@ object SparkSubmit {
           }
           if (args.principal != null) {
             require(args.keytab != null, "Keytab must be specified when the keytab is specified")
    +        sysProps.put("spark.yarn.keytab", args.keytab)
    +        sysProps.put("spark.yarn.principal", args.principal)
    --- End diff --
    
    Hi folks, 
    Yarn Client.scala checks the setting from argStrings (passed from SparkSubmit) and sparkConf (which loads java system properties starting with spark.* as well). The args settings (--principal, --keytab) will not be available in spark sql, but we can take advantage of SparkConf with system properties. Updated the pull request and tested. Please advice if this is what you are looking for, Thank you!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-156758037
  
    **[Test build #45937 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45937/consoleFull)** for PR 9272 at commit [`1fbc372`](https://github.com/apache/spark/commit/1fbc3724d3ba7b90a1c412ac3d85d7a7eec9304c).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-154876577
  
    **[Test build #45308 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45308/consoleFull)** for PR 9272 at commit [`7d09f5d`](https://github.com/apache/spark/commit/7d09f5df98a2b2d9941e416a6495add2b8620867).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by steveloughran <gi...@git.apache.org>.
Github user steveloughran commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-153688412
  
    This is unrelated to the SPARK-11265 patch; that's all about getting reflection to find the right methods. This is about UGI setup.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yolandagao <gi...@git.apache.org>.
Github user yolandagao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r43717742
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -520,6 +520,8 @@ object SparkSubmit {
           }
           if (args.principal != null) {
             require(args.keytab != null, "Keytab must be specified when the keytab is specified")
    --- End diff --
    
    Sure. Corrected the error message. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-150965943
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-156745289
  
    **[Test build #45937 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45937/consoleFull)** for PR 9272 at commit [`1fbc372`](https://github.com/apache/spark/commit/1fbc3724d3ba7b90a1c412ac3d85d7a7eec9304c).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-155219421
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by steveloughran <gi...@git.apache.org>.
Github user steveloughran commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r44013419
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/ClientWrapper.scala ---
    @@ -150,6 +152,14 @@ private[hive] class ClientWrapper(
         val original = Thread.currentThread().getContextClassLoader
         // Switch to the initClassLoader.
         Thread.currentThread().setContextClassLoader(initClassLoader)
    +
    +    val sparkConf = new SparkConf
    +    if (sparkConf.contains("spark.yarn.principal") && sparkConf.contains("spark.yarn.keytab")) {
    +      UserGroupInformation.loginUserFromKeytab(
    +        sparkConf.get("spark.yarn.principal"),
    --- End diff --
    
    OK. just include the check (actually, UGI itself should do that check shouldn't it? Lazy)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-156774463
  
    **[Test build #45944 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45944/consoleFull)** for PR 9272 at commit [`caf51a7`](https://github.com/apache/spark/commit/caf51a723591b0820d9c48a0e07fec50f511296b).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by steveloughran <gi...@git.apache.org>.
Github user steveloughran commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r43867986
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/ClientWrapper.scala ---
    @@ -150,6 +152,14 @@ private[hive] class ClientWrapper(
         val original = Thread.currentThread().getContextClassLoader
         // Switch to the initClassLoader.
         Thread.currentThread().setContextClassLoader(initClassLoader)
    +
    +    val sparkConf = new SparkConf
    +    if (sparkConf.contains("spark.yarn.principal") && sparkConf.contains("spark.yarn.keytab")) {
    +      UserGroupInformation.loginUserFromKeytab(
    +        sparkConf.get("spark.yarn.principal"),
    --- End diff --
    
    actually, you should call {{ SparkHadoopUtil.get.loginUserFromKeytab(principalName, keytabFilename)}}; maybe the check could be added there.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r44863760
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/ClientWrapper.scala ---
    @@ -20,6 +20,8 @@ package org.apache.spark.sql.hive.client
     import java.io.{File, PrintStream}
     import java.util.{Map => JMap}
     
    +import org.apache.hadoop.security.UserGroupInformation
    --- End diff --
    
    Let's move this import down to the place where we have other hadoop related imports. https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide#SparkCodeStyleGuide-Imports is the doc about import ordering.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yolandagao <gi...@git.apache.org>.
Github user yolandagao commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-156774495
  
    Thank you Yin for the review. Updated the comments accordingly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yolandagao <gi...@git.apache.org>.
Github user yolandagao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r44742954
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/ClientWrapper.scala ---
    @@ -150,6 +152,21 @@ private[hive] class ClientWrapper(
         val original = Thread.currentThread().getContextClassLoader
         // Switch to the initClassLoader.
         Thread.currentThread().setContextClassLoader(initClassLoader)
    +
    +    val sparkConf = new SparkConf
    --- End diff --
    
    @yhuai Sorry for the late response. I did the testing after changing to SparkEnv.get.conf but it didn't work. The reason is that Yarn Client.scala resets property spark.yarn.keytab by appending some random strings to keytab file name during setupCredentials, which will be used as the link name in distributed cache. This one should be actually separated from the original keytab setting, e.g. using different property names.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yolandagao <gi...@git.apache.org>.
Github user yolandagao commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-153287881
  
    Hi folks, 
    Yarn Client.scala checks the setting from argStrings (passed from SparkSubmit) and sparkConf (which loads java system properties starting with spark.* as well). The args settings (--principal, --keytab) will not be available in spark sql, but we can take advantage of SparkConf with system properties. Updated the pull request and tested. Please advise if this is what you are looking for, Thank you!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yolandagao <gi...@git.apache.org>.
Github user yolandagao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r44863090
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/ClientWrapper.scala ---
    @@ -150,6 +152,21 @@ private[hive] class ClientWrapper(
         val original = Thread.currentThread().getContextClassLoader
         // Switch to the initClassLoader.
         Thread.currentThread().setContextClassLoader(initClassLoader)
    +
    +    val sparkConf = new SparkConf
    --- End diff --
    
    @yhuai Sure. I should have done this earlier, to make everything clearer:) Added some comments there, and please help review. Thank you!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r44863646
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -521,8 +521,18 @@ object SparkSubmit {
             sysProps.put("spark.yarn.isPython", "true")
           }
           if (args.principal != null) {
    -        require(args.keytab != null, "Keytab must be specified when the keytab is specified")
    -        UserGroupInformation.loginUserFromKeytab(args.principal, args.keytab)
    +        require(args.keytab != null, "Keytab must be specified when principal is specified")
    +        if (!new File(args.keytab).exists()) {
    +          throw new SparkException(s"Keytab file: ${args.keytab} does not exist")
    +        } else {
    +          // Add keytab and principal configurations in sysProps to make them available
    +          // for later use (e.g. by spark sql). These Configurations will be set as
    --- End diff --
    
    Let's be more specific on the use case of spark sql. Let's say the isolated class loader used to talk to HiveMetastore will use these settings.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r44814575
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/ClientWrapper.scala ---
    @@ -150,6 +152,21 @@ private[hive] class ClientWrapper(
         val original = Thread.currentThread().getContextClassLoader
         // Switch to the initClassLoader.
         Thread.currentThread().setContextClassLoader(initClassLoader)
    +
    +    val sparkConf = new SparkConf
    --- End diff --
    
    @yolandagao Thanks for the explanation. Can you add comments to your code (including why we need to put those confs to sysProps and why we need to create a new SparkConf at here)? Basically, we need to document the flow of how these confs get propagated. Otherwise, it is not obvious why we need to do this change. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yolandagao <gi...@git.apache.org>.
Github user yolandagao commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-154014784
  
    Updated and tested the change - in yarn client and cluster mode, with and without keytab/principal parameters. Please take a look. Thank you!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yolandagao <gi...@git.apache.org>.
Github user yolandagao commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-154941484
  
    The org.apache.spark.sql.hive.thriftserver.CliSuite timeout failure seems not related to the change but some environment issue. Ran the test suite on my mac machine several times with the same sbt command, and it all passed.
    
    Can we retest?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-155219229
  
    **[Test build #45392 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45392/consoleFull)** for PR 9272 at commit [`7d09f5d`](https://github.com/apache/spark/commit/7d09f5df98a2b2d9941e416a6495add2b8620867).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-155152136
  
    @yolandagao you can try saying "jenkins retest this please" in a comment (I know if you've been whitelisted it will trigger a retest less certain if not yet whitelisted).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-154858809
  
    Merged build started.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-151257965
  
    This this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-156866902
  
    Thanks! Merging to master and branch 1.6.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-156775010
  
    LGTM pending jenkins.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-156784631
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yolandagao <gi...@git.apache.org>.
Github user yolandagao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r43974652
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/ClientWrapper.scala ---
    @@ -150,6 +152,14 @@ private[hive] class ClientWrapper(
         val original = Thread.currentThread().getContextClassLoader
         // Switch to the initClassLoader.
         Thread.currentThread().setContextClassLoader(initClassLoader)
    +
    +    val sparkConf = new SparkConf
    +    if (sparkConf.contains("spark.yarn.principal") && sparkConf.contains("spark.yarn.keytab")) {
    +      UserGroupInformation.loginUserFromKeytab(
    +        sparkConf.get("spark.yarn.principal"),
    --- End diff --
    
    @steveloughran Good point. Better to check the existence of the keytab file before make the login call, as if the keytab doesn't exist the UGI call will definitely fail but with some indirect message like "login failed... no keys found..." ect. Added the check.
    
    However, calling SparkHadoopUtil.get.loginUserFromKeytab instead of UserGroupInformation.loginUserFromKeytab in ClientWrapper will not solve the problem as SparkHadoopUtil is shared and the UserGroupInformation class it includes is not the same one used by SessionState.start in ClientWrapper. Therefore, the program still fails with no tgt exception when connecting to metastore. Also not able to replace the UGI call in SparkSubmit either, as incorrect type of SparkHadoopUtil instance might get created due to yarn mode isn't set in the system until it flows to yarn Client.scala.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by steveloughran <gi...@git.apache.org>.
Github user steveloughran commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9272#discussion_r43867512
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/ClientWrapper.scala ---
    @@ -150,6 +152,14 @@ private[hive] class ClientWrapper(
         val original = Thread.currentThread().getContextClassLoader
         // Switch to the initClassLoader.
         Thread.currentThread().setContextClassLoader(initClassLoader)
    +
    +    val sparkConf = new SparkConf
    +    if (sparkConf.contains("spark.yarn.principal") && sparkConf.contains("spark.yarn.keytab")) {
    +      UserGroupInformation.loginUserFromKeytab(
    --- End diff --
    
    before calling this, actually verify that the keytab file exists and fail with a message including the property name, to help people debug the problem. UGI internal exceptions are rarely informative enough


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10181][SQL] Do kerberos login for crede...

Posted by yolandagao <gi...@git.apache.org>.
Github user yolandagao commented on the pull request:

    https://github.com/apache/spark/pull/9272#issuecomment-152904592
  
    @yhuai Hi Yin, SPARK-11265 is a different issue - in yarn Client.scala code, hive metastore token is obtained when kerberos is enabled to set up AM container launch context, during which Hive and HiveConf instantiation failed due to a new change in Hive 1.2.1. 
        YarnSparkHadoopUtil.get.obtainTokensForNamenodes(nns, hadoopConf, credentials)
        obtainTokenForHiveMetastore(hadoopConf, credentials)
        obtainTokenForHBase(hadoopConf, credentials)
        ....
        setupSecurityToken(amContainer)
        UserGroupInformation.getCurrentUser().addCredentials(credentials)
    
    SPARK-11265 fix will not solve the problem here. In yarn-client mode, Hive client with IsolatedClientLoader has no access to these delegation tokens and kerberos tickets (set up via UGI.loginUserFromKeytab in SparkSubmit), as it has a pure new UserGroupInformation class loaded.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org