You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by vanzin <gi...@git.apache.org> on 2018/10/31 18:53:29 UTC
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
GitHub user vanzin opened a pull request:
https://github.com/apache/spark/pull/22911
[SPARK-25815][k8s] Support kerberos in client mode, keytab-based token renewal.
This change hooks up the k8s backed to the updated HadoopDelegationTokenManager,
so that delegation tokens are also available in client mode.
It also closes the remaining gap in kerberos support which is to support
the current keytab-based token renewal supported by YARN (and partially by
Mesos). That is done by stashing the keytab in a secret on the driver pod,
and then providing it to the HadoopDelegationTokenManager when the
driver starts.
Tested in cluster mode with and without a keytab, and also in client
mode.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vanzin/spark SPARK-25815
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22911.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #22911
----
commit 7f86e231922648a9da07ae73026a3a430d2c83f5
Author: Marcelo Vanzin <va...@...>
Date: 2018-10-19T20:33:47Z
[SPARK-25815][k8s] Support kerbers in client mode, keytab-based token renewal.
This change hooks up the k8s backed to the updated HadoopDelegationTokenManager,
so that delegation tokens are also available in client mode.
It also closes the remaining gap in kerberos support which is to support
the current keytab-based token renewal supported by YARN (and partially by
Mesos). That is done by stashing the keytab in a secret on the driver pod,
and then providing it to the HadoopDelegationTokenManager when the
driver starts.
Tested in cluster mode with and without a keytab, and also in client
mode.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233588339
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/DelegationTokenFeatureStep.scala ---
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s.features
+
+import io.fabric8.kubernetes.api.model.{ContainerBuilder, HasMetadata, PodBuilder, SecretBuilder}
+import org.apache.commons.codec.binary.Base64
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.deploy.SparkHadoopUtil
+import org.apache.spark.deploy.k8s.{KubernetesConf, KubernetesUtils, SparkPod}
+import org.apache.spark.deploy.k8s.Config._
+import org.apache.spark.deploy.k8s.Constants._
+import org.apache.spark.deploy.security.HadoopDelegationTokenManager
+import org.apache.spark.internal.config._
+
+/**
+ * Delegation token support for Spark apps on kubernetes.
+ *
+ * When preparing driver resources, this step will generate delegation tokens for the app if
+ * they're needed.
+ *
+ * When preparing pods, this step will mount the delegation token secret (either pre-defined,
+ * or generated by this step when preparing the driver).
+ */
+private[spark] class DelegationTokenFeatureStep(conf: KubernetesConf[_], isDriver: Boolean)
--- End diff --
This is what I meant above when I said that the `HadoopKerberosLogin` logic could be deleted. The assumption here is that the secret should not be created as the keytab will use the HadoopDelegationTokenManager logic. The only secret that should be _created_ would be the keytab. However, I personally thought that we should point to a secretName that is either the delegationToken or the keytab. Hence why I suggested that the secretName and secretItemKey remain.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98333/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233642625
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala ---
@@ -144,6 +144,10 @@ private[spark] class BasicExecutorFeatureStep(
.addToLimits("memory", executorMemoryQuantity)
.addToRequests("cpu", executorCpuQuantity)
.endResources()
+ .addNewEnv()
+ .withName(ENV_SPARK_USER)
--- End diff --
I see that you noted that this is always done across resource managers. What is the reason for that, just wondering? as I introduced it exclusively in the HadoopSteps
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233643587
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala ---
@@ -144,6 +144,10 @@ private[spark] class BasicExecutorFeatureStep(
.addToLimits("memory", executorMemoryQuantity)
.addToRequests("cpu", executorCpuQuantity)
.endResources()
+ .addNewEnv()
+ .withName(ENV_SPARK_USER)
--- End diff --
If you don't do this, whatever is the OS user in the container will become the identity used to talk to Hadoop services (when kerberos is not on).
In YARN, for example, that would be the "yarn" user.
In k8s, with the current image, that would be "root".
You probably don't want that by default. We're talking about non-secured Hadoop here, so users can easily override this stuff, but by default let's at least try to identify the user correctly.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5866/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/22911
To address the comments I had to change a whole bunch of things; mainly because I noticed that the client and cluster paths were really different, when they should for the most part be similar. Check the updated PR description for details.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by rvesse <gi...@git.apache.org>.
Github user rvesse commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r231195413
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -126,20 +134,53 @@ private[spark] class KerberosConfDriverFeatureStep(
HadoopBootstrapUtil.bootstrapSparkUserPod(
kubeTokenManager.getCurrentUser.getShortUserName,
hadoopBasedSparkPod))
+
+ if (keytab.isDefined) {
+ val podWitKeytab = new PodBuilder(kerberizedPod.pod)
--- End diff --
Typo - `Wit` -> `With`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98833/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98880/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98885/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r231348306
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/KubernetesClusterSchedulerBackend.scala ---
@@ -123,7 +126,11 @@ private[spark] class KubernetesClusterSchedulerBackend(
}
override def createDriverEndpoint(properties: Seq[(String, String)]): DriverEndpoint = {
- new KubernetesDriverEndpoint(rpcEnv, properties)
+ new KubernetesDriverEndpoint(sc.env.rpcEnv, properties)
+ }
+
+ override protected def createTokenManager(): Option[HadoopDelegationTokenManager] = {
+ Some(new HadoopDelegationTokenManager(conf, sc.hadoopConfiguration))
--- End diff --
Ah, ok I get it now. I can do that. I'll try to include support for (3) but it depends on how much I have to touch other parts of the code. Hopefully not much.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98880 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98880/testReport)** for PR 22911 at commit [`78b76a8`](https://github.com/apache/spark/commit/78b76a8ace35b77a6f005dfb26a6e2fd836c1ff4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5030/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233652949
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -69,97 +72,191 @@ private[spark] class KerberosConfDriverFeatureStep(
"If a Kerberos keytab is specified you must also specify a Kerberos principal")
KubernetesUtils.requireBothOrNeitherDefined(
- existingSecretName,
- existingSecretItemKey,
+ existingDtSecret,
+ existingDtItemKey,
"If a secret data item-key where the data of the Kerberos Delegation Token is specified" +
" you must also specify the name of the secret",
"If a secret storing a Kerberos Delegation Token is specified you must also" +
" specify the item-key where the data is stored")
- private val hadoopConfigurationFiles = hadoopConfDirSpec.hadoopConfDir.map { hConfDir =>
- HadoopBootstrapUtil.getHadoopConfFiles(hConfDir)
+ if (!hasKerberosConf) {
+ logInfo("You have not specified a krb5.conf file locally or via a ConfigMap. " +
+ "Make sure that you have the krb5.conf locally on the driver image.")
}
- private val newHadoopConfigMapName =
- if (hadoopConfDirSpec.hadoopConfigMapName.isEmpty) {
- Some(kubernetesConf.hadoopConfigMapName)
- } else {
- None
- }
- // Either use pre-existing secret or login to create new Secret with DT stored within
- private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
- secretName <- existingSecretName
- secretItemKey <- existingSecretItemKey
- } yield {
- KerberosConfigSpec(
- dtSecret = None,
- dtSecretName = secretName,
- dtSecretItemKey = secretItemKey,
- jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
- }).orElse(
- if (isKerberosEnabled) {
- Some(HadoopKerberosLogin.buildSpec(
- conf,
- kubernetesConf.appResourceNamePrefix,
- kubeTokenManager))
- } else {
- None
+ // Create delegation tokens if needed. This is a lazy val so that it's not populated
+ // unnecessarily. But it needs to be accessible to different methods in this class,
+ // since it's not clear based solely on available configuration options that delegation
+ // tokens are needed when other credentials are not available.
+ private lazy val delegationTokens: Array[Byte] = if (keytab.isEmpty && existingDtSecret.isEmpty) {
+ createDelegationTokens()
+ } else {
+ null
+ }
+
+ private def needKeytabUpload: Boolean = keytab.exists(!Utils.isLocalUri(_))
+
+ private def dtSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-delegation-tokens"
+
+ private def ktSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-kerberos-keytab"
+
+ private def hasKerberosConf: Boolean = krb5CMap.isDefined | krb5File.isDefined
+
+ override def configurePod(original: SparkPod): SparkPod = {
+ original.transform { case pod if hasKerberosConf =>
+ val configMapVolume = if (krb5CMap.isDefined) {
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(krb5CMap.get)
+ .endConfigMap()
+ .build()
+ } else {
+ val krb5Conf = new File(krb5File.get)
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(kubernetesConf.krbConfigMapName)
+ .withItems(new KeyToPathBuilder()
+ .withKey(krb5Conf.getName())
+ .withPath(krb5Conf.getName())
+ .build())
+ .endConfigMap()
+ .build()
+ }
+
+ val podWithVolume = new PodBuilder(pod.pod)
+ .editSpec()
+ .addNewVolumeLike(configMapVolume)
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithMount = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KRB_FILE_VOLUME)
+ .withMountPath(KRB_FILE_DIR_PATH + "/krb5.conf")
+ .withSubPath("krb5.conf")
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWithVolume, containerWithMount)
+ }.transform {
+ case pod if needKeytabUpload =>
+ // If keytab is defined and is a submission-local file (not local: URI), then create a
+ // secret for it. The keytab data will be stored in this secret below.
+ val podWitKeytab = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withNewSecret()
+ .withSecretName(ktSecretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithKeytab = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withMountPath(KERBEROS_KEYTAB_MOUNT_POINT)
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWitKeytab, containerWithKeytab)
+
+ case pod if existingDtSecret.isDefined | delegationTokens != null =>
+ val secretName = existingDtSecret.getOrElse(dtSecretName)
+ val itemKey = existingDtItemKey.getOrElse(KERBEROS_SECRET_KEY)
+
+ val podWithTokens = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withNewSecret()
+ .withSecretName(secretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithTokens = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withMountPath(SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR)
+ .endVolumeMount()
+ .addNewEnv()
+ .withName(ENV_HADOOP_TOKEN_FILE_LOCATION)
+ .withValue(s"$SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR/$itemKey")
+ .endEnv()
+ .build()
+
+ SparkPod(podWithTokens, containerWithTokens)
}
- )
-
- override def configurePod(pod: SparkPod): SparkPod = {
- val hadoopBasedSparkPod = HadoopBootstrapUtil.bootstrapHadoopConfDir(
- hadoopConfDirSpec.hadoopConfDir,
- newHadoopConfigMapName,
- hadoopConfDirSpec.hadoopConfigMapName,
- pod)
- kerberosConfSpec.map { hSpec =>
- HadoopBootstrapUtil.bootstrapKerberosPod(
- hSpec.dtSecretName,
- hSpec.dtSecretItemKey,
- hSpec.jobUserName,
- krb5File,
- Some(kubernetesConf.krbConfigMapName),
- krb5CMap,
- hadoopBasedSparkPod)
- }.getOrElse(
- HadoopBootstrapUtil.bootstrapSparkUserPod(
- kubeTokenManager.getCurrentUser.getShortUserName,
- hadoopBasedSparkPod))
}
override def getAdditionalPodSystemProperties(): Map[String, String] = {
- val resolvedConfValues = kerberosConfSpec.map { hSpec =>
- Map(KERBEROS_DT_SECRET_NAME -> hSpec.dtSecretName,
- KERBEROS_DT_SECRET_KEY -> hSpec.dtSecretItemKey,
- KERBEROS_SPARK_USER_NAME -> hSpec.jobUserName,
- KRB5_CONFIG_MAP_NAME -> krb5CMap.getOrElse(kubernetesConf.krbConfigMapName))
- }.getOrElse(
- Map(KERBEROS_SPARK_USER_NAME ->
- kubeTokenManager.getCurrentUser.getShortUserName))
- Map(HADOOP_CONFIG_MAP_NAME ->
- hadoopConfDirSpec.hadoopConfigMapName.getOrElse(
- kubernetesConf.hadoopConfigMapName)) ++ resolvedConfValues
+ // If a submission-local keytab is provided, update the Spark config so that it knows the
+ // path of the keytab in the driver container.
+ if (needKeytabUpload) {
+ val ktName = new File(keytab.get).getName()
+ Map(KEYTAB.key -> s"$KERBEROS_KEYTAB_MOUNT_POINT/$ktName")
+ } else {
+ Map.empty
+ }
}
override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
- val hadoopConfConfigMap = for {
- hName <- newHadoopConfigMapName
- hFiles <- hadoopConfigurationFiles
- } yield {
- HadoopBootstrapUtil.buildHadoopConfigMap(hName, hFiles)
+ Seq[HasMetadata]() ++ {
--- End diff --
> the style, same reasoning as the transform method I added. It avoids having to create a bunch of local variables and then concatenate them at the end.
The `transform` I like because it gets rid of:
```
val someIntermediateName = someOption.map { blah =>
// create the updated pod
}.getOrElse(previousPodName)
```
But that is not what is happening here exactly. Personally, I thought that the previous approach (with local variables + concatonation) was quite clear and didn't really require a change. But its a matter of opinion.
> Now there's only one step that does that stuff, so the util class became unnecessary.
Okay I guess from a re-usability perspective that is true, but imo seems like a lot of logic going into this step. but once again, just a matter of opinion
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #99630 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99630/testReport)** for PR 22911 at commit [`05333ea`](https://github.com/apache/spark/commit/05333ea11bc50ade8f3ba8e1d2116bad9d3d7c89).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/4680/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233649110
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -69,97 +72,191 @@ private[spark] class KerberosConfDriverFeatureStep(
"If a Kerberos keytab is specified you must also specify a Kerberos principal")
KubernetesUtils.requireBothOrNeitherDefined(
- existingSecretName,
- existingSecretItemKey,
+ existingDtSecret,
+ existingDtItemKey,
"If a secret data item-key where the data of the Kerberos Delegation Token is specified" +
" you must also specify the name of the secret",
"If a secret storing a Kerberos Delegation Token is specified you must also" +
" specify the item-key where the data is stored")
- private val hadoopConfigurationFiles = hadoopConfDirSpec.hadoopConfDir.map { hConfDir =>
- HadoopBootstrapUtil.getHadoopConfFiles(hConfDir)
+ if (!hasKerberosConf) {
+ logInfo("You have not specified a krb5.conf file locally or via a ConfigMap. " +
+ "Make sure that you have the krb5.conf locally on the driver image.")
}
- private val newHadoopConfigMapName =
- if (hadoopConfDirSpec.hadoopConfigMapName.isEmpty) {
- Some(kubernetesConf.hadoopConfigMapName)
- } else {
- None
- }
- // Either use pre-existing secret or login to create new Secret with DT stored within
- private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
- secretName <- existingSecretName
- secretItemKey <- existingSecretItemKey
- } yield {
- KerberosConfigSpec(
- dtSecret = None,
- dtSecretName = secretName,
- dtSecretItemKey = secretItemKey,
- jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
- }).orElse(
- if (isKerberosEnabled) {
- Some(HadoopKerberosLogin.buildSpec(
- conf,
- kubernetesConf.appResourceNamePrefix,
- kubeTokenManager))
- } else {
- None
+ // Create delegation tokens if needed. This is a lazy val so that it's not populated
+ // unnecessarily. But it needs to be accessible to different methods in this class,
+ // since it's not clear based solely on available configuration options that delegation
+ // tokens are needed when other credentials are not available.
+ private lazy val delegationTokens: Array[Byte] = if (keytab.isEmpty && existingDtSecret.isEmpty) {
+ createDelegationTokens()
+ } else {
+ null
+ }
+
+ private def needKeytabUpload: Boolean = keytab.exists(!Utils.isLocalUri(_))
+
+ private def dtSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-delegation-tokens"
+
+ private def ktSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-kerberos-keytab"
+
+ private def hasKerberosConf: Boolean = krb5CMap.isDefined | krb5File.isDefined
+
+ override def configurePod(original: SparkPod): SparkPod = {
+ original.transform { case pod if hasKerberosConf =>
+ val configMapVolume = if (krb5CMap.isDefined) {
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(krb5CMap.get)
+ .endConfigMap()
+ .build()
+ } else {
+ val krb5Conf = new File(krb5File.get)
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(kubernetesConf.krbConfigMapName)
+ .withItems(new KeyToPathBuilder()
+ .withKey(krb5Conf.getName())
+ .withPath(krb5Conf.getName())
+ .build())
+ .endConfigMap()
+ .build()
+ }
+
+ val podWithVolume = new PodBuilder(pod.pod)
+ .editSpec()
+ .addNewVolumeLike(configMapVolume)
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithMount = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KRB_FILE_VOLUME)
+ .withMountPath(KRB_FILE_DIR_PATH + "/krb5.conf")
+ .withSubPath("krb5.conf")
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWithVolume, containerWithMount)
+ }.transform {
+ case pod if needKeytabUpload =>
+ // If keytab is defined and is a submission-local file (not local: URI), then create a
+ // secret for it. The keytab data will be stored in this secret below.
+ val podWitKeytab = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withNewSecret()
+ .withSecretName(ktSecretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithKeytab = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withMountPath(KERBEROS_KEYTAB_MOUNT_POINT)
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWitKeytab, containerWithKeytab)
+
+ case pod if existingDtSecret.isDefined | delegationTokens != null =>
+ val secretName = existingDtSecret.getOrElse(dtSecretName)
+ val itemKey = existingDtItemKey.getOrElse(KERBEROS_SECRET_KEY)
+
+ val podWithTokens = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withNewSecret()
+ .withSecretName(secretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithTokens = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withMountPath(SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR)
+ .endVolumeMount()
+ .addNewEnv()
+ .withName(ENV_HADOOP_TOKEN_FILE_LOCATION)
+ .withValue(s"$SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR/$itemKey")
+ .endEnv()
+ .build()
+
+ SparkPod(podWithTokens, containerWithTokens)
}
- )
-
- override def configurePod(pod: SparkPod): SparkPod = {
- val hadoopBasedSparkPod = HadoopBootstrapUtil.bootstrapHadoopConfDir(
- hadoopConfDirSpec.hadoopConfDir,
- newHadoopConfigMapName,
- hadoopConfDirSpec.hadoopConfigMapName,
- pod)
- kerberosConfSpec.map { hSpec =>
- HadoopBootstrapUtil.bootstrapKerberosPod(
- hSpec.dtSecretName,
- hSpec.dtSecretItemKey,
- hSpec.jobUserName,
- krb5File,
- Some(kubernetesConf.krbConfigMapName),
- krb5CMap,
- hadoopBasedSparkPod)
- }.getOrElse(
- HadoopBootstrapUtil.bootstrapSparkUserPod(
- kubeTokenManager.getCurrentUser.getShortUserName,
- hadoopBasedSparkPod))
}
override def getAdditionalPodSystemProperties(): Map[String, String] = {
- val resolvedConfValues = kerberosConfSpec.map { hSpec =>
- Map(KERBEROS_DT_SECRET_NAME -> hSpec.dtSecretName,
- KERBEROS_DT_SECRET_KEY -> hSpec.dtSecretItemKey,
- KERBEROS_SPARK_USER_NAME -> hSpec.jobUserName,
- KRB5_CONFIG_MAP_NAME -> krb5CMap.getOrElse(kubernetesConf.krbConfigMapName))
- }.getOrElse(
- Map(KERBEROS_SPARK_USER_NAME ->
- kubeTokenManager.getCurrentUser.getShortUserName))
- Map(HADOOP_CONFIG_MAP_NAME ->
- hadoopConfDirSpec.hadoopConfigMapName.getOrElse(
- kubernetesConf.hadoopConfigMapName)) ++ resolvedConfValues
+ // If a submission-local keytab is provided, update the Spark config so that it knows the
+ // path of the keytab in the driver container.
+ if (needKeytabUpload) {
+ val ktName = new File(keytab.get).getName()
+ Map(KEYTAB.key -> s"$KERBEROS_KEYTAB_MOUNT_POINT/$ktName")
+ } else {
+ Map.empty
+ }
}
override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
- val hadoopConfConfigMap = for {
- hName <- newHadoopConfigMapName
- hFiles <- hadoopConfigurationFiles
- } yield {
- HadoopBootstrapUtil.buildHadoopConfigMap(hName, hFiles)
+ Seq[HasMetadata]() ++ {
--- End diff --
nit: is there a particular advantage gained by this style of Seq() building? I prefer the .toSeq approach as I personally don't like `if {...} else { Nil }`. Once again, why was the `Util` class removed? Isn't this a lot of logic to throw into this one step?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98833 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98833/testReport)** for PR 22911 at commit [`d8416cd`](https://github.com/apache/spark/commit/d8416cda0e23af43a86b129a4a102f37bcbdaf4c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5033/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98333 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98333/testReport)** for PR 22911 at commit [`7f86e23`](https://github.com/apache/spark/commit/7f86e231922648a9da07ae73026a3a430d2c83f5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r231256397
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/KubernetesClusterSchedulerBackend.scala ---
@@ -123,7 +126,11 @@ private[spark] class KubernetesClusterSchedulerBackend(
}
override def createDriverEndpoint(properties: Seq[(String, String)]): DriverEndpoint = {
- new KubernetesDriverEndpoint(rpcEnv, properties)
+ new KubernetesDriverEndpoint(sc.env.rpcEnv, properties)
+ }
+
+ override protected def createTokenManager(): Option[HadoopDelegationTokenManager] = {
+ Some(new HadoopDelegationTokenManager(conf, sc.hadoopConfiguration))
--- End diff --
I'm not sure I follow your train of thought here so I'll comment on what I understand.
First, the code that creates the secret is in `KerberosConfDriverFeatureStep`. As far as I know, that class is not used in client mode. In client mode the keytab stays in the client machine, with the driver, and the driver just sends DTs to executors. So the whole discussion about secrets is irrelevant in that case.
In cluster mode, you need the driver to have access to the keytab for this feature to work. There are a few ways to achieve that:
- the current YARN mode, which is the keytab lives in the submission host, and is distributed with the application. In k8s this would amount to what I have here: the submission code creates a secret for the driver pod and stashes the keytab in it.
- add the ability to store the keytab in an external place (like HDFS or an HTTP server). That has drawbacks (e.g. people probably wouldn't like that, and there's a chicken & egg problem in HDFS, so you'd still need a kerberos TGT to bootstrap things).
- add a k8s-specific feature of mounting a pre-defined secret in the driver pod. I believe this is what you're suggesting?
I think supporting the first is easy as this change shows, and keeps feature parity with what's already supported in YARN. Unless there's a glaring issue with using secrets that I'm not aware of, I don't see a reason for not doing it.
The third option (pre-defined secret) could also be added. My hope is that you could do it with pre-existing configs (`spark.kubernetes.driver.secrets.` & company), but I don't know how you'd set the `spark.kerberos.keytab` and `spark.kerberos.principal` configs just in the driver - and not in the submission client. So it seems we'd need at least a little bit of code here to support that scenario.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233654819
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -69,97 +72,191 @@ private[spark] class KerberosConfDriverFeatureStep(
"If a Kerberos keytab is specified you must also specify a Kerberos principal")
KubernetesUtils.requireBothOrNeitherDefined(
- existingSecretName,
- existingSecretItemKey,
+ existingDtSecret,
+ existingDtItemKey,
"If a secret data item-key where the data of the Kerberos Delegation Token is specified" +
" you must also specify the name of the secret",
"If a secret storing a Kerberos Delegation Token is specified you must also" +
" specify the item-key where the data is stored")
- private val hadoopConfigurationFiles = hadoopConfDirSpec.hadoopConfDir.map { hConfDir =>
- HadoopBootstrapUtil.getHadoopConfFiles(hConfDir)
+ if (!hasKerberosConf) {
+ logInfo("You have not specified a krb5.conf file locally or via a ConfigMap. " +
+ "Make sure that you have the krb5.conf locally on the driver image.")
}
- private val newHadoopConfigMapName =
- if (hadoopConfDirSpec.hadoopConfigMapName.isEmpty) {
- Some(kubernetesConf.hadoopConfigMapName)
- } else {
- None
- }
- // Either use pre-existing secret or login to create new Secret with DT stored within
- private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
- secretName <- existingSecretName
- secretItemKey <- existingSecretItemKey
- } yield {
- KerberosConfigSpec(
- dtSecret = None,
- dtSecretName = secretName,
- dtSecretItemKey = secretItemKey,
- jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
- }).orElse(
- if (isKerberosEnabled) {
- Some(HadoopKerberosLogin.buildSpec(
- conf,
- kubernetesConf.appResourceNamePrefix,
- kubeTokenManager))
- } else {
- None
+ // Create delegation tokens if needed. This is a lazy val so that it's not populated
+ // unnecessarily. But it needs to be accessible to different methods in this class,
+ // since it's not clear based solely on available configuration options that delegation
+ // tokens are needed when other credentials are not available.
+ private lazy val delegationTokens: Array[Byte] = if (keytab.isEmpty && existingDtSecret.isEmpty) {
+ createDelegationTokens()
+ } else {
+ null
+ }
+
+ private def needKeytabUpload: Boolean = keytab.exists(!Utils.isLocalUri(_))
+
+ private def dtSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-delegation-tokens"
+
+ private def ktSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-kerberos-keytab"
+
+ private def hasKerberosConf: Boolean = krb5CMap.isDefined | krb5File.isDefined
+
+ override def configurePod(original: SparkPod): SparkPod = {
+ original.transform { case pod if hasKerberosConf =>
+ val configMapVolume = if (krb5CMap.isDefined) {
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(krb5CMap.get)
+ .endConfigMap()
+ .build()
+ } else {
+ val krb5Conf = new File(krb5File.get)
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(kubernetesConf.krbConfigMapName)
+ .withItems(new KeyToPathBuilder()
+ .withKey(krb5Conf.getName())
+ .withPath(krb5Conf.getName())
+ .build())
+ .endConfigMap()
+ .build()
+ }
+
+ val podWithVolume = new PodBuilder(pod.pod)
+ .editSpec()
+ .addNewVolumeLike(configMapVolume)
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithMount = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KRB_FILE_VOLUME)
+ .withMountPath(KRB_FILE_DIR_PATH + "/krb5.conf")
+ .withSubPath("krb5.conf")
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWithVolume, containerWithMount)
+ }.transform {
+ case pod if needKeytabUpload =>
+ // If keytab is defined and is a submission-local file (not local: URI), then create a
+ // secret for it. The keytab data will be stored in this secret below.
+ val podWitKeytab = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withNewSecret()
+ .withSecretName(ktSecretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithKeytab = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withMountPath(KERBEROS_KEYTAB_MOUNT_POINT)
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWitKeytab, containerWithKeytab)
+
+ case pod if existingDtSecret.isDefined | delegationTokens != null =>
+ val secretName = existingDtSecret.getOrElse(dtSecretName)
+ val itemKey = existingDtItemKey.getOrElse(KERBEROS_SECRET_KEY)
+
+ val podWithTokens = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withNewSecret()
+ .withSecretName(secretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithTokens = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withMountPath(SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR)
+ .endVolumeMount()
+ .addNewEnv()
+ .withName(ENV_HADOOP_TOKEN_FILE_LOCATION)
+ .withValue(s"$SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR/$itemKey")
+ .endEnv()
+ .build()
+
+ SparkPod(podWithTokens, containerWithTokens)
}
- )
-
- override def configurePod(pod: SparkPod): SparkPod = {
- val hadoopBasedSparkPod = HadoopBootstrapUtil.bootstrapHadoopConfDir(
- hadoopConfDirSpec.hadoopConfDir,
- newHadoopConfigMapName,
- hadoopConfDirSpec.hadoopConfigMapName,
- pod)
- kerberosConfSpec.map { hSpec =>
- HadoopBootstrapUtil.bootstrapKerberosPod(
- hSpec.dtSecretName,
- hSpec.dtSecretItemKey,
- hSpec.jobUserName,
- krb5File,
- Some(kubernetesConf.krbConfigMapName),
- krb5CMap,
- hadoopBasedSparkPod)
- }.getOrElse(
- HadoopBootstrapUtil.bootstrapSparkUserPod(
- kubeTokenManager.getCurrentUser.getShortUserName,
- hadoopBasedSparkPod))
}
override def getAdditionalPodSystemProperties(): Map[String, String] = {
- val resolvedConfValues = kerberosConfSpec.map { hSpec =>
- Map(KERBEROS_DT_SECRET_NAME -> hSpec.dtSecretName,
- KERBEROS_DT_SECRET_KEY -> hSpec.dtSecretItemKey,
- KERBEROS_SPARK_USER_NAME -> hSpec.jobUserName,
- KRB5_CONFIG_MAP_NAME -> krb5CMap.getOrElse(kubernetesConf.krbConfigMapName))
- }.getOrElse(
- Map(KERBEROS_SPARK_USER_NAME ->
- kubeTokenManager.getCurrentUser.getShortUserName))
- Map(HADOOP_CONFIG_MAP_NAME ->
- hadoopConfDirSpec.hadoopConfigMapName.getOrElse(
- kubernetesConf.hadoopConfigMapName)) ++ resolvedConfValues
+ // If a submission-local keytab is provided, update the Spark config so that it knows the
+ // path of the keytab in the driver container.
+ if (needKeytabUpload) {
+ val ktName = new File(keytab.get).getName()
+ Map(KEYTAB.key -> s"$KERBEROS_KEYTAB_MOUNT_POINT/$ktName")
+ } else {
+ Map.empty
+ }
}
override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
- val hadoopConfConfigMap = for {
- hName <- newHadoopConfigMapName
- hFiles <- hadoopConfigurationFiles
- } yield {
- HadoopBootstrapUtil.buildHadoopConfigMap(hName, hFiles)
+ Seq[HasMetadata]() ++ {
+ krb5File.map { path =>
+ val file = new File(path)
+ new ConfigMapBuilder()
+ .withNewMetadata()
+ .withName(kubernetesConf.krbConfigMapName)
+ .endMetadata()
+ .addToData(
+ Map(file.getName() -> Files.toString(file, StandardCharsets.UTF_8)).asJava)
+ .build()
+ }
+ } ++ {
+ // If a submission-local keytab is provided, stash it in a secret.
+ if (needKeytabUpload) {
+ val kt = new File(keytab.get)
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(ktSecretName)
+ .endMetadata()
+ .addToData(kt.getName(), Base64.encodeBase64String(Files.toByteArray(kt)))
+ .build())
+ } else {
+ Nil
+ }
+ } ++ {
+ if (delegationTokens != null) {
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(dtSecretName)
+ .endMetadata()
+ .addToData(KERBEROS_SECRET_KEY, Base64.encodeBase64String(delegationTokens))
+ .build())
+ } else {
+ Nil
+ }
}
+ }
- val krb5ConfigMap = krb5File.map { fileLocation =>
- HadoopBootstrapUtil.buildkrb5ConfigMap(
- kubernetesConf.krbConfigMapName,
- fileLocation)
+ // Visible for testing.
+ def createDelegationTokens(): Array[Byte] = {
--- End diff --
> I'm mocking the creation of delegation tokens
BTW there are other ways to achieve that, e.g. with `UserGroupInformation.createTestUser()` and `doAs`, but I think the current test is easier to understand.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #99404 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99404/testReport)** for PR 22911 at commit [`88f1bb5`](https://github.com/apache/spark/commit/88f1bb516fa6098125fb7f77fd548a1d423cf745).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233541147
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/SparkPod.scala ---
@@ -18,7 +18,30 @@ package org.apache.spark.deploy.k8s
import io.fabric8.kubernetes.api.model.{Container, ContainerBuilder, Pod, PodBuilder}
-private[spark] case class SparkPod(pod: Pod, container: Container)
+private[spark] case class SparkPod(pod: Pod, container: Container) {
+
+ /**
+ * Convenience method to apply a series of chained transformations to a pod.
+ *
+ * Use it like:
+ *
+ * original.modify { case pod =>
+ * // update pod and return new one
+ * }.modify { case pod =>
+ * // more changes that create a new pod
+ * }.modify {
+ * case pod if someCondition => // new pod
+ * }
+ *
+ * This makes it cleaner to apply multiple transformations, avoiding having to create
+ * a bunch of awkwardly-named local variables. Since the argument is a partial function,
+ * it can do matching without needing to exhaust all the possibilities. If the function
+ * is not applied, then the original pod will be kept.
+ */
+ def transform(fn: PartialFunction[SparkPod, SparkPod]): SparkPod = fn.lift(this).getOrElse(this)
--- End diff --
I added this because I started to get tired of code like the following:
```
val someIntermediateName = someOption.map { blah =>
// create the updated pod
}.getOrElse(previousPodName)
// lather, rinse, repeat
```
To me that's hard to follow and brittle, and this pattern makes things clearer IMO.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5866/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5477/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5036/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98842 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98842/testReport)** for PR 22911 at commit [`7e9b3a2`](https://github.com/apache/spark/commit/7e9b3a2218359846176136cc98d63eb841d97f15).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r230860519
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/KubernetesClusterSchedulerBackend.scala ---
@@ -123,7 +126,11 @@ private[spark] class KubernetesClusterSchedulerBackend(
}
override def createDriverEndpoint(properties: Seq[(String, String)]): DriverEndpoint = {
- new KubernetesDriverEndpoint(rpcEnv, properties)
+ new KubernetesDriverEndpoint(sc.env.rpcEnv, properties)
+ }
+
+ override protected def createTokenManager(): Option[HadoopDelegationTokenManager] = {
+ Some(new HadoopDelegationTokenManager(conf, sc.hadoopConfiguration))
--- End diff --
If we are introducing this change, I think it is important that we talk about the future of secret creation upon using `--keytab` + `principle`. Right now, secrets are created when a keytab is used by the client or for client-mode by the driver; this was used primarily for testing (on my end) but also because this logic wasn't previously generalized for all cluster-managers. Should we create an option for the user to create a secret or get rid of it as a whole, as delegation token logic is handled via the UpdateDelegationToken message passing framework. In essence, if we leave the ability to create a secret we are twice obtaining a DT which is extraneous. And if we are removing it, it is sensible to refactor the KerberosConfig logic to account for this removal. I was planning to do this in my token renewal PR where I was also introducing this change, but it seems that this will probably get merged in before mine, as such, here would be a better place to refactor. Or maybe a sepe
rate PR that introduces this line and does the refactor, and then this and my PR could be introduced subsequently.
thoughts, @vanzin ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98842/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5477/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by shaneknapp <gi...@git.apache.org>.
Github user shaneknapp commented on the issue:
https://github.com/apache/spark/pull/22911
test this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233650735
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesDriverBuilder.scala ---
@@ -81,14 +81,14 @@ private[spark] class KubernetesDriverBuilder(
val driverCommandStep = provideDriverCommandStep(kubernetesConf)
- val maybeHadoopConfigStep =
- kubernetesConf.hadoopConfSpec.map { _ =>
- provideHadoopGlobalStep(kubernetesConf)}
+ val otherSteps = Seq(
--- End diff --
All this code will go away when I get to SPARK-25877, so I'm not super concerned with the names here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233578429
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/DelegationTokenFeatureStep.scala ---
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s.features
+
+import io.fabric8.kubernetes.api.model.{ContainerBuilder, HasMetadata, PodBuilder, SecretBuilder}
+import org.apache.commons.codec.binary.Base64
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.deploy.SparkHadoopUtil
+import org.apache.spark.deploy.k8s.{KubernetesConf, KubernetesUtils, SparkPod}
+import org.apache.spark.deploy.k8s.Config._
+import org.apache.spark.deploy.k8s.Constants._
+import org.apache.spark.deploy.security.HadoopDelegationTokenManager
+import org.apache.spark.internal.config._
+
+/**
+ * Delegation token support for Spark apps on kubernetes.
+ *
+ * When preparing driver resources, this step will generate delegation tokens for the app if
+ * they're needed.
+ *
+ * When preparing pods, this step will mount the delegation token secret (either pre-defined,
+ * or generated by this step when preparing the driver).
+ */
+private[spark] class DelegationTokenFeatureStep(conf: KubernetesConf[_], isDriver: Boolean)
--- End diff --
During the recent discussion in the sig meeting I was thinking about this guy. I think it may not be necessary to mount this secret in executors, and let the driver propagate the tokens to executors through its normal means.
e.g. later if/when adding code to monitor delegation tokens for updates, that would mean only the driver has to do it.
I'll git this a try; if it works, it might be feasible to re-merge this code with the kerberos step.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #99627 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99627/testReport)** for PR 22911 at commit [`05333ea`](https://github.com/apache/spark/commit/05333ea11bc50ade8f3ba8e1d2116bad9d3d7c89).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233542062
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/SparkPod.scala ---
@@ -18,7 +18,30 @@ package org.apache.spark.deploy.k8s
import io.fabric8.kubernetes.api.model.{Container, ContainerBuilder, Pod, PodBuilder}
-private[spark] case class SparkPod(pod: Pod, container: Container)
+private[spark] case class SparkPod(pod: Pod, container: Container) {
+
+ /**
+ * Convenience method to apply a series of chained transformations to a pod.
+ *
+ * Use it like:
+ *
+ * original.modify { case pod =>
+ * // update pod and return new one
+ * }.modify { case pod =>
+ * // more changes that create a new pod
+ * }.modify {
+ * case pod if someCondition => // new pod
+ * }
+ *
+ * This makes it cleaner to apply multiple transformations, avoiding having to create
+ * a bunch of awkwardly-named local variables. Since the argument is a partial function,
+ * it can do matching without needing to exhaust all the possibilities. If the function
+ * is not applied, then the original pod will be kept.
+ */
+ def transform(fn: PartialFunction[SparkPod, SparkPod]): SparkPod = fn.lift(this).getOrElse(this)
--- End diff --
I would think that this change is out of the scope of this PR, but I do love the use of a PartialFunction here. Thanks for this!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r231344398
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/KubernetesClusterSchedulerBackend.scala ---
@@ -123,7 +126,11 @@ private[spark] class KubernetesClusterSchedulerBackend(
}
override def createDriverEndpoint(properties: Seq[(String, String)]): DriverEndpoint = {
- new KubernetesDriverEndpoint(rpcEnv, properties)
+ new KubernetesDriverEndpoint(sc.env.rpcEnv, properties)
+ }
+
+ override protected def createTokenManager(): Option[HadoopDelegationTokenManager] = {
+ Some(new HadoopDelegationTokenManager(conf, sc.hadoopConfiguration))
--- End diff --
Oh, I was referencing the creation of `Delegation Token` secret when a `--keytab` is specified. I believe that you are right in that in client-mode you would not need to worry about running this step. But I think the 3rd option would be good to include here. I think that with the introduction of `HadoopDelegationTokenManager` we should remove the creation of the `dtSecret`, and that should be included in this PR if you are introducing this. Therefore, I think it is sensible to refactor the `KerberosConfigSpec` to have a generic `secret`, `secretName`, `secretKey`, that would either contain a `DelegationToken` or a `keytab`. Such that the code block:
```
private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
secretName <- existingSecretName
secretItemKey <- existingSecretItemKey
} yield {
KerberosConfigSpec(
secret = None,
secretName = secretName,
secretItemKey = secretItemKey,
jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
}).orElse(
if (isKerberosEnabled) {
keytab.map { ..... }
} else {
None
}
```
would return a kerberosConfSpec that would account for either case. Erm, that would also mean that you could delete the `HadoopKerberosLogin` method.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233650398
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -69,97 +72,191 @@ private[spark] class KerberosConfDriverFeatureStep(
"If a Kerberos keytab is specified you must also specify a Kerberos principal")
KubernetesUtils.requireBothOrNeitherDefined(
- existingSecretName,
- existingSecretItemKey,
+ existingDtSecret,
+ existingDtItemKey,
"If a secret data item-key where the data of the Kerberos Delegation Token is specified" +
" you must also specify the name of the secret",
"If a secret storing a Kerberos Delegation Token is specified you must also" +
" specify the item-key where the data is stored")
- private val hadoopConfigurationFiles = hadoopConfDirSpec.hadoopConfDir.map { hConfDir =>
- HadoopBootstrapUtil.getHadoopConfFiles(hConfDir)
+ if (!hasKerberosConf) {
+ logInfo("You have not specified a krb5.conf file locally or via a ConfigMap. " +
+ "Make sure that you have the krb5.conf locally on the driver image.")
}
- private val newHadoopConfigMapName =
- if (hadoopConfDirSpec.hadoopConfigMapName.isEmpty) {
- Some(kubernetesConf.hadoopConfigMapName)
- } else {
- None
- }
- // Either use pre-existing secret or login to create new Secret with DT stored within
- private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
- secretName <- existingSecretName
- secretItemKey <- existingSecretItemKey
- } yield {
- KerberosConfigSpec(
- dtSecret = None,
- dtSecretName = secretName,
- dtSecretItemKey = secretItemKey,
- jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
- }).orElse(
- if (isKerberosEnabled) {
- Some(HadoopKerberosLogin.buildSpec(
- conf,
- kubernetesConf.appResourceNamePrefix,
- kubeTokenManager))
- } else {
- None
+ // Create delegation tokens if needed. This is a lazy val so that it's not populated
+ // unnecessarily. But it needs to be accessible to different methods in this class,
+ // since it's not clear based solely on available configuration options that delegation
+ // tokens are needed when other credentials are not available.
+ private lazy val delegationTokens: Array[Byte] = if (keytab.isEmpty && existingDtSecret.isEmpty) {
+ createDelegationTokens()
+ } else {
+ null
+ }
+
+ private def needKeytabUpload: Boolean = keytab.exists(!Utils.isLocalUri(_))
+
+ private def dtSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-delegation-tokens"
+
+ private def ktSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-kerberos-keytab"
+
+ private def hasKerberosConf: Boolean = krb5CMap.isDefined | krb5File.isDefined
+
+ override def configurePod(original: SparkPod): SparkPod = {
+ original.transform { case pod if hasKerberosConf =>
+ val configMapVolume = if (krb5CMap.isDefined) {
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(krb5CMap.get)
+ .endConfigMap()
+ .build()
+ } else {
+ val krb5Conf = new File(krb5File.get)
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(kubernetesConf.krbConfigMapName)
+ .withItems(new KeyToPathBuilder()
+ .withKey(krb5Conf.getName())
+ .withPath(krb5Conf.getName())
+ .build())
+ .endConfigMap()
+ .build()
+ }
+
+ val podWithVolume = new PodBuilder(pod.pod)
+ .editSpec()
+ .addNewVolumeLike(configMapVolume)
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithMount = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KRB_FILE_VOLUME)
+ .withMountPath(KRB_FILE_DIR_PATH + "/krb5.conf")
+ .withSubPath("krb5.conf")
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWithVolume, containerWithMount)
+ }.transform {
+ case pod if needKeytabUpload =>
+ // If keytab is defined and is a submission-local file (not local: URI), then create a
+ // secret for it. The keytab data will be stored in this secret below.
+ val podWitKeytab = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withNewSecret()
+ .withSecretName(ktSecretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithKeytab = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withMountPath(KERBEROS_KEYTAB_MOUNT_POINT)
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWitKeytab, containerWithKeytab)
+
+ case pod if existingDtSecret.isDefined | delegationTokens != null =>
+ val secretName = existingDtSecret.getOrElse(dtSecretName)
+ val itemKey = existingDtItemKey.getOrElse(KERBEROS_SECRET_KEY)
+
+ val podWithTokens = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withNewSecret()
+ .withSecretName(secretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithTokens = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withMountPath(SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR)
+ .endVolumeMount()
+ .addNewEnv()
+ .withName(ENV_HADOOP_TOKEN_FILE_LOCATION)
+ .withValue(s"$SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR/$itemKey")
+ .endEnv()
+ .build()
+
+ SparkPod(podWithTokens, containerWithTokens)
}
- )
-
- override def configurePod(pod: SparkPod): SparkPod = {
- val hadoopBasedSparkPod = HadoopBootstrapUtil.bootstrapHadoopConfDir(
- hadoopConfDirSpec.hadoopConfDir,
- newHadoopConfigMapName,
- hadoopConfDirSpec.hadoopConfigMapName,
- pod)
- kerberosConfSpec.map { hSpec =>
- HadoopBootstrapUtil.bootstrapKerberosPod(
- hSpec.dtSecretName,
- hSpec.dtSecretItemKey,
- hSpec.jobUserName,
- krb5File,
- Some(kubernetesConf.krbConfigMapName),
- krb5CMap,
- hadoopBasedSparkPod)
- }.getOrElse(
- HadoopBootstrapUtil.bootstrapSparkUserPod(
- kubeTokenManager.getCurrentUser.getShortUserName,
- hadoopBasedSparkPod))
}
override def getAdditionalPodSystemProperties(): Map[String, String] = {
- val resolvedConfValues = kerberosConfSpec.map { hSpec =>
- Map(KERBEROS_DT_SECRET_NAME -> hSpec.dtSecretName,
- KERBEROS_DT_SECRET_KEY -> hSpec.dtSecretItemKey,
- KERBEROS_SPARK_USER_NAME -> hSpec.jobUserName,
- KRB5_CONFIG_MAP_NAME -> krb5CMap.getOrElse(kubernetesConf.krbConfigMapName))
- }.getOrElse(
- Map(KERBEROS_SPARK_USER_NAME ->
- kubeTokenManager.getCurrentUser.getShortUserName))
- Map(HADOOP_CONFIG_MAP_NAME ->
- hadoopConfDirSpec.hadoopConfigMapName.getOrElse(
- kubernetesConf.hadoopConfigMapName)) ++ resolvedConfValues
+ // If a submission-local keytab is provided, update the Spark config so that it knows the
+ // path of the keytab in the driver container.
+ if (needKeytabUpload) {
+ val ktName = new File(keytab.get).getName()
+ Map(KEYTAB.key -> s"$KERBEROS_KEYTAB_MOUNT_POINT/$ktName")
+ } else {
+ Map.empty
+ }
}
override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
- val hadoopConfConfigMap = for {
- hName <- newHadoopConfigMapName
- hFiles <- hadoopConfigurationFiles
- } yield {
- HadoopBootstrapUtil.buildHadoopConfigMap(hName, hFiles)
+ Seq[HasMetadata]() ++ {
--- End diff --
re: the style, same reasoning as the `transform` method I added. It avoids having to create a bunch of local variables and then concatenate them at the end.
re: the Util class, it existed because the methods were being called from multiple feature steps. Now there's only one step that does that stuff, so the util class became unnecessary.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5686/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5686/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by suxingfate <gi...@git.apache.org>.
Github user suxingfate commented on the issue:
https://github.com/apache/spark/pull/22911
--keytab is a simpler way IMO to enable long running spark on k8s than DT renew approach
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233660132
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -69,97 +72,191 @@ private[spark] class KerberosConfDriverFeatureStep(
"If a Kerberos keytab is specified you must also specify a Kerberos principal")
KubernetesUtils.requireBothOrNeitherDefined(
- existingSecretName,
- existingSecretItemKey,
+ existingDtSecret,
+ existingDtItemKey,
"If a secret data item-key where the data of the Kerberos Delegation Token is specified" +
" you must also specify the name of the secret",
"If a secret storing a Kerberos Delegation Token is specified you must also" +
" specify the item-key where the data is stored")
- private val hadoopConfigurationFiles = hadoopConfDirSpec.hadoopConfDir.map { hConfDir =>
- HadoopBootstrapUtil.getHadoopConfFiles(hConfDir)
+ if (!hasKerberosConf) {
+ logInfo("You have not specified a krb5.conf file locally or via a ConfigMap. " +
+ "Make sure that you have the krb5.conf locally on the driver image.")
}
- private val newHadoopConfigMapName =
- if (hadoopConfDirSpec.hadoopConfigMapName.isEmpty) {
- Some(kubernetesConf.hadoopConfigMapName)
- } else {
- None
- }
- // Either use pre-existing secret or login to create new Secret with DT stored within
- private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
- secretName <- existingSecretName
- secretItemKey <- existingSecretItemKey
- } yield {
- KerberosConfigSpec(
- dtSecret = None,
- dtSecretName = secretName,
- dtSecretItemKey = secretItemKey,
- jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
- }).orElse(
- if (isKerberosEnabled) {
- Some(HadoopKerberosLogin.buildSpec(
- conf,
- kubernetesConf.appResourceNamePrefix,
- kubeTokenManager))
- } else {
- None
+ // Create delegation tokens if needed. This is a lazy val so that it's not populated
+ // unnecessarily. But it needs to be accessible to different methods in this class,
+ // since it's not clear based solely on available configuration options that delegation
+ // tokens are needed when other credentials are not available.
+ private lazy val delegationTokens: Array[Byte] = if (keytab.isEmpty && existingDtSecret.isEmpty) {
+ createDelegationTokens()
+ } else {
+ null
+ }
+
+ private def needKeytabUpload: Boolean = keytab.exists(!Utils.isLocalUri(_))
+
+ private def dtSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-delegation-tokens"
+
+ private def ktSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-kerberos-keytab"
+
+ private def hasKerberosConf: Boolean = krb5CMap.isDefined | krb5File.isDefined
+
+ override def configurePod(original: SparkPod): SparkPod = {
+ original.transform { case pod if hasKerberosConf =>
+ val configMapVolume = if (krb5CMap.isDefined) {
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(krb5CMap.get)
+ .endConfigMap()
+ .build()
+ } else {
+ val krb5Conf = new File(krb5File.get)
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(kubernetesConf.krbConfigMapName)
+ .withItems(new KeyToPathBuilder()
+ .withKey(krb5Conf.getName())
+ .withPath(krb5Conf.getName())
+ .build())
+ .endConfigMap()
+ .build()
+ }
+
+ val podWithVolume = new PodBuilder(pod.pod)
+ .editSpec()
+ .addNewVolumeLike(configMapVolume)
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithMount = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KRB_FILE_VOLUME)
+ .withMountPath(KRB_FILE_DIR_PATH + "/krb5.conf")
+ .withSubPath("krb5.conf")
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWithVolume, containerWithMount)
+ }.transform {
+ case pod if needKeytabUpload =>
+ // If keytab is defined and is a submission-local file (not local: URI), then create a
+ // secret for it. The keytab data will be stored in this secret below.
+ val podWitKeytab = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withNewSecret()
+ .withSecretName(ktSecretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithKeytab = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withMountPath(KERBEROS_KEYTAB_MOUNT_POINT)
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWitKeytab, containerWithKeytab)
+
+ case pod if existingDtSecret.isDefined | delegationTokens != null =>
+ val secretName = existingDtSecret.getOrElse(dtSecretName)
+ val itemKey = existingDtItemKey.getOrElse(KERBEROS_SECRET_KEY)
+
+ val podWithTokens = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withNewSecret()
+ .withSecretName(secretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithTokens = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withMountPath(SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR)
+ .endVolumeMount()
+ .addNewEnv()
+ .withName(ENV_HADOOP_TOKEN_FILE_LOCATION)
+ .withValue(s"$SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR/$itemKey")
+ .endEnv()
+ .build()
+
+ SparkPod(podWithTokens, containerWithTokens)
}
- )
-
- override def configurePod(pod: SparkPod): SparkPod = {
- val hadoopBasedSparkPod = HadoopBootstrapUtil.bootstrapHadoopConfDir(
- hadoopConfDirSpec.hadoopConfDir,
- newHadoopConfigMapName,
- hadoopConfDirSpec.hadoopConfigMapName,
- pod)
- kerberosConfSpec.map { hSpec =>
- HadoopBootstrapUtil.bootstrapKerberosPod(
- hSpec.dtSecretName,
- hSpec.dtSecretItemKey,
- hSpec.jobUserName,
- krb5File,
- Some(kubernetesConf.krbConfigMapName),
- krb5CMap,
- hadoopBasedSparkPod)
- }.getOrElse(
- HadoopBootstrapUtil.bootstrapSparkUserPod(
- kubeTokenManager.getCurrentUser.getShortUserName,
- hadoopBasedSparkPod))
}
override def getAdditionalPodSystemProperties(): Map[String, String] = {
- val resolvedConfValues = kerberosConfSpec.map { hSpec =>
- Map(KERBEROS_DT_SECRET_NAME -> hSpec.dtSecretName,
- KERBEROS_DT_SECRET_KEY -> hSpec.dtSecretItemKey,
- KERBEROS_SPARK_USER_NAME -> hSpec.jobUserName,
- KRB5_CONFIG_MAP_NAME -> krb5CMap.getOrElse(kubernetesConf.krbConfigMapName))
- }.getOrElse(
- Map(KERBEROS_SPARK_USER_NAME ->
- kubeTokenManager.getCurrentUser.getShortUserName))
- Map(HADOOP_CONFIG_MAP_NAME ->
- hadoopConfDirSpec.hadoopConfigMapName.getOrElse(
- kubernetesConf.hadoopConfigMapName)) ++ resolvedConfValues
+ // If a submission-local keytab is provided, update the Spark config so that it knows the
+ // path of the keytab in the driver container.
+ if (needKeytabUpload) {
+ val ktName = new File(keytab.get).getName()
+ Map(KEYTAB.key -> s"$KERBEROS_KEYTAB_MOUNT_POINT/$ktName")
+ } else {
+ Map.empty
+ }
}
override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
- val hadoopConfConfigMap = for {
- hName <- newHadoopConfigMapName
- hFiles <- hadoopConfigurationFiles
- } yield {
- HadoopBootstrapUtil.buildHadoopConfigMap(hName, hFiles)
+ Seq[HasMetadata]() ++ {
+ krb5File.map { path =>
+ val file = new File(path)
+ new ConfigMapBuilder()
+ .withNewMetadata()
+ .withName(kubernetesConf.krbConfigMapName)
+ .endMetadata()
+ .addToData(
+ Map(file.getName() -> Files.toString(file, StandardCharsets.UTF_8)).asJava)
+ .build()
+ }
+ } ++ {
+ // If a submission-local keytab is provided, stash it in a secret.
+ if (needKeytabUpload) {
+ val kt = new File(keytab.get)
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(ktSecretName)
+ .endMetadata()
+ .addToData(kt.getName(), Base64.encodeBase64String(Files.toByteArray(kt)))
+ .build())
+ } else {
+ Nil
+ }
+ } ++ {
+ if (delegationTokens != null) {
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(dtSecretName)
+ .endMetadata()
+ .addToData(KERBEROS_SECRET_KEY, Base64.encodeBase64String(delegationTokens))
+ .build())
+ } else {
+ Nil
+ }
}
+ }
- val krb5ConfigMap = krb5File.map { fileLocation =>
- HadoopBootstrapUtil.buildkrb5ConfigMap(
- kubernetesConf.krbConfigMapName,
- fileLocation)
+ // Visible for testing.
+ def createDelegationTokens(): Array[Byte] = {
--- End diff --
> Yes. And the test here is not interested in whether DT creation works or not.
Hmm, I somewhat disagree, but maybe I am just overly opinionated. I think that the function definition should be:
`def createDelegationTokens(creds: Credentials, tm: HadoopDelegationTokenManager)` so that you can, most importantly imo, mock the `tm` to ensure that the result coming from that method results in the necessary tokens (and _if it is necessary_ to test that: if it returns nothing that no secret is created)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/22911
there was a seemingly corrupt xml file in the jenkins worker, I removed it and will retest.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5477/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98885 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98885/testReport)** for PR 22911 at commit [`78b76a8`](https://github.com/apache/spark/commit/78b76a8ace35b77a6f005dfb26a6e2fd836c1ff4).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98832 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98832/testReport)** for PR 22911 at commit [`a129314`](https://github.com/apache/spark/commit/a129314abb45413d7b46e8510b2a2ea7d61fa916).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5060/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on the issue:
https://github.com/apache/spark/pull/22911
Just a note, this PR has some overlaps with https://github.com/apache/spark/pull/22915. But just after a brief skim, I am a bit hesitant on storing keytabs in secrets, but it might have to be done for client mode support.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5058/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233648073
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -69,97 +72,191 @@ private[spark] class KerberosConfDriverFeatureStep(
"If a Kerberos keytab is specified you must also specify a Kerberos principal")
KubernetesUtils.requireBothOrNeitherDefined(
- existingSecretName,
- existingSecretItemKey,
+ existingDtSecret,
+ existingDtItemKey,
"If a secret data item-key where the data of the Kerberos Delegation Token is specified" +
" you must also specify the name of the secret",
"If a secret storing a Kerberos Delegation Token is specified you must also" +
" specify the item-key where the data is stored")
- private val hadoopConfigurationFiles = hadoopConfDirSpec.hadoopConfDir.map { hConfDir =>
- HadoopBootstrapUtil.getHadoopConfFiles(hConfDir)
+ if (!hasKerberosConf) {
+ logInfo("You have not specified a krb5.conf file locally or via a ConfigMap. " +
+ "Make sure that you have the krb5.conf locally on the driver image.")
}
- private val newHadoopConfigMapName =
- if (hadoopConfDirSpec.hadoopConfigMapName.isEmpty) {
- Some(kubernetesConf.hadoopConfigMapName)
- } else {
- None
- }
- // Either use pre-existing secret or login to create new Secret with DT stored within
- private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
- secretName <- existingSecretName
- secretItemKey <- existingSecretItemKey
- } yield {
- KerberosConfigSpec(
- dtSecret = None,
- dtSecretName = secretName,
- dtSecretItemKey = secretItemKey,
- jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
- }).orElse(
- if (isKerberosEnabled) {
- Some(HadoopKerberosLogin.buildSpec(
- conf,
- kubernetesConf.appResourceNamePrefix,
- kubeTokenManager))
- } else {
- None
+ // Create delegation tokens if needed. This is a lazy val so that it's not populated
+ // unnecessarily. But it needs to be accessible to different methods in this class,
+ // since it's not clear based solely on available configuration options that delegation
+ // tokens are needed when other credentials are not available.
+ private lazy val delegationTokens: Array[Byte] = if (keytab.isEmpty && existingDtSecret.isEmpty) {
+ createDelegationTokens()
+ } else {
+ null
+ }
+
+ private def needKeytabUpload: Boolean = keytab.exists(!Utils.isLocalUri(_))
+
+ private def dtSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-delegation-tokens"
+
+ private def ktSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-kerberos-keytab"
+
+ private def hasKerberosConf: Boolean = krb5CMap.isDefined | krb5File.isDefined
+
+ override def configurePod(original: SparkPod): SparkPod = {
+ original.transform { case pod if hasKerberosConf =>
+ val configMapVolume = if (krb5CMap.isDefined) {
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(krb5CMap.get)
+ .endConfigMap()
+ .build()
+ } else {
+ val krb5Conf = new File(krb5File.get)
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(kubernetesConf.krbConfigMapName)
+ .withItems(new KeyToPathBuilder()
+ .withKey(krb5Conf.getName())
+ .withPath(krb5Conf.getName())
+ .build())
+ .endConfigMap()
+ .build()
+ }
+
+ val podWithVolume = new PodBuilder(pod.pod)
+ .editSpec()
+ .addNewVolumeLike(configMapVolume)
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithMount = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KRB_FILE_VOLUME)
+ .withMountPath(KRB_FILE_DIR_PATH + "/krb5.conf")
+ .withSubPath("krb5.conf")
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWithVolume, containerWithMount)
+ }.transform {
+ case pod if needKeytabUpload =>
+ // If keytab is defined and is a submission-local file (not local: URI), then create a
+ // secret for it. The keytab data will be stored in this secret below.
+ val podWitKeytab = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withNewSecret()
+ .withSecretName(ktSecretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithKeytab = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withMountPath(KERBEROS_KEYTAB_MOUNT_POINT)
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWitKeytab, containerWithKeytab)
+
+ case pod if existingDtSecret.isDefined | delegationTokens != null =>
+ val secretName = existingDtSecret.getOrElse(dtSecretName)
+ val itemKey = existingDtItemKey.getOrElse(KERBEROS_SECRET_KEY)
+
+ val podWithTokens = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withNewSecret()
+ .withSecretName(secretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithTokens = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withMountPath(SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR)
+ .endVolumeMount()
+ .addNewEnv()
+ .withName(ENV_HADOOP_TOKEN_FILE_LOCATION)
+ .withValue(s"$SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR/$itemKey")
+ .endEnv()
+ .build()
+
+ SparkPod(podWithTokens, containerWithTokens)
}
- )
-
- override def configurePod(pod: SparkPod): SparkPod = {
- val hadoopBasedSparkPod = HadoopBootstrapUtil.bootstrapHadoopConfDir(
- hadoopConfDirSpec.hadoopConfDir,
- newHadoopConfigMapName,
- hadoopConfDirSpec.hadoopConfigMapName,
- pod)
- kerberosConfSpec.map { hSpec =>
- HadoopBootstrapUtil.bootstrapKerberosPod(
- hSpec.dtSecretName,
- hSpec.dtSecretItemKey,
- hSpec.jobUserName,
- krb5File,
- Some(kubernetesConf.krbConfigMapName),
- krb5CMap,
- hadoopBasedSparkPod)
- }.getOrElse(
- HadoopBootstrapUtil.bootstrapSparkUserPod(
- kubeTokenManager.getCurrentUser.getShortUserName,
- hadoopBasedSparkPod))
}
override def getAdditionalPodSystemProperties(): Map[String, String] = {
- val resolvedConfValues = kerberosConfSpec.map { hSpec =>
- Map(KERBEROS_DT_SECRET_NAME -> hSpec.dtSecretName,
- KERBEROS_DT_SECRET_KEY -> hSpec.dtSecretItemKey,
- KERBEROS_SPARK_USER_NAME -> hSpec.jobUserName,
- KRB5_CONFIG_MAP_NAME -> krb5CMap.getOrElse(kubernetesConf.krbConfigMapName))
- }.getOrElse(
- Map(KERBEROS_SPARK_USER_NAME ->
- kubeTokenManager.getCurrentUser.getShortUserName))
- Map(HADOOP_CONFIG_MAP_NAME ->
- hadoopConfDirSpec.hadoopConfigMapName.getOrElse(
- kubernetesConf.hadoopConfigMapName)) ++ resolvedConfValues
+ // If a submission-local keytab is provided, update the Spark config so that it knows the
+ // path of the keytab in the driver container.
+ if (needKeytabUpload) {
+ val ktName = new File(keytab.get).getName()
+ Map(KEYTAB.key -> s"$KERBEROS_KEYTAB_MOUNT_POINT/$ktName")
+ } else {
+ Map.empty
+ }
}
override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
- val hadoopConfConfigMap = for {
- hName <- newHadoopConfigMapName
- hFiles <- hadoopConfigurationFiles
- } yield {
- HadoopBootstrapUtil.buildHadoopConfigMap(hName, hFiles)
+ Seq[HasMetadata]() ++ {
+ krb5File.map { path =>
+ val file = new File(path)
+ new ConfigMapBuilder()
+ .withNewMetadata()
+ .withName(kubernetesConf.krbConfigMapName)
+ .endMetadata()
+ .addToData(
+ Map(file.getName() -> Files.toString(file, StandardCharsets.UTF_8)).asJava)
+ .build()
+ }
+ } ++ {
+ // If a submission-local keytab is provided, stash it in a secret.
+ if (needKeytabUpload) {
+ val kt = new File(keytab.get)
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(ktSecretName)
+ .endMetadata()
+ .addToData(kt.getName(), Base64.encodeBase64String(Files.toByteArray(kt)))
+ .build())
+ } else {
+ Nil
+ }
+ } ++ {
+ if (delegationTokens != null) {
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(dtSecretName)
+ .endMetadata()
+ .addToData(KERBEROS_SECRET_KEY, Base64.encodeBase64String(delegationTokens))
+ .build())
+ } else {
+ Nil
+ }
}
+ }
- val krb5ConfigMap = krb5File.map { fileLocation =>
- HadoopBootstrapUtil.buildkrb5ConfigMap(
- kubernetesConf.krbConfigMapName,
- fileLocation)
+ // Visible for testing.
+ def createDelegationTokens(): Array[Byte] = {
--- End diff --
nit: Why isn't this logic done in a `KubernetesDelegationTokenManager` or some Util class? It seems we are over-running this step with a lot of functionality? no? You are also just mocking via
`when(step).createDelegationToken()`, I personally don't agree with the coverage brought up by this method of unit-testing but this is an extension of the discussion on a separate PR. i.e. I would mock the tokenManager and test to ensure that the `obtainDelegationTokens` take in a particular `creds` as an argument (also, so that one could test the if statement below).`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #99842 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99842/testReport)** for PR 22911 at commit [`4143603`](https://github.com/apache/spark/commit/414360315a484d7a32e6817ff2197cd1f3f8f43c).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class ArrowCollectSerializer(Serializer):`
* `class CSVInferSchema(val options: CSVOptions) extends Serializable `
* `class InterpretedSafeProjection(expressions: Seq[Expression]) extends Projection `
* `sealed trait DateTimeFormatter `
* `class Iso8601DateTimeFormatter(`
* `class LegacyDateTimeFormatter(`
* `class LegacyFallbackDateTimeFormatter(`
* `sealed trait DateFormatter `
* `class Iso8601DateFormatter(`
* `class LegacyDateFormatter(`
* `class LegacyFallbackDateFormatter(`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5032/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #99627 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99627/testReport)** for PR 22911 at commit [`05333ea`](https://github.com/apache/spark/commit/05333ea11bc50ade8f3ba8e1d2116bad9d3d7c89).
* This patch **fails build dependency tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/22911
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5686/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98846 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98846/testReport)** for PR 22911 at commit [`78b76a8`](https://github.com/apache/spark/commit/78b76a8ace35b77a6f005dfb26a6e2fd836c1ff4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #99630 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99630/testReport)** for PR 22911 at commit [`05333ea`](https://github.com/apache/spark/commit/05333ea11bc50ade8f3ba8e1d2116bad9d3d7c89).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98842 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98842/testReport)** for PR 22911 at commit [`7e9b3a2`](https://github.com/apache/spark/commit/7e9b3a2218359846176136cc98d63eb841d97f15).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98832/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233649612
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -69,97 +72,191 @@ private[spark] class KerberosConfDriverFeatureStep(
"If a Kerberos keytab is specified you must also specify a Kerberos principal")
KubernetesUtils.requireBothOrNeitherDefined(
- existingSecretName,
- existingSecretItemKey,
+ existingDtSecret,
+ existingDtItemKey,
"If a secret data item-key where the data of the Kerberos Delegation Token is specified" +
" you must also specify the name of the secret",
"If a secret storing a Kerberos Delegation Token is specified you must also" +
" specify the item-key where the data is stored")
- private val hadoopConfigurationFiles = hadoopConfDirSpec.hadoopConfDir.map { hConfDir =>
- HadoopBootstrapUtil.getHadoopConfFiles(hConfDir)
+ if (!hasKerberosConf) {
+ logInfo("You have not specified a krb5.conf file locally or via a ConfigMap. " +
+ "Make sure that you have the krb5.conf locally on the driver image.")
}
- private val newHadoopConfigMapName =
- if (hadoopConfDirSpec.hadoopConfigMapName.isEmpty) {
- Some(kubernetesConf.hadoopConfigMapName)
- } else {
- None
- }
- // Either use pre-existing secret or login to create new Secret with DT stored within
- private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
- secretName <- existingSecretName
- secretItemKey <- existingSecretItemKey
- } yield {
- KerberosConfigSpec(
- dtSecret = None,
- dtSecretName = secretName,
- dtSecretItemKey = secretItemKey,
- jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
- }).orElse(
- if (isKerberosEnabled) {
- Some(HadoopKerberosLogin.buildSpec(
- conf,
- kubernetesConf.appResourceNamePrefix,
- kubeTokenManager))
- } else {
- None
+ // Create delegation tokens if needed. This is a lazy val so that it's not populated
+ // unnecessarily. But it needs to be accessible to different methods in this class,
+ // since it's not clear based solely on available configuration options that delegation
+ // tokens are needed when other credentials are not available.
+ private lazy val delegationTokens: Array[Byte] = if (keytab.isEmpty && existingDtSecret.isEmpty) {
+ createDelegationTokens()
+ } else {
+ null
+ }
+
+ private def needKeytabUpload: Boolean = keytab.exists(!Utils.isLocalUri(_))
+
+ private def dtSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-delegation-tokens"
+
+ private def ktSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-kerberos-keytab"
+
+ private def hasKerberosConf: Boolean = krb5CMap.isDefined | krb5File.isDefined
+
+ override def configurePod(original: SparkPod): SparkPod = {
+ original.transform { case pod if hasKerberosConf =>
+ val configMapVolume = if (krb5CMap.isDefined) {
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(krb5CMap.get)
+ .endConfigMap()
+ .build()
+ } else {
+ val krb5Conf = new File(krb5File.get)
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(kubernetesConf.krbConfigMapName)
+ .withItems(new KeyToPathBuilder()
+ .withKey(krb5Conf.getName())
+ .withPath(krb5Conf.getName())
+ .build())
+ .endConfigMap()
+ .build()
+ }
+
+ val podWithVolume = new PodBuilder(pod.pod)
+ .editSpec()
+ .addNewVolumeLike(configMapVolume)
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithMount = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KRB_FILE_VOLUME)
+ .withMountPath(KRB_FILE_DIR_PATH + "/krb5.conf")
+ .withSubPath("krb5.conf")
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWithVolume, containerWithMount)
+ }.transform {
+ case pod if needKeytabUpload =>
+ // If keytab is defined and is a submission-local file (not local: URI), then create a
+ // secret for it. The keytab data will be stored in this secret below.
+ val podWitKeytab = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withNewSecret()
+ .withSecretName(ktSecretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithKeytab = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withMountPath(KERBEROS_KEYTAB_MOUNT_POINT)
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWitKeytab, containerWithKeytab)
+
+ case pod if existingDtSecret.isDefined | delegationTokens != null =>
+ val secretName = existingDtSecret.getOrElse(dtSecretName)
+ val itemKey = existingDtItemKey.getOrElse(KERBEROS_SECRET_KEY)
+
+ val podWithTokens = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withNewSecret()
+ .withSecretName(secretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithTokens = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withMountPath(SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR)
+ .endVolumeMount()
+ .addNewEnv()
+ .withName(ENV_HADOOP_TOKEN_FILE_LOCATION)
+ .withValue(s"$SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR/$itemKey")
+ .endEnv()
+ .build()
+
+ SparkPod(podWithTokens, containerWithTokens)
}
- )
-
- override def configurePod(pod: SparkPod): SparkPod = {
- val hadoopBasedSparkPod = HadoopBootstrapUtil.bootstrapHadoopConfDir(
- hadoopConfDirSpec.hadoopConfDir,
- newHadoopConfigMapName,
- hadoopConfDirSpec.hadoopConfigMapName,
- pod)
- kerberosConfSpec.map { hSpec =>
- HadoopBootstrapUtil.bootstrapKerberosPod(
- hSpec.dtSecretName,
- hSpec.dtSecretItemKey,
- hSpec.jobUserName,
- krb5File,
- Some(kubernetesConf.krbConfigMapName),
- krb5CMap,
- hadoopBasedSparkPod)
- }.getOrElse(
- HadoopBootstrapUtil.bootstrapSparkUserPod(
- kubeTokenManager.getCurrentUser.getShortUserName,
- hadoopBasedSparkPod))
}
override def getAdditionalPodSystemProperties(): Map[String, String] = {
- val resolvedConfValues = kerberosConfSpec.map { hSpec =>
- Map(KERBEROS_DT_SECRET_NAME -> hSpec.dtSecretName,
- KERBEROS_DT_SECRET_KEY -> hSpec.dtSecretItemKey,
- KERBEROS_SPARK_USER_NAME -> hSpec.jobUserName,
- KRB5_CONFIG_MAP_NAME -> krb5CMap.getOrElse(kubernetesConf.krbConfigMapName))
- }.getOrElse(
- Map(KERBEROS_SPARK_USER_NAME ->
- kubeTokenManager.getCurrentUser.getShortUserName))
- Map(HADOOP_CONFIG_MAP_NAME ->
- hadoopConfDirSpec.hadoopConfigMapName.getOrElse(
- kubernetesConf.hadoopConfigMapName)) ++ resolvedConfValues
+ // If a submission-local keytab is provided, update the Spark config so that it knows the
+ // path of the keytab in the driver container.
+ if (needKeytabUpload) {
+ val ktName = new File(keytab.get).getName()
+ Map(KEYTAB.key -> s"$KERBEROS_KEYTAB_MOUNT_POINT/$ktName")
+ } else {
+ Map.empty
+ }
}
override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
- val hadoopConfConfigMap = for {
- hName <- newHadoopConfigMapName
- hFiles <- hadoopConfigurationFiles
- } yield {
- HadoopBootstrapUtil.buildHadoopConfigMap(hName, hFiles)
+ Seq[HasMetadata]() ++ {
+ krb5File.map { path =>
+ val file = new File(path)
+ new ConfigMapBuilder()
+ .withNewMetadata()
+ .withName(kubernetesConf.krbConfigMapName)
+ .endMetadata()
+ .addToData(
+ Map(file.getName() -> Files.toString(file, StandardCharsets.UTF_8)).asJava)
+ .build()
+ }
+ } ++ {
+ // If a submission-local keytab is provided, stash it in a secret.
+ if (needKeytabUpload) {
+ val kt = new File(keytab.get)
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(ktSecretName)
+ .endMetadata()
+ .addToData(kt.getName(), Base64.encodeBase64String(Files.toByteArray(kt)))
+ .build())
+ } else {
+ Nil
+ }
+ } ++ {
+ if (delegationTokens != null) {
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(dtSecretName)
+ .endMetadata()
+ .addToData(KERBEROS_SECRET_KEY, Base64.encodeBase64String(delegationTokens))
+ .build())
+ } else {
+ Nil
+ }
}
+ }
- val krb5ConfigMap = krb5File.map { fileLocation =>
- HadoopBootstrapUtil.buildkrb5ConfigMap(
- kubernetesConf.krbConfigMapName,
- fileLocation)
+ // Visible for testing.
+ def createDelegationTokens(): Array[Byte] = {
--- End diff --
> Why isn't this logic done in a KubernetesDelegationTokenManager or some Util class?
Because that's not needed. What exactly would doing that achieve?
> I would mock the tokenManager and test to ensure that the obtainDelegationTokens take in a particular creds
That's not what this code is testing. This code is testing that when delegation tokens are created, they are stashed in a secret. For that I'm mocking the creation of delegation tokens, because without any services to talk to, delegation tokens will not be created.
(BTW, that's what mocks should be used for.)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5684/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98880 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98880/testReport)** for PR 22911 at commit [`78b76a8`](https://github.com/apache/spark/commit/78b76a8ace35b77a6f005dfb26a6e2fd836c1ff4).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98838 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98838/testReport)** for PR 22911 at commit [`d4a84bc`](https://github.com/apache/spark/commit/d4a84bc32d6e07c91b72d83913a24719e411369a).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99627/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98846 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98846/testReport)** for PR 22911 at commit [`78b76a8`](https://github.com/apache/spark/commit/78b76a8ace35b77a6f005dfb26a6e2fd836c1ff4).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99404/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #99404 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99404/testReport)** for PR 22911 at commit [`88f1bb5`](https://github.com/apache/spark/commit/88f1bb516fa6098125fb7f77fd548a1d423cf745).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by mccheah <gi...@git.apache.org>.
Github user mccheah commented on the issue:
https://github.com/apache/spark/pull/22911
@ifilonenko @skonto
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/22911
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #99842 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99842/testReport)** for PR 22911 at commit [`4143603`](https://github.com/apache/spark/commit/414360315a484d7a32e6817ff2197cd1f3f8f43c).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5029/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5030/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/4680/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233658128
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -69,97 +72,191 @@ private[spark] class KerberosConfDriverFeatureStep(
"If a Kerberos keytab is specified you must also specify a Kerberos principal")
KubernetesUtils.requireBothOrNeitherDefined(
- existingSecretName,
- existingSecretItemKey,
+ existingDtSecret,
+ existingDtItemKey,
"If a secret data item-key where the data of the Kerberos Delegation Token is specified" +
" you must also specify the name of the secret",
"If a secret storing a Kerberos Delegation Token is specified you must also" +
" specify the item-key where the data is stored")
- private val hadoopConfigurationFiles = hadoopConfDirSpec.hadoopConfDir.map { hConfDir =>
- HadoopBootstrapUtil.getHadoopConfFiles(hConfDir)
+ if (!hasKerberosConf) {
+ logInfo("You have not specified a krb5.conf file locally or via a ConfigMap. " +
+ "Make sure that you have the krb5.conf locally on the driver image.")
}
- private val newHadoopConfigMapName =
- if (hadoopConfDirSpec.hadoopConfigMapName.isEmpty) {
- Some(kubernetesConf.hadoopConfigMapName)
- } else {
- None
- }
- // Either use pre-existing secret or login to create new Secret with DT stored within
- private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
- secretName <- existingSecretName
- secretItemKey <- existingSecretItemKey
- } yield {
- KerberosConfigSpec(
- dtSecret = None,
- dtSecretName = secretName,
- dtSecretItemKey = secretItemKey,
- jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
- }).orElse(
- if (isKerberosEnabled) {
- Some(HadoopKerberosLogin.buildSpec(
- conf,
- kubernetesConf.appResourceNamePrefix,
- kubeTokenManager))
- } else {
- None
+ // Create delegation tokens if needed. This is a lazy val so that it's not populated
+ // unnecessarily. But it needs to be accessible to different methods in this class,
+ // since it's not clear based solely on available configuration options that delegation
+ // tokens are needed when other credentials are not available.
+ private lazy val delegationTokens: Array[Byte] = if (keytab.isEmpty && existingDtSecret.isEmpty) {
+ createDelegationTokens()
+ } else {
+ null
+ }
+
+ private def needKeytabUpload: Boolean = keytab.exists(!Utils.isLocalUri(_))
+
+ private def dtSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-delegation-tokens"
+
+ private def ktSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-kerberos-keytab"
+
+ private def hasKerberosConf: Boolean = krb5CMap.isDefined | krb5File.isDefined
+
+ override def configurePod(original: SparkPod): SparkPod = {
+ original.transform { case pod if hasKerberosConf =>
+ val configMapVolume = if (krb5CMap.isDefined) {
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(krb5CMap.get)
+ .endConfigMap()
+ .build()
+ } else {
+ val krb5Conf = new File(krb5File.get)
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(kubernetesConf.krbConfigMapName)
+ .withItems(new KeyToPathBuilder()
+ .withKey(krb5Conf.getName())
+ .withPath(krb5Conf.getName())
+ .build())
+ .endConfigMap()
+ .build()
+ }
+
+ val podWithVolume = new PodBuilder(pod.pod)
+ .editSpec()
+ .addNewVolumeLike(configMapVolume)
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithMount = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KRB_FILE_VOLUME)
+ .withMountPath(KRB_FILE_DIR_PATH + "/krb5.conf")
+ .withSubPath("krb5.conf")
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWithVolume, containerWithMount)
+ }.transform {
+ case pod if needKeytabUpload =>
+ // If keytab is defined and is a submission-local file (not local: URI), then create a
+ // secret for it. The keytab data will be stored in this secret below.
+ val podWitKeytab = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withNewSecret()
+ .withSecretName(ktSecretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithKeytab = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withMountPath(KERBEROS_KEYTAB_MOUNT_POINT)
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWitKeytab, containerWithKeytab)
+
+ case pod if existingDtSecret.isDefined | delegationTokens != null =>
+ val secretName = existingDtSecret.getOrElse(dtSecretName)
+ val itemKey = existingDtItemKey.getOrElse(KERBEROS_SECRET_KEY)
+
+ val podWithTokens = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withNewSecret()
+ .withSecretName(secretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithTokens = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withMountPath(SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR)
+ .endVolumeMount()
+ .addNewEnv()
+ .withName(ENV_HADOOP_TOKEN_FILE_LOCATION)
+ .withValue(s"$SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR/$itemKey")
+ .endEnv()
+ .build()
+
+ SparkPod(podWithTokens, containerWithTokens)
}
- )
-
- override def configurePod(pod: SparkPod): SparkPod = {
- val hadoopBasedSparkPod = HadoopBootstrapUtil.bootstrapHadoopConfDir(
- hadoopConfDirSpec.hadoopConfDir,
- newHadoopConfigMapName,
- hadoopConfDirSpec.hadoopConfigMapName,
- pod)
- kerberosConfSpec.map { hSpec =>
- HadoopBootstrapUtil.bootstrapKerberosPod(
- hSpec.dtSecretName,
- hSpec.dtSecretItemKey,
- hSpec.jobUserName,
- krb5File,
- Some(kubernetesConf.krbConfigMapName),
- krb5CMap,
- hadoopBasedSparkPod)
- }.getOrElse(
- HadoopBootstrapUtil.bootstrapSparkUserPod(
- kubeTokenManager.getCurrentUser.getShortUserName,
- hadoopBasedSparkPod))
}
override def getAdditionalPodSystemProperties(): Map[String, String] = {
- val resolvedConfValues = kerberosConfSpec.map { hSpec =>
- Map(KERBEROS_DT_SECRET_NAME -> hSpec.dtSecretName,
- KERBEROS_DT_SECRET_KEY -> hSpec.dtSecretItemKey,
- KERBEROS_SPARK_USER_NAME -> hSpec.jobUserName,
- KRB5_CONFIG_MAP_NAME -> krb5CMap.getOrElse(kubernetesConf.krbConfigMapName))
- }.getOrElse(
- Map(KERBEROS_SPARK_USER_NAME ->
- kubeTokenManager.getCurrentUser.getShortUserName))
- Map(HADOOP_CONFIG_MAP_NAME ->
- hadoopConfDirSpec.hadoopConfigMapName.getOrElse(
- kubernetesConf.hadoopConfigMapName)) ++ resolvedConfValues
+ // If a submission-local keytab is provided, update the Spark config so that it knows the
+ // path of the keytab in the driver container.
+ if (needKeytabUpload) {
+ val ktName = new File(keytab.get).getName()
+ Map(KEYTAB.key -> s"$KERBEROS_KEYTAB_MOUNT_POINT/$ktName")
+ } else {
+ Map.empty
+ }
}
override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
- val hadoopConfConfigMap = for {
- hName <- newHadoopConfigMapName
- hFiles <- hadoopConfigurationFiles
- } yield {
- HadoopBootstrapUtil.buildHadoopConfigMap(hName, hFiles)
+ Seq[HasMetadata]() ++ {
+ krb5File.map { path =>
+ val file = new File(path)
+ new ConfigMapBuilder()
+ .withNewMetadata()
+ .withName(kubernetesConf.krbConfigMapName)
+ .endMetadata()
+ .addToData(
+ Map(file.getName() -> Files.toString(file, StandardCharsets.UTF_8)).asJava)
+ .build()
+ }
+ } ++ {
+ // If a submission-local keytab is provided, stash it in a secret.
+ if (needKeytabUpload) {
+ val kt = new File(keytab.get)
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(ktSecretName)
+ .endMetadata()
+ .addToData(kt.getName(), Base64.encodeBase64String(Files.toByteArray(kt)))
+ .build())
+ } else {
+ Nil
+ }
+ } ++ {
+ if (delegationTokens != null) {
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(dtSecretName)
+ .endMetadata()
+ .addToData(KERBEROS_SECRET_KEY, Base64.encodeBase64String(delegationTokens))
+ .build())
+ } else {
+ Nil
+ }
}
+ }
- val krb5ConfigMap = krb5File.map { fileLocation =>
- HadoopBootstrapUtil.buildkrb5ConfigMap(
- kubernetesConf.krbConfigMapName,
- fileLocation)
+ // Visible for testing.
+ def createDelegationTokens(): Array[Byte] = {
--- End diff --
> that is the function of a DelegationTokenManager. To handle nuances related to DT management
Yes. And the test here is not interested in whether DT creation works or not. It's interested in that if they are created, then they are stashed in a secret.
> then it would make sense to mock a KubernetesDelegtionTokenManager and just call kdtm.obtainTokens()
Would it make you happier if instead of having instead of the current method, have this:
```
def createDelegationTokens(creds: Credentials)
```
The only extra thing you'd really be testing is the call to `SparkHadoopUtil.get.serialize`. I don't really see a lot of gains there.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98833 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98833/testReport)** for PR 22911 at commit [`d8416cd`](https://github.com/apache/spark/commit/d8416cda0e23af43a86b129a4a102f37bcbdaf4c).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5029/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5866/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/22911
on a non-testing not, any further feedback here?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233653814
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -69,97 +72,191 @@ private[spark] class KerberosConfDriverFeatureStep(
"If a Kerberos keytab is specified you must also specify a Kerberos principal")
KubernetesUtils.requireBothOrNeitherDefined(
- existingSecretName,
- existingSecretItemKey,
+ existingDtSecret,
+ existingDtItemKey,
"If a secret data item-key where the data of the Kerberos Delegation Token is specified" +
" you must also specify the name of the secret",
"If a secret storing a Kerberos Delegation Token is specified you must also" +
" specify the item-key where the data is stored")
- private val hadoopConfigurationFiles = hadoopConfDirSpec.hadoopConfDir.map { hConfDir =>
- HadoopBootstrapUtil.getHadoopConfFiles(hConfDir)
+ if (!hasKerberosConf) {
+ logInfo("You have not specified a krb5.conf file locally or via a ConfigMap. " +
+ "Make sure that you have the krb5.conf locally on the driver image.")
}
- private val newHadoopConfigMapName =
- if (hadoopConfDirSpec.hadoopConfigMapName.isEmpty) {
- Some(kubernetesConf.hadoopConfigMapName)
- } else {
- None
- }
- // Either use pre-existing secret or login to create new Secret with DT stored within
- private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
- secretName <- existingSecretName
- secretItemKey <- existingSecretItemKey
- } yield {
- KerberosConfigSpec(
- dtSecret = None,
- dtSecretName = secretName,
- dtSecretItemKey = secretItemKey,
- jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
- }).orElse(
- if (isKerberosEnabled) {
- Some(HadoopKerberosLogin.buildSpec(
- conf,
- kubernetesConf.appResourceNamePrefix,
- kubeTokenManager))
- } else {
- None
+ // Create delegation tokens if needed. This is a lazy val so that it's not populated
+ // unnecessarily. But it needs to be accessible to different methods in this class,
+ // since it's not clear based solely on available configuration options that delegation
+ // tokens are needed when other credentials are not available.
+ private lazy val delegationTokens: Array[Byte] = if (keytab.isEmpty && existingDtSecret.isEmpty) {
+ createDelegationTokens()
+ } else {
+ null
+ }
+
+ private def needKeytabUpload: Boolean = keytab.exists(!Utils.isLocalUri(_))
+
+ private def dtSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-delegation-tokens"
+
+ private def ktSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-kerberos-keytab"
+
+ private def hasKerberosConf: Boolean = krb5CMap.isDefined | krb5File.isDefined
+
+ override def configurePod(original: SparkPod): SparkPod = {
+ original.transform { case pod if hasKerberosConf =>
+ val configMapVolume = if (krb5CMap.isDefined) {
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(krb5CMap.get)
+ .endConfigMap()
+ .build()
+ } else {
+ val krb5Conf = new File(krb5File.get)
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(kubernetesConf.krbConfigMapName)
+ .withItems(new KeyToPathBuilder()
+ .withKey(krb5Conf.getName())
+ .withPath(krb5Conf.getName())
+ .build())
+ .endConfigMap()
+ .build()
+ }
+
+ val podWithVolume = new PodBuilder(pod.pod)
+ .editSpec()
+ .addNewVolumeLike(configMapVolume)
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithMount = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KRB_FILE_VOLUME)
+ .withMountPath(KRB_FILE_DIR_PATH + "/krb5.conf")
+ .withSubPath("krb5.conf")
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWithVolume, containerWithMount)
+ }.transform {
+ case pod if needKeytabUpload =>
+ // If keytab is defined and is a submission-local file (not local: URI), then create a
+ // secret for it. The keytab data will be stored in this secret below.
+ val podWitKeytab = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withNewSecret()
+ .withSecretName(ktSecretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithKeytab = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withMountPath(KERBEROS_KEYTAB_MOUNT_POINT)
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWitKeytab, containerWithKeytab)
+
+ case pod if existingDtSecret.isDefined | delegationTokens != null =>
+ val secretName = existingDtSecret.getOrElse(dtSecretName)
+ val itemKey = existingDtItemKey.getOrElse(KERBEROS_SECRET_KEY)
+
+ val podWithTokens = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withNewSecret()
+ .withSecretName(secretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithTokens = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withMountPath(SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR)
+ .endVolumeMount()
+ .addNewEnv()
+ .withName(ENV_HADOOP_TOKEN_FILE_LOCATION)
+ .withValue(s"$SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR/$itemKey")
+ .endEnv()
+ .build()
+
+ SparkPod(podWithTokens, containerWithTokens)
}
- )
-
- override def configurePod(pod: SparkPod): SparkPod = {
- val hadoopBasedSparkPod = HadoopBootstrapUtil.bootstrapHadoopConfDir(
- hadoopConfDirSpec.hadoopConfDir,
- newHadoopConfigMapName,
- hadoopConfDirSpec.hadoopConfigMapName,
- pod)
- kerberosConfSpec.map { hSpec =>
- HadoopBootstrapUtil.bootstrapKerberosPod(
- hSpec.dtSecretName,
- hSpec.dtSecretItemKey,
- hSpec.jobUserName,
- krb5File,
- Some(kubernetesConf.krbConfigMapName),
- krb5CMap,
- hadoopBasedSparkPod)
- }.getOrElse(
- HadoopBootstrapUtil.bootstrapSparkUserPod(
- kubeTokenManager.getCurrentUser.getShortUserName,
- hadoopBasedSparkPod))
}
override def getAdditionalPodSystemProperties(): Map[String, String] = {
- val resolvedConfValues = kerberosConfSpec.map { hSpec =>
- Map(KERBEROS_DT_SECRET_NAME -> hSpec.dtSecretName,
- KERBEROS_DT_SECRET_KEY -> hSpec.dtSecretItemKey,
- KERBEROS_SPARK_USER_NAME -> hSpec.jobUserName,
- KRB5_CONFIG_MAP_NAME -> krb5CMap.getOrElse(kubernetesConf.krbConfigMapName))
- }.getOrElse(
- Map(KERBEROS_SPARK_USER_NAME ->
- kubeTokenManager.getCurrentUser.getShortUserName))
- Map(HADOOP_CONFIG_MAP_NAME ->
- hadoopConfDirSpec.hadoopConfigMapName.getOrElse(
- kubernetesConf.hadoopConfigMapName)) ++ resolvedConfValues
+ // If a submission-local keytab is provided, update the Spark config so that it knows the
+ // path of the keytab in the driver container.
+ if (needKeytabUpload) {
+ val ktName = new File(keytab.get).getName()
+ Map(KEYTAB.key -> s"$KERBEROS_KEYTAB_MOUNT_POINT/$ktName")
+ } else {
+ Map.empty
+ }
}
override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
- val hadoopConfConfigMap = for {
- hName <- newHadoopConfigMapName
- hFiles <- hadoopConfigurationFiles
- } yield {
- HadoopBootstrapUtil.buildHadoopConfigMap(hName, hFiles)
+ Seq[HasMetadata]() ++ {
--- End diff --
> seems like a lot of logic going into this step
The logic itself isn't really different from before. The only difference is that the "what is done" for each combination of configs now also lives here.
It could be broken down into separate steps, but then you'd have 3 related steps that need to agree on how to treat the configuration of the driver; e.g. if the step that adds they keytab runs, then the step that creates delegation tokens shouldn't run. And that makes the code harder to follow, because you have to look at multiple places to understand how this works.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5684/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98885 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98885/testReport)** for PR 22911 at commit [`78b76a8`](https://github.com/apache/spark/commit/78b76a8ace35b77a6f005dfb26a6e2fd836c1ff4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on the issue:
https://github.com/apache/spark/pull/22911
> But whether to store it in secrets is not a question. You either store it in a secret or you don't support the keytab/principal feature in Spark at all, and we can delete a bunch of code here.
But can we, for a specific mode, client or cluster, tune whether the keytab is put into a secret? Right now, if a keytab is specified, regardless, a secret is created. I do wish to support keytab/principal so I think that storage in secrets should be fine.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98838 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98838/testReport)** for PR 22911 at commit [`d4a84bc`](https://github.com/apache/spark/commit/d4a84bc32d6e07c91b72d83913a24719e411369a).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98846/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233667509
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -69,97 +72,191 @@ private[spark] class KerberosConfDriverFeatureStep(
"If a Kerberos keytab is specified you must also specify a Kerberos principal")
KubernetesUtils.requireBothOrNeitherDefined(
- existingSecretName,
- existingSecretItemKey,
+ existingDtSecret,
+ existingDtItemKey,
"If a secret data item-key where the data of the Kerberos Delegation Token is specified" +
" you must also specify the name of the secret",
"If a secret storing a Kerberos Delegation Token is specified you must also" +
" specify the item-key where the data is stored")
- private val hadoopConfigurationFiles = hadoopConfDirSpec.hadoopConfDir.map { hConfDir =>
- HadoopBootstrapUtil.getHadoopConfFiles(hConfDir)
+ if (!hasKerberosConf) {
+ logInfo("You have not specified a krb5.conf file locally or via a ConfigMap. " +
+ "Make sure that you have the krb5.conf locally on the driver image.")
}
- private val newHadoopConfigMapName =
- if (hadoopConfDirSpec.hadoopConfigMapName.isEmpty) {
- Some(kubernetesConf.hadoopConfigMapName)
- } else {
- None
- }
- // Either use pre-existing secret or login to create new Secret with DT stored within
- private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
- secretName <- existingSecretName
- secretItemKey <- existingSecretItemKey
- } yield {
- KerberosConfigSpec(
- dtSecret = None,
- dtSecretName = secretName,
- dtSecretItemKey = secretItemKey,
- jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
- }).orElse(
- if (isKerberosEnabled) {
- Some(HadoopKerberosLogin.buildSpec(
- conf,
- kubernetesConf.appResourceNamePrefix,
- kubeTokenManager))
- } else {
- None
+ // Create delegation tokens if needed. This is a lazy val so that it's not populated
+ // unnecessarily. But it needs to be accessible to different methods in this class,
+ // since it's not clear based solely on available configuration options that delegation
+ // tokens are needed when other credentials are not available.
+ private lazy val delegationTokens: Array[Byte] = if (keytab.isEmpty && existingDtSecret.isEmpty) {
+ createDelegationTokens()
+ } else {
+ null
+ }
+
+ private def needKeytabUpload: Boolean = keytab.exists(!Utils.isLocalUri(_))
+
+ private def dtSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-delegation-tokens"
+
+ private def ktSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-kerberos-keytab"
+
+ private def hasKerberosConf: Boolean = krb5CMap.isDefined | krb5File.isDefined
+
+ override def configurePod(original: SparkPod): SparkPod = {
+ original.transform { case pod if hasKerberosConf =>
+ val configMapVolume = if (krb5CMap.isDefined) {
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(krb5CMap.get)
+ .endConfigMap()
+ .build()
+ } else {
+ val krb5Conf = new File(krb5File.get)
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(kubernetesConf.krbConfigMapName)
+ .withItems(new KeyToPathBuilder()
+ .withKey(krb5Conf.getName())
+ .withPath(krb5Conf.getName())
+ .build())
+ .endConfigMap()
+ .build()
+ }
+
+ val podWithVolume = new PodBuilder(pod.pod)
+ .editSpec()
+ .addNewVolumeLike(configMapVolume)
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithMount = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KRB_FILE_VOLUME)
+ .withMountPath(KRB_FILE_DIR_PATH + "/krb5.conf")
+ .withSubPath("krb5.conf")
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWithVolume, containerWithMount)
+ }.transform {
+ case pod if needKeytabUpload =>
+ // If keytab is defined and is a submission-local file (not local: URI), then create a
+ // secret for it. The keytab data will be stored in this secret below.
+ val podWitKeytab = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withNewSecret()
+ .withSecretName(ktSecretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithKeytab = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withMountPath(KERBEROS_KEYTAB_MOUNT_POINT)
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWitKeytab, containerWithKeytab)
+
+ case pod if existingDtSecret.isDefined | delegationTokens != null =>
+ val secretName = existingDtSecret.getOrElse(dtSecretName)
+ val itemKey = existingDtItemKey.getOrElse(KERBEROS_SECRET_KEY)
+
+ val podWithTokens = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withNewSecret()
+ .withSecretName(secretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithTokens = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withMountPath(SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR)
+ .endVolumeMount()
+ .addNewEnv()
+ .withName(ENV_HADOOP_TOKEN_FILE_LOCATION)
+ .withValue(s"$SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR/$itemKey")
+ .endEnv()
+ .build()
+
+ SparkPod(podWithTokens, containerWithTokens)
}
- )
-
- override def configurePod(pod: SparkPod): SparkPod = {
- val hadoopBasedSparkPod = HadoopBootstrapUtil.bootstrapHadoopConfDir(
- hadoopConfDirSpec.hadoopConfDir,
- newHadoopConfigMapName,
- hadoopConfDirSpec.hadoopConfigMapName,
- pod)
- kerberosConfSpec.map { hSpec =>
- HadoopBootstrapUtil.bootstrapKerberosPod(
- hSpec.dtSecretName,
- hSpec.dtSecretItemKey,
- hSpec.jobUserName,
- krb5File,
- Some(kubernetesConf.krbConfigMapName),
- krb5CMap,
- hadoopBasedSparkPod)
- }.getOrElse(
- HadoopBootstrapUtil.bootstrapSparkUserPod(
- kubeTokenManager.getCurrentUser.getShortUserName,
- hadoopBasedSparkPod))
}
override def getAdditionalPodSystemProperties(): Map[String, String] = {
- val resolvedConfValues = kerberosConfSpec.map { hSpec =>
- Map(KERBEROS_DT_SECRET_NAME -> hSpec.dtSecretName,
- KERBEROS_DT_SECRET_KEY -> hSpec.dtSecretItemKey,
- KERBEROS_SPARK_USER_NAME -> hSpec.jobUserName,
- KRB5_CONFIG_MAP_NAME -> krb5CMap.getOrElse(kubernetesConf.krbConfigMapName))
- }.getOrElse(
- Map(KERBEROS_SPARK_USER_NAME ->
- kubeTokenManager.getCurrentUser.getShortUserName))
- Map(HADOOP_CONFIG_MAP_NAME ->
- hadoopConfDirSpec.hadoopConfigMapName.getOrElse(
- kubernetesConf.hadoopConfigMapName)) ++ resolvedConfValues
+ // If a submission-local keytab is provided, update the Spark config so that it knows the
+ // path of the keytab in the driver container.
+ if (needKeytabUpload) {
+ val ktName = new File(keytab.get).getName()
+ Map(KEYTAB.key -> s"$KERBEROS_KEYTAB_MOUNT_POINT/$ktName")
+ } else {
+ Map.empty
+ }
}
override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
- val hadoopConfConfigMap = for {
- hName <- newHadoopConfigMapName
- hFiles <- hadoopConfigurationFiles
- } yield {
- HadoopBootstrapUtil.buildHadoopConfigMap(hName, hFiles)
+ Seq[HasMetadata]() ++ {
+ krb5File.map { path =>
+ val file = new File(path)
+ new ConfigMapBuilder()
+ .withNewMetadata()
+ .withName(kubernetesConf.krbConfigMapName)
+ .endMetadata()
+ .addToData(
+ Map(file.getName() -> Files.toString(file, StandardCharsets.UTF_8)).asJava)
+ .build()
+ }
+ } ++ {
+ // If a submission-local keytab is provided, stash it in a secret.
+ if (needKeytabUpload) {
+ val kt = new File(keytab.get)
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(ktSecretName)
+ .endMetadata()
+ .addToData(kt.getName(), Base64.encodeBase64String(Files.toByteArray(kt)))
+ .build())
+ } else {
+ Nil
+ }
+ } ++ {
+ if (delegationTokens != null) {
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(dtSecretName)
+ .endMetadata()
+ .addToData(KERBEROS_SECRET_KEY, Base64.encodeBase64String(delegationTokens))
+ .build())
+ } else {
+ Nil
+ }
}
+ }
- val krb5ConfigMap = krb5File.map { fileLocation =>
- HadoopBootstrapUtil.buildkrb5ConfigMap(
- kubernetesConf.krbConfigMapName,
- fileLocation)
+ // Visible for testing.
+ def createDelegationTokens(): Array[Byte] = {
--- End diff --
> Hmm, I somewhat disagree, but maybe I am just overly opinionated.
The code that creates delegation tokens is not in this class. If delegation token creation is to be tested, it needs to be tested in the test code for `HadoopDelegationTokenManager`.
All this class does is take the tokens that other class creates and stash them in a secret.
I'll change this to not need a mock at all, mostly to make a point.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99630/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233650115
--- Diff: resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStepSuite.scala ---
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s.features
+
+import java.io.File
+import java.nio.charset.StandardCharsets.UTF_8
+
+import scala.collection.JavaConverters._
+
+import com.google.common.io.Files
+import io.fabric8.kubernetes.api.model.{ConfigMap, Secret}
+import org.mockito.Mockito._
+
+import org.apache.spark.{SparkConf, SparkFunSuite}
+import org.apache.spark.deploy.k8s._
+import org.apache.spark.deploy.k8s.Config._
+import org.apache.spark.deploy.k8s.Constants._
+import org.apache.spark.deploy.k8s.submit.JavaMainAppResource
+import org.apache.spark.internal.config._
+import org.apache.spark.util.Utils
+
+class KerberosConfDriverFeatureStepSuite extends SparkFunSuite {
+
+ import KubernetesFeaturesTestUtils._
+ import SecretVolumeUtils._
+
+ private val tmpDir = Utils.createTempDir()
+
+ test("mount krb5 config map if defined") {
+ val configMap = "testConfigMap"
+ val step = createStep(
+ new SparkConf(false).set(KUBERNETES_KERBEROS_KRB5_CONFIG_MAP, configMap))
+
+ checkPodForKrbConf(step.configurePod(SparkPod.initialPod()), configMap)
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ assert(filter[ConfigMap](step.getAdditionalKubernetesResources()).isEmpty)
+ }
+
+ test("create krb5.conf config map if local config provided") {
+ val krbConf = File.createTempFile("krb5", ".conf", tmpDir)
+ Files.write("some data", krbConf, UTF_8)
+
+ val sparkConf = new SparkConf(false)
+ .set(KUBERNETES_KERBEROS_KRB5_FILE, krbConf.getAbsolutePath())
+ val step = createStep(sparkConf)
+
+ val confMap = filter[ConfigMap](step.getAdditionalKubernetesResources()).head
+ assert(confMap.getData().keySet().asScala === Set(krbConf.getName()))
+
+ checkPodForKrbConf(step.configurePod(SparkPod.initialPod()), confMap.getMetadata().getName())
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ }
+
+ test("create keytab secret if client keytab file used") {
+ val keytab = File.createTempFile("keytab", ".bin", tmpDir)
+ Files.write("some data", keytab, UTF_8)
+
+ val sparkConf = new SparkConf(false)
+ .set(KEYTAB, keytab.getAbsolutePath())
+ .set(PRINCIPAL, "alice")
+ val step = createStep(sparkConf)
+
+ val pod = step.configurePod(SparkPod.initialPod())
+ assert(podHasVolume(pod.pod, KERBEROS_KEYTAB_VOLUME))
+ assert(containerHasVolume(pod.container, KERBEROS_KEYTAB_VOLUME, KERBEROS_KEYTAB_MOUNT_POINT))
+
+ assert(step.getAdditionalPodSystemProperties().keys === Set(KEYTAB.key))
+
+ val secret = filter[Secret](step.getAdditionalKubernetesResources()).head
+ assert(secret.getData().keySet().asScala === Set(keytab.getName()))
+ }
+
+ test("do nothing if container-local keytab used") {
+ val sparkConf = new SparkConf(false)
+ .set(KEYTAB, "local:/my.keytab")
+ .set(PRINCIPAL, "alice")
+ val step = createStep(sparkConf)
+
+ val initial = SparkPod.initialPod()
+ assert(step.configurePod(initial) === initial)
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ assert(step.getAdditionalKubernetesResources().isEmpty)
+ }
+
+ test("mount delegation tokens if provided") {
+ val dtSecret = "tokenSecret"
+ val sparkConf = new SparkConf(false)
+ .set(KUBERNETES_KERBEROS_DT_SECRET_NAME, dtSecret)
+ .set(KUBERNETES_KERBEROS_DT_SECRET_ITEM_KEY, "dtokens")
+ val step = createStep(sparkConf)
+
+ checkPodForTokens(step.configurePod(SparkPod.initialPod()), dtSecret)
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ assert(step.getAdditionalKubernetesResources().isEmpty)
+ }
+
+ test("create delegation tokens if needed") {
+ val step = spy(createStep(new SparkConf(false)))
+ doReturn(Array[Byte](0x4, 0x2)).when(step).createDelegationTokens()
--- End diff --
Should we check the contents of the token in the unit test?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233657016
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStep.scala ---
@@ -69,97 +72,191 @@ private[spark] class KerberosConfDriverFeatureStep(
"If a Kerberos keytab is specified you must also specify a Kerberos principal")
KubernetesUtils.requireBothOrNeitherDefined(
- existingSecretName,
- existingSecretItemKey,
+ existingDtSecret,
+ existingDtItemKey,
"If a secret data item-key where the data of the Kerberos Delegation Token is specified" +
" you must also specify the name of the secret",
"If a secret storing a Kerberos Delegation Token is specified you must also" +
" specify the item-key where the data is stored")
- private val hadoopConfigurationFiles = hadoopConfDirSpec.hadoopConfDir.map { hConfDir =>
- HadoopBootstrapUtil.getHadoopConfFiles(hConfDir)
+ if (!hasKerberosConf) {
+ logInfo("You have not specified a krb5.conf file locally or via a ConfigMap. " +
+ "Make sure that you have the krb5.conf locally on the driver image.")
}
- private val newHadoopConfigMapName =
- if (hadoopConfDirSpec.hadoopConfigMapName.isEmpty) {
- Some(kubernetesConf.hadoopConfigMapName)
- } else {
- None
- }
- // Either use pre-existing secret or login to create new Secret with DT stored within
- private val kerberosConfSpec: Option[KerberosConfigSpec] = (for {
- secretName <- existingSecretName
- secretItemKey <- existingSecretItemKey
- } yield {
- KerberosConfigSpec(
- dtSecret = None,
- dtSecretName = secretName,
- dtSecretItemKey = secretItemKey,
- jobUserName = kubeTokenManager.getCurrentUser.getShortUserName)
- }).orElse(
- if (isKerberosEnabled) {
- Some(HadoopKerberosLogin.buildSpec(
- conf,
- kubernetesConf.appResourceNamePrefix,
- kubeTokenManager))
- } else {
- None
+ // Create delegation tokens if needed. This is a lazy val so that it's not populated
+ // unnecessarily. But it needs to be accessible to different methods in this class,
+ // since it's not clear based solely on available configuration options that delegation
+ // tokens are needed when other credentials are not available.
+ private lazy val delegationTokens: Array[Byte] = if (keytab.isEmpty && existingDtSecret.isEmpty) {
+ createDelegationTokens()
+ } else {
+ null
+ }
+
+ private def needKeytabUpload: Boolean = keytab.exists(!Utils.isLocalUri(_))
+
+ private def dtSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-delegation-tokens"
+
+ private def ktSecretName: String = s"${kubernetesConf.appResourceNamePrefix}-kerberos-keytab"
+
+ private def hasKerberosConf: Boolean = krb5CMap.isDefined | krb5File.isDefined
+
+ override def configurePod(original: SparkPod): SparkPod = {
+ original.transform { case pod if hasKerberosConf =>
+ val configMapVolume = if (krb5CMap.isDefined) {
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(krb5CMap.get)
+ .endConfigMap()
+ .build()
+ } else {
+ val krb5Conf = new File(krb5File.get)
+ new VolumeBuilder()
+ .withName(KRB_FILE_VOLUME)
+ .withNewConfigMap()
+ .withName(kubernetesConf.krbConfigMapName)
+ .withItems(new KeyToPathBuilder()
+ .withKey(krb5Conf.getName())
+ .withPath(krb5Conf.getName())
+ .build())
+ .endConfigMap()
+ .build()
+ }
+
+ val podWithVolume = new PodBuilder(pod.pod)
+ .editSpec()
+ .addNewVolumeLike(configMapVolume)
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithMount = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KRB_FILE_VOLUME)
+ .withMountPath(KRB_FILE_DIR_PATH + "/krb5.conf")
+ .withSubPath("krb5.conf")
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWithVolume, containerWithMount)
+ }.transform {
+ case pod if needKeytabUpload =>
+ // If keytab is defined and is a submission-local file (not local: URI), then create a
+ // secret for it. The keytab data will be stored in this secret below.
+ val podWitKeytab = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withNewSecret()
+ .withSecretName(ktSecretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithKeytab = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(KERBEROS_KEYTAB_VOLUME)
+ .withMountPath(KERBEROS_KEYTAB_MOUNT_POINT)
+ .endVolumeMount()
+ .build()
+
+ SparkPod(podWitKeytab, containerWithKeytab)
+
+ case pod if existingDtSecret.isDefined | delegationTokens != null =>
+ val secretName = existingDtSecret.getOrElse(dtSecretName)
+ val itemKey = existingDtItemKey.getOrElse(KERBEROS_SECRET_KEY)
+
+ val podWithTokens = new PodBuilder(pod.pod)
+ .editOrNewSpec()
+ .addNewVolume()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withNewSecret()
+ .withSecretName(secretName)
+ .endSecret()
+ .endVolume()
+ .endSpec()
+ .build()
+
+ val containerWithTokens = new ContainerBuilder(pod.container)
+ .addNewVolumeMount()
+ .withName(SPARK_APP_HADOOP_SECRET_VOLUME_NAME)
+ .withMountPath(SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR)
+ .endVolumeMount()
+ .addNewEnv()
+ .withName(ENV_HADOOP_TOKEN_FILE_LOCATION)
+ .withValue(s"$SPARK_APP_HADOOP_CREDENTIALS_BASE_DIR/$itemKey")
+ .endEnv()
+ .build()
+
+ SparkPod(podWithTokens, containerWithTokens)
}
- )
-
- override def configurePod(pod: SparkPod): SparkPod = {
- val hadoopBasedSparkPod = HadoopBootstrapUtil.bootstrapHadoopConfDir(
- hadoopConfDirSpec.hadoopConfDir,
- newHadoopConfigMapName,
- hadoopConfDirSpec.hadoopConfigMapName,
- pod)
- kerberosConfSpec.map { hSpec =>
- HadoopBootstrapUtil.bootstrapKerberosPod(
- hSpec.dtSecretName,
- hSpec.dtSecretItemKey,
- hSpec.jobUserName,
- krb5File,
- Some(kubernetesConf.krbConfigMapName),
- krb5CMap,
- hadoopBasedSparkPod)
- }.getOrElse(
- HadoopBootstrapUtil.bootstrapSparkUserPod(
- kubeTokenManager.getCurrentUser.getShortUserName,
- hadoopBasedSparkPod))
}
override def getAdditionalPodSystemProperties(): Map[String, String] = {
- val resolvedConfValues = kerberosConfSpec.map { hSpec =>
- Map(KERBEROS_DT_SECRET_NAME -> hSpec.dtSecretName,
- KERBEROS_DT_SECRET_KEY -> hSpec.dtSecretItemKey,
- KERBEROS_SPARK_USER_NAME -> hSpec.jobUserName,
- KRB5_CONFIG_MAP_NAME -> krb5CMap.getOrElse(kubernetesConf.krbConfigMapName))
- }.getOrElse(
- Map(KERBEROS_SPARK_USER_NAME ->
- kubeTokenManager.getCurrentUser.getShortUserName))
- Map(HADOOP_CONFIG_MAP_NAME ->
- hadoopConfDirSpec.hadoopConfigMapName.getOrElse(
- kubernetesConf.hadoopConfigMapName)) ++ resolvedConfValues
+ // If a submission-local keytab is provided, update the Spark config so that it knows the
+ // path of the keytab in the driver container.
+ if (needKeytabUpload) {
+ val ktName = new File(keytab.get).getName()
+ Map(KEYTAB.key -> s"$KERBEROS_KEYTAB_MOUNT_POINT/$ktName")
+ } else {
+ Map.empty
+ }
}
override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
- val hadoopConfConfigMap = for {
- hName <- newHadoopConfigMapName
- hFiles <- hadoopConfigurationFiles
- } yield {
- HadoopBootstrapUtil.buildHadoopConfigMap(hName, hFiles)
+ Seq[HasMetadata]() ++ {
+ krb5File.map { path =>
+ val file = new File(path)
+ new ConfigMapBuilder()
+ .withNewMetadata()
+ .withName(kubernetesConf.krbConfigMapName)
+ .endMetadata()
+ .addToData(
+ Map(file.getName() -> Files.toString(file, StandardCharsets.UTF_8)).asJava)
+ .build()
+ }
+ } ++ {
+ // If a submission-local keytab is provided, stash it in a secret.
+ if (needKeytabUpload) {
+ val kt = new File(keytab.get)
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(ktSecretName)
+ .endMetadata()
+ .addToData(kt.getName(), Base64.encodeBase64String(Files.toByteArray(kt)))
+ .build())
+ } else {
+ Nil
+ }
+ } ++ {
+ if (delegationTokens != null) {
+ Seq(new SecretBuilder()
+ .withNewMetadata()
+ .withName(dtSecretName)
+ .endMetadata()
+ .addToData(KERBEROS_SECRET_KEY, Base64.encodeBase64String(delegationTokens))
+ .build())
+ } else {
+ Nil
+ }
}
+ }
- val krb5ConfigMap = krb5File.map { fileLocation =>
- HadoopBootstrapUtil.buildkrb5ConfigMap(
- kubernetesConf.krbConfigMapName,
- fileLocation)
+ // Visible for testing.
+ def createDelegationTokens(): Array[Byte] = {
--- End diff --
> Because that's not needed. What exactly would doing that achieve?
I mean that is the function of a DelegationTokenManager, no? To handle nuances related to DT management, both for creation and renewal. _shrug_
> This code is testing that when delegation tokens are created, they are stashed in a secret
Well there is logic being introduced here beyond just `tokenManager.obtainDelegationToken()`, right? And if the purpose of the tests is to take the results from delegation token creation and put into a secret, then it would make sense to mock a `KubernetesDelegtionTokenManager` and just call `kdtm.obtainTokens()`, where this logic would be housed. Else, if we were to keep this logic within this step, i'd prefer that all parts of the logic are mocked.
Once again, a matter of opinion, that differs between unit-testing strategy. I'll loop in @mccheah
> I think the current test is easier to understand.
I agree, but is it comprehensive is what I am talking about here?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r231359962
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/KubernetesClusterSchedulerBackend.scala ---
@@ -123,7 +126,11 @@ private[spark] class KubernetesClusterSchedulerBackend(
}
override def createDriverEndpoint(properties: Seq[(String, String)]): DriverEndpoint = {
- new KubernetesDriverEndpoint(rpcEnv, properties)
+ new KubernetesDriverEndpoint(sc.env.rpcEnv, properties)
+ }
+
+ override protected def createTokenManager(): Option[HadoopDelegationTokenManager] = {
+ Some(new HadoopDelegationTokenManager(conf, sc.hadoopConfiguration))
--- End diff --
Yeah, I can always throw up a follow-up for that. No worries
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5684/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98838/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/22911
It is actually not needed for client mode because only the driver needs the keytab.
But whether to store it in secrets is not a question. You either store it in a secret or you don't support the keytab/principal feature in Spark at all, and we can delete a bunch of code here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233649498
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesDriverBuilder.scala ---
@@ -81,14 +81,14 @@ private[spark] class KubernetesDriverBuilder(
val driverCommandStep = provideDriverCommandStep(kubernetesConf)
- val maybeHadoopConfigStep =
- kubernetesConf.hadoopConfSpec.map { _ =>
- provideHadoopGlobalStep(kubernetesConf)}
+ val otherSteps = Seq(
--- End diff --
nit: `otherStep` isn't very descriptive to the contents of the steps. I prefer `hadoopSteps` if anything
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r238770191
--- Diff: resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStepSuite.scala ---
@@ -0,0 +1,171 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s.features
+
+import java.io.File
+import java.nio.charset.StandardCharsets.UTF_8
+import java.security.PrivilegedExceptionAction
+
+import scala.collection.JavaConverters._
+
+import com.google.common.io.Files
+import io.fabric8.kubernetes.api.model.{ConfigMap, Secret}
+import org.apache.commons.codec.binary.Base64
+import org.apache.hadoop.io.Text
+import org.apache.hadoop.security.{Credentials, UserGroupInformation}
+
+import org.apache.spark.{SparkConf, SparkFunSuite}
+import org.apache.spark.deploy.SparkHadoopUtil
+import org.apache.spark.deploy.k8s._
+import org.apache.spark.deploy.k8s.Config._
+import org.apache.spark.deploy.k8s.Constants._
+import org.apache.spark.deploy.k8s.submit.JavaMainAppResource
+import org.apache.spark.internal.config._
+import org.apache.spark.util.Utils
+
+class KerberosConfDriverFeatureStepSuite extends SparkFunSuite {
+
+ import KubernetesFeaturesTestUtils._
+ import SecretVolumeUtils._
+
+ private val tmpDir = Utils.createTempDir()
+
+ test("mount krb5 config map if defined") {
+ val configMap = "testConfigMap"
+ val step = createStep(
+ new SparkConf(false).set(KUBERNETES_KERBEROS_KRB5_CONFIG_MAP, configMap))
+
+ checkPodForKrbConf(step.configurePod(SparkPod.initialPod()), configMap)
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ assert(filter[ConfigMap](step.getAdditionalKubernetesResources()).isEmpty)
+ }
+
+ test("create krb5.conf config map if local config provided") {
+ val krbConf = File.createTempFile("krb5", ".conf", tmpDir)
+ Files.write("some data", krbConf, UTF_8)
+
+ val sparkConf = new SparkConf(false)
+ .set(KUBERNETES_KERBEROS_KRB5_FILE, krbConf.getAbsolutePath())
+ val step = createStep(sparkConf)
+
+ val confMap = filter[ConfigMap](step.getAdditionalKubernetesResources()).head
+ assert(confMap.getData().keySet().asScala === Set(krbConf.getName()))
+
+ checkPodForKrbConf(step.configurePod(SparkPod.initialPod()), confMap.getMetadata().getName())
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ }
+
+ test("create keytab secret if client keytab file used") {
+ val keytab = File.createTempFile("keytab", ".bin", tmpDir)
+ Files.write("some data", keytab, UTF_8)
+
+ val sparkConf = new SparkConf(false)
+ .set(KEYTAB, keytab.getAbsolutePath())
+ .set(PRINCIPAL, "alice")
+ val step = createStep(sparkConf)
+
+ val pod = step.configurePod(SparkPod.initialPod())
+ assert(podHasVolume(pod.pod, KERBEROS_KEYTAB_VOLUME))
+ assert(containerHasVolume(pod.container, KERBEROS_KEYTAB_VOLUME, KERBEROS_KEYTAB_MOUNT_POINT))
+
+ assert(step.getAdditionalPodSystemProperties().keys === Set(KEYTAB.key))
+
+ val secret = filter[Secret](step.getAdditionalKubernetesResources()).head
+ assert(secret.getData().keySet().asScala === Set(keytab.getName()))
+ }
+
+ test("do nothing if container-local keytab used") {
+ val sparkConf = new SparkConf(false)
+ .set(KEYTAB, "local:/my.keytab")
+ .set(PRINCIPAL, "alice")
+ val step = createStep(sparkConf)
+
+ val initial = SparkPod.initialPod()
+ assert(step.configurePod(initial) === initial)
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ assert(step.getAdditionalKubernetesResources().isEmpty)
+ }
+
+ test("mount delegation tokens if provided") {
+ val dtSecret = "tokenSecret"
+ val sparkConf = new SparkConf(false)
+ .set(KUBERNETES_KERBEROS_DT_SECRET_NAME, dtSecret)
+ .set(KUBERNETES_KERBEROS_DT_SECRET_ITEM_KEY, "dtokens")
+ val step = createStep(sparkConf)
+
+ checkPodForTokens(step.configurePod(SparkPod.initialPod()), dtSecret)
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ assert(step.getAdditionalKubernetesResources().isEmpty)
+ }
+
+ test("create delegation tokens if needed") {
+ // Since HadoopDelegationTokenManager does not create any tokens without proper configs and
+ // services, start with a test user that already has some tokens that will just be piped
+ // through to the driver.
+ val testUser = UserGroupInformation.createUserForTesting("k8s", Array())
+ testUser.doAs(new PrivilegedExceptionAction[Unit]() {
+ override def run(): Unit = {
+ val creds = testUser.getCredentials()
+ creds.addSecretKey(new Text("K8S_TEST_KEY"), Array[Byte](0x4, 0x2))
+ testUser.addCredentials(creds)
+
+ val tokens = SparkHadoopUtil.get.serialize(creds)
--- End diff --
The reason I am particular about using mocks is in the case that the internal logic changes (i.e. we remove using `SparkHadoopUtil` in future iterations, or whatnot.) But tbh, I am not hard-set on this as I see your point, and such changes aren't probable surrounding this code. So I am guess I am fine with it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233651011
--- Diff: resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/KerberosConfDriverFeatureStepSuite.scala ---
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s.features
+
+import java.io.File
+import java.nio.charset.StandardCharsets.UTF_8
+
+import scala.collection.JavaConverters._
+
+import com.google.common.io.Files
+import io.fabric8.kubernetes.api.model.{ConfigMap, Secret}
+import org.mockito.Mockito._
+
+import org.apache.spark.{SparkConf, SparkFunSuite}
+import org.apache.spark.deploy.k8s._
+import org.apache.spark.deploy.k8s.Config._
+import org.apache.spark.deploy.k8s.Constants._
+import org.apache.spark.deploy.k8s.submit.JavaMainAppResource
+import org.apache.spark.internal.config._
+import org.apache.spark.util.Utils
+
+class KerberosConfDriverFeatureStepSuite extends SparkFunSuite {
+
+ import KubernetesFeaturesTestUtils._
+ import SecretVolumeUtils._
+
+ private val tmpDir = Utils.createTempDir()
+
+ test("mount krb5 config map if defined") {
+ val configMap = "testConfigMap"
+ val step = createStep(
+ new SparkConf(false).set(KUBERNETES_KERBEROS_KRB5_CONFIG_MAP, configMap))
+
+ checkPodForKrbConf(step.configurePod(SparkPod.initialPod()), configMap)
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ assert(filter[ConfigMap](step.getAdditionalKubernetesResources()).isEmpty)
+ }
+
+ test("create krb5.conf config map if local config provided") {
+ val krbConf = File.createTempFile("krb5", ".conf", tmpDir)
+ Files.write("some data", krbConf, UTF_8)
+
+ val sparkConf = new SparkConf(false)
+ .set(KUBERNETES_KERBEROS_KRB5_FILE, krbConf.getAbsolutePath())
+ val step = createStep(sparkConf)
+
+ val confMap = filter[ConfigMap](step.getAdditionalKubernetesResources()).head
+ assert(confMap.getData().keySet().asScala === Set(krbConf.getName()))
+
+ checkPodForKrbConf(step.configurePod(SparkPod.initialPod()), confMap.getMetadata().getName())
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ }
+
+ test("create keytab secret if client keytab file used") {
+ val keytab = File.createTempFile("keytab", ".bin", tmpDir)
+ Files.write("some data", keytab, UTF_8)
+
+ val sparkConf = new SparkConf(false)
+ .set(KEYTAB, keytab.getAbsolutePath())
+ .set(PRINCIPAL, "alice")
+ val step = createStep(sparkConf)
+
+ val pod = step.configurePod(SparkPod.initialPod())
+ assert(podHasVolume(pod.pod, KERBEROS_KEYTAB_VOLUME))
+ assert(containerHasVolume(pod.container, KERBEROS_KEYTAB_VOLUME, KERBEROS_KEYTAB_MOUNT_POINT))
+
+ assert(step.getAdditionalPodSystemProperties().keys === Set(KEYTAB.key))
+
+ val secret = filter[Secret](step.getAdditionalKubernetesResources()).head
+ assert(secret.getData().keySet().asScala === Set(keytab.getName()))
+ }
+
+ test("do nothing if container-local keytab used") {
+ val sparkConf = new SparkConf(false)
+ .set(KEYTAB, "local:/my.keytab")
+ .set(PRINCIPAL, "alice")
+ val step = createStep(sparkConf)
+
+ val initial = SparkPod.initialPod()
+ assert(step.configurePod(initial) === initial)
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ assert(step.getAdditionalKubernetesResources().isEmpty)
+ }
+
+ test("mount delegation tokens if provided") {
+ val dtSecret = "tokenSecret"
+ val sparkConf = new SparkConf(false)
+ .set(KUBERNETES_KERBEROS_DT_SECRET_NAME, dtSecret)
+ .set(KUBERNETES_KERBEROS_DT_SECRET_ITEM_KEY, "dtokens")
+ val step = createStep(sparkConf)
+
+ checkPodForTokens(step.configurePod(SparkPod.initialPod()), dtSecret)
+ assert(step.getAdditionalPodSystemProperties().isEmpty)
+ assert(step.getAdditionalKubernetesResources().isEmpty)
+ }
+
+ test("create delegation tokens if needed") {
+ val step = spy(createStep(new SparkConf(false)))
+ doReturn(Array[Byte](0x4, 0x2)).when(step).createDelegationTokens()
--- End diff --
Seems redundant, but why not.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22911: [SPARK-25815][k8s] Support kerberos in client mod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22911#discussion_r233591350
--- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/DelegationTokenFeatureStep.scala ---
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s.features
+
+import io.fabric8.kubernetes.api.model.{ContainerBuilder, HasMetadata, PodBuilder, SecretBuilder}
+import org.apache.commons.codec.binary.Base64
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.deploy.SparkHadoopUtil
+import org.apache.spark.deploy.k8s.{KubernetesConf, KubernetesUtils, SparkPod}
+import org.apache.spark.deploy.k8s.Config._
+import org.apache.spark.deploy.k8s.Constants._
+import org.apache.spark.deploy.security.HadoopDelegationTokenManager
+import org.apache.spark.internal.config._
+
+/**
+ * Delegation token support for Spark apps on kubernetes.
+ *
+ * When preparing driver resources, this step will generate delegation tokens for the app if
+ * they're needed.
+ *
+ * When preparing pods, this step will mount the delegation token secret (either pre-defined,
+ * or generated by this step when preparing the driver).
+ */
+private[spark] class DelegationTokenFeatureStep(conf: KubernetesConf[_], isDriver: Boolean)
--- End diff --
I think you're missing one use case there. There are 3 use cases:
- keytab: keytab is provided to driver, driver handles kerberos login, creates tokens, distributes tokens to executors
- pre-defined token secret: secret is mounted in the driver, env variable is set, driver loads them and distributes to executors
- default kerberos case: submission client generates delegation tokens, creates a secret for them, then this behaves like the bullet above
The third use case is actually the most common. In your reply above you're only covering the other two. My code covers all three.
I'm just saying that this code actually doesn't need to do anything on the executor side, because the driver takes care of everything when the credentials are provided.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5030/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5029/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99842/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98832 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98832/testReport)** for PR 22911 at commit [`a129314`](https://github.com/apache/spark/commit/a129314abb45413d7b46e8510b2a2ea7d61fa916).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22911
**[Test build #98333 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98333/testReport)** for PR 22911 at commit [`7f86e23`](https://github.com/apache/spark/commit/7f86e231922648a9da07ae73026a3a430d2c83f5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22911: [SPARK-25815][k8s] Support kerberos in client mode, keyt...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4680/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org