Spark driver's exit code should be exit code of driver pod

[ash211]

As @kimoonkim pointed out in #119 (comment) the exit code of the driver JVM is being ignored by the rest server:

I see the driver exit code (the return value of process.waitFor below) being ignored. Maybe we should let the rest server exit using the driver exit code if it is non-zero?

                waitForProcessCompleteExecutor.submit(new Runnable {
                  override def run(): Unit = {
                    process.waitFor
                    SERVLET_LOCK.synchronized {
                      logInfo("Spark application complete. Shutting down submission server...")
                      KubernetesSparkRestServer.this.stop
                      shutdownLock.countDown()
                    }
                  }
                })

[kimoonkim]

Thanks @ash211 for adding this issue. I was playing with the code to see if there is a quick fix. I'll update here when I have more findings.

[ash211]

@kimoonkim I threw something together at #138 -- is that similar to what you were looking at?

[kimoonkim]

@ash211 Yes, #138 is very similar to mine down to variable names. And it looks good :-)

I am glad that you wrote the change, because we can now talk about the next issue I observed. When the rest server exits using a non-zero exit code, it gets restarted automatically by kubernetes. I think this is because we specify the restart policy OnFailure:

    kubernetesClient.pods().createNew()
      .withNewMetadata()
        .withName(kubernetesAppId)
        .withLabels(driverKubernetesSelectors)
        .endMetadata()
      .withNewSpec()
        .withRestartPolicy("OnFailure")

Since the pod never completed, LoggingPodStatusWatcher keeps going reporting Running status.

...
2017-02-22 15:33:18 INFO  LoggingPodStatusWatcher:54 - Application status for spark-pi-1487806168626 (phase: Running)
2017-02-22 15:33:19 INFO  LoggingPodStatusWatcher:54 - Application status for spark-pi-1487806168626 (phase: Running)
2017-02-22 15:33:20 INFO  LoggingPodStatusWatcher:54 - Application status for spark-pi-1487806168626 (phase: Running)
2017-02-22 15:33:21 INFO  LoggingPodStatusWatcher:54 - Application status for spark-pi-1487806168626 (phase: Running)
2017-02-22 15:33:22 INFO  LoggingPodStatusWatcher:54 - Application status for spark-pi-1487806168626 (phase: Running)

The only visible difference outside is that the restart count increased from 0 to 1. (It was like spark-pi-1487806168626 1/1 Running 0 1m before)

$ kubectl get pods
NAME                                  READY     STATUS    RESTARTS   AGE
spark-pi-1487806168626                1/1       Running   1          3m

The pod now has only the rest server in it. And the client does not try to relaunch the driver. So it is pointless to keep the pod running.

I wonder if we want to change the restart policy to Never, at least after the client launched the driver.

[kimoonkim]

Quickly played with setting the restart policy Never. This leads to a much better behavior. The client correctly recognizes the driver failure:

2017-02-23 10:11:03 INFO  LoggingPodStatusWatcher:54 - Application status for spark-pi-1487873291140 (phase: Running)
2017-02-23 10:11:03 INFO  LoggingPodStatusWatcher:54 - Application status for spark-pi-1487873291140 (phase: Failed)
2017-02-23 10:11:03 INFO  LoggingPodStatusWatcher:54 - Phase changed, new state:
	 pod name: spark-pi-1487873291140
	 namespace: default
	 labels: spark-app-id -> spark-pi-1487873291140, spark-app-name -> spark-pi, spark-driver -> spark-pi-1487873291140
	 pod uid: 0bdbdc69-f9f3-11e6-a88c-02f2c310e88c
	 creation time: 2017-02-23T18:08:15Z
	 service account name: default
	 volumes: spark-submission-secret-volume, default-token-7eejh
	 node name: kube-n2.pepperdata.com
	 start time: 2017-02-23T18:08:15Z
	 container images: docker:5000/spark-driver:kimoon-crash
	 phase: Failed
2017-02-23 10:11:03 INFO  Client:54 - Application spark-pi-1487873291140 finished.

kubectl get pods also behaves reasonably:

$ kubectl get pods
...
  info: 1 completed object(s) was(were) not shown in pods list. Pass --show-all to see all objects.

$ kubectl get pods --show-all
...
spark-pi-1487873291140                0/1       Error     0          5m

At this point, one could do kubectl logs spark-pi-1487873291140 to look at the driver and rest server log. (Previously, the failure messages disappeared because the pod restarted)

Did we have reasons to use the OnFailure restart policy? Maybe we're worried the pod may fail to launch at the start?

[ash211]

I think Never is probably better behavior for the driver pod anyway -- because it depends on state injected into it from the launcher, it can't be restarted anyway. So probably that PR should also change the driver pod's restart policy to Never. Does that seem reasonable?

[kimoonkim]

SGTM. But I wonder what others think who originally chose OnFailure. Would switching to Never cause any regressions? If nobody recalls, I think we should use Never.

[ash211]

Fixed in #138

I'll file a separate issue for changing the driver pod restart policy.