-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-22839] [K8s] Refactor to unify driver and executor pod builder APIs #20910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
f1b8c08
80e1562
c3460ae
6d1711b
4036d72
d46d671
2936aa5
d2751b6
430fbb2
fd3e8e6
f0ea6d9
67e9ca1
4c944c4
27b8634
9c67016
f3540f8
33f9d56
9b6cc05
a5f08bb
fbde25d
dff0089
02bbcbc
7d65875
041a240
f446868
df75a9c
518fb2a
7b339c3
dbe35fa
4b92989
7807c9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,17 +14,18 @@ | |
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.spark.deploy.k8s.submit.steps | ||
| package org.apache.spark.deploy.k8s | ||
|
|
||
| import org.apache.spark.deploy.k8s.submit.KubernetesDriverSpec | ||
| import io.fabric8.kubernetes.api.model.HasMetadata | ||
|
|
||
| /** | ||
| * Represents a step in configuring the Spark driver pod. | ||
| */ | ||
| private[spark] trait DriverConfigurationStep { | ||
| private[k8s] case class KubernetesSpec( | ||
| pod: SparkPod, | ||
| additionalDriverKubernetesResources: Seq[HasMetadata], | ||
| podJavaSystemProperties: Map[String, String]) | ||
|
|
||
| /** | ||
| * Apply some transformation to the previous state of the driver to add a new feature to it. | ||
| */ | ||
| def configureDriver(driverSpec: KubernetesDriverSpec): KubernetesDriverSpec | ||
| private[k8s] object KubernetesSpec { | ||
|
||
| def initialSpec(initialProps: Map[String, String]): KubernetesSpec = KubernetesSpec( | ||
| SparkPod.initialPod(), | ||
| Seq.empty, | ||
| initialProps) | ||
| } | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,12 +27,10 @@ import scala.util.control.NonFatal | |
|
|
||
| import org.apache.spark.SparkConf | ||
| import org.apache.spark.deploy.SparkApplication | ||
| import org.apache.spark.deploy.k8s.{KubernetesConf, KubernetesDriverSpecificConf, SparkKubernetesClientFactory} | ||
| import org.apache.spark.deploy.k8s.Config._ | ||
| import org.apache.spark.deploy.k8s.Constants._ | ||
| import org.apache.spark.deploy.k8s.SparkKubernetesClientFactory | ||
| import org.apache.spark.deploy.k8s.submit.steps.DriverConfigurationStep | ||
| import org.apache.spark.internal.Logging | ||
| import org.apache.spark.internal.config.ConfigBuilder | ||
| import org.apache.spark.util.Utils | ||
|
|
||
| /** | ||
|
|
@@ -80,40 +78,31 @@ private[spark] object ClientArguments { | |
| * watcher that monitors and logs the application status. Waits for the application to terminate if | ||
| * spark.kubernetes.submission.waitAppCompletion is true. | ||
| * | ||
| * @param submissionSteps steps that collectively configure the driver | ||
| * @param sparkConf the submission client Spark configuration | ||
| * @param builder Responsible for building the base driver pod based on a composition of | ||
| * implemented features. | ||
| * @param kubernetesConf application configuration | ||
| * @param kubernetesClient the client to talk to the Kubernetes API server | ||
| * @param waitForAppCompletion a flag indicating whether the client should wait for the application | ||
| * to complete | ||
| * @param appName the application name | ||
| * @param watcher a watcher that monitors and logs the application status | ||
| */ | ||
| private[spark] class Client( | ||
| submissionSteps: Seq[DriverConfigurationStep], | ||
| sparkConf: SparkConf, | ||
| kubernetesClient: KubernetesClient, | ||
| waitForAppCompletion: Boolean, | ||
| appName: String, | ||
| watcher: LoggingPodStatusWatcher, | ||
| kubernetesResourceNamePrefix: String) extends Logging { | ||
| builder: KubernetesDriverBuilder, | ||
| kubernetesConf: KubernetesConf[KubernetesDriverSpecificConf], | ||
| kubernetesClient: KubernetesClient, | ||
| waitForAppCompletion: Boolean, | ||
| appName: String, | ||
| watcher: LoggingPodStatusWatcher, | ||
| kubernetesResourceNamePrefix: String) extends Logging { | ||
|
|
||
| /** | ||
| * Run command that initializes a DriverSpec that will be updated after each | ||
| * DriverConfigurationStep in the sequence that is passed in. The final KubernetesDriverSpec | ||
| * will be used to build the Driver Container, Driver Pod, and Kubernetes Resources | ||
| */ | ||
| def run(): Unit = { | ||
| var currentDriverSpec = KubernetesDriverSpec.initialSpec(sparkConf) | ||
| // submissionSteps contain steps necessary to take, to resolve varying | ||
| // client arguments that are passed in, created by orchestrator | ||
| for (nextStep <- submissionSteps) { | ||
| currentDriverSpec = nextStep.configureDriver(currentDriverSpec) | ||
| } | ||
| val resolvedDriverSpec = builder.buildFromFeatures(kubernetesConf) | ||
| val configMapName = s"$kubernetesResourceNamePrefix-driver-conf-map" | ||
| val configMap = buildConfigMap(configMapName, currentDriverSpec.driverSparkConf) | ||
| val configMap = buildConfigMap(configMapName, resolvedDriverSpec.podJavaSystemProperties) | ||
| // The include of the ENV_VAR for "SPARK_CONF_DIR" is to allow for the | ||
| // Spark command builder to pickup on the Java Options present in the ConfigMap | ||
| val resolvedDriverContainer = new ContainerBuilder(currentDriverSpec.driverContainer) | ||
| val resolvedDriverContainer = new ContainerBuilder(resolvedDriverSpec.pod.container) | ||
| .addNewEnv() | ||
| .withName(ENV_SPARK_CONF_DIR) | ||
| .withValue(SPARK_CONF_DIR_INTERNAL) | ||
|
|
@@ -123,7 +112,7 @@ private[spark] class Client( | |
| .withMountPath(SPARK_CONF_DIR_INTERNAL) | ||
| .endVolumeMount() | ||
| .build() | ||
| val resolvedDriverPod = new PodBuilder(currentDriverSpec.driverPod) | ||
| val resolvedDriverPod = new PodBuilder(resolvedDriverSpec.pod.pod) | ||
| .editSpec() | ||
| .addToContainers(resolvedDriverContainer) | ||
| .addNewVolume() | ||
|
|
@@ -141,12 +130,10 @@ private[spark] class Client( | |
| .watch(watcher)) { _ => | ||
| val createdDriverPod = kubernetesClient.pods().create(resolvedDriverPod) | ||
| try { | ||
| if (currentDriverSpec.otherKubernetesResources.nonEmpty) { | ||
| val otherKubernetesResources = | ||
| currentDriverSpec.otherKubernetesResources ++ Seq(configMap) | ||
| addDriverOwnerReference(createdDriverPod, otherKubernetesResources) | ||
| kubernetesClient.resourceList(otherKubernetesResources: _*).createOrReplace() | ||
| } | ||
| val otherKubernetesResources = | ||
| resolvedDriverSpec.additionalDriverKubernetesResources ++ Seq(configMap) | ||
| addDriverOwnerReference(createdDriverPod, otherKubernetesResources) | ||
| kubernetesClient.resourceList(otherKubernetesResources: _*).createOrReplace() | ||
| } catch { | ||
| case NonFatal(e) => | ||
| kubernetesClient.pods().delete(createdDriverPod) | ||
|
|
@@ -180,20 +167,17 @@ private[spark] class Client( | |
| } | ||
|
|
||
| // Build a Config Map that will house spark conf properties in a single file for spark-submit | ||
| private def buildConfigMap(configMapName: String, conf: SparkConf): ConfigMap = { | ||
| private def buildConfigMap(configMapName: String, conf: Map[String, String]): ConfigMap = { | ||
| val properties = new Properties() | ||
| conf.getAll.foreach { case (k, v) => | ||
| conf.foreach { case (k, v) => | ||
| properties.setProperty(k, v) | ||
| } | ||
| val propertiesWriter = new StringWriter() | ||
| properties.store(propertiesWriter, | ||
| s"Java properties built from Kubernetes config map with name: $configMapName") | ||
|
|
||
| val namespace = conf.get(KUBERNETES_NAMESPACE) | ||
| new ConfigMapBuilder() | ||
| .withNewMetadata() | ||
| .withName(configMapName) | ||
| .withNamespace(namespace) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why removed this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not necessary to set namespaces on these objects because the kubernetes client itself is namespaced. |
||
| .endMetadata() | ||
| .addToData(SPARK_CONF_FILE_NAME, propertiesWriter.toString) | ||
| .build() | ||
|
|
@@ -211,34 +195,34 @@ private[spark] class KubernetesClientApplication extends SparkApplication { | |
| } | ||
|
|
||
| private def run(clientArguments: ClientArguments, sparkConf: SparkConf): Unit = { | ||
| val namespace = sparkConf.get(KUBERNETES_NAMESPACE) | ||
| val appName = sparkConf.getOption("spark.app.name").getOrElse("spark") | ||
| // For constructing the app ID, we can't use the Spark application name, as the app ID is going | ||
| // to be added as a label to group resources belonging to the same application. Label values are | ||
| // considerably restrictive, e.g. must be no longer than 63 characters in length. So we generate | ||
| // a unique app ID (captured by spark.app.id) in the format below. | ||
| val kubernetesAppId = s"spark-${UUID.randomUUID().toString.replaceAll("-", "")}" | ||
| val launchTime = System.currentTimeMillis() | ||
| val waitForAppCompletion = sparkConf.get(WAIT_FOR_APP_COMPLETION) | ||
| val appName = sparkConf.getOption("spark.app.name").getOrElse("spark") | ||
| val kubernetesResourceNamePrefix = { | ||
| s"$appName-$launchTime".toLowerCase.replaceAll("\\.", "-") | ||
| } | ||
| val kubernetesConf = KubernetesConf.createDriverConf( | ||
| sparkConf, | ||
| appName, | ||
| kubernetesResourceNamePrefix, | ||
| kubernetesAppId, | ||
| clientArguments.mainAppResource, | ||
| clientArguments.mainClass, | ||
| clientArguments.driverArgs) | ||
| val orchestrator = new KubernetesDriverBuilder | ||
|
||
| val namespace = kubernetesConf.namespace() | ||
| // The master URL has been checked for validity already in SparkSubmit. | ||
| // We just need to get rid of the "k8s://" prefix here. | ||
| val master = sparkConf.get("spark.master").substring("k8s://".length) | ||
| val loggingInterval = if (waitForAppCompletion) Some(sparkConf.get(REPORT_INTERVAL)) else None | ||
|
|
||
| val watcher = new LoggingPodStatusWatcherImpl(kubernetesAppId, loggingInterval) | ||
|
|
||
| val orchestrator = new DriverConfigOrchestrator( | ||
| kubernetesAppId, | ||
| kubernetesResourceNamePrefix, | ||
| clientArguments.mainAppResource, | ||
| appName, | ||
| clientArguments.mainClass, | ||
| clientArguments.driverArgs, | ||
| sparkConf) | ||
|
|
||
| Utils.tryWithResource(SparkKubernetesClientFactory.createKubernetesClient( | ||
| master, | ||
| Some(namespace), | ||
|
|
@@ -247,8 +231,8 @@ private[spark] class KubernetesClientApplication extends SparkApplication { | |
| None, | ||
| None)) { kubernetesClient => | ||
| val client = new Client( | ||
| orchestrator.getAllConfigurationSteps, | ||
| sparkConf, | ||
| orchestrator, | ||
| kubernetesConf, | ||
| kubernetesClient, | ||
| waitForAppCompletion, | ||
| appName, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| /* | ||
| * 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.submit | ||
|
|
||
| import org.apache.spark.deploy.k8s.{KubernetesConf, KubernetesDriverSpecificConf, KubernetesRoleSpecificConf, KubernetesSpec} | ||
| import org.apache.spark.deploy.k8s.features.{BasicDriverFeatureStep, DriverKubernetesCredentialsFeatureStep, DriverServiceFeatureStep, MountSecretsFeatureStep} | ||
|
|
||
| private[spark] class KubernetesDriverBuilder( | ||
| provideBasicStep: (KubernetesConf[KubernetesDriverSpecificConf]) => BasicDriverFeatureStep = | ||
| new BasicDriverFeatureStep(_), | ||
| provideCredentialsStep: (KubernetesConf[KubernetesDriverSpecificConf]) | ||
| => DriverKubernetesCredentialsFeatureStep = | ||
| new DriverKubernetesCredentialsFeatureStep(_), | ||
| provideServiceStep: (KubernetesConf[KubernetesDriverSpecificConf]) => DriverServiceFeatureStep = | ||
| new DriverServiceFeatureStep(_), | ||
| provideSecretsStep: (KubernetesConf[_ <: KubernetesRoleSpecificConf] | ||
| => MountSecretsFeatureStep) = | ||
| new MountSecretsFeatureStep(_)) { | ||
|
|
||
| def buildFromFeatures( | ||
| kubernetesConf: KubernetesConf[KubernetesDriverSpecificConf]): KubernetesSpec = { | ||
| val baseFeatures = Seq( | ||
| provideBasicStep(kubernetesConf), | ||
| provideCredentialsStep(kubernetesConf), | ||
| provideServiceStep(kubernetesConf)) | ||
| val allFeatures = if (kubernetesConf.roleSecretNamesToMountPaths.nonEmpty) { | ||
| baseFeatures ++ Seq(provideSecretsStep(kubernetesConf)) | ||
| } else baseFeatures | ||
| var spec = KubernetesSpec.initialSpec(kubernetesConf.sparkConf.getAll.toMap) | ||
|
||
| for (feature <- allFeatures) { | ||
| val configuredPod = feature.configurePod(spec.pod) | ||
| val addedSystemProperties = feature.getAdditionalPodSystemProperties() | ||
| val addedResources = feature.getAdditionalKubernetesResources() | ||
| spec = KubernetesSpec( | ||
| configuredPod, | ||
| spec.additionalDriverKubernetesResources ++ addedResources, | ||
| spec.podJavaSystemProperties ++ addedSystemProperties) | ||
| } | ||
| spec | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we shorten the name to just
systemProperties? One of the most frequent types of comments I got while working on the upstreaming was to use short names.