HwangDongJun commented on code in PR #58187:
URL: https://github.com/apache/spark/pull/58187#discussion_r4310450858
##########
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/submit/ClientSuite.scala:
##########
@@ -221,14 +306,216 @@ class ClientSuite extends SparkFunSuite with
BeforeAndAfter {
assert(secrets.nonEmpty)
assert(configMaps.nonEmpty)
val configMap = configMaps.head
- assert(configMap.getMetadata.getName ===
- KubernetesClientUtils.configMapNameDriver)
+ // SPARK-38079: configMapNameDriver is a `def` -- see its own comment --
so this cannot
+ // compare against a freshly-generated name (that call would return a
different one); it
+ // can only check the name has the shape configMapNameDriver is documented
to produce.
+ assert(configMap.getMetadata.getName.matches("spark-drv-\\S+-conf-map"))
assert(configMap.getImmutable())
assert(configMap.getData.containsKey(SPARK_CONF_FILE_NAME))
assert(configMap.getData.get(SPARK_CONF_FILE_NAME).contains("conf1key=conf1value"))
assert(configMap.getData.get(SPARK_CONF_FILE_NAME).contains("conf2key=conf2value"))
}
+ test("SPARK-38079: driver pod is created (still scheduling-gated) before its
own config " +
+ "map, and the config map's single create call already carries the owner
reference") {
+ val submissionClient = new Client(
+ kconf,
+ driverBuilder,
+ kubernetesClient,
+ loggingPodStatusWatcher)
+ submissionClient.run()
+ // The pod itself must be created before any pre-resource resourceList()
call, since a
+ // pre-resource's owner reference (set below) needs the pod's UID.
+ assert(gatedPod === fullExpectedPod(gatedConfigMapName))
+ verify(namedPods).create()
+
+ // The (single) resourceList(...) call creating the driver's own config
map must already
+ // carry the owner reference -- there is no separate, later "refresh"
call. The pod stays
+ // scheduling-gated (so kubelet cannot attempt to mount it) for as long as
this has not yet
+ // happened, avoiding the "configmap ... not found" mount race
(SPARK-38079) without ever
+ // creating the config map ownerless.
+ val resourceListCall = createdResourcesArgumentCaptor.getAllValues.get(0)
+ val configMaps = resourceListCall
+ .filter(_.isInstanceOf[ConfigMap]).map(_.asInstanceOf[ConfigMap])
+ assert(configMaps.nonEmpty,
+ "the driver's own config map must be created as a pre-resource")
+ val ownerReferences = configMaps.head.getMetadata.getOwnerReferences
Review Comment:
Both points are addressed. `resourceList(...)` is now stubbed with a
`doAnswer` that records each call as it happens, including whether every
resource already has an owner reference at that moment, and the tests assert
the whole sequence (`create pod`, `list(... +owner)`, `apply`, `remove gate`).
The `any[Array[HasMetadata]]()` matcher is no longer used in the suite.
Swapping the owner-reference step with the apply fails three tests, and
moving the pre-resources after the pod fails four.
--
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]
