You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/03/05 20:36:59 UTC
[GitHub] [spark] viirya opened a new pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
viirya opened a new pull request #31761:
URL: https://github.com/apache/spark/pull/31761
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This patch adds a config `spark.kerberos.renewal.exclude.hadoopFileSystems` which lists the filesystems to be excluded from delegation token renewal.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
MapReduce jobs can instruct YARN to skip renewal of tokens obtained from certain hosts by specifying the hosts with configuration mapreduce.job.hdfs-servers.token-renewal.exclude=<host1>,<host2>,..,<hostN>.
But seems Spark lacks of similar option. So the job submission fails if YARN fails to renew DelegationToken for any of the remote HDFS cluster. The failure in DT renewal can happen due to many reason like Remote HDFS does not trust Kerberos identity of YARN etc. We have a customer facing such issue.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No, if the config is not set. Yes, as users can use this config to instruct YARN not to renew delegation token from certain filesystems.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
It is hard to do unit test for this. We did verify it work from the customer using this fix in the production environment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791893355
cc @tgravescs
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791719065
**[Test build #135820 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135820/testReport)** for PR 31761 at commit [`e11cdb9`](https://github.com/apache/spark/commit/e11cdb9af8f6dc4dcefb5e81abe86dfa3bcfa611).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588953689
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,14 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs for which to exclude from delegation token renewal.")
Review comment:
Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791799867
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135820/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589882783
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The hosts on which the file systems to be excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
+ // RM skips renewing token with empty renewer
Review comment:
Hmm does it only apply to YARN though? It seems Spark has its own `HadoopDelegationTokenManager` which is separated from YARN. Also, it seems Spark has its own renewal logic and the empty string for renewer approach won't work. See `HadoopDelegationTokenManager.scheduleRenewal`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r595391725
##########
File path: docs/security.md
##########
@@ -838,6 +838,17 @@ The following options provides finer-grained control for this feature:
</td>
<td>3.0.0</td>
</tr>
+<tr>
+ <td><code>spark.kerberos.renewal.exclude.hadoopFileSystems</code></td>
+ <td>(none)</td>
+ <td>
+ A comma-separated list of Hadoop filesystems for whose hosts will be excluded from from delegation
+ token renewal at resource scheduler. For example, <code>spark.kerberos.renewal.exclude.hadoopFileSystems=hdfs://nn1.com:8032,
+ hdfs://nn2.com:8032</code>. This is known to work under YARN for now, so YARN Resource Manager won't renew tokens for the application.
+ Note that as resource scheduler does not renew token, the application might not be long running once the token expires.
Review comment:
I had a suggestion above for rephrasing the text here, something more like:
Note that as resource scheduler does not renew token, so any application running longer than the original token expiration that tries to use that token will likely fail.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-799839438
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136086/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-804114195
sorry for my delay, I was out, mostly good, I think if we just move docs to yarn section its good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588969225
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The filesystems excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
Review comment:
No, we can accept hosts directly. It doesn't matter here. Either URL or host can achieve the same result we want.
It takes file system URLs because I let it to be consistent with other configs like `KERBEROS_FILESYSTEMS_TO_ACCESS`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r591059181
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
If I understand correctly, it is the same code path for obtaining delegation token and renewing. Actually the so called renew is that Spark periodically obtain delegation token.
So I guess it doesn't make sense for a case to get delegation token but not renew it. At least in Spark I don't see such case is possible. I may wrong here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791768736
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40402/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588955475
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The filesystems excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
Review comment:
I'm also wondering why unlike the MR config we don't use hosts directly but rather URLs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792714252
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135862/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792071350
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135835/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792054274
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40417/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588972518
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
It makes sense for `spark.kerberos.access.hadoopFileSystems` to be URLs since Spark needs to instantiate `FileSystem`s from them. But for this case I'm not sure if it's necessary: we can just parse the config into a set of host names and check whether the file systems above contain them:
```scala
val hostsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE).toSet
filesystems.filter(fs => !hostsToExclude.contains(fs.getUri.getHost).foreach { fs =>
...
}
```
I'm fine either way though since it also makes sense to keep it consistent with `spark.kerberos.access.hadoopFileSystems`. BTW I think we'll need to update security.md for the new config.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792054274
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40417/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792221074
**[Test build #135838 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135838/testReport)** for PR 31761 at commit [`faa85fd`](https://github.com/apache/spark/commit/faa85fd44e088756f320aa9b25354855ee6982ec).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-803462935
**[Test build #136289 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/136289/testReport)** for PR 31761 at commit [`71acd5c`](https://github.com/apache/spark/commit/71acd5ca7cb0687cc9991f563cbf559703a48ca2).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-799166105
Thanks all for the review. I updated the config doc and security.md. Hopefully it addresses the concerns.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589467185
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
I guess it could be the staging or default Filesystem as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-801254875
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136171/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-804397486
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40947/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792047964
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40417/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-804397451
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40947/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-804433757
**[Test build #136362 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/136362/testReport)** for PR 31761 at commit [`cf8da75`](https://github.com/apache/spark/commit/cf8da757045d23f5657ec1fb68e20f4ca0247772).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r592889372
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
Ah OK I think I missed some part of @tgravescs 's comment. My bad.
His point wasn't that the new config is not necessary at all. His point was that the problem will occur from only defaultFs and/or stageFs, so the new config could be simplified instead of being general but a bit verbose.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r595473936
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,18 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
Review comment:
`spark.yarn.kerberos.renewal.excludeHadoopFileSystems` looks okay to me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588896903
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The filesystems excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
Review comment:
Since this excludes all file systems on that host, shall we revise the description in line 107?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792235761
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135838/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589308695
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
Review comment:
I see the point. On the other hand the other use-case is that Spark user lets Spark to obtain tokens and such case the added configuration has effect. In the mentioned case the name is misleading because it has effect on the initial obtain too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589881790
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
It's hard to imagine that this applies to defaultFS/stagingDir since it means the YARN cluster (or other types of clusters) is not configured to be in the same Kerberos realm as these which could cause other more serious issues. But, just as `spark.kerberos.access.hadoopFileSystems`, I guess nothing stops users from putting in the host for defaultFS/stagingDir in the config as well, and Spark will just do what it's told to do.
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The hosts on which the file systems to be excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
+ // RM skips renewing token with empty renewer
Review comment:
Hmm does it only apply to YARN though? It seems Spark has its own `HadoopDelegationTokenManager` which is separated from YARN. It has its own renewal logic as well and I'm not sure we even need to use "" for renewer.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-804332367
Thank you @tgravescs.
I'll still leave this open for one or two days. Then I will merge it if no more comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r590889740
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The hosts on which the file systems to be excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
+ // RM skips renewing token with empty renewer
Review comment:
We only test it under YARN and verify it works to skip the renewal from YARN if we give an empty renewer. I'm not sure if other resource scheduler follows this behavior. So I can just document the config to mention it is known to work for YARN.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589099378
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The hosts on which the file systems to be excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
+ // RM skips renewing token with empty renewer
Review comment:
Just to understand properly, does `RM` mean Resource Manager? Now the delegation token handling is not only done for Spark on yarn but also for other resource schedulers as well, so probably better to remove specific resource scheduler's term/details.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589606273
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The hosts on which the file systems to be excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
Review comment:
nit: we don't have to get file system here, maybe just
```scala
map(new Path(_).toUri.getHost)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r591358375
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The hosts on which the file systems to be excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
+ // RM skips renewing token with empty renewer
+ logInfo(s"getting token for: $fs with empty renewer to skip renewal")
+ fs.addDelegationTokens("", creds)
+ } else {
+ logInfo(s"getting token for: $fs with renewer $renewer")
Review comment:
This is not required here but lately I've seen that `$fs` produces a really cryptic out (which was 0 help in debugging) and maybe we can print out the `Uri` or something more meaningful. But again, this is a nice to have separately.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791743359
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40402/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792714252
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135862/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791799867
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135820/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792366731
I haven't encountered such so don't feel I'm qualified to approve, but I see what the PR is trying to do and it makes sense. Generally looks OK.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792235761
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135838/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589281157
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
Review comment:
Since `HadoopFSDelegationTokenProvider` obtains the initial tokens and not just renewing them why do we call it `...renewal...`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791897203
> According to the PR description, is this a bug, @viirya ?
I think it is not a bug. Spark currently just doesn't support to exclude specific filesystems from DT renewal. But MapReduce supports it. So if the user has such requirement, the user cannot submit Spark job to YARN because YARN is unable to renew the DT.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya edited a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya edited a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791897203
I think it is not a bug. Spark currently just doesn't support to exclude specific filesystems from DT renewal. But MapReduce supports it. So if the user has such requirement, the user cannot submit Spark job to YARN because YARN is unable to renew the DT.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-801254875
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136171/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588972957
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
Thanks. Let me update security.md.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r595401992
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,18 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
Review comment:
I also think we should change the name fo this. we have spark.kerberos.renewal.credentials that is talking about how Spark renewals. This has the same prefix but is not about spark renewal, its about resource scheduler renewal.
Honestly at this point if no one knows of a k8s component that could do this I would almost say we make the config yarn specific - spark.yarn.kerberos.renewal.excludeHadoopFileSystems. I would actually like to see rm or resourceScheduler in the name to make it even more clear but that gets really long. Open to other opinions but we can always make a generic one later. If someone has an idea how this would be used with others we can leave it generic to not yarn but I want something else in the name to show its resource scheduler specific.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588911535
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,14 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs for which to exclude from delegation token renewal.")
Review comment:
<del>This configuration doesn't ban the file system, but excludes it from DT renewal. The user needs to use the file system but YARN cannot renew it.</del>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-804334403
**[Test build #136362 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/136362/testReport)** for PR 31761 at commit [`cf8da75`](https://github.com/apache/spark/commit/cf8da757045d23f5657ec1fb68e20f4ca0247772).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-799853853
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40669/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792711924
**[Test build #135862 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135862/testReport)** for PR 31761 at commit [`397ebd2`](https://github.com/apache/spark/commit/397ebd298d239cc900044fbd392e365ac21c0601).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589100405
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -119,7 +133,7 @@ private[deploy] class HadoopFSDelegationTokenProvider
val renewer = UserGroupInformation.getCurrentUser().getUserName()
val creds = new Credentials()
- fetchDelegationTokens(renewer, filesystems, creds)
+ fetchDelegationTokens(renewer, filesystems, creds, hadoopConf, sparkConf)
val renewIntervals = creds.getAllTokens.asScala.filter {
Review comment:
Just curious, suppose we have a fs which we exclude the renewal, then my understanding on behavior is it fetches the delegation token but won't renew. Do I understand correctly? Just curious about how these credentials will be handled from here, like whether they're not `AbstractDelegationTokenIdentifier`, or `token.renew` fails but Try will swallow.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-803477181
**[Test build #136289 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/136289/testReport)** for PR 31761 at commit [`71acd5c`](https://github.com/apache/spark/commit/71acd5ca7cb0687cc9991f563cbf559703a48ca2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-799856640
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40669/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-799850349
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40669/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r592897399
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
> so just to be clear, these fileystems are ones the user specified in spark.kerberos.access.hadoopFileSystems so it wants to get initial tokens for them but then later we don't want to renew, correct? We should perhaps update doc to mention that.
> It would be nice to know if this works with other cluster managers like k8s - @ifilonenko @mccheah maybe?
Hmm, for the customer case, the requirement is to prevent YARN from renewing the delegation token obtained. It doesn't matter if the file system is defaultFs/stageFs or the ones specified in `spark.kerberos.access.hadoopFileSystems`. That said, Spark still can obtain the token for the ones in `spark.kerberos.access.hadoopFileSystems`, but we don't want YARN to renew it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-804334403
**[Test build #136362 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/136362/testReport)** for PR 31761 at commit [`cf8da75`](https://github.com/apache/spark/commit/cf8da757045d23f5657ec1fb68e20f4ca0247772).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-799198684
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589501811
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
@tgravescs with the actual code Spark doesn't obtain the initial tokens when an FS is listed in "spark.kerberos.renewal.exclude.hadoopFileSystems". Please see my comment below...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r594652477
##########
File path: docs/security.md
##########
@@ -838,6 +838,17 @@ The following options provides finer-grained control for this feature:
</td>
<td>3.0.0</td>
</tr>
+<tr>
+ <td><code>spark.kerberos.renewal.exclude.hadoopFileSystems</code></td>
+ <td>(none)</td>
+ <td>
+ A comma-separated list of Hadoop filesystems for whose hosts will be excluded from from delegation
+ token renewal at resource scheduler. For example, <code>spark.kerberos.renewal.exclude.hadoopFileSystems=hdfs://nn1.com:8032,
+ hdfs://nn2.com:8032</code>. This is known to work under YARN for now, so YARN Resource Manager won't renew tokens for the application.
+ Note that as resource scheduler does not renew token, the application might not be long running once the token expires.
Review comment:
As far as I understand, this config doesn't affect Spark's renewal of tokens, but avoid YARN RM's renewal. That is different. The token might be invalid if Spark doesn't renew it before expiration. Whether Spark renews the token is depending on https://github.com/apache/spark/pull/31761#discussion_r591350982.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r591012019
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
Review comment:
Sorry I am confused. :) So you said this configuration also affects initial obtain too. It means if users set a file system in the configuration, Spark won't obtain initial token for it? Or I misunderstood above comments?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791773894
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40402/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-803462935
**[Test build #136289 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/136289/testReport)** for PR 31761 at commit [`71acd5c`](https://github.com/apache/spark/commit/71acd5ca7cb0687cc9991f563cbf559703a48ca2).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
mridulm commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-796198220
If enabling this means the application be long running, is that documented some place @viirya ? If not, can you add a note ? Thx.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792644070
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40445/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589637483
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
Hm? @gaborgsomogyi I think this config doesn't affect Spark obtain the initial tokens?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792071350
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135835/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-804454165
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136362/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r591350982
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
@HeartSaVioR Spark actually behaves this way (w/o this change): if one DT is created then it's renewed all the time except in the following cases:
* `keytab` used but no `principal` provided
* `ccache` used but it has no kerberos credentials
Please see the code here: https://github.com/apache/spark/blob/a916690dd9aac40df38922dbea233785354a2f2a/core/src/main/scala/org/apache/spark/deploy/security/HadoopDelegationTokenManager.scala#L83-L88
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r598995765
##########
File path: docs/security.md
##########
@@ -838,6 +838,18 @@ The following options provides finer-grained control for this feature:
</td>
<td>3.0.0</td>
</tr>
+<tr>
+ <td><code>spark.yarn.kerberos.renewal.excludeHadoopFileSystems</code></td>
Review comment:
Moved. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792229807
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40420/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792212023
cc @HeartSaVioR and @gaborgsomogyi FYI
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-799198684
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40636/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r594749886
##########
File path: docs/security.md
##########
@@ -838,6 +838,17 @@ The following options provides finer-grained control for this feature:
</td>
<td>3.0.0</td>
</tr>
+<tr>
+ <td><code>spark.kerberos.renewal.exclude.hadoopFileSystems</code></td>
+ <td>(none)</td>
+ <td>
+ A comma-separated list of Hadoop filesystems for whose hosts will be excluded from from delegation
+ token renewal at resource scheduler. For example, <code>spark.kerberos.renewal.exclude.hadoopFileSystems=hdfs://nn1.com:8032,
+ hdfs://nn2.com:8032</code>. This is known to work under YARN for now, so YARN Resource Manager won't renew tokens for the application.
+ Note that as resource scheduler does not renew token, the application might not be long running once the token expires.
Review comment:
Okay, this sounds a good point to me. I think I should not change the behavior of `getTokenRenewalInterval` because it is not related to the issue here. The renew call in `getTokenRenewalInterval` is just used for obtaining renewal interval for all FS tokens.
I assume this config is only for YARN-specific behavior. I documented it explicitly about YARN in the config doc/security.md. Is any other thing I should do for it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-804397486
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40947/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589638371
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
Review comment:
Do you mean with empty renewer `addDelegationTokens` doesn't work to obtain the initial token if the token is not present?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589465996
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
so just to be clear, these fileystems are ones the user specified in spark.kerberos.access.hadoopFileSystems so it wants to get initial tokens for them but then later we don't want to renew, correct? We should perhaps update doc to mention that.
It would be nice to know if this works with other cluster managers like k8s - @ifilonenko @mccheah maybe?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya closed pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
viirya closed pull request #31761:
URL: https://github.com/apache/spark/pull/31761
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-803461496
@tgravescs Thanks for the review. Do you have any other comment I need to address?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588956359
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
Is this only restricted to HDFS URLs? can "hosts" either be nameservice name or namenode name? some examples might help.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589299302
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
Review comment:
It seems related to YARN behavior. YARN will try to renew delegation token when the job is submitted to YARN. For the customer case, the delegation token is already obtained when calling `spark-submit`. But YARN is unable to renew the DT and refuses the job. This config is for such case to avoid YARN to renew the DT.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r591340918
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The hosts on which the file systems to be excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
+ // RM skips renewing token with empty renewer
Review comment:
Oh gosh, maybe I'm working on too many stuff in parallel and was watching bad TV here :)
Today morning I've put other tasks away and has taken the time and re-checked/re-tested the code and it has no influence on the initial obtain.
This can be resolved and I'm fine with the code as-is.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r590952411
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
I'm not sure I understand correctly so just consider it as ignorant question.
> Anything else doesn't get renewed unless its in spark.kerberos.access.hadoopFileSystems and in that case just don't include it in spark.kerberos.access.hadoopFileSystems.
Is it possible if someone wants to get initial delegation token for them but doesn't renew them, and does this change make difference from here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589638371
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
Review comment:
Do you mean with empty renewer `addDelegationTokens` doesn't work to obtain the token?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792052738
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40417/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r590891367
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
From the user perspective, I think it is still possible they want to avoid renewal for defaultFs. If the defaultFs is just a remote HDFS that whose tokens cannot be renew from YARN?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-805595373
Thanks all for the review. Merging to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r594168255
##########
File path: docs/security.md
##########
@@ -838,6 +838,17 @@ The following options provides finer-grained control for this feature:
</td>
<td>3.0.0</td>
</tr>
+<tr>
+ <td><code>spark.kerberos.renewal.exclude.hadoopFileSystems</code></td>
+ <td>(none)</td>
+ <td>
+ A comma-separated list of Hadoop filesystems for whose hosts will be excluded from from delegation
+ token renewal at resource scheduler. For example, <code>spark.kerberos.renewal.exclude.hadoopFileSystems=hdfs://nn1.com:8032,
+ hdfs://nn2.com:8032</code>. This is known to work under YARN for now, so YARN Resource Manager won't renew tokens for the application.
+ Note that as resource scheduler does not renew token, the application might not be long running once the token expires.
Review comment:
`might not be long running` is a little bit soft in my view. Can you imagine a situation where token expires and the workload goes on successfully?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792068030
**[Test build #135835 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135835/testReport)** for PR 31761 at commit [`d6c8f9e`](https://github.com/apache/spark/commit/d6c8f9e3b422b99ecbb560b96f52376c77166592).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-796356582
> If enabling this means the application cannot be long running, is that documented some place @viirya ? If not, can you add a note ? Thx.
This should only be applied on the token which is obtained before calling `spark-submit` and the token is unable to renew by YARN. So excluding from renewal sounds it not for long-running applications? I can add a note to the config doc if you think it is better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589658117
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
Right, the point I was getting at is that we only renew the ones in spark.kerberos.access.hadoopFileSystems + defaultFs + staging fs. So I assume its basically defaultFs then? I was kind of wanting to make sure it wasn't something silly that they specified in both. It seems odd for the defaultFs since this is the Hadoop filesystem which supports renewal. I can obviously see this if its a separate name node but it would require you to specify it in the config for us to try to renew. So I'm assuming this is perhaps an odd config that you can't renew on the current default fs. Just seems a bit odd case so wanted to make sure I wasn't missing something.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-799227623
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136053/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r594749886
##########
File path: docs/security.md
##########
@@ -838,6 +838,17 @@ The following options provides finer-grained control for this feature:
</td>
<td>3.0.0</td>
</tr>
+<tr>
+ <td><code>spark.kerberos.renewal.exclude.hadoopFileSystems</code></td>
+ <td>(none)</td>
+ <td>
+ A comma-separated list of Hadoop filesystems for whose hosts will be excluded from from delegation
+ token renewal at resource scheduler. For example, <code>spark.kerberos.renewal.exclude.hadoopFileSystems=hdfs://nn1.com:8032,
+ hdfs://nn2.com:8032</code>. This is known to work under YARN for now, so YARN Resource Manager won't renew tokens for the application.
+ Note that as resource scheduler does not renew token, the application might not be long running once the token expires.
Review comment:
Okay, this sounds a good point to me. I think I should not change the behavior of `getTokenRenewalInterval` because it is not related to the issue here. The renew call in `getTokenRenewalInterval` is just used for obtaining renewal interval for all FS tokens.
Throwing exception during individual `renew` call will be ignored. The actual renewal internal will be the minimum among all internals. But yes this could be a behavior change if the token with empty renewer is actually the minimum one and we cannot get it now because of the exception.
Let me restore the behavior of `getTokenRenewalInterval` to make it safer.
I assume this config is only for YARN-specific behavior. I documented it explicitly about YARN in the config doc/security.md. Is any other thing I should do for it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-803469447
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-803461375
retest this please
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589882783
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The hosts on which the file systems to be excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
+ // RM skips renewing token with empty renewer
Review comment:
Hmm does it only apply to YARN though? It seems Spark has its own `HadoopDelegationTokenManager` which is separated from YARN. Also, it seems Spark has its own renewal logic and the empty string for renewer approach won't work.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792229807
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40420/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-804454165
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136362/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-803487534
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136289/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r590905476
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
yes its true like I mentioned above. I'm not against it just want to understand and make sure this config makes sense. For instance instead of having this config we could have one that says don't include defaultFs or stageFs or both in renewal. I think those are really the only 2 filesystems that make sense to put into this config. Anything else doesn't get renewed unless its in spark.kerberos.access.hadoopFileSystems and in that case just don't include it in spark.kerberos.access.hadoopFileSystems. The nice things about this approach is its generic if things change in the future. The downside is I have to know namenode path for the defaultFs and staging fs to add to this config.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r593361450
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
Okay, thanks @tgravescs. I think in short this config doesn't affect Spark token handling, but only prevent YARN from renewing tokens, no matter the tokens are obtained in advance or by Spark. Let me update security.md and try to explain it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588897073
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,14 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs for which to exclude from delegation token renewal.")
Review comment:
This might mislead the users because this configuration bans all file system from that host.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792644070
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40445/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-803469447
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40871/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792624085
**[Test build #135862 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135862/testReport)** for PR 31761 at commit [`397ebd2`](https://github.com/apache/spark/commit/397ebd298d239cc900044fbd392e365ac21c0601).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589663480
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
Note I'm not against it just making sure the problem is understood and we document it appropriately. I guess it could be stage FS as well, which perhaps might make more sense if on a separate namenoxde
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589239763
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -119,7 +133,7 @@ private[deploy] class HadoopFSDelegationTokenProvider
val renewer = UserGroupInformation.getCurrentUser().getUserName()
val creds = new Credentials()
- fetchDelegationTokens(renewer, filesystems, creds)
+ fetchDelegationTokens(renewer, filesystems, creds, hadoopConf, sparkConf)
val renewIntervals = creds.getAllTokens.asScala.filter {
Review comment:
The customer uses spark-submit to submit Spark job to YARN, I think the customer already obtains delegation token for the remote HDFS. YARN will try to renew the delegation token when we submit job to it, but the YARN cluster is not able to renew it and fails.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-797442051
Not renewing specific tokens implicitly means the workload won't work after it's validity period (so I don't feel it must be added), though it doesn't hurt to add such note to be 100% clear. Good examples are the cases I've mentioned [here](https://github.com/apache/spark/pull/31761#discussion_r591350982).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791773894
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40402/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791719065
**[Test build #135820 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135820/testReport)** for PR 31761 at commit [`e11cdb9`](https://github.com/apache/spark/commit/e11cdb9af8f6dc4dcefb5e81abe86dfa3bcfa611).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm edited a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
mridulm edited a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-796198220
If enabling this means the application cannot be long running, is that documented some place @viirya ? If not, can you add a note ? Thx.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-799856640
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40669/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
mridulm commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-794690457
Curious about this usecase @viirya ... how does the DT get renewed ? Or is the DT required only at initial job submission time ?
Any reasonably long running job will fail if the acquired token cant be renewed, right ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r592889372
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
Ah OK I think I missed some part of @tgravescs 's comment. My bad.
His point wasn't that the config is not necessary at all. His point was that the problem will occur from only defaultFs and/or stageFs, so the new config could be simplified instead of being general but a bit verbose.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-799839438
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136086/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r594697177
##########
File path: docs/security.md
##########
@@ -838,6 +838,17 @@ The following options provides finer-grained control for this feature:
</td>
<td>3.0.0</td>
</tr>
+<tr>
+ <td><code>spark.kerberos.renewal.exclude.hadoopFileSystems</code></td>
+ <td>(none)</td>
+ <td>
+ A comma-separated list of Hadoop filesystems for whose hosts will be excluded from from delegation
+ token renewal at resource scheduler. For example, <code>spark.kerberos.renewal.exclude.hadoopFileSystems=hdfs://nn1.com:8032,
+ hdfs://nn2.com:8032</code>. This is known to work under YARN for now, so YARN Resource Manager won't renew tokens for the application.
+ Note that as resource scheduler does not renew token, the application might not be long running once the token expires.
Review comment:
oh right. Please update the description on the issue to be what you are really trying to do here.
I'm not sure that is completely true though either. getTokenRenewalInterval fetches the delegation tokens in fetchDelegationTokens and then calls renew on them to get the next expiration time. I'm assuming your changes in fetchDelegationTokens makes renew not work. What does renew return in this case that renewer is empty? It looks like it should throw an exception going by the base Hadoop classes I looked at.
Assuming its not the actual next renewal time since you say it doesn't get renewed if this tokens expiration is actually less than all the other tokens than we won't properly renew it on the Spark side either. That is in the case the key tab was specified. If it throws an exception that is even worse.
I'm also wondering out loud if this config would ever apply to anything other than YARN. there is no renewal component in k8s that I'm aware of
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-804393426
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40947/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-801253287
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40753/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r594343928
##########
File path: docs/security.md
##########
@@ -838,6 +838,17 @@ The following options provides finer-grained control for this feature:
</td>
<td>3.0.0</td>
</tr>
+<tr>
+ <td><code>spark.kerberos.renewal.exclude.hadoopFileSystems</code></td>
+ <td>(none)</td>
+ <td>
+ A comma-separated list of Hadoop filesystems for whose hosts will be excluded from from delegation
+ token renewal at resource scheduler. For example, <code>spark.kerberos.renewal.exclude.hadoopFileSystems=hdfs://nn1.com:8032,
+ hdfs://nn2.com:8032</code>. This is known to work under YARN for now, so YARN Resource Manager won't renew tokens for the application.
+ Note that as resource scheduler does not renew token, the application might not be long running once the token expires.
Review comment:
the workload can definitely go on until it tries to access that filesystem. There is no reason it couldn't fully succeed depending on where its writing data or if it writes data at all.
We definitely could clarify though, perhaps something more like:
, so any application running longer than the original token expiration that tries to use that token will fail.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r594652477
##########
File path: docs/security.md
##########
@@ -838,6 +838,17 @@ The following options provides finer-grained control for this feature:
</td>
<td>3.0.0</td>
</tr>
+<tr>
+ <td><code>spark.kerberos.renewal.exclude.hadoopFileSystems</code></td>
+ <td>(none)</td>
+ <td>
+ A comma-separated list of Hadoop filesystems for whose hosts will be excluded from from delegation
+ token renewal at resource scheduler. For example, <code>spark.kerberos.renewal.exclude.hadoopFileSystems=hdfs://nn1.com:8032,
+ hdfs://nn2.com:8032</code>. This is known to work under YARN for now, so YARN Resource Manager won't renew tokens for the application.
+ Note that as resource scheduler does not renew token, the application might not be long running once the token expires.
Review comment:
As far as I understand, this config doesn't affect Spark's renewal of tokens, but avoid YARN RM's renewal. That is different. The token might be invalid if Spark doesn't renew it before expiration. Whether Spark renews the token is depending on https://github.com/apache/spark/pull/31761#discussion_r591350982. Please correct me if I misunderstand it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-801253287
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40753/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589244417
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The hosts on which the file systems to be excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
+ // RM skips renewing token with empty renewer
Review comment:
Actually you remind me that I should document it this is only for YARN. As the PR description mentions, this is for YARN behavior. I am not sure if other resource scheduler follows the same behavior here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588911535
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,14 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs for which to exclude from delegation token renewal.")
Review comment:
This configuration doesn't ban the file system, but excludes it from DT renewal. The user needs to use the file system but YARN cannot renew it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-803467530
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40871/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r591340918
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The hosts on which the file systems to be excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
+ // RM skips renewing token with empty renewer
Review comment:
Oh gosh, maybe I'm working on too many stuff in parallel and was watching bad TV here :)
Today morning I've put other tasks away and has taken the time and re-checked/re-tested the code and it has no influence on the initial obtain.
The comment can be resolved and I'm fine with the code as-is.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gaborgsomogyi commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r590112061
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
Review comment:
I mean the code works because I've tested it. My suggestion is not to name the config variable `...renewal...` because it has effect in the initial obtain too. The code is good as-is and good job :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792624085
**[Test build #135862 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135862/testReport)** for PR 31761 at commit [`397ebd2`](https://github.com/apache/spark/commit/397ebd298d239cc900044fbd392e365ac21c0601).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r593257557
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
I was simply trying to clarify the exact case being hit here and make sure there wasn't an alternate solution and make sure the docs are clear. The specific case being hit could affect the solution.
I think there are comments on this review that are very confusing, thus why I wanted to clarification. Some indicate that Spark doesn't get initial tokens, others saying in this case the tokens were already acquired, etc.
In the end my comment ends up being I think we should update the security.md doc to mention renewal in the kerberos section for Hadoop filesystems to help explain to users.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588911645
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The filesystems excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
Review comment:
`The hosts on which the file systems to be excluded from token renewal`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792221074
**[Test build #135838 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135838/testReport)** for PR 31761 at commit [`faa85fd`](https://github.com/apache/spark/commit/faa85fd44e088756f320aa9b25354855ee6982ec).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-803468033
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40871/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792234628
**[Test build #135838 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135838/testReport)** for PR 31761 at commit [`faa85fd`](https://github.com/apache/spark/commit/faa85fd44e088756f320aa9b25354855ee6982ec).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792035220
**[Test build #135835 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135835/testReport)** for PR 31761 at commit [`d6c8f9e`](https://github.com/apache/spark/commit/d6c8f9e3b422b99ecbb560b96f52376c77166592).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal at YARN
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r598773784
##########
File path: docs/security.md
##########
@@ -838,6 +838,18 @@ The following options provides finer-grained control for this feature:
</td>
<td>3.0.0</td>
</tr>
+<tr>
+ <td><code>spark.yarn.kerberos.renewal.excludeHadoopFileSystems</code></td>
Review comment:
this should be in the yarn specific kerberos section -> YARN-specific Kerberos Configuration (http://spark.apache.org/docs/latest/running-on-yarn.html). and we should link to it from the security.md doc i
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588953655
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The filesystems excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
Review comment:
Ya. Maybe.
BTW, we are accept `hadoopFileSystems` via 'spark.kerberos.renewal.exclude.hadoopFileSystem' and convert it to hosts?
Could you describe some examples where we cannot accept `hosts` directly?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588914495
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,14 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs for which to exclude from delegation token renewal.")
Review comment:
I think I got your point. Update the config doc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r588969592
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -691,6 +691,15 @@ package object config {
.toSequence
.createWithDefault(Nil)
+ private[spark] val KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE =
+ ConfigBuilder("spark.kerberos.renewal.exclude.hadoopFileSystems")
+ .doc("The list of Hadoop filesystem URLs whose hosts will be excluded from " +
Review comment:
Because the configuration value will be parsed as file system URLs to get file system. A simple host name doesn't work here.
e.g.
```scala
scala> new Path("hdfs.namenode.net").getFileSystem(new Configuration()).getUri.getHost
res9: String = null
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-792035220
**[Test build #135835 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135835/testReport)** for PR 31761 at commit [`d6c8f9e`](https://github.com/apache/spark/commit/d6c8f9e3b422b99ecbb560b96f52376c77166592).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-794693848
> Curious about this usecase @viirya ... how does the DT get renewed ? Or is the DT required only at initial job submission time ?
> Any reasonably long running job will fail if the acquired token cant be renewed, right ?
Yea, I think the customer doesn't care about renew and the job is finished before the DT expires. Currently the job cannot be submitted successfully to YARN.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r594351479
##########
File path: docs/security.md
##########
@@ -838,6 +838,17 @@ The following options provides finer-grained control for this feature:
</td>
<td>3.0.0</td>
</tr>
+<tr>
+ <td><code>spark.kerberos.renewal.exclude.hadoopFileSystems</code></td>
+ <td>(none)</td>
+ <td>
+ A comma-separated list of Hadoop filesystems for whose hosts will be excluded from from delegation
+ token renewal at resource scheduler. For example, <code>spark.kerberos.renewal.exclude.hadoopFileSystems=hdfs://nn1.com:8032,
+ hdfs://nn2.com:8032</code>. This is known to work under YARN for now, so YARN Resource Manager won't renew tokens for the application.
+ Note that as resource scheduler does not renew token, the application might not be long running once the token expires.
Review comment:
so my intent was actually to add more description up on like line 792 and add to the "When using a Hadoop filesystem " bit.
Perhaps something like:
service hosting the user's home directory and staging directory. Spark will renew Hadoop filesystem delegation tokens before their expiration unless the token is excluded via spark.kerberos.renewal.exclude.hadoopFileSystems. Please note that if the token is not renewed, any application that attempts to access the filesystem associated with that token after it expired will likely fail.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31761:
URL: https://github.com/apache/spark/pull/31761#issuecomment-791790957
**[Test build #135820 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135820/testReport)** for PR 31761 at commit [`e11cdb9`](https://github.com/apache/spark/commit/e11cdb9af8f6dc4dcefb5e81abe86dfa3bcfa611).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #31761: [SPARK-34295][CORE] Exclude filesystems from token renewal
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #31761:
URL: https://github.com/apache/spark/pull/31761#discussion_r589882783
##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -99,11 +100,24 @@ private[deploy] class HadoopFSDelegationTokenProvider
private def fetchDelegationTokens(
renewer: String,
filesystems: Set[FileSystem],
- creds: Credentials): Credentials = {
+ creds: Credentials,
+ hadoopConf: Configuration,
+ sparkConf: SparkConf): Credentials = {
+
+ // The hosts on which the file systems to be excluded from token renewal
+ val fsToExclude = sparkConf.get(KERBEROS_FILESYSTEM_RENEWAL_EXCLUDE)
+ .map(new Path(_).getFileSystem(hadoopConf).getUri.getHost)
+ .toSet
filesystems.foreach { fs =>
- logInfo(s"getting token for: $fs with renewer $renewer")
- fs.addDelegationTokens(renewer, creds)
+ if (fsToExclude.contains(fs.getUri.getHost)) {
+ // RM skips renewing token with empty renewer
Review comment:
Hmm does it only apply to YARN though? It seems Spark has its own `HadoopDelegationTokenManager` which is separated from YARN. Also, it seems Spark has its own renewal logic and I'm not sure how the empty string for renewer approach work in the case. See `HadoopDelegationTokenManager.scheduleRenewal`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org