Skip to content

[SPARK-35969][K8S] Make the pod prefix more readable and tallied with K8S DNS Label Names#33171

Closed
yaooqinn wants to merge 2 commits intoapache:masterfrom
yaooqinn:SPARK-35969
Closed

[SPARK-35969][K8S] Make the pod prefix more readable and tallied with K8S DNS Label Names#33171
yaooqinn wants to merge 2 commits intoapache:masterfrom
yaooqinn:SPARK-35969

Conversation

@yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented Jul 1, 2021

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> "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

Make the pod prefix more readable and tallied with K8S DNS Label Names
@SparkQA
Copy link

SparkQA commented Jul 1, 2021

Test build #140504 has finished for PR 33171 at commit 9c33988.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 1, 2021

Test build #140509 has finished for PR 33171 at commit ac79194.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 1, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45016/

@yaooqinn
Copy link
Member Author

yaooqinn commented Jul 1, 2021

cc @dongjoon-hyun @holdenk thanks

@SparkQA
Copy link

SparkQA commented Jul 1, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45016/

@SparkQA
Copy link

SparkQA commented Jul 1, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45022/

@SparkQA
Copy link

SparkQA commented Jul 1, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45022/

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @yaooqinn .
Merged to master for Apache Spark 3.2.0.

@yaooqinn yaooqinn deleted the SPARK-35969 branch July 1, 2021 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants