-
Notifications
You must be signed in to change notification settings - Fork 28.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-30970][K8S][Core] Fix NPE while resolving k8s master url #27721
Conversation
cc @dongjoon-hyun @srowen, thanks for reviewing |
Kubernetes integration test starting |
Kubernetes integration test status failure |
Test build #119022 has finished for PR 27721 at commit
|
Kubernetes integration test starting |
Kubernetes integration test status failure |
retest this please |
Test build #119028 has finished for PR 27721 at commit
|
Kubernetes integration test starting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I buy it.
Kubernetes integration test status success |
Test build #119046 has finished for PR 27721 at commit
|
retest this please |
Kubernetes integration test starting |
Kubernetes integration test status failure |
Test build #119062 has finished for PR 27721 at commit
|
retest this please |
Kubernetes integration test starting |
Kubernetes integration test status failure |
retest this please, the k8s it test seems quite flaky. |
Test build #119067 has finished for PR 27721 at commit
|
cc @holdenk for the flaky test which is blocking this PR.
I created SPARK-30981 for the flaky test. |
Kubernetes integration test starting |
Kubernetes integration test status success |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM. Now all tests passed.
- [SPARK-30970][K8S][Core] Fix NPE while resolving k8s master url #27721 (comment)
- [SPARK-30970][K8S][Core] Fix NPE while resolving k8s master url #27721 (comment)
Merged to master/3.0.
### What changes were proposed in this pull request? ``` bin/spark-sql --master k8s:///https://kubernetes.docker.internal:6443 --conf spark.kubernetes.container.image=yaooqinn/spark:v2.4.4 Exception in thread "main" java.lang.NullPointerException at org.apache.spark.util.Utils$.checkAndGetK8sMasterUrl(Utils.scala:2739) at org.apache.spark.deploy.SparkSubmit.prepareSubmitEnvironment(SparkSubmit.scala:261) at org.apache.spark.deploy.SparkSubmit.org$apache$spark$deploy$SparkSubmit$$runMain(SparkSubmit.scala:774) at org.apache.spark.deploy.SparkSubmit.doRunMain$1(SparkSubmit.scala:161) at org.apache.spark.deploy.SparkSubmit.submit(SparkSubmit.scala:184) at org.apache.spark.deploy.SparkSubmit.doSubmit(SparkSubmit.scala:86) at org.apache.spark.deploy.SparkSubmit$$anon$2.doSubmit(SparkSubmit.scala:920) at org.apache.spark.deploy.SparkSubmit$.main(SparkSubmit.scala:929) at org.apache.spark.deploy.SparkSubmit.main(SparkSubmit.scala) ``` Althrough `k8s:///https://kubernetes.docker.internal:6443` is a wrong master url but should not throw npe The `case null` will never be touched. https://github.com/apache/spark/blob/3f4060c340d6bac412e8819c4388ccba226efcf3/core/src/main/scala/org/apache/spark/util/Utils.scala#L2772-L2776 ### Why are the changes needed? bug fix ### Does this PR introduce any user-facing change? no ### How was this patch tested? add ut case Closes #27721 from yaooqinn/SPARK-30970. Authored-by: Kent Yao <yaooqinn@hotmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit 1383bd4) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Hi, @yaooqinn . Could you make a PR to |
Test build #119071 has finished for PR 27721 at commit
|
OK @dongjoon-hyun and thanks for merging |
### What changes were proposed in this pull request? backport #27721 ``` bin/spark-sql --master k8s:///https://kubernetes.docker.internal:6443 --conf spark.kubernetes.container.image=yaooqinn/spark:v2.4.4 Exception in thread "main" java.lang.NullPointerException at org.apache.spark.util.Utils$.checkAndGetK8sMasterUrl(Utils.scala:2739) at org.apache.spark.deploy.SparkSubmit.prepareSubmitEnvironment(SparkSubmit.scala:261) at org.apache.spark.deploy.SparkSubmit.org$apache$spark$deploy$SparkSubmit$$runMain(SparkSubmit.scala:774) at org.apache.spark.deploy.SparkSubmit.doRunMain$1(SparkSubmit.scala:161) at org.apache.spark.deploy.SparkSubmit.submit(SparkSubmit.scala:184) at org.apache.spark.deploy.SparkSubmit.doSubmit(SparkSubmit.scala:86) at org.apache.spark.deploy.SparkSubmit$$anon$2.doSubmit(SparkSubmit.scala:920) at org.apache.spark.deploy.SparkSubmit$.main(SparkSubmit.scala:929) at org.apache.spark.deploy.SparkSubmit.main(SparkSubmit.scala) ``` Althrough `k8s:///https://kubernetes.docker.internal:6443` is a wrong master url but should not throw npe The `case null` will never be touched. https://github.com/apache/spark/blob/3f4060c340d6bac412e8819c4388ccba226efcf3/core/src/main/scala/org/apache/spark/util/Utils.scala#L2772-L2776 ### Why are the changes needed? bug fix ### Does this PR introduce any user-facing change? no ### How was this patch tested? add ut case Closes #27736 from yaooqinn/SPARK-30970-2. Authored-by: Kent Yao <yaooqinn@hotmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
### What changes were proposed in this pull request? ``` bin/spark-sql --master k8s:///https://kubernetes.docker.internal:6443 --conf spark.kubernetes.container.image=yaooqinn/spark:v2.4.4 Exception in thread "main" java.lang.NullPointerException at org.apache.spark.util.Utils$.checkAndGetK8sMasterUrl(Utils.scala:2739) at org.apache.spark.deploy.SparkSubmit.prepareSubmitEnvironment(SparkSubmit.scala:261) at org.apache.spark.deploy.SparkSubmit.org$apache$spark$deploy$SparkSubmit$$runMain(SparkSubmit.scala:774) at org.apache.spark.deploy.SparkSubmit.doRunMain$1(SparkSubmit.scala:161) at org.apache.spark.deploy.SparkSubmit.submit(SparkSubmit.scala:184) at org.apache.spark.deploy.SparkSubmit.doSubmit(SparkSubmit.scala:86) at org.apache.spark.deploy.SparkSubmit$$anon$2.doSubmit(SparkSubmit.scala:920) at org.apache.spark.deploy.SparkSubmit$.main(SparkSubmit.scala:929) at org.apache.spark.deploy.SparkSubmit.main(SparkSubmit.scala) ``` Althrough `k8s:///https://kubernetes.docker.internal:6443` is a wrong master url but should not throw npe The `case null` will never be touched. https://github.com/apache/spark/blob/3f4060c340d6bac412e8819c4388ccba226efcf3/core/src/main/scala/org/apache/spark/util/Utils.scala#L2772-L2776 ### Why are the changes needed? bug fix ### Does this PR introduce any user-facing change? no ### How was this patch tested? add ut case Closes apache#27721 from yaooqinn/SPARK-30970. Authored-by: Kent Yao <yaooqinn@hotmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
What changes were proposed in this pull request?
Althrough
k8s:///https://kubernetes.docker.internal:6443
is a wrong master url but should not throw npeThe
case null
will never be touched.spark/core/src/main/scala/org/apache/spark/util/Utils.scala
Lines 2772 to 2776 in 3f4060c
Why are the changes needed?
bug fix
Does this PR introduce any user-facing change?
no
How was this patch tested?
add ut case