You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by do...@apache.org on 2021/07/01 15:15:51 UTC
[spark] branch master updated: [SPARK-35969][K8S] Make the pod
prefix more readable and tallied with K8S DNS Label Names
This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 94c1e3c [SPARK-35969][K8S] Make the pod prefix more readable and tallied with K8S DNS Label Names
94c1e3c is described below
commit 94c1e3c38cfcc7444aad6efa38a7d2c3ed9f9f4a
Author: Kent Yao <ya...@apache.org>
AuthorDate: Thu Jul 1 08:15:00 2021 -0700
[SPARK-35969][K8S] Make the pod prefix more readable and tallied with K8S DNS Label Names
### What changes were proposed in this pull request?
By default, the executor pod prefix is generated by the app name. It handles characters that match [^a-z0-9\\-] differently. The '.' and all whitespaces will be converted to '-', but other ones to empty string. Especially, characters like '_', '|' are commonly used as a word separator in many languages.
According to the K8S DNS Label Names, see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names, we can convert all special characters to `-`.
For example,
```
scala> "xyz_abc_i_am_a_app_name_w/_some_abbrs".replaceAll("[^a-z0-9\\-]", "-").replaceAll("-+", "-")
res11: String = xyz-abc-i-am-a-app-name-w-some-abbrs
scala> "xyz_abc_i_am_a_app_name_w/_some_abbrs".replaceAll("\\s+", "-").replaceAll("\\.", "-").replaceAll("[^a-z0-9\\-]", "").replaceAll("-+", "-")
res12: String = xyzabciamaappnamewsomeabbrs
```
```scala
scala> "time.is%the￥most$valuable_——————thing,it's about time.".replaceAll("[^a-z0-9\\-]", "-").replaceAll("-+", "-")
res9: String = time-is-the-most-valuable-thing-it-s-about-time-
scala> "time.is%the￥most$valuable_——————thing,it's about time.".replaceAll("\\s+", "-").replaceAll("\\.", "-").replaceAll("[^a-z0-9\\-]", "").replaceAll("-+", "-")
res10: String = time-isthemostvaluablethingits-about-time-
```
### Why are the changes needed?
For better UX
### Does this PR introduce _any_ user-facing change?
yes, the executor pod name might look better
### How was this patch tested?
add new ones
Closes #33171 from yaooqinn/SPARK-35969.
Authored-by: Kent Yao <ya...@apache.org>
Signed-off-by: Dongjoon Hyun <do...@apache.org>
---
.../apache/spark/deploy/k8s/KubernetesConf.scala | 4 +---
.../features/BasicExecutorFeatureStepSuite.scala | 23 ++++++++++++++++++++++
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesConf.scala b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesConf.scala
index 087eeee..937c5f5 100644
--- a/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesConf.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesConf.scala
@@ -230,9 +230,7 @@ private[spark] object KubernetesConf {
s"$appName-$id"
.trim
.toLowerCase(Locale.ROOT)
- .replaceAll("\\s+", "-")
- .replaceAll("\\.", "-")
- .replaceAll("[^a-z0-9\\-]", "")
+ .replaceAll("[^a-z0-9\\-]", "-")
.replaceAll("-+", "-")
}
diff --git a/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStepSuite.scala b/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStepSuite.scala
index b7c97bb..1aa34aa 100644
--- a/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStepSuite.scala
+++ b/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStepSuite.scala
@@ -370,6 +370,29 @@ class BasicExecutorFeatureStepSuite extends SparkFunSuite with BeforeAndAfter {
}
}
+ test("SPARK-35969: Make the pod prefix more readable and tallied with K8S DNS Label Names") {
+ baseConf.remove(KUBERNETES_EXECUTOR_POD_NAME_PREFIX)
+ baseConf.set("spark.app.name", "xyz.abc _i_am_a_app_name_w/_some_abbrs")
+ val baseDriverPod = SparkPod.initialPod()
+ val step1 = new BasicExecutorFeatureStep(newExecutorConf(), new SecurityManager(baseConf),
+ defaultProfile)
+ val podConfigured1 = step1.configurePod(baseDriverPod)
+ assert(podConfigured1.pod.getMetadata.getName
+ .startsWith("xyz-abc-i-am-a-app-name-w-some-abbrs"))
+
+ // scalastyle:off nonascii
+ baseConf.set("spark.app.name", "time.is the#most￥valuable_—thing。it's&about?time.")
+ // scalastyle:on
+
+ val step2 = new BasicExecutorFeatureStep(newExecutorConf(), new SecurityManager(baseConf),
+ defaultProfile)
+
+ val podConfigured2 = step2.configurePod(baseDriverPod)
+ assert(podConfigured2.pod.getMetadata.getName
+ .startsWith("time-is-the-most-valuable-thing-it-s-about-time-"))
+
+ }
+
// There is always exactly one controller reference, and it points to the driver pod.
private def checkOwnerReferences(executor: Pod, driverPodUid: String): Unit = {
assert(executor.getMetadata.getOwnerReferences.size() === 1)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org