Skip to content

Commit

Permalink
[SPARK-44906][K8S] Make Kubernetes[Driver|Executor]Conf.annotations
Browse files Browse the repository at this point in the history
… substitute annotations instead of feature steps

### What changes were proposed in this pull request?

Move `Utils. SubstituteAppNExecIds` logic  into `KubernetesConf.annotations` as the default logic,

### Why are the changes needed?

Easy for users to reuse, rather than to rewrite it again at the same logic.

When user write custom feature step and using annotations, before this pr, they should call `Utils. SubstituteAppNExecIds` once.

### Does this PR introduce _any_ user-facing change?

Yes, but no sense for user to use annotations.

### How was this patch tested?

Add unit test

### Was this patch authored or co-authored using generative AI tooling?

No

Closes apache#42600 from zwangsheng/SPARK-44906.

Lead-authored-by: zwangsheng <2213335496@qq.com>
Co-authored-by: Kent Yao <yao@apache.org>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
  • Loading branch information
2 people authored and dongjoon-hyun committed Aug 23, 2023
1 parent 2d0a0a0 commit 2a3aec1
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ private[spark] class KubernetesDriverConf(

override def annotations: Map[String, String] = {
KubernetesUtils.parsePrefixedKeyValuePairs(sparkConf, KUBERNETES_DRIVER_ANNOTATION_PREFIX)
.map { case (k, v) => (k, Utils.substituteAppNExecIds(v, appId, "")) }
}

def serviceLabels: Map[String, String] = {
Expand Down Expand Up @@ -188,6 +189,7 @@ private[spark] class KubernetesExecutorConf(

override def annotations: Map[String, String] = {
KubernetesUtils.parsePrefixedKeyValuePairs(sparkConf, KUBERNETES_EXECUTOR_ANNOTATION_PREFIX)
.map { case(k, v) => (k, Utils.substituteAppNExecIds(v, appId, executorId)) }
}

override def secretNamesToMountPaths: Map[String, String] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ private[spark] class BasicDriverFeatureStep(conf: KubernetesDriverConf)
.editOrNewMetadata()
.withName(driverPodName)
.addToLabels(conf.labels.asJava)
.addToAnnotations(conf.annotations.map { case (k, v) =>
(k, Utils.substituteAppNExecIds(v, conf.appId, "")) }.asJava)
.addToAnnotations(conf.annotations.asJava)
.endMetadata()
.editOrNewSpec()
.withRestartPolicy("Never")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,14 +255,12 @@ private[spark] class BasicExecutorFeatureStep(
case "statefulset" => "Always"
case _ => "Never"
}
val annotations = kubernetesConf.annotations.map { case (k, v) =>
(k, Utils.substituteAppNExecIds(v, kubernetesConf.appId, kubernetesConf.executorId))
}

val executorPodBuilder = new PodBuilder(pod.pod)
.editOrNewMetadata()
.withName(name)
.addToLabels(kubernetesConf.labels.asJava)
.addToAnnotations(annotations.asJava)
.addToAnnotations(kubernetesConf.annotations.asJava)
.addToOwnerReferences(ownerReference.toSeq: _*)
.endMetadata()
.editOrNewSpec()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import org.apache.spark.deploy.k8s.Config._
import org.apache.spark.deploy.k8s.Constants._
import org.apache.spark.deploy.k8s.submit._
import org.apache.spark.resource.ResourceProfile.DEFAULT_RESOURCE_PROFILE_ID
import org.apache.spark.util.Utils

class KubernetesConfSuite extends SparkFunSuite {

Expand All @@ -42,7 +43,9 @@ class KubernetesConfSuite extends SparkFunSuite {
"customLabel2Key" -> "customLabel2Value")
private val CUSTOM_ANNOTATIONS = Map(
"customAnnotation1Key" -> "customAnnotation1Value",
"customAnnotation2Key" -> "customAnnotation2Value")
"customAnnotation2Key" -> "customAnnotation2Value",
"customAnnotation3Key" -> "{{APP_ID}}",
"customAnnotation4Key" -> "{{EXECUTOR_ID}}")
private val SECRET_NAMES_TO_MOUNT_PATHS = Map(
"secret1" -> "/mnt/secrets/secret1",
"secret2" -> "/mnt/secrets/secret2")
Expand Down Expand Up @@ -93,7 +96,9 @@ class KubernetesConfSuite extends SparkFunSuite {
SPARK_APP_NAME_LABEL -> KubernetesConf.getAppNameLabel(conf.appName),
SPARK_ROLE_LABEL -> SPARK_POD_DRIVER_ROLE) ++
CUSTOM_LABELS)
assert(conf.annotations === CUSTOM_ANNOTATIONS)
assert(conf.annotations === CUSTOM_ANNOTATIONS.map {
case (k, v) => (k, Utils.substituteAppNExecIds(v, conf.appId, ""))
})
assert(conf.secretNamesToMountPaths === SECRET_NAMES_TO_MOUNT_PATHS)
assert(conf.secretEnvNamesToKeyRefs === SECRET_ENV_VARS)
assert(conf.environment === CUSTOM_ENVS)
Expand Down Expand Up @@ -161,7 +166,9 @@ class KubernetesConfSuite extends SparkFunSuite {
SPARK_APP_NAME_LABEL -> KubernetesConf.getAppNameLabel(conf.appName),
SPARK_ROLE_LABEL -> SPARK_POD_EXECUTOR_ROLE,
SPARK_RESOURCE_PROFILE_ID_LABEL -> DEFAULT_RESOURCE_PROFILE_ID.toString) ++ CUSTOM_LABELS)
assert(conf.annotations === CUSTOM_ANNOTATIONS)
assert(conf.annotations === CUSTOM_ANNOTATIONS.map {
case (k, v) => (k, Utils.substituteAppNExecIds(v, conf.appId, EXECUTOR_ID))
})
assert(conf.secretNamesToMountPaths === SECRET_NAMES_TO_MOUNT_PATHS)
assert(conf.secretEnvNamesToKeyRefs === SECRET_ENV_VARS)
}
Expand Down

0 comments on commit 2a3aec1

Please sign in to comment.