Skip to content

[SPARK-15754] [yarn] Not letting the credentials containing hdfs delegation tokens to be added in current user credential. #13499

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.

Sign up for GitHub

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
subrotosanyal wants to merge 3 commits into from
Closed

[SPARK-15754] [yarn] Not letting the credentials containing hdfs delegation tokens to be added in current user credential. #13499

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
16ce28c
SPARK-15754 Not letting the credentials containing hdfs delegation to…
Jun 3, 2016
d53ad55
SPARK-15754 Review comment fix: scala coding guideline related to imp…
Jun 3, 2016
3a4c04a
SPARK-15754 Review comment fix: Reverting back the change
Jun 3, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,6 @@ private[spark] class Client(
amContainer.setApplicationACLs(
YarnSparkHadoopUtil.getApplicationAclsForYarn(securityManager).asJava)
setupSecurityToken(amContainer)
UserGroupInformation.getCurrentUser().addCredentials(credentials)

amContainer
}
Expand All @@ -1001,7 +1000,8 @@ private[spark] class Client(
sparkConf.set(KEYTAB.key, keytabFileName)
sparkConf.set(PRINCIPAL.key, principal)
}
credentials = UserGroupInformation.getCurrentUser.getCredentials
// Defensive copy of the credentials
credentials = new Credentials(UserGroupInformation.getCurrentUser.getCredentials)
}

/**
Expand Down
10 changes: 7 additions & 3 deletions yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnClientSchedulerBackend.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,13 @@ import scala.collection.mutable.ArrayBuffer

import org.apache.hadoop.yarn.api.records.YarnApplicationState

import org.apache.spark.{SparkContext, SparkException}
import org.apache.spark.deploy.yarn.{Client, ClientArguments, YarnSparkHadoopUtil}
import org.apache.spark.SparkContext

@vanzin vanzin Jun 3, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are unnecessary, and in fact we prefer the previous style.

import org.apache.spark.SparkException
import org.apache.spark.deploy.yarn.Client
import org.apache.spark.deploy.yarn.ClientArguments
import org.apache.spark.deploy.yarn.YarnSparkHadoopUtil
import org.apache.spark.internal.Logging
import org.apache.spark.internal.config.PRINCIPAL
import org.apache.spark.launcher.SparkAppHandle
import org.apache.spark.scheduler.TaskSchedulerImpl

Expand Down Expand Up @@ -64,7 +68,7 @@ private[spark] class YarnClientSchedulerBackend(
// SPARK-8851: In yarn-client mode, the AM still does the credentials refresh. The driver
// reads the credentials from HDFS, just like the executors and updates its own credentials
// cache.
if (conf.contains("spark.yarn.credentials.file")) {
if (!conf.contains(PRINCIPAL.key) && conf.contains("spark.yarn.credentials.file")) {

@vanzin vanzin Jun 3, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're touching this, you could also use CREDENTIALS_FILE_PATH.

BTW I'm not entirely sure why this change is needed, or related to the issue you reported. You want to start the token renewer code exactly when PRINCIPAL is defined. You're pretty much disabling this code now, because CREDENTIALS_FILE_PATH is never set when PRINCIPAL is not.

@subrotosanyal subrotosanyal Jun 3, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @vanzin
the method: org.apache.spark.deploy.yarn.ExecutorDelegationTokenUpdater.updateCredentialsIfRequired() invokes the same operation of updating the current user credential and correct me if I am wrong; if user has logged in through a keytab file already then ideally there shouldn't be any need to update the credentials at regular intervals. This thread of updating the credential might be useful for executors where there is only CREDENTAILS_FILE or delegation token.

@vanzin vanzin Jun 3, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a different issue, and I'm not sure it's really an issue, given what the comment above this line says.

This code is here for when Spark is managing your kerberos credentials and delegation tokens, i.e., when you pass --principal and --keytab. In that case, as the comment above says, the AM is tasked with renewing the credentials and tokens periodically, and this code exists so that the driver (which is not running in the AM in client mode) can get the new tokens.

You're basically breaking that feature by changing this. If your app is managing the kerberos login, you'd never pass --principal and --keytab (or the equivalent settings) to Spark, so you wouldn't run into this problem.

YarnSparkHadoopUtil.get.startExecutorDelegationTokenRenewer(conf)
}
monitorThread = asyncMonitorApplication()
Expand Down