eolivelli commented on code in PR #54336:
URL: https://github.com/apache/spark/pull/54336#discussion_r2813088995
##########
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocatorSuite.scala:
##########
@@ -1021,6 +1021,31 @@ class ExecutorPodsAllocatorSuite extends SparkFunSuite
with BeforeAndAfter {
assert(podsAllocatorUnderTest.invokePrivate(numOutstandingPods).get() == 0)
}
+ test("SPARK-55496: replacePVCsIfNeeded should re-use disks with larger
storage") {
Review Comment:
what about adding a test case on the negative case ? there is a disk but it
is smaller than expected
the other test case I would add is about having more than one PVC
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala:
##########
@@ -499,8 +499,8 @@ class ExecutorPodsAllocator(
// Find one with the same storage class and size.
val index = reusablePVCs.indexWhere { p =>
p.getSpec.getStorageClassName == pvc.getSpec.getStorageClassName &&
- p.getSpec.getResources.getRequests.get("storage") ==
- pvc.getSpec.getResources.getRequests.get("storage")
+ p.getSpec.getResources.getRequests.get("storage")
Review Comment:
what about adding a comment here ?
To explain that it is allowed to see a disk that is larger because the
autoscaler may have increased the size
--
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]
