dongjoon-hyun commented on code in PR #58730:
URL: https://github.com/apache/spark/pull/58730#discussion_r4047637201
##########
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/KubernetesVolumeUtilsSuite.scala:
##########
@@ -257,6 +257,38 @@ class KubernetesVolumeUtilsSuite extends SparkFunSuite {
assert(e.getMessage.contains("nfs.volumeName.options.server"))
}
+ test("Parses csi volumes correctly") {
+ val sparkConf = new SparkConf(false)
+ sparkConf.set("test.csi.volumeName.mount.path", "/path")
+ sparkConf.set("test.csi.volumeName.mount.readOnly", "true")
+ sparkConf.set("test.csi.volumeName.options.driver", "csi.example.com")
+ sparkConf.set("test.csi.volumeName.options.fsType", "ext4")
+ sparkConf.set("test.csi.volumeName.options.nodePublishSecretName",
"csi-secret")
+ sparkConf.set("test.csi.volumeName.options.volumeAttributes.foo", "bar")
+ // Only `options.volumeAttributes.*` become CSI volume attributes; any
other unknown
+ // `options.*` key is ignored rather than swept in as an attribute.
+ sparkConf.set("test.csi.volumeName.options.unknownOption", "ignored")
+
+ val volumeSpec = KubernetesVolumeUtils.parseVolumesWithPrefix(sparkConf,
"test.").head
+ assert(volumeSpec.volumeName === "volumeName")
+ assert(volumeSpec.mountPath === "/path")
+ assert(volumeSpec.mountReadOnly === true)
+ assert(volumeSpec.volumeConf.asInstanceOf[KubernetesCSIVolumeConf] ===
+ KubernetesCSIVolumeConf("csi.example.com", Map("foo" -> "bar"),
+ fsType = Some("ext4"), nodePublishSecretName = Some("csi-secret")))
+ }
+
+ test("Fails on missing csi driver option") {
+ val sparkConf = new SparkConf(false)
+ sparkConf.set("test.csi.volumeName.mount.path", "/path")
+ sparkConf.set("test.csi.volumeName.options.volumeAttributes.foo", "bar")
+
+ val e = intercept[NoSuchElementException] {
+ KubernetesVolumeUtils.parseVolumesWithPrefix(sparkConf, "test.")
+ }
+ assert(e.getMessage.contains("csi.volumeName.options.driver"))
Review Comment:
nit: `e.getMessage.contains("csi.volumeName.options.driver")` also passes
with the raw `key not found: csi.volumeName.options.driver` message, so this
doesn't actually verify that `verifyOptionKey` is used. How about asserting on
`"csi.volumeName.options.driver is required for csi"`?
##########
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStepSuite.scala:
##########
@@ -573,4 +573,40 @@ class MountVolumesFeatureStepSuite extends SparkFunSuite {
assert(configuredPod.pod.getSpec.getVolumes.size() === 2)
assert(configuredPod.container.getVolumeMounts.size() === 2)
}
+
+ test("Mounts csi") {
+ val volumeConf = KubernetesVolumeSpec(
+ "testVolume",
+ "/mnt/disk1",
+ "",
+ "",
+ false,
Review Comment:
nit: Since propagating `mount.readOnly` to `csi.readOnly` was one of the
fixes in this PR, could you use `mountReadOnly = true` here (or add a case) so
that the test fails if `.withReadOnly(...)` is dropped? A driver-only case
asserting `getFsType == null` and `getNodePublishSecretRef == null` would also
be nice.
##########
docs/running-on-kubernetes.md:
##########
@@ -355,6 +356,15 @@ And, the claim name of a `persistentVolumeClaim` with
volume name `checkpointpvc
spark.kubernetes.driver.volumes.persistentVolumeClaim.checkpointpvc.options.claimName=check-point-pvc-claim
```
+And, a `csi` volume with volume name `data` can be specified using the
following properties. `options.driver` is required; `options.fsType`,
`options.nodePublishSecretName` (the name of a `Secret` in the pod's namespace)
and any `options.volumeAttributes.*` are optional:
+
+```
+spark.kubernetes.driver.volumes.csi.data.options.driver=file.csi.azure.com
+spark.kubernetes.driver.volumes.csi.data.options.fsType=ext4
Review Comment:
nit: `file.csi.azure.com` is an SMB/NFS file share driver, so
`options.fsType=ext4` in the same example is a bit misleading. Shall we use a
generic driver name (e.g. `csi.example.com`) or drop the `fsType` line?
--
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]
