HwangDongJun commented on code in PR #58187:
URL: https://github.com/apache/spark/pull/58187#discussion_r4310502371
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala:
##########
@@ -131,48 +147,80 @@ private[spark] class Client(
.withName(configMapName)
.endConfigMap()
.endVolume()
+ .addToNodeSelector(
+ pinnedNodeName.map(nodeName => Map("kubernetes.io/hostname" ->
nodeName))
+ .getOrElse(Map.empty[String, String]).asJava)
+ // SPARK-38079: holds the pod unschedulable -- so kubelet cannot
attempt to mount
+ // anything on it -- until its pre-resources exist (see below). Only
removed once
+ // that is true, right before the watch loop; see the "Remove the
pre-resources
+ // scheduling gate" step below for why this makes an
ownerless-resource window,
+ // and its consequences (a shutdown hook), unnecessary.
+ .addNewSchedulingGate(PRE_RESOURCES_SCHEDULING_GATE)
.endSpec()
.build()
val driverPodName = resolvedDriverPod.getMetadata.getName
- // setup resources before pod creation
- val preKubernetesResources =
resolvedDriverSpec.driverPreKubernetesResources
+ // SPARK-38079: the driver's own base config map (mounted as
SPARK_CONF_VOLUME_DRIVER
+ // above) must also be created before the pod is schedulable, 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)
Review Comment:
Done in three parts. The cases at L171 and L135 are removed by the order and
`nodeName` changes above. For the one that remains, the `patch` verb, I added
`spark.kubernetes.submission.useDriverSchedulingGate` (default `true`), which
restores the previous flow when set to `false`; the name follows the boolean
guideline in `ConfigEntry.scala` and it declares
`ConfigBindingPolicy.NOT_APPLICABLE`. `docs/core-migration-guide.md` has the
entry with the "To restore the legacy behavior" wording, and the PR title and
description are rewritten.
I am not sure the switch is wanted. If documenting the `patch` requirement
is enough, I will remove it.
--
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]
