HwangDongJun commented on code in PR #58187:
URL: https://github.com/apache/spark/pull/58187#discussion_r4163622553
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala:
##########
@@ -136,43 +188,75 @@ private[spark] class Client(
val driverPodName = resolvedDriverPod.getMetadata.getName
// setup resources before pod creation
- val preKubernetesResources =
resolvedDriverSpec.driverPreKubernetesResources
- try {
- kubernetesClient.resourceList(preKubernetesResources:
_*).forceConflicts().serverSideApply()
- } catch {
- case NonFatal(e) =>
- logError("Please check \"kubectl auth can-i create [resource]\"
first." +
- " It should be yes. And please also check your feature step
implementation.")
- kubernetesClient.resourceList(preKubernetesResources: _*).delete()
- throw e
- }
+ // SPARK-38079: the driver's own base config map (mounted as
SPARK_CONF_VOLUME_DRIVER
+ // above) must also be created before the pod itself, to avoid a
"configmap ... not
+ // found" mount race between the driver pod and the config map it depends
on.
+ val preKubernetesResources =
resolvedDriverSpec.driverPreKubernetesResources ++ Seq(configMap)
+
+ // SPARK-38079: some of the pre-resources above (e.g. the Kerberos
keytab/delegation token
Review Comment:
The whole shutdown-hook mechanism these comments were explaining is gone
(replaced by `spec.schedulingGates`, see [the reply
above](https://github.com/apache/spark/pull/58187#discussion_r4129910563)), so
the repeated rationale and the specific inaccurate/stale phrasing you quoted
went with it:
- All four of the quoted phrases ("today's behavior", "is now a
pre-resource", "unchanged by SPARK-38079", "would only be printed to stderr")
no longer appear anywhere in the module.
- The three `logError` hints in `KubernetesClientApplication.scala` are now
distinct per failure point, not duplicated.
- Re-measuring the new code against the same yardstick:
`KubernetesClientApplication.scala`'s `run()`/`deletePodAndPreResources` area
is now comment-light relative to the code, nowhere near the ~1:1 ratio you
measured on the old hook.
I also found (and fixed, in a follow-up commit) a couple of comments in
`ClientSuite.scala` that had the same diff-describing/comment-ID-reference
pattern reintroduced during this round of changes -- thanks for the standard to
check against.
--
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]
