[SPARK-52997] Fixes wrong worker assignment if multiple clusters are deployed to the same namespace #291
Conversation
schmaxXximilian
changed the title
schmaxXximilian
force-pushed
the
main
branch
2 times, most recently
from
a373739 to
62a3b2d
Compare
…o wrong worker assignment
|
Hey @dongjoon-hyun, would be great if you could have a look at this. Our team is using the operator and we are grateful to the great work you folks have been doing. |
|
Thank you for making a PR, @schmaxXximilian . cc @peter-toth |
...-submission-worker/src/main/java/org/apache/spark/k8s/operator/SparkClusterResourceSpec.java
Outdated
Show resolved
Hide resolved
| .withClusterIP("None") | ||
| .withSelector( | ||
| Collections.singletonMap(LABEL_SPARK_ROLE_NAME, LABEL_SPARK_ROLE_MASTER_VALUE)) | ||
| Map.of( |
There was a problem hiding this comment.
May I ask why we need to use Map.of instead of Collections.singletonMap?
There was a problem hiding this comment.
Hi @dongjoon-hyun, I'm afraid, I'm not quire sure what you are suggesting. Collections.singletonMap only creates a map with a single element.
I changed my initial commit to below approach. Do you think it would be more suitable?
.addToSelector(Collections.singletonMap(LABEL_SPARK_CLUSTER_NAME, name))
.addToSelector(
Collections.singletonMap(LABEL_SPARK_ROLE_NAME, LABEL_SPARK_ROLE_MASTER_VALUE))
There was a problem hiding this comment.
I left a few comments including https://github.com/apache/spark-kubernetes-operator/pull/291/files#r2243410142 .
In general, I understand your requirements although this is not recommended for HPA-enabled Spark Clusters. Let me play with this for a while.
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM. Thank you, @schmaxXximilian . Sorry for being delayed.
Merged to main.
|
I added you to the Apache Spark contributor group (of ASF JIRA) and assigned SPARK-52997 to you. Welcome to the Apache Spark community. |
…deployed to the same namespace ### What changes were proposed in this pull request? Updated the podSelector for master and worker services to include both clusterRole and name labels. ### Why are the changes needed? Using only clusterRole caused service misrouting when multiple Spark clusters were deployed in the same namespace. Adding name ensures correct pod targeting. ### Does this PR introduce any user-facing change? No, this is an internal fix to service selectors. ### How was this patch tested? Tested with multiple clusters in the same namespace. Verified each service only matched its own pods via kubectl describe service. Also adapted unit tests to reflect new behaviour ### Was this patch authored or co-authored using generative AI tooling? Yes, PR metadata was assisted by AI, but code changes were made manually. Closes apache#291 from schmaxXximilian/main. Authored-by: Schmöller Maximilian <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 7915164)
2 participants
What changes were proposed in this pull request?
Updated the podSelector for master and worker services to include both clusterRole and name labels.
Why are the changes needed?
Using only clusterRole caused service misrouting when multiple Spark clusters were deployed in the same namespace. Adding name ensures correct pod targeting.
Does this PR introduce any user-facing change?
No, this is an internal fix to service selectors.
How was this patch tested?
Tested with multiple clusters in the same namespace. Verified each service only matched its own pods via kubectl describe service.
Also adapted unit tests to reflect new behaviour
Was this patch authored or co-authored using generative AI tooling?
Yes, PR metadata was assisted by AI, but code changes were made manually.