[
https://issues.apache.org/jira/browse/SPARK-58910?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Yang Jie updated SPARK-58910:
-----------------------------
Affects Version/s: 4.0.0
(was: 5.0.0)
> Executor pod template's serviceAccountName is overwritten when a service
> account is configured
> ----------------------------------------------------------------------------------------------
>
> Key: SPARK-58910
> URL: https://issues.apache.org/jira/browse/SPARK-58910
> Project: Spark
> Issue Type: Bug
> Components: Kubernetes
> Affects Versions: 4.0.0
> Reporter: Yang Jie
> Priority: Major
>
> {{ExecutorKubernetesCredentialsFeatureStep.configurePod}} decides whether the
> executor pod already carries a service account by reading a single field:
> {code:scala}
> pod = if (Option(pod.pod.getSpec.getServiceAccount).isEmpty) {
> buildPodWithServiceAccount(executorServiceAccount
> .orElse(driverServiceAccount), pod).getOrElse(pod.pod)
> } else {
> pod.pod
> }
> {code}
> {{serviceAccount}} is the field Kubernetes marks as deprecated.
> {{serviceAccountName}} is the current one, and on {{PodSpec}} the two are
> independent fields with their own setters.
> {{KubernetesUtils.loadPodFromTemplate}} deserializes a pod template
> client-side, without the defaulting an API server would apply, so a template
> naming only {{serviceAccountName}} leaves {{serviceAccount}} null.
> The guard therefore reads such a template as having no service account at all
> and calls {{buildPodWithServiceAccount}}, which writes both fields:
> {code:scala}
> new PodBuilder(pod.pod)
> .editOrNewSpec()
> .withServiceAccount(account)
> .withServiceAccountName(account)
> {code}
> The template's account is replaced and the executors run under a different
> identity from the one the template asked for. That is the opposite of what
> the step's own comment promises: "if not setup by the pod template, fallback
> to the executor's sa".
> h3. Reproduction
> Executor pod template:
> {code:yaml}
> spec:
> serviceAccountName: team-sa
> {code}
> Submitted with:
> {code}
> --conf
> spark.kubernetes.executor.podTemplateFile=/path/to/executor-template.yaml
> --conf spark.kubernetes.authenticate.driver.serviceAccountName=spark
> {code}
> The executor pods run as {{spark}}, not {{team-sa}}. Using
> {{spark.kubernetes.authenticate.executor.serviceAccountName}} instead gives
> the same result. With neither config set, {{buildPodWithServiceAccount}}
> returns {{None}} and the template value survives, so reaching the bug takes
> both a template that names the account and a configured account.
> Writing {{serviceAccount}} in the template, or both fields, behaves
> correctly, since then the guard sees the account and takes the {{else}}
> branch.
> h3. Why the current tests do not catch it
> The three cases in {{ExecutorKubernetesCredentialsFeatureStepSuite}} all
> start from {{SparkPod.initialPod()}} with an empty {{SparkConf}}, so both
> spec fields are null and the {{isEmpty}} branch always wins. None of them
> pre-sets an account on the pod. A case whose pod names only
> {{serviceAccountName}} fails today.
> h3. Suggested fix
> Read both fields, as SPARK-58872 does on the driver side:
> {code:scala}
> val spec = pod.pod.getSpec
> val existing = Option(spec.getServiceAccountName).filter(_.nonEmpty)
> .orElse(Option(spec.getServiceAccount).filter(_.nonEmpty))
> {code}
> The empty-string filter matches {{SetDefaults_PodSpec}} in
> {{pkg/apis/core/v1/defaults.go}}, which treats an empty
> {{serviceAccountName}} as unset and copies the deprecated alias up.
> h3. History
> The guard arrived with SPARK-27872 ({{7912ab85a6f}}, 2019). SPARK-30122
> ({{f9f06eee985}}) later added the fallback to the driver's account. The pod
> template override table in {{docs/running-on-kubernetes.md}} states that
> Spark overrides {{serviceAccount}} and {{serviceAccountName}} on driver pods
> only and that executor pods remain unaffected, which holds except in this
> case.
> Found while reviewing SPARK-58872, which fixes the same single-field read on
> the driver side. There the consequence is only a spurious warning; here the
> executors change identity.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
