-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-25887][K8S] Configurable K8S context support #22904
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
Changes from all commits
7fa5097
8fc744d
f8b0632
c76f752
5e4c340
7832480
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,19 +21,21 @@ import java.io.File | |
| import com.google.common.base.Charsets | ||
| import com.google.common.io.Files | ||
| import io.fabric8.kubernetes.client.{ConfigBuilder, DefaultKubernetesClient, KubernetesClient} | ||
| import io.fabric8.kubernetes.client.Config.autoConfigure | ||
| import io.fabric8.kubernetes.client.utils.HttpClientUtils | ||
| import okhttp3.Dispatcher | ||
|
|
||
| import org.apache.spark.SparkConf | ||
| import org.apache.spark.deploy.k8s.Config._ | ||
| import org.apache.spark.internal.Logging | ||
| import org.apache.spark.util.ThreadUtils | ||
|
|
||
| /** | ||
| * Spark-opinionated builder for Kubernetes clients. It uses a prefix plus common suffixes to | ||
| * parse configuration keys, similar to the manner in which Spark's SecurityManager parses SSL | ||
| * options for different components. | ||
| */ | ||
| private[spark] object SparkKubernetesClientFactory { | ||
| private[spark] object SparkKubernetesClientFactory extends Logging { | ||
|
|
||
| def createKubernetesClient( | ||
| master: String, | ||
|
|
@@ -42,9 +44,6 @@ private[spark] object SparkKubernetesClientFactory { | |
| sparkConf: SparkConf, | ||
| defaultServiceAccountToken: Option[File], | ||
| defaultServiceAccountCaCert: Option[File]): KubernetesClient = { | ||
|
|
||
| // TODO [SPARK-25887] Support configurable context | ||
|
|
||
| val oauthTokenFileConf = s"$kubernetesAuthConfPrefix.$OAUTH_TOKEN_FILE_CONF_SUFFIX" | ||
| val oauthTokenConf = s"$kubernetesAuthConfPrefix.$OAUTH_TOKEN_CONF_SUFFIX" | ||
| val oauthTokenFile = sparkConf.getOption(oauthTokenFileConf) | ||
|
|
@@ -67,8 +66,16 @@ private[spark] object SparkKubernetesClientFactory { | |
| val dispatcher = new Dispatcher( | ||
| ThreadUtils.newDaemonCachedThreadPool("kubernetes-dispatcher")) | ||
|
|
||
| // TODO [SPARK-25887] Create builder in a way that respects configurable context | ||
| val config = new ConfigBuilder() | ||
| // Allow for specifying a context used to auto-configure from the users K8S config file | ||
| val kubeContext = sparkConf.get(KUBERNETES_CONTEXT).filter(_.nonEmpty) | ||
| logInfo(s"Auto-configuring K8S client using " + | ||
| kubeContext.map("context " + _).getOrElse("current context") + | ||
| s" from users K8S config file") | ||
|
|
||
| // Start from an auto-configured config with the desired context | ||
| // Fabric 8 uses null to indicate that the users current context should be used so if no | ||
| // explicit setting pass null | ||
| val config = new ConfigBuilder(autoConfigure(kubeContext.getOrElse(null))) | ||
|
||
| .withApiVersion("v1") | ||
| .withMasterUrl(master) | ||
| .withWebsocketPingInterval(0) | ||
|
|
||
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 would add more details about what configuration we can reuse from the context. For example, would
spark.masterbe taken from kubernetes master? I'm trying to understand what's the net new benefit for somebody using this in their day-to-day work.Also, now that we have client mode, I anticipate many users switching to that to emulate a typical Mesos deployment (e.g. driver in client mode, kept alive by Marathon). I am assuming this does not apply in client mode, where all the options need to be explicitly specified.
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.
To be frank I'm not really sure why there are different config options for client vs cluster mode and this may have changed with some of the cleanup that @vanzin has been doing lately to simplify the configuration code.
Personally I have never needed to use any of the additional configuration properties in either client/cluster mode as the auto-configuration from my K8S config file has always been sufficient. At worst I've needed to set
KUBECONFIGto select the correct config file for the cluster I want to submit to.Note that the core behaviour (the auto-configuration) has always existed implicitly in the K8S backend but was just not called out explicitly previously in the docs. This PR primarily just makes it more explicit and flexible for users who have multiple contexts in their config files.
WRT
spark.masterSpark in general requires that to always be set and will use that to override whatever is present in the K8S config file regardless.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.
Ok, I just opened https://issues.apache.org/jira/browse/SPARK-26295 and @vanzin redirected me to this thread. Would love your eyes on that issue, see if we can use your work here to close that too.
In short, If there is code that propagates the kube context along this path, I'm not aware of it, would love to see some documentation:
There is no kubectl or "kube context" in the docker container, it's just the spark distro and my jars. So where would the driver pod get the account from?
PS: agreed that there are too many config options on the auth side, maybe we could consolidate them more.