[SPARK-27872][K8s][2.4] Fix executor service account inconsistency

[nssalian]

What changes were proposed in this pull request?

Similar patch to #24748 but applied to the branch-2.4.
Backporting the fix to releases 2.4.x.
Please let me know if I missed some step; I haven't contributed to spark in a long time.

[nssalian]

@erikerlandson , could you help review this, when you get a chance?

[dongjoon-hyun]

ok to test

[SparkQA]

Test build #129079 has finished for PR 29844 at commit 853e1c7.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[erikerlandson]

Is there a test for executor SA that is different than the driver SA?

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33700/

[SparkQA]

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33700/

[SparkQA]

Test build #129085 has finished for PR 29844 at commit 1e6c509.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33706/

[SparkQA]

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33706/

[nssalian]

Thanks @dongjoon-hyun and @erikerlandson for the review. Let me know if you have more comments that I can address.

[SparkQA]

Test build #129091 has finished for PR 29844 at commit 9cb5851.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33712/

[SparkQA]

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33712/

[erikerlandson]

@nssalian this LGTM - it looks like you exceeded 100 character line length in a few places and that needs to be fixed to pass the build checks. Once it passes the linting and testing I can merge

[SparkQA]

Test build #129093 has finished for PR 29844 at commit cfed34c.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #129094 has finished for PR 29844 at commit 46bcdff.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33714/

[SparkQA]

Test build #129096 has finished for PR 29844 at commit 0a47a8e.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33715/

[SparkQA]

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33714/

[SparkQA]

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33715/

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33717/

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33718/

[SparkQA]

Test build #129097 has finished for PR 29844 at commit 81205a5.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33717/

[nssalian]

@erikerlandson, tests passed. Thanks for your review.

[SparkQA]

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33718/

[erikerlandson]

@nssalian merged to branch-2.4, thanks for the patch!

[erikerlandson]

passes tests and linting

[dongjoon-hyun]

@erikerlandson Does this conf exist in Apache Spark 3.0.1?

[dongjoon-hyun]

This merge silently creates a future regression because Apache Spark 3.0 doesn't have this conf.

~s:master $ git grep KUBERNETES_AUTH_EXECUTOR_CONF_PREFIX.serviceAccountName
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala:    ConfigBuilder(s"$KUBERNETES_AUTH_EXECUTOR_CONF_PREFIX.serviceAccountName")

~s:branch-3.0 $ git grep KUBERNETES_AUTH_EXECUTOR_CONF_PREFIX.serviceAccountName

~s:branch-2.4 $ git grep KUBERNETES_AUTH_EXECUTOR_CONF_PREFIX.serviceAccountName
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala:    ConfigBuilder(s"$KUBERNETES_AUTH_EXECUTOR_CONF_PREFIX.serviceAccountName")

[dongjoon-hyun]

Sorry guys, @erikerlandson and @nssalian .

@erikerlandson . Could you revert this commit?

@nssalian . You can make a correct PR if you want SPARK-27872. Please don't piggy-back the other improvements silently. It breaks Apache Spark release policy.

[dongjoon-hyun]

cc @viirya , too.

[nssalian]

@dongjoon-hyun wasn't aware of the PR you mentioned. I can correct this.

[nssalian]

@dongjoon-hyun @erikerlandson I added another PR to fix these changes, easier than a revert. -> #29876

[dongjoon-hyun]

Thanks for swift replying, @nssalian . However, your addition PR isn't better than a clean revert in the perspective of downstreams. We had better have a clean and correct commit after reverting.

[dongjoon-hyun]

Since @erikerlandson seems to be busy, I'll revert the problematic commit first.

[erikerlandson]

@dongjoon-hyun the changes backported correspond to #24748, and SPARK-27872 was tagged as a bug fix (although a credible argument could be made it was a new feature).

[dongjoon-hyun]

Yep. It looks like that. I understand the situation, @erikerlandson . The committers had better be more careful because the contributors make this kind of mistakes sometime. BTW, I'm not against SPARK-27872. I'm against SPARK-30122 based on the PR content.