You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/04/18 06:24:21 UTC

[GitHub] [spark] dongjoon-hyun opened a new pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

dongjoon-hyun opened a new pull request #32221:
URL: https://github.com/apache/spark/pull/32221


   ### What changes were proposed in this pull request?
   
   This PR aims to upgrade K8s client to 5.3.0 to support K8s 1.20 officially.
   - https://github.com/fabric8io/kubernetes-client#compatibility-matrix
   
   The following are the notable breaking API changes.
   
   1. Remove Doneable (5.0+):
       - https://github.com/fabric8io/kubernetes-client/pull/2571
   2. Change Watcher.onClose signature (5.0+):
       - https://github.com/fabric8io/kubernetes-client/pull/2616
       - https://github.com/fabric8io/kubernetes-client/pull/2616
   3. Change Readiness (5.1+)
       - https://github.com/fabric8io/kubernetes-client/pull/2796
   
   ### Why are the changes needed?
   
   According to the compatibility matrix, this makes Apache Spark and its extension support K8s 1.20 officially for Apache Spark 3.2.0.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes, this is a dev dependency change which affects K8s cluster extension users.
   
   ### How was this patch tested?
   
   Pass the CIs.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] attilapiros commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
attilapiros commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821999302


   In the description you are referring twice to to the same k8s client PR, here:
   > 2. Change Watcher.onClose signature (5.0+):
   > - fabric8io/kubernetes-client#2616
   > - fabric8io/kubernetes-client#2616
   
   Is there any reason for this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] attilapiros commented on a change in pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #32221:
URL: https://github.com/apache/spark/pull/32221#discussion_r615667822



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala
##########
@@ -58,10 +58,14 @@ private[spark] class ExecutorPodsWatchSnapshotSource(
       snapshotsStore.updatePod(pod)
     }
 
-    override def onClose(e: KubernetesClientException): Unit = {
+    override def onClose(e: WatcherException): Unit = {
       logWarning("Kubernetes client has been closed (this is expected if the application is" +

Review comment:
       > Actually, that's what @attilapiros meant, but it's not in reality. As of now, it's frequently possible to observe K8s side exceptions, especially HTTP-related ones, during application termination after `SparkContext.close()` starts to doing several clean-ups.
   
   In this case I am fine to leave it as it is.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822022897


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137532/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821950233


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42101/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822265951


   **[Test build #137590 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137590/testReport)** for PR 32221 at commit [`e1c8707`](https://github.com/apache/spark/commit/e1c8707001abae1150d672f2fba34ef270769dba).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821995488


   Could you review this please, @viirya and @attilapiros ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822409754


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137590/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822059652


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42110/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822068005


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137535/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821995500


   **[Test build #137532 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137532/testReport)** for PR 32221 at commit [`a902250`](https://github.com/apache/spark/commit/a902250e4a6cc88d20097ab356884bc2813edba3).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] viirya commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822615978


   lgtm too


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun edited a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822043113


   The following are addressed.
   - Revise the PR description by removing a duplicate link.
   - Add `private val readiness = Readiness.getInstance()` and reuse it.
   - Add a corresponding `override def onClose(): Unit` consistently for all places in order not to forget it.
   - The warning message is revised in case of `onClose()` (no exception version).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822409754


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137590/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821944379


   **[Test build #137527 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137527/testReport)** for PR 32221 at commit [`dc1ad22`](https://github.com/apache/spark/commit/dc1ad22bf185e2c6ef8f0a3aa5ba1d96d755aeb9).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821965371


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137527/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822068005


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137535/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822042717


   **[Test build #137535 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137535/testReport)** for PR 32221 at commit [`9c5e70e`](https://github.com/apache/spark/commit/9c5e70e45aec3bb42dd6af1717af242980cfcb18).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822004017


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42106/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32221:
URL: https://github.com/apache/spark/pull/32221#discussion_r615452336



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala
##########
@@ -58,10 +58,14 @@ private[spark] class ExecutorPodsWatchSnapshotSource(
       snapshotsStore.updatePod(pod)
     }
 
-    override def onClose(e: KubernetesClientException): Unit = {
+    override def onClose(e: WatcherException): Unit = {
       logWarning("Kubernetes client has been closed (this is expected if the application is" +

Review comment:
       Although `expected` was there, the message doesn't mean that `Exception will happen always`. Instead, it means you may see `Exception` some cases. In this context of `onClose`, `WatcherException` already happened. So, `logWarning` says the users that this exception is not **unexpected** one.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32221:
URL: https://github.com/apache/spark/pull/32221#discussion_r615451924



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala
##########
@@ -58,10 +58,14 @@ private[spark] class ExecutorPodsWatchSnapshotSource(
       snapshotsStore.updatePod(pod)
     }
 
-    override def onClose(e: KubernetesClientException): Unit = {
+    override def onClose(e: WatcherException): Unit = {
       logWarning("Kubernetes client has been closed (this is expected if the application is" +

Review comment:
       Actually, that's what @attilapiros meant, but it's not in reality. As of now, it's frequently possible to observe K8s side exceptions, especially HTTP-related ones, during application termination after `SparkContext.close()` starts to doing several clean-ups.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822064309


   **[Test build #137535 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137535/testReport)** for PR 32221 at commit [`9c5e70e`](https://github.com/apache/spark/commit/9c5e70e45aec3bb42dd6af1717af242980cfcb18).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822397239


   **[Test build #137590 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137590/testReport)** for PR 32221 at commit [`e1c8707`](https://github.com/apache/spark/commit/e1c8707001abae1150d672f2fba34ef270769dba).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] viirya commented on a change in pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #32221:
URL: https://github.com/apache/spark/pull/32221#discussion_r615426803



##########
File path: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/SparkReadinessWatcher.scala
##########
@@ -30,12 +30,12 @@ private[spark] class SparkReadinessWatcher[T <: HasMetadata] extends Watcher[T]
 
   override def eventReceived(action: Action, resource: T): Unit = {
     if ((action == Action.MODIFIED || action == Action.ADDED) &&
-        Readiness.isReady(resource)) {
+        Readiness.getInstance().isReady(resource)) {

Review comment:
       do we need to pull `Readiness.getInstance()` as private variable? Is it cheap to call `Readiness.getInstance()` every call of `eventReceived`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822050749


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42109/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822039517


   Thank you for your review, @attilapiros and @viirya . I'll update according your comments!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32221:
URL: https://github.com/apache/spark/pull/32221#discussion_r615452466



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala
##########
@@ -58,10 +58,14 @@ private[spark] class ExecutorPodsWatchSnapshotSource(
       snapshotsStore.updatePod(pod)
     }
 
-    override def onClose(e: KubernetesClientException): Unit = {
+    override def onClose(e: WatcherException): Unit = {
       logWarning("Kubernetes client has been closed (this is expected if the application is" +

Review comment:
       BTW, @attilapiros and @viirya . If that is still your concern, I'd suggest to handle it in a separate PR. That looks pretty much orthogonal to `K8s client 5.3.0`. WDYT?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822057059






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] attilapiros edited a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
attilapiros edited a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822000621


   Reading that K8s client PR about `onClose()` I think we have to be careful which one is to chosen to be implemented: as there is one without any argument and there is another with one argument (WatcherException):
   
   > The interface now provides a dedicated default argument-less onClose() method. This method should always be called upon Watcher completion. Implementations should override this method in case operations (e.g. cleanup) should be performed once the Watcher is closed.
   >
   > The previous onClose(WatcherException cause) is kept and only invoked whenever an exception occurs during the Watcher life-cycle (once the watch connection is open and established).
   
   For example here I think the argumentless version should be used instead:
   https://github.com/apache/spark/blob/a902250e4a6cc88d20097ab356884bc2813edba3/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala#L61-L65
   
   WDYT?
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32221:
URL: https://github.com/apache/spark/pull/32221#discussion_r615452466



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala
##########
@@ -58,10 +58,14 @@ private[spark] class ExecutorPodsWatchSnapshotSource(
       snapshotsStore.updatePod(pod)
     }
 
-    override def onClose(e: KubernetesClientException): Unit = {
+    override def onClose(e: WatcherException): Unit = {
       logWarning("Kubernetes client has been closed (this is expected if the application is" +

Review comment:
       BTW, @attilapiros and @viirya . If that is still your concerning, I'd suggest to handle it in a separate PR. That looks pretty much orthogonal to `K8s client 5.3.0`. WDYT?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822048461


   Kubernetes integration test unable to build dist.
   
   exiting with code: 1
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42109/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822004017


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42106/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821948589


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] viirya commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822023718


   Good point @attilapiros. But maybe we should implement both argumentless and `onClose(WatcherException cause)`? At least I think we should log the exception like currently we did.
   
   
   > For example here I think the argumentless version should be used instead:
   > 
   > https://github.com/apache/spark/blob/a902250e4a6cc88d20097ab356884bc2813edba3/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala#L61-L65
   > 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822022897


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137532/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822050912


   **[Test build #137536 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137536/testReport)** for PR 32221 at commit [`e1c8707`](https://github.com/apache/spark/commit/e1c8707001abae1150d672f2fba34ef270769dba).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822043113


   The following are addressed.
   - Add `private val readiness = Readiness.getInstance()` and reuse it.
   - Add a corresponding `override def onClose(): Unit` consistently for all places not to forget it.
   - The warning message is revised in case `onClose()` (no exception version).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822265951


   **[Test build #137590 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137590/testReport)** for PR 32221 at commit [`e1c8707`](https://github.com/apache/spark/commit/e1c8707001abae1150d672f2fba34ef270769dba).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] attilapiros commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
attilapiros commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822000621


   Reading that K8s client PR about `onClose()` I think we have to be careful which one is to chose  when implementing one: as there is one without any argument and there is another with one argument (WatcherException):
   
   > The interface now provides a dedicated default argument-less onClose() method. This method should always be called upon Watcher completion. Implementations should override this method in case operations (e.g. cleanup) should be performed once the Watcher is closed.
   >
   > The previous onClose(WatcherException cause) is kept and only invoked whenever an exception occurs during the Watcher life-cycle (once the watch connection is open and established).
   
   For example here I think the argumentless version should be used instead:
   https://github.com/apache/spark/blob/a902250e4a6cc88d20097ab356884bc2813edba3/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala#L61-L65
   
   WDYT?
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821950233


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42101/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] viirya edited a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
viirya edited a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822023718


   Good point @attilapiros. But maybe we should implement both argumentless and `onClose(WatcherException cause)`? At least I think we should log the exception like currently we did.
   
   Actually if we don't have cleanup operation to do, maybe we don't need to override argumentless one?
   
   > For example here I think the argumentless version should be used instead:
   > 
   > https://github.com/apache/spark/blob/a902250e4a6cc88d20097ab356884bc2813edba3/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala#L61-L65
   > 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822001132






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822042717


   **[Test build #137535 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137535/testReport)** for PR 32221 at commit [`9c5e70e`](https://github.com/apache/spark/commit/9c5e70e45aec3bb42dd6af1717af242980cfcb18).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821995500


   **[Test build #137532 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137532/testReport)** for PR 32221 at commit [`a902250`](https://github.com/apache/spark/commit/a902250e4a6cc88d20097ab356884bc2813edba3).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] attilapiros edited a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
attilapiros edited a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822026876


   @viirya 
   > Actually if we don't have cleanup operation to do, maybe we don't need to override argumentless one?
   
   As I see the current log message is misleading. If this method with the exception is called then  "(this is expected if the application is shutting down.)" part is not true. So I think we should at least change that.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822519359


   Thank you so much, @attilapiros and @viirya .
   Merged to master for Apache Spark 3.2.0.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun edited a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822043113


   The following are addressed.
   - Add `private val readiness = Readiness.getInstance()` and reuse it.
   - Add a corresponding `override def onClose(): Unit` consistently for all places in order not to forget it.
   - The warning message is revised in case `onClose()` (no exception version).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun closed pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #32221:
URL: https://github.com/apache/spark/pull/32221


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] attilapiros commented on a change in pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #32221:
URL: https://github.com/apache/spark/pull/32221#discussion_r615667822



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala
##########
@@ -58,10 +58,14 @@ private[spark] class ExecutorPodsWatchSnapshotSource(
       snapshotsStore.updatePod(pod)
     }
 
-    override def onClose(e: KubernetesClientException): Unit = {
+    override def onClose(e: WatcherException): Unit = {
       logWarning("Kubernetes client has been closed (this is expected if the application is" +

Review comment:
       > Actually, that's what @attilapiros meant, but it's not in reality. As of now, it's frequently possible to observe K8s side exceptions, especially HTTP-related ones, during application termination after `SparkContext.close()` starts to doing several clean-ups.
   
   In this case I am fine to keep it as it is.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822071255


   **[Test build #137536 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137536/testReport)** for PR 32221 at commit [`e1c8707`](https://github.com/apache/spark/commit/e1c8707001abae1150d672f2fba34ef270769dba).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822050912


   **[Test build #137536 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137536/testReport)** for PR 32221 at commit [`e1c8707`](https://github.com/apache/spark/commit/e1c8707001abae1150d672f2fba34ef270769dba).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821949015


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822076396


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137536/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822050749


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42109/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821965371


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137527/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun edited a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822043113


   The following are addressed.
   - Add `private val readiness = Readiness.getInstance()` and reuse it.
   - Add a corresponding `override def onClose(): Unit` consistently for all places in order not to forget it.
   - The warning message is revised in case of `onClose()` (no exception version).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] attilapiros commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
attilapiros commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822026876


   > Actually if we don't have cleanup operation to do, maybe we don't need to override argumentless one?
   
   As I see the current log message is misleading. If this method with the exception is called then  "(this is expected if the application is shutting down.)" part is not true. So I think we should at least change that.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822076396


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137536/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821961017


   **[Test build #137527 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137527/testReport)** for PR 32221 at commit [`dc1ad22`](https://github.com/apache/spark/commit/dc1ad22bf185e2c6ef8f0a3aa5ba1d96d755aeb9).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32221:
URL: https://github.com/apache/spark/pull/32221#discussion_r615477900



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala
##########
@@ -58,10 +58,14 @@ private[spark] class ExecutorPodsWatchSnapshotSource(
       snapshotsStore.updatePod(pod)
     }
 
-    override def onClose(e: KubernetesClientException): Unit = {
+    override def onClose(e: WatcherException): Unit = {
       logWarning("Kubernetes client has been closed (this is expected if the application is" +

Review comment:
       BTW, in this PR, the warning message of `onClose()` (without exception args) doesn't have `(this is expected if the application is...)` part.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-821944379


   **[Test build #137527 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137527/testReport)** for PR 32221 at commit [`dc1ad22`](https://github.com/apache/spark/commit/dc1ad22bf185e2c6ef8f0a3aa5ba1d96d755aeb9).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822017950


   **[Test build #137532 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137532/testReport)** for PR 32221 at commit [`a902250`](https://github.com/apache/spark/commit/a902250e4a6cc88d20097ab356884bc2813edba3).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] viirya commented on a change in pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #32221:
URL: https://github.com/apache/spark/pull/32221#discussion_r615445958



##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala
##########
@@ -58,10 +58,14 @@ private[spark] class ExecutorPodsWatchSnapshotSource(
       snapshotsStore.updatePod(pod)
     }
 
-    override def onClose(e: KubernetesClientException): Unit = {
+    override def onClose(e: WatcherException): Unit = {
       logWarning("Kubernetes client has been closed (this is expected if the application is" +

Review comment:
       > As I see the current log message is misleading. If this method with the exception is called then "(this is expected if the application is shutting down.)" part is not true. So I think we should at least change that.
   
   I guess @attilapiros comment is we are better to revise this log message? As this is exception case, it may not "this is expected".




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #32221: [SPARK-35125][K8S] Upgrade K8s client to 5.3.0 to support K8s 1.20

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32221:
URL: https://github.com/apache/spark/pull/32221#issuecomment-822059652


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42110/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org