[GitHub] [spark] Yikun commented on a diff in pull request #37802: [SPARK-40350][Kubernetes] Use spark config to configure the parameters of volcano podgroup

[GitBox <gi...@apache.org>]

Yikun commented on code in PR #37802:
URL: https://github.com/apache/spark/pull/37802#discussion_r963695606


##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala:
##########
@@ -54,11 +56,33 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
     metadata.setName(podGroupName)
     metadata.setNamespace(namespace)
     pg.setMetadata(metadata)
-
     var spec = pg.getSpec
     if (spec == null) spec = new PodGroupSpec
+    val queue = kubernetesConf.getOption(POD_GROUP_SPEC_QUEUE)
+    if (queue.isDefined) {
+      spec.setQueue(queue.get)
+    }
+    val minResourceCPU = kubernetesConf.getOption(POD_GROUP_SPEC_MIN_RESOURCE_CPU)
+    val minResourceMemory = kubernetesConf.getOption(POD_GROUP_SPEC_MIN_RESOURCE_MEMORY)
+    if (minResourceCPU.isDefined || minResourceMemory.isDefined) {
+      val minResources = new util.HashMap[String, Quantity]
+      if (minResourceCPU.isDefined) {
+        minResources.put("cpu", new Quantity(minResourceCPU.get))
+      }
+      if (minResourceMemory.isDefined) {
+        minResources.put("memory", new Quantity(minResourceMemory.get))
+      }
+      spec.setMinResources(minResources)

Review Comment:
   This will force to replace existing MinResources in template and ignore existing template
   
   



##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/VolcanoFeatureStep.scala:
##########
@@ -54,11 +56,33 @@ private[spark] class VolcanoFeatureStep extends KubernetesDriverCustomFeatureCon
     metadata.setName(podGroupName)
     metadata.setNamespace(namespace)
     pg.setMetadata(metadata)
-
     var spec = pg.getSpec
     if (spec == null) spec = new PodGroupSpec
+    val queue = kubernetesConf.getOption(POD_GROUP_SPEC_QUEUE)

Review Comment:
   ```scala
   val pg = template.map(client.podGroups.load(_).get).getOrElse(new PodGroup())
   val builder = new PodGroupBuilder(pg)
     .editOrNewMetadata()
       .withName(podGroupName)
       .withNamespace(namespace)
     .endMetadata()
     .editOrNewSpec()
     .endSpec()
   
   if (xxx) {
     builder.editOrNewSpec().endSpec()
   }
   
   return Seq(builder.build())
   ```
   
   If we decided to add parameters support, the builder style might more stable and easy to maintainance.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org