dongjoon-hyun commented on PR #828: URL: https://github.com/apache/spark-kubernetes-operator/pull/828#issuecomment-5689157868
Thank you for the careful follow-up, @viirya. Both points are addressed in 67d1cfc. **Selection before the name check**: `SparkAppContext` now has `getCurrentAttemptDriverPod()`. It collects every driver-labeled pod from the informer cache and applies the current-attempt predicate to all of them before selecting one, instead of checking only the single pod that `getDriverPod().findAny()` happens to return. `AppInitStep` uses that method; the name comparison is gone from the step. `SparkAppContextTest.currentAttemptDriverPodIsFoundBehindPreviousAttemptPod` has the older pod first in the stream and asserts the current one is found. **Reused pod names (`spark.app.id` / `spark.kubernetes.driver.pod.name`)**: rather than assuming the name carries the attempt id, the predicate now requires the pod to have the desired driver pod spec's name **and** no `deletionTimestamp`. `AppCleanUpStep` deletes the previous attempt's driver before the application transitions to `ScheduledToRestart`, and `deleteResourceIfExists` rethrows anything but 404 so the transition cannot happen without the delete being accepted. Once the app is in `ScheduledToRestart`, the previous attempt's pod is therefore either gone or terminating, even when its name is identical to the current spec's. `Submitted` has no previous attempt. `previousAttemptDriverPodDoesNotBypassSuspend` now uses a terminating pod with the very same name as the spec, and `SparkAppContextTest.terminatingPreviousAttemptPodWithSameNameIsNotCurrentAttemptDriver` covers the selection for that case. The remaining window is an informer cache that has not yet reflected the acce pted delete, which is the same class of lag the missing-driver grace period already tolerates. I considered labeling the driver pod with the attempt id, which would make the identification exact regardless of naming. That changes every driver pod the operator creates and is independent of `spec.suspend`, so it is better done as its own change if we want it. The javadoc and the PR description describe the current rule. The `pi-with-gluten` failure on the previous run was a state-transition timeout in that group's own assertion, unrelated to this change. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
