HwangDongJun commented on code in PR #58187:
URL: https://github.com/apache/spark/pull/58187#discussion_r4345302450
##########
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/submit/ClientSuite.scala:
##########
@@ -223,12 +277,210 @@ class ClientSuite extends SparkFunSuite with
BeforeAndAfter {
val configMap = configMaps.head
assert(configMap.getMetadata.getName ===
KubernetesClientUtils.configMapNameDriver)
+ assert(configMap.getMetadata.getName === mountedConfigMapName)
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: the driver pod is created gated, and the gate is removed
last") {
+ newClient().run()
+
+ val configMapName = KubernetesClientUtils.configMapNameDriver
+ assert(events.toSeq === Seq(
+ "list()", "apply",
+ "create pod",
+ "list()", "apply",
+ s"list(Secret/secret+owner,ConfigMap/$configMapName+owner)", "apply",
+ "remove gate"))
+
assert(createdPodArgumentCaptor.getValue.getSpec.getSchedulingGates.asScala.map(_.getName)
===
+ Seq(DRIVER_RESOURCES_SCHEDULING_GATE))
+ }
+
+ test("SPARK-38079: pre resources are applied before the gated pod and owned
after it") {
+ val preResKconf = KubernetesTestConf.createDriverConf(
+ resourceNamePrefix = Some(KUBERNETES_RESOURCE_PREFIX))
+ when(driverBuilder.buildFromFeatures(preResKconf, kubernetesClient))
+ .thenReturn(BUILT_KUBERNETES_SPEC_WITH_PRERES)
+ kconf = preResKconf
+
+ newClient().run()
+
+ val configMapName = KubernetesClientUtils.configMapNameDriver
+ assert(events.toSeq === Seq(
+ "list(CustomResourceDefinition/preCRD)", "apply",
+ "create pod",
+ "list(CustomResourceDefinition/preCRD+owner)", "apply",
+ s"list(Secret/secret+owner,ConfigMap/$configMapName+owner)", "apply",
+ "remove gate"))
+ }
+
+ test("SPARK-38079: the gate is removed by name with an idempotent strategic
merge patch") {
+ newClient().run()
+
+ verify(namedPods).patch(
+ ArgumentMatchers.any[PatchContext](), ArgumentMatchers.any[String]())
+ assert(patchContextCaptor.getValue.getPatchType ===
PatchType.STRATEGIC_MERGE)
+ assert(patchBodyCaptor.getValue ===
+ s"""{"spec":{"schedulingGates":[{"$$patch":"delete",""" +
+ s""""name":"$DRIVER_RESOURCES_SCHEDULING_GATE"}]}}""")
+ }
+
+ test("SPARK-38079: gates already on the pod template are kept next to the
Spark gate") {
+ val templateGate = "example.com/keep-me"
+ val podWithGate = new PodBuilder(BUILT_DRIVER_POD)
+ .editSpec()
+ .addNewSchedulingGate(templateGate)
+ .endSpec()
+ .build()
+ when(driverBuilder.buildFromFeatures(kconf, kubernetesClient)).thenReturn(
+ KubernetesDriverSpec(
+ SparkPod(podWithGate, BUILT_DRIVER_CONTAINER),
+ Nil,
+ ADDITIONAL_RESOURCES,
+ RESOLVED_JAVA_OPTIONS))
+
+ newClient().run()
+
+
assert(createdPodArgumentCaptor.getValue.getSpec.getSchedulingGates.asScala.map(_.getName)
===
+ Seq(templateGate, DRIVER_RESOURCES_SCHEDULING_GATE))
+ assert(!patchBodyCaptor.getValue.contains(templateGate))
+ }
+
+ test("SPARK-38079: the gate patch leaves a gate it does not name in place") {
+ // A fake API server that applies the patch body: only the named gate may
disappear.
Review Comment:
Fixed both.
- The `ObjectMapper` test is gone, together with the module's only
`ObjectMapper`. What it was meant to show is in the test for template gates,
which asserts the gate names on the created pod, the patch type and that the
patch body does not name the template's gate. The comment there now says only
that the patch names Spark's gate. Switching to `PatchType.JSON_MERGE` fails
the patch test and the template gate test.
- The comment about "the one this submission created" is now "The delete
goes through the pod returned by create().", which is what the test checks.
--
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]
