Skip to content

Commit 2e6f7db

Browse files
committed
Addressed comments
1 parent 9be26a8 commit 2e6f7db

File tree

4 files changed

+28
-33
lines changed

4 files changed

+28
-33
lines changed

resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/MountSecretsBootstrap.scala

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,32 +24,36 @@ import io.fabric8.kubernetes.api.model.{Container, ContainerBuilder, Pod, PodBui
2424
private[spark] class MountSecretsBootstrap(secretNamesToMountPaths: Map[String, String]) {
2525

2626
/**
27-
* Mounts Kubernetes secrets as secret volumes into the given container in the given pod.
27+
* Add new secret volumes for the secrets specified in secretNamesToMountPaths into the given pod.
2828
*
2929
* @param pod the pod into which the secret volumes are being added.
30-
* @param container the container into which the secret volumes are being mounted.
31-
* @param addNewVolumes whether to add new secret volumes for the secrets.
32-
* @return the updated pod and container with the secrets mounted.
30+
* @return the updated pod with the secret volumes added.
3331
*/
34-
def mountSecrets(
35-
pod: Pod,
36-
container: Container,
37-
addNewVolumes: Boolean): (Pod, Container) = {
32+
def addSecretVolumes(pod: Pod): Pod = {
3833
var podBuilder = new PodBuilder(pod)
39-
if (addNewVolumes) {
40-
secretNamesToMountPaths.keys.foreach { name =>
41-
podBuilder = podBuilder
42-
.editOrNewSpec()
34+
secretNamesToMountPaths.keys.foreach { name =>
35+
podBuilder = podBuilder
36+
.editOrNewSpec()
4337
.addNewVolume()
4438
.withName(secretVolumeName(name))
4539
.withNewSecret()
4640
.withSecretName(name)
4741
.endSecret()
4842
.endVolume()
4943
.endSpec()
50-
}
5144
}
5245

46+
podBuilder.build()
47+
}
48+
49+
/**
50+
* Mounts Kubernetes secret volumes of the secrets specified in secretNamesToMountPaths into the
51+
* given container.
52+
*
53+
* @param container the container into which the secret volumes are being mounted.
54+
* @return the updated container with the secrets mounted.
55+
*/
56+
def mountSecrets(container: Container): Container = {
5357
var containerBuilder = new ContainerBuilder(container)
5458
secretNamesToMountPaths.foreach { case (name, path) =>
5559
containerBuilder = containerBuilder
@@ -59,7 +63,7 @@ private[spark] class MountSecretsBootstrap(secretNamesToMountPaths: Map[String,
5963
.endVolumeMount()
6064
}
6165

62-
(podBuilder.build(), containerBuilder.build())
66+
containerBuilder.build()
6367
}
6468

6569
private def secretVolumeName(secretName: String): String = {

resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/steps/DriverMountSecretsStep.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ private[spark] class DriverMountSecretsStep(
2828
bootstrap: MountSecretsBootstrap) extends DriverConfigurationStep {
2929

3030
override def configureDriver(driverSpec: KubernetesDriverSpec): KubernetesDriverSpec = {
31-
val (pod, container) = bootstrap.mountSecrets(
32-
driverSpec.driverPod, driverSpec.driverContainer, addNewVolumes = true)
31+
val pod = bootstrap.addSecretVolumes(driverSpec.driverPod)
32+
val container = bootstrap.mountSecrets(driverSpec.driverContainer)
3333
driverSpec.copy(
3434
driverPod = pod,
3535
driverContainer = container

resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/steps/initcontainer/InitContainerMountSecretsStep.scala

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,9 @@ private[spark] class InitContainerMountSecretsStep(
2828
bootstrap: MountSecretsBootstrap) extends InitContainerConfigurationStep {
2929

3030
override def configureInitContainer(spec: InitContainerSpec) : InitContainerSpec = {
31-
// Skip adding new secret volumes for the secrets as the volumes have already been added when
32-
// mounting the secrets into the main driver container.
33-
val (driverPod, initContainer) = bootstrap.mountSecrets(
34-
spec.driverPod,
35-
spec.initContainer,
36-
addNewVolumes = false)
37-
spec.copy(
38-
driverPod = driverPod,
39-
initContainer = initContainer
40-
)
31+
// Mount the secret volumes given that the volumes have already been added to the driver pod
32+
// when mounting the secrets into the main driver container.
33+
val initContainer = bootstrap.mountSecrets(spec.initContainer)
34+
spec.copy(initContainer = initContainer)
4135
}
4236
}

resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodFactory.scala

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ private[spark] class ExecutorPodFactory(
214214

215215
val (maybeSecretsMountedPod, maybeSecretsMountedContainer) =
216216
mountSecretsBootstrap.map { bootstrap =>
217-
bootstrap.mountSecrets(executorPod, containerWithLimitCores, addNewVolumes = true)
217+
(bootstrap.addSecretVolumes(executorPod), bootstrap.mountSecrets(containerWithLimitCores))
218218
}.getOrElse((executorPod, containerWithLimitCores))
219219

220220
val (bootstrappedPod, bootstrappedContainer) =
@@ -227,12 +227,9 @@ private[spark] class ExecutorPodFactory(
227227

228228
val (pod, mayBeSecretsMountedInitContainer) =
229229
initContainerMountSecretsBootstrap.map { bootstrap =>
230-
// Skip adding new secret volumes for the secrets as the volumes have already been added
231-
// above when mounting the secrets into the main executor container.
232-
bootstrap.mountSecrets(
233-
podWithInitContainer.pod,
234-
podWithInitContainer.initContainer,
235-
addNewVolumes = false)
230+
// Mount the secret volumes given that the volumes have already been added to the
231+
// executor pod when mounting the secrets into the main executor container.
232+
(podWithInitContainer.pod, bootstrap.mountSecrets(podWithInitContainer.initContainer))
236233
}.getOrElse((podWithInitContainer.pod, podWithInitContainer.initContainer))
237234

238235
val bootstrappedPod = KubernetesUtils.appendInitContainer(

0 commit comments

Comments
 (0)