[GitHub] spark pull request #21095: [SPARK-23529][K8s] Support mounting hostPath volu...

2018-07-18 Thread asfgit 
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/21095


---

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

[GitHub] spark pull request #21095: [SPARK-23529][K8s] Support mounting hostPath volu...

2018-05-03 Thread liyinan926 
Github user liyinan926 commented on a diff in the pull request:

https://github.com/apache/spark/pull/21095#discussion_r185961170
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesVolumeUtils.scala
 ---
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s
+
+import scala.collection.mutable.HashMap
+
+import io.fabric8.kubernetes.api.model._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.deploy.k8s.Config._
+
+private[spark] object KubernetesVolumeUtils {
+
+  /**
+   * Given hostPath volume specs, add volume to pod and volume mount to 
container.
+   *
+   * @param pod original specification of the pod
+   * @param container original specification of the container
+   * @param sparkConf Spark configuration
+   * @param prefix the prefix for volume configuration
+   * @return a tuple of (pod with the volume(s) added, container with 
mount(s) added)
+   */
+  def addVolumes(
+  pod: Pod,
+  container: Container,
+  sparkConf: SparkConf,
+  prefix : String): (Pod, Container) = {
+val hostPathVolumeSpecs = parseHostPathVolumesWithPrefix(sparkConf, 
prefix)
+addHostPathVolumes(pod, container, hostPathVolumeSpecs)
+  }
+
+  /**
+   * Extract Spark volume configuration properties with a given name 
prefix.
+   *
+   * @param sparkConf Spark configuration
+   * @param prefix the given property name prefix
+   * @param volumeTypeKey the given property name prefix
+   * @return a Map storing with volume name as key and spec as value
+   */
+  def parseVolumesWithPrefix(
+  sparkConf: SparkConf,
+  prefix: String,
+  volumeTypeKey: String): Map[String, KubernetesVolumeSpec] = {
+val volumes = HashMap[String, KubernetesVolumeSpec]()
+val properties = 
sparkConf.getAllWithPrefix(s"$prefix$volumeTypeKey.").toList
+// Extract volume names
+properties.foreach {
+  k =>
+val keys = k._1.split("\\.")
+if (keys.nonEmpty && !volumes.contains(keys(0))) {
+  volumes.update(keys(0), KubernetesVolumeSpec.emptySpec())
+}
+}
+// Populate spec
+volumes.foreach {
+  case (name, spec) =>
+properties.foreach {
+  k =>
+k._1.split("\\.") match {
+  case Array(`name`, KUBERNETES_VOLUMES_MOUNT_KEY, 
KUBERNETES_VOLUMES_PATH_KEY) =>
+spec.mountPath = Some(k._2)
+  case Array(`name`, KUBERNETES_VOLUMES_MOUNT_KEY, 
KUBERNETES_VOLUMES_READONLY_KEY) =>
+spec.mountReadOnly = Some(k._2.toBoolean)
+  case Array(`name`, KUBERNETES_VOLUMES_OPTIONS_KEY, option) =>
+spec.optionsSpec.update(option, k._2)
+  case _ =>
+None
+}
+}
+}
+volumes.toMap
+  }
+
+  /**
+   * Extract Spark hostPath volume configuration properties with a given 
name prefix and
+   * return the result as a Map.
+   *
+   * @param sparkConf Spark configuration
+   * @param prefix the given property name prefix
+   * @return a Map storing with volume name as key and spec as value
+   */
+  def parseHostPathVolumesWithPrefix(
+  sparkConf: SparkConf,
+  prefix: String): Map[String, KubernetesVolumeSpec] = {
+parseVolumesWithPrefix(sparkConf, prefix, 
KUBERNETES_VOLUMES_HOSTPATH_KEY)
+  }
+
+  /**
+   * Given hostPath volume specs, add volume to pod and volume mount to 
container.
+   *
+   * @param pod original specification of the pod
+   * @param container original specification of the container
+   * @param volumes list of named volume specs
+   * @return a tuple of (pod with the volume(s) added, container with 
mount(s) added)
+   */
+  def addHostPathVolumes(
+  pod: Pod,
+

[GitHub] spark pull request #21095: [SPARK-23529][K8s] Support mounting hostPath volu...

2018-05-03 Thread liyinan926 
Github user liyinan926 commented on a diff in the pull request:

https://github.com/apache/spark/pull/21095#discussion_r185961495
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesVolumeUtils.scala
 ---
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s
+
+import scala.collection.mutable.HashMap
+
+import io.fabric8.kubernetes.api.model._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.deploy.k8s.Config._
+
+private[spark] object KubernetesVolumeUtils {
+
+  /**
+   * Given hostPath volume specs, add volume to pod and volume mount to 
container.
+   *
+   * @param pod original specification of the pod
+   * @param container original specification of the container
+   * @param sparkConf Spark configuration
+   * @param prefix the prefix for volume configuration
+   * @return a tuple of (pod with the volume(s) added, container with 
mount(s) added)
+   */
+  def addVolumes(
+  pod: Pod,
+  container: Container,
+  sparkConf: SparkConf,
+  prefix : String): (Pod, Container) = {
+val hostPathVolumeSpecs = parseHostPathVolumesWithPrefix(sparkConf, 
prefix)
+addHostPathVolumes(pod, container, hostPathVolumeSpecs)
+  }
+
+  /**
+   * Extract Spark volume configuration properties with a given name 
prefix.
+   *
+   * @param sparkConf Spark configuration
+   * @param prefix the given property name prefix
+   * @param volumeTypeKey the given property name prefix
+   * @return a Map storing with volume name as key and spec as value
+   */
+  def parseVolumesWithPrefix(
+  sparkConf: SparkConf,
+  prefix: String,
+  volumeTypeKey: String): Map[String, KubernetesVolumeSpec] = {
+val volumes = HashMap[String, KubernetesVolumeSpec]()
+val properties = 
sparkConf.getAllWithPrefix(s"$prefix$volumeTypeKey.").toList
+// Extract volume names
+properties.foreach {
+  k =>
+val keys = k._1.split("\\.")
+if (keys.nonEmpty && !volumes.contains(keys(0))) {
+  volumes.update(keys(0), KubernetesVolumeSpec.emptySpec())
+}
+}
+// Populate spec
+volumes.foreach {
+  case (name, spec) =>
+properties.foreach {
+  k =>
+k._1.split("\\.") match {
+  case Array(`name`, KUBERNETES_VOLUMES_MOUNT_KEY, 
KUBERNETES_VOLUMES_PATH_KEY) =>
+spec.mountPath = Some(k._2)
+  case Array(`name`, KUBERNETES_VOLUMES_MOUNT_KEY, 
KUBERNETES_VOLUMES_READONLY_KEY) =>
+spec.mountReadOnly = Some(k._2.toBoolean)
+  case Array(`name`, KUBERNETES_VOLUMES_OPTIONS_KEY, option) =>
+spec.optionsSpec.update(option, k._2)
+  case _ =>
+None
+}
+}
+}
+volumes.toMap
+  }
+
+  /**
+   * Extract Spark hostPath volume configuration properties with a given 
name prefix and
+   * return the result as a Map.
+   *
+   * @param sparkConf Spark configuration
+   * @param prefix the given property name prefix
+   * @return a Map storing with volume name as key and spec as value
+   */
+  def parseHostPathVolumesWithPrefix(
+  sparkConf: SparkConf,
+  prefix: String): Map[String, KubernetesVolumeSpec] = {
+parseVolumesWithPrefix(sparkConf, prefix, 
KUBERNETES_VOLUMES_HOSTPATH_KEY)
+  }
+
+  /**
+   * Given hostPath volume specs, add volume to pod and volume mount to 
container.
+   *
+   * @param pod original specification of the pod
+   * @param container original specification of the container
+   * @param volumes list of named volume specs
+   * @return a tuple of (pod with the volume(s) added, container with 
mount(s) added)
+   */
+  def addHostPathVolumes(
+  pod: Pod,
+

[GitHub] spark pull request #21095: [SPARK-23529][K8s] Support mounting hostPath volu...

2018-05-03 Thread liyinan926 
Github user liyinan926 commented on a diff in the pull request:

https://github.com/apache/spark/pull/21095#discussion_r185960781
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesVolumeUtils.scala
 ---
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s
+
+import scala.collection.mutable.HashMap
+
+import io.fabric8.kubernetes.api.model._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.deploy.k8s.Config._
+
+private[spark] object KubernetesVolumeUtils {
+
+  /**
+   * Given hostPath volume specs, add volume to pod and volume mount to 
container.
+   *
+   * @param pod original specification of the pod
+   * @param container original specification of the container
+   * @param sparkConf Spark configuration
+   * @param prefix the prefix for volume configuration
+   * @return a tuple of (pod with the volume(s) added, container with 
mount(s) added)
+   */
+  def addVolumes(
+  pod: Pod,
+  container: Container,
+  sparkConf: SparkConf,
+  prefix : String): (Pod, Container) = {
+val hostPathVolumeSpecs = parseHostPathVolumesWithPrefix(sparkConf, 
prefix)
+addHostPathVolumes(pod, container, hostPathVolumeSpecs)
+  }
+
+  /**
+   * Extract Spark volume configuration properties with a given name 
prefix.
+   *
+   * @param sparkConf Spark configuration
+   * @param prefix the given property name prefix
+   * @param volumeTypeKey the given property name prefix
+   * @return a Map storing with volume name as key and spec as value
+   */
+  def parseVolumesWithPrefix(
+  sparkConf: SparkConf,
+  prefix: String,
+  volumeTypeKey: String): Map[String, KubernetesVolumeSpec] = {
+val volumes = HashMap[String, KubernetesVolumeSpec]()
+val properties = 
sparkConf.getAllWithPrefix(s"$prefix$volumeTypeKey.").toList
+// Extract volume names
+properties.foreach {
+  k =>
+val keys = k._1.split("\\.")
+if (keys.nonEmpty && !volumes.contains(keys(0))) {
+  volumes.update(keys(0), KubernetesVolumeSpec.emptySpec())
+}
+}
+// Populate spec
+volumes.foreach {
+  case (name, spec) =>
+properties.foreach {
+  k =>
+k._1.split("\\.") match {
+  case Array(`name`, KUBERNETES_VOLUMES_MOUNT_KEY, 
KUBERNETES_VOLUMES_PATH_KEY) =>
+spec.mountPath = Some(k._2)
+  case Array(`name`, KUBERNETES_VOLUMES_MOUNT_KEY, 
KUBERNETES_VOLUMES_READONLY_KEY) =>
+spec.mountReadOnly = Some(k._2.toBoolean)
+  case Array(`name`, KUBERNETES_VOLUMES_OPTIONS_KEY, option) =>
+spec.optionsSpec.update(option, k._2)
+  case _ =>
+None
+}
+}
+}
+volumes.toMap
+  }
+
+  /**
+   * Extract Spark hostPath volume configuration properties with a given 
name prefix and
+   * return the result as a Map.
+   *
+   * @param sparkConf Spark configuration
+   * @param prefix the given property name prefix
+   * @return a Map storing with volume name as key and spec as value
+   */
+  def parseHostPathVolumesWithPrefix(
+  sparkConf: SparkConf,
+  prefix: String): Map[String, KubernetesVolumeSpec] = {
+parseVolumesWithPrefix(sparkConf, prefix, 
KUBERNETES_VOLUMES_HOSTPATH_KEY)
+  }
+
+  /**
+   * Given hostPath volume specs, add volume to pod and volume mount to 
container.
+   *
+   * @param pod original specification of the pod
+   * @param container original specification of the container
+   * @param volumes list of named volume specs
+   * @return a tuple of (pod with the volume(s) added, container with 
mount(s) added)
+   */
+  def addHostPathVolumes(
+  pod: Pod,
+

[GitHub] spark pull request #21095: [SPARK-23529][K8s] Support mounting hostPath volu...

2018-05-03 Thread liyinan926 
Github user liyinan926 commented on a diff in the pull request:

https://github.com/apache/spark/pull/21095#discussion_r185980118
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesVolumeUtils.scala
 ---
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s
+
+import scala.collection.mutable.HashMap
+
+import io.fabric8.kubernetes.api.model._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.deploy.k8s.Config._
+
+private[spark] object KubernetesVolumeUtils {
+
+  /**
+   * Given hostPath volume specs, add volume to pod and volume mount to 
container.
+   *
+   * @param pod original specification of the pod
+   * @param container original specification of the container
+   * @param sparkConf Spark configuration
+   * @param prefix the prefix for volume configuration
+   * @return a tuple of (pod with the volume(s) added, container with 
mount(s) added)
+   */
+  def addVolumes(
+  pod: Pod,
+  container: Container,
+  sparkConf: SparkConf,
+  prefix : String): (Pod, Container) = {
+val hostPathVolumeSpecs = parseHostPathVolumesWithPrefix(sparkConf, 
prefix)
+addHostPathVolumes(pod, container, hostPathVolumeSpecs)
+  }
+
+  /**
+   * Extract Spark volume configuration properties with a given name 
prefix.
+   *
+   * @param sparkConf Spark configuration
+   * @param prefix the given property name prefix
+   * @param volumeTypeKey the given property name prefix
+   * @return a Map storing with volume name as key and spec as value
+   */
+  def parseVolumesWithPrefix(
+  sparkConf: SparkConf,
+  prefix: String,
+  volumeTypeKey: String): Map[String, KubernetesVolumeSpec] = {
+val volumes = HashMap[String, KubernetesVolumeSpec]()
+val properties = 
sparkConf.getAllWithPrefix(s"$prefix$volumeTypeKey.").toList
+// Extract volume names
+properties.foreach {
+  k =>
+val keys = k._1.split("\\.")
+if (keys.nonEmpty && !volumes.contains(keys(0))) {
+  volumes.update(keys(0), KubernetesVolumeSpec.emptySpec())
+}
+}
+// Populate spec
+volumes.foreach {
+  case (name, spec) =>
+properties.foreach {
+  k =>
+k._1.split("\\.") match {
+  case Array(`name`, KUBERNETES_VOLUMES_MOUNT_KEY, 
KUBERNETES_VOLUMES_PATH_KEY) =>
+spec.mountPath = Some(k._2)
+  case Array(`name`, KUBERNETES_VOLUMES_MOUNT_KEY, 
KUBERNETES_VOLUMES_READONLY_KEY) =>
+spec.mountReadOnly = Some(k._2.toBoolean)
+  case Array(`name`, KUBERNETES_VOLUMES_OPTIONS_KEY, option) =>
+spec.optionsSpec.update(option, k._2)
+  case _ =>
+None
+}
+}
+}
+volumes.toMap
+  }
+
+  /**
+   * Extract Spark hostPath volume configuration properties with a given 
name prefix and
+   * return the result as a Map.
+   *
+   * @param sparkConf Spark configuration
+   * @param prefix the given property name prefix
+   * @return a Map storing with volume name as key and spec as value
+   */
+  def parseHostPathVolumesWithPrefix(
--- End diff --

Looks like you don't really need this function as it's just a wrapper of 
`parseVolumesWithPrefix `. 


---

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

[GitHub] spark pull request #21095: [SPARK-23529][K8s] Support mounting hostPath volu...

2018-05-03 Thread liyinan926 
Github user liyinan926 commented on a diff in the pull request:

https://github.com/apache/spark/pull/21095#discussion_r185980514
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicDriverFeatureStep.scala
 ---
@@ -109,7 +109,15 @@ private[spark] class BasicDriverFeatureStep(
 .addToImagePullSecrets(conf.imagePullSecrets(): _*)
 .endSpec()
   .build()
-SparkPod(driverPod, driverContainer)
+
+val (driverPodWithVolumes, driverContainerVolumes) =
--- End diff --

Instead of putting the logic of volume mounting in `BasicDriverFeatureStep` 
and `BasicExecutorFeatureStep`, we should add a new step for mounting volumes, 
similarly to how we handle secrets, e.g., `MountVolumesFeatureStep` where the 
logic of `addVolumes` should be. This feature step can be used for both the 
driver and executors.


---

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

[GitHub] spark pull request #21095: [SPARK-23529][K8s] Support mounting hostPath volu...

2018-05-03 Thread liyinan926 
Github user liyinan926 commented on a diff in the pull request:

https://github.com/apache/spark/pull/21095#discussion_r185960630
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesVolumeUtils.scala
 ---
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s
+
+import scala.collection.mutable.HashMap
+
+import io.fabric8.kubernetes.api.model._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.deploy.k8s.Config._
+
+private[spark] object KubernetesVolumeUtils {
+
+  /**
+   * Given hostPath volume specs, add volume to pod and volume mount to 
container.
+   *
+   * @param pod original specification of the pod
+   * @param container original specification of the container
+   * @param sparkConf Spark configuration
+   * @param prefix the prefix for volume configuration
+   * @return a tuple of (pod with the volume(s) added, container with 
mount(s) added)
+   */
+  def addVolumes(
+  pod: Pod,
+  container: Container,
+  sparkConf: SparkConf,
+  prefix : String): (Pod, Container) = {
+val hostPathVolumeSpecs = parseHostPathVolumesWithPrefix(sparkConf, 
prefix)
+addHostPathVolumes(pod, container, hostPathVolumeSpecs)
+  }
+
+  /**
+   * Extract Spark volume configuration properties with a given name 
prefix.
+   *
+   * @param sparkConf Spark configuration
+   * @param prefix the given property name prefix
+   * @param volumeTypeKey the given property name prefix
+   * @return a Map storing with volume name as key and spec as value
+   */
+  def parseVolumesWithPrefix(
+  sparkConf: SparkConf,
+  prefix: String,
+  volumeTypeKey: String): Map[String, KubernetesVolumeSpec] = {
+val volumes = HashMap[String, KubernetesVolumeSpec]()
+val properties = 
sparkConf.getAllWithPrefix(s"$prefix$volumeTypeKey.").toList
+// Extract volume names
+properties.foreach {
+  k =>
+val keys = k._1.split("\\.")
+if (keys.nonEmpty && !volumes.contains(keys(0))) {
+  volumes.update(keys(0), KubernetesVolumeSpec.emptySpec())
+}
+}
+// Populate spec
+volumes.foreach {
+  case (name, spec) =>
--- End diff --

The `case` line should be merged into the previous line according to the 
Spark code convention, e.g., `volumes.foreach { case (name, spec) =>`. 


---

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

[GitHub] spark pull request #21095: [SPARK-23529][K8s] Support mounting hostPath volu...

2018-05-03 Thread liyinan926 
Github user liyinan926 commented on a diff in the pull request:

https://github.com/apache/spark/pull/21095#discussion_r185960149
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesVolumeUtils.scala
 ---
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s
+
+import scala.collection.mutable.HashMap
+
+import io.fabric8.kubernetes.api.model._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.deploy.k8s.Config._
+
+private[spark] object KubernetesVolumeUtils {
+
+  /**
+   * Given hostPath volume specs, add volume to pod and volume mount to 
container.
+   *
+   * @param pod original specification of the pod
+   * @param container original specification of the container
+   * @param sparkConf Spark configuration
+   * @param prefix the prefix for volume configuration
+   * @return a tuple of (pod with the volume(s) added, container with 
mount(s) added)
+   */
+  def addVolumes(
--- End diff --

The Scaladoc should not mention hostPath as this function is not hostPath 
exclusively.


---

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

[GitHub] spark pull request #21095: [SPARK-23529][K8s] Support mounting hostPath volu...

2018-05-03 Thread liyinan926 
Github user liyinan926 commented on a diff in the pull request:

https://github.com/apache/spark/pull/21095#discussion_r185960658
  
--- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesVolumeUtils.scala
 ---
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark.deploy.k8s
+
+import scala.collection.mutable.HashMap
+
+import io.fabric8.kubernetes.api.model._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.deploy.k8s.Config._
+
+private[spark] object KubernetesVolumeUtils {
+
+  /**
+   * Given hostPath volume specs, add volume to pod and volume mount to 
container.
+   *
+   * @param pod original specification of the pod
+   * @param container original specification of the container
+   * @param sparkConf Spark configuration
+   * @param prefix the prefix for volume configuration
+   * @return a tuple of (pod with the volume(s) added, container with 
mount(s) added)
+   */
+  def addVolumes(
+  pod: Pod,
+  container: Container,
+  sparkConf: SparkConf,
+  prefix : String): (Pod, Container) = {
+val hostPathVolumeSpecs = parseHostPathVolumesWithPrefix(sparkConf, 
prefix)
+addHostPathVolumes(pod, container, hostPathVolumeSpecs)
+  }
+
+  /**
+   * Extract Spark volume configuration properties with a given name 
prefix.
+   *
+   * @param sparkConf Spark configuration
+   * @param prefix the given property name prefix
+   * @param volumeTypeKey the given property name prefix
+   * @return a Map storing with volume name as key and spec as value
+   */
+  def parseVolumesWithPrefix(
+  sparkConf: SparkConf,
+  prefix: String,
+  volumeTypeKey: String): Map[String, KubernetesVolumeSpec] = {
+val volumes = HashMap[String, KubernetesVolumeSpec]()
+val properties = 
sparkConf.getAllWithPrefix(s"$prefix$volumeTypeKey.").toList
+// Extract volume names
+properties.foreach {
+  k =>
+val keys = k._1.split("\\.")
+if (keys.nonEmpty && !volumes.contains(keys(0))) {
+  volumes.update(keys(0), KubernetesVolumeSpec.emptySpec())
+}
+}
+// Populate spec
+volumes.foreach {
+  case (name, spec) =>
+properties.foreach {
+  k =>
--- End diff --

Ditto.


---

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

[GitHub] spark pull request #21095: [SPARK-23529][K8s] Support mounting hostPath volu...

2018-04-18 Thread madanadit 
GitHub user madanadit opened a pull request:

https://github.com/apache/spark/pull/21095

[SPARK-23529][K8s] Support mounting hostPath volumes

## What changes were proposed in this pull request?

This PR introduces a new config `spark.kubernetes.driver/executor.volumes` 
taking a values of the format documented 
[here](https://docs.google.com/document/d/15-mk7UnOYNTXoF6EKaVlelWYc9DTrTXrYoodwDuAwY4/edit?usp=sharing)

The use case is to enable short-circuit writes to distributed storage on 
k8s. The Alluxio File System uses domain sockets to enable short-circuit writes 
from the client to worker memory when co-located on the same host machine. A 
directory, lets say /tmp/domain on the host, is mounted on the Alluxio worker 
container as well as the Alluxio client ( = Spark executor) container. The 
worker creates a domain socket /tmp/domain/d and if the client container mounts 
the same directory, it can write directly to the Alluxio worker w/o passing 
through network stack. The end result is faster data access when data is local.

## How was this patch tested?

Manual testing on a k8s v1.8 cluster. Unit tests added to 
`Driver/ExecutorPodFactorySuite`.

This PR replaces https://github.com/apache/spark/pull/21032.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/madanadit/spark k8s-vol

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/21095.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #21095


commit 51a877a7d86b6c0f18d381ba9300de18ccef9189
Author: madanadit 
Date:   2018-04-05T01:02:25Z

Support mounting hostPath volumes for executors

commit 4ada724ab2f6e79553fa05475b216884f9ba127a
Author: madanadit 
Date:   2018-04-05T20:57:45Z

Read mode for mounted volumes

commit 17258a3967b6d8b2170c3c7d7186449c3155fef1
Author: madanadit 
Date:   2018-04-05T22:05:07Z

Refactor

commit 6fff716cbe42e184059660cf2009f594048a6420
Author: madanadit 
Date:   2018-04-06T18:18:09Z

Fix style

commit f961b33aa34ad7908ac6d77ded251e18bf6e835a
Author: madanadit 
Date:   2018-04-06T19:57:45Z

Add unit tests

commit af4d9baae8d637215d2be645b377fa5dbd714e94
Author: madanadit 
Date:   2018-04-06T20:00:18Z

Update comment

commit 13e10493fa6f3b40c3eb406d0dc2f88d09ce20b8
Author: madanadit 
Date:   2018-04-06T23:56:31Z

Fix unit tests

commit 7a25d76e386c0d8157eba0ccc0517bea3b7f7f0e
Author: madanadit 
Date:   2018-04-16T22:41:26Z

Fix typo

commit a5a277a065dec156e976478de7767062bcf1da13
Author: madanadit 
Date:   2018-04-17T21:51:58Z

Change configuration format

commit 772128e13895d6f313efa3cbc38947db51c3e493
Author: madanadit 
Date:   2018-04-17T23:34:37Z

Fix build

commit a393b92c93d60bdcf96025ae24d6c1fbecf17f9b
Author: madanadit 
Date:   2018-04-17T23:44:08Z

Fix test

commit b9b3dcb22ded628f0abe1caa0beb2b15da6ccc49
Author: madanadit 
Date:   2018-04-18T00:39:14Z

Fetch properties correctly

commit 81a7811b06a0195e84ffeddc135875da6c500a7e
Author: madanadit 
Date:   2018-04-18T05:21:11Z

Fix test cases

commit 95d1b0d8c681cd46f47c8ab1692172d0b3b0aba8
Author: madanadit 
Date:   2018-04-18T06:42:56Z

Abstract tests

commit facde97b365a7acb02c41e5ef076a9ea0f1edff9
Author: madanadit 
Date:   2018-04-18T06:59:43Z

Add readOnly option

commit ccdc7990ca8995ff86f46647f8a2949848f06380
Author: madanadit 
Date:   2018-04-18T07:06:17Z

Fix test

commit 7c1be8aff51a462bf96012fafbbfec765424de53
Author: madanadit 
Date:   2018-04-18T07:24:55Z

Driver hostPath volumes with tests




---

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