-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-21890] Credentials not being passed to add the tokens #19140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ebbcb7f
Credentials not being passed to gather the tokens
0cfca50
Feel fetchCreds is appropriate naming convention
5424972
Fix scala style tests
d72c08f
style tests
075de5d
add back PRINCIPAL check aka spark.yarn.principal for renewing tokens
1184a73
move spark conf to group scala style tests
98f0ff2
fix style tests again
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here the diff in spark2.2 and master
=> is missing PRINCPAL(aka spark.yarn.principal) config. Not sure if we need to do this now. Let me know your opinion @vanzin @tgravescs
sparkConf.get(PRINCIPAL).flatMap { renewer =>
val creds = new Credentials()
hadoopFSsToAccess(hadoopConf, sparkConf).foreach { dst =>
val dstFs = dst.getFileSystem(hadoopConf)
dstFs.addDelegationTokens(renewer, creds)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That code was in
getTokenRenewalInterval; that call is only needed when principal and keytab are provided, so adding the code back should be ok. It shouldn't cause any issues if it's not there, though, aside from a wasted round trip to the NNs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to not call it if we don't need to so as long as adding the config back doesn't mess with the mesos side of things (since this is now common code) I think that would be good. the PRINCIPAL config is yarn specific config, but looking at SparkSubmit it appears to be using for mesos as well.
@vanzin do you happen to know if mesos is using that as well, I haven't kept up with mesos kerberos support. so not sure if more is going to happen there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure Mesos is not currently hooked up to the principal / keytab stuff. It just picks up the initial delegation token set, and when those expire, things stop working.
Adding the check back here is the right thing; it shouldn't affect Mesos when it adds support for principal / keytab (or if it does, it can be fixed at that time).