[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/22362 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22362#discussion_r220364426
--- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala ---
@@ -726,7 +726,11 @@ private[spark] object SparkConf extends Logging {
DRIVER_MEMORY_OVERHEAD.key -> Seq(
AlternateConfig("spark.yarn.driver.memoryOverhead", "2.3")),
EXECUTOR_MEMORY_OVERHEAD.key -> Seq(
- AlternateConfig("spark.yarn.executor.memoryOverhead", "2.3"))
+ AlternateConfig("spark.yarn.executor.memoryOverhead", "2.3")),
+KEYTAB.key -> Seq(
+ AlternateConfig("spark.yarn.keytab", "2.4")),
--- End diff --
Okay. I will then point to 2.5.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22362#discussion_r220360201
--- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala ---
@@ -726,7 +726,11 @@ private[spark] object SparkConf extends Logging {
DRIVER_MEMORY_OVERHEAD.key -> Seq(
AlternateConfig("spark.yarn.driver.memoryOverhead", "2.3")),
EXECUTOR_MEMORY_OVERHEAD.key -> Seq(
- AlternateConfig("spark.yarn.executor.memoryOverhead", "2.3"))
+ AlternateConfig("spark.yarn.executor.memoryOverhead", "2.3")),
+KEYTAB.key -> Seq(
+ AlternateConfig("spark.yarn.keytab", "2.4")),
--- End diff --
This change doesn't make a whole lot of sense without the rest of the k8s
kerberos work, and that change is a lot more controversial to pull into 2.4 at
this point. So I'm leaning towards no.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22362#discussion_r220356187
--- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala ---
@@ -726,7 +726,11 @@ private[spark] object SparkConf extends Logging {
DRIVER_MEMORY_OVERHEAD.key -> Seq(
AlternateConfig("spark.yarn.driver.memoryOverhead", "2.3")),
EXECUTOR_MEMORY_OVERHEAD.key -> Seq(
- AlternateConfig("spark.yarn.executor.memoryOverhead", "2.3"))
+ AlternateConfig("spark.yarn.executor.memoryOverhead", "2.3")),
+KEYTAB.key -> Seq(
+ AlternateConfig("spark.yarn.keytab", "2.4")),
--- End diff --
Thank you for the test! I appreciate it. As the final RC Is not cut, it
could be added if you think it should.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22362#discussion_r220354878
--- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala ---
@@ -726,7 +726,11 @@ private[spark] object SparkConf extends Logging {
DRIVER_MEMORY_OVERHEAD.key -> Seq(
AlternateConfig("spark.yarn.driver.memoryOverhead", "2.3")),
EXECUTOR_MEMORY_OVERHEAD.key -> Seq(
- AlternateConfig("spark.yarn.executor.memoryOverhead", "2.3"))
+ AlternateConfig("spark.yarn.executor.memoryOverhead", "2.3")),
+KEYTAB.key -> Seq(
+ AlternateConfig("spark.yarn.keytab", "2.4")),
--- End diff --
This should be 2.5, also below. This change won't go into 2.4 at this
point...
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
Github user ifilonenko commented on a diff in the pull request: https://github.com/apache/spark/pull/22362#discussion_r219638207 --- Diff: R/pkg/R/sparkR.R --- @@ -624,8 +624,8 @@ sparkConfToSubmitOps[["spark.driver.extraClassPath"]] <- "--driver-class-path" sparkConfToSubmitOps[["spark.driver.extraJavaOptions"]] <- "--driver-java-options" sparkConfToSubmitOps[["spark.driver.extraLibraryPath"]] <- "--driver-library-path" sparkConfToSubmitOps[["spark.master"]] <- "--master" -sparkConfToSubmitOps[["spark.yarn.keytab"]] <- "--keytab" --- End diff -- Added backwards compatibility --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/22362#discussion_r216122659
--- Diff:
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -199,8 +199,8 @@ private[deploy] class SparkSubmitArguments(args:
Seq[String], env: Map[String, S
numExecutors = Option(numExecutors)
.getOrElse(sparkProperties.get("spark.executor.instances").orNull)
queue =
Option(queue).orElse(sparkProperties.get("spark.yarn.queue")).orNull
-keytab =
Option(keytab).orElse(sparkProperties.get("spark.yarn.keytab")).orNull
-principal =
Option(principal).orElse(sparkProperties.get("spark.yarn.principal")).orNull
+keytab =
Option(keytab).orElse(sparkProperties.get("spark.kerberos.keytab")).orNull
--- End diff --
agreed, shouldn't the "old" config still work? `spark.yarn.keytab` etc
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22362#discussion_r216108106
--- Diff:
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -199,8 +199,8 @@ private[deploy] class SparkSubmitArguments(args:
Seq[String], env: Map[String, S
numExecutors = Option(numExecutors)
.getOrElse(sparkProperties.get("spark.executor.instances").orNull)
queue =
Option(queue).orElse(sparkProperties.get("spark.yarn.queue")).orNull
-keytab =
Option(keytab).orElse(sparkProperties.get("spark.yarn.keytab")).orNull
-principal =
Option(principal).orElse(sparkProperties.get("spark.yarn.principal")).orNull
+keytab =
Option(keytab).orElse(sparkProperties.get("spark.kerberos.keytab")).orNull
--- End diff --
This might be easier to fix though; `prepareSubmitEnvironment` (where these
things are used) could just read them from `SparkConf` at that point. Something
like this in that method, instead of this code:
Option(args.keytab).orElse(sparkConf.get(KEYTAB))
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22362#discussion_r216107597
--- Diff:
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -199,8 +199,8 @@ private[deploy] class SparkSubmitArguments(args:
Seq[String], env: Map[String, S
numExecutors = Option(numExecutors)
.getOrElse(sparkProperties.get("spark.executor.instances").orNull)
queue =
Option(queue).orElse(sparkProperties.get("spark.yarn.queue")).orNull
-keytab =
Option(keytab).orElse(sparkProperties.get("spark.yarn.keytab")).orNull
-principal =
Option(principal).orElse(sparkProperties.get("spark.yarn.principal")).orNull
+keytab =
Option(keytab).orElse(sparkProperties.get("spark.kerberos.keytab")).orNull
--- End diff --
This has a similar issue as the R code above re: backwards compatibility.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22362#discussion_r216107505
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
@@ -520,6 +520,10 @@ private[spark] class SparkSubmit extends Logging {
confKey = "spark.driver.extraJavaOptions"),
OptionAssigner(args.driverExtraLibraryPath, ALL_CLUSTER_MGRS,
ALL_DEPLOY_MODES,
confKey = "spark.driver.extraLibraryPath"),
+ OptionAssigner(args.principal, ALL_CLUSTER_MGRS, ALL_DEPLOY_MODES,
+confKey = "spark.kerberos.principal"),
--- End diff --
Since you're changing this, could you use `PRINCIPAL.key` and `KEYTAB.key`
in the places where the config name is referenced?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
Github user vanzin commented on a diff in the pull request: https://github.com/apache/spark/pull/22362#discussion_r216107385 --- Diff: R/pkg/R/sparkR.R --- @@ -624,8 +624,8 @@ sparkConfToSubmitOps[["spark.driver.extraClassPath"]] <- "--driver-class-path" sparkConfToSubmitOps[["spark.driver.extraJavaOptions"]] <- "--driver-java-options" sparkConfToSubmitOps[["spark.driver.extraLibraryPath"]] <- "--driver-library-path" sparkConfToSubmitOps[["spark.master"]] <- "--master" -sparkConfToSubmitOps[["spark.yarn.keytab"]] <- "--keytab" --- End diff -- @felixcheung This seems wrong, since existing apps are using the old config, not the new one, so you may need some backwards compatibility code. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22362: [SPARK-25372][YARN][K8S] Deprecate and generalize...
GitHub user ifilonenko opened a pull request: https://github.com/apache/spark/pull/22362 [SPARK-25372][YARN][K8S] Deprecate and generalize keytab / principal config ## What changes were proposed in this pull request? SparkSubmit already logs in the user if a keytab is provided, the only issue is that it uses the existing configs which have "yarn" in their name. As such, the configs were changed to: `spark.kerberos.keytab` and `spark.kerberos.principal`. ## How was this patch tested? Will be tested with K8S tests, but needs to be tested with Yarn - [ ] K8S Secure HDFS tests - [ ] Yarn Secure HDFS tests @vanzin You can merge this pull request into a Git repository by running: $ git pull https://github.com/ifilonenko/spark SPARK-25372 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/22362.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 #22362 commit 30286f85ac315cad2578fe2e11c8718b8883630a Author: Ilan Filonenko Date: 2018-09-07T22:28:25Z first step --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
