[GitHub] spark pull request #20722: [SPARK-23571][K8S] Delete auxiliary Kubernetes re...

2018-07-13 Thread liyinan926
Github user liyinan926 closed the pull request at:

https://github.com/apache/spark/pull/20722


---

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



[GitHub] spark pull request #20722: [SPARK-23571][K8S] Delete auxiliary Kubernetes re...

2018-03-02 Thread liyinan926
Github user liyinan926 commented on a diff in the pull request:

https://github.com/apache/spark/pull/20722#discussion_r171975765
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala
 ---
@@ -159,6 +159,19 @@ private[spark] class Client(
 logInfo(s"Waiting for application $appName to finish...")
 watcher.awaitCompletion()
 logInfo(s"Application $appName finished.")
+try {
--- End diff --

Per the discussion offline, I think the right solution is move resource 
management to the driver pod. This way, resource cleanup is guaranteed 
regardless of the deployment mode.


---

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



[GitHub] spark pull request #20722: [SPARK-23571][K8S] Delete auxiliary Kubernetes re...

2018-03-02 Thread foxish
Github user foxish commented on a diff in the pull request:

https://github.com/apache/spark/pull/20722#discussion_r171972187
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala
 ---
@@ -159,6 +159,19 @@ private[spark] class Client(
 logInfo(s"Waiting for application $appName to finish...")
 watcher.awaitCompletion()
 logInfo(s"Application $appName finished.")
+try {
--- End diff --

I see! It was the RBAC rules and downscoping them that led us here. I'm 
concerned not all jobs will actually use this interactive mode of launching. 
What do you think of just granting more permissions to the driver and allowing 
cleanup there? 


---

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



[GitHub] spark pull request #20722: [SPARK-23571][K8S] Delete auxiliary Kubernetes re...

2018-03-02 Thread liyinan926
Github user liyinan926 commented on a diff in the pull request:

https://github.com/apache/spark/pull/20722#discussion_r171969867
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala
 ---
@@ -159,6 +159,19 @@ private[spark] class Client(
 logInfo(s"Waiting for application $appName to finish...")
 watcher.awaitCompletion()
 logInfo(s"Application $appName finished.")
+try {
--- End diff --

We talked about this before. The main reason is this requires giving the 
driver extra permissions to delete things. 


---

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



[GitHub] spark pull request #20722: [SPARK-23571][K8S] Delete auxiliary Kubernetes re...

2018-03-02 Thread foxish
Github user foxish commented on a diff in the pull request:

https://github.com/apache/spark/pull/20722#discussion_r171968886
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala
 ---
@@ -159,6 +159,19 @@ private[spark] class Client(
 logInfo(s"Waiting for application $appName to finish...")
 watcher.awaitCompletion()
 logInfo(s"Application $appName finished.")
+try {
--- End diff --

(also because this code path isn't invoked in fire-and-forget mode IIUC)


---

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



[GitHub] spark pull request #20722: [SPARK-23571][K8S] Delete auxiliary Kubernetes re...

2018-03-02 Thread foxish
Github user foxish commented on a diff in the pull request:

https://github.com/apache/spark/pull/20722#discussion_r171968410
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala
 ---
@@ -159,6 +159,19 @@ private[spark] class Client(
 logInfo(s"Waiting for application $appName to finish...")
 watcher.awaitCompletion()
 logInfo(s"Application $appName finished.")
+try {
--- End diff --

Why are we doing this in client code? Driver shutdown is the right place to 
perform cleanup right?


---

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



[GitHub] spark pull request #20722: [SPARK-23571][K8S] Delete auxiliary Kubernetes re...

2018-03-02 Thread liyinan926
GitHub user liyinan926 opened a pull request:

https://github.com/apache/spark/pull/20722

[SPARK-23571][K8S] Delete auxiliary Kubernetes resources upon application 
completion

## What changes were proposed in this pull request?

Spark on Kubernetes creates some auxiliary Kubernetes resources such as the 
headless driver service used by the executors to connect to the driver and the 
ConfigMap that carries configuration for the init-container. Such resources are 
no longer needed once an application completes, but they are still persisted by 
the API server and there should be deleted upon completion. This PR handles the 
cases when the submission waits for the application to complete, which is the 
default behavior as `spark.kubernetes.submission.waitAppCompletion` defaults to 
`true`. 

## How was this patch tested?

Unit tested.

@felixcheung @foxish

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/liyinan926/spark-k8s SPARK-23571

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/20722.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #20722


commit 9890ebc543eb226f395391fb14b877d4b1bf0560
Author: Yinan Li 
Date:   2018-03-02T19:57:35Z

[SPARK-23571][K8S] Delete auxiliary Kubernetes resources upon application 
completion




---

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