[SPARK-27702][K8S] Allow using some alternatives for service accounts #24601

Udbhav30 · 2019-05-14T10:48:21Z

What changes were proposed in this pull request?

To allow alternatives to serviceaccounts

Why are the changes needed?

Although we provide some authentication configuration, such as spark.kubernetes.authenticate.driver.mounted.oauthTokenFile, spark.kubernetes.authenticate.driver.mounted.caCertFile, etc.
But there is a bug as we forced the service account so when we use one of them, driver still use the KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH file, and the error look like bellow:

the KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH serviceAccount not exists

Does this PR introduce any user-facing change?

Yes user can now use spark.kubernetes.authenticate.driver.mounted.caCertFile
or token file by spark.kubernetes.authenticate.driver.mounted.oauthTokenFile

How was this patch tested?

Manually passed the certificates using spark.kubernetes.authenticate.driver.mounted.caCertFile
or token file by spark.kubernetes.authenticate.driver.mounted.oauthTokenFile if there is no default service account available.

dongjoon-hyun · 2019-05-20T05:42:42Z

...es/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/KubernetesClusterManager.scala

@Udbhav30 . Could you add a test case this your PR?

dongjoon-hyun · 2019-05-28T18:56:32Z

Gentle ping, @Udbhav30 .

Udbhav30 · 2019-05-29T11:18:27Z

@dongjoon-hyun sorry for the late reply , yes i will check how can i add a test-case for it and update you

Udbhav30 · 2019-05-31T08:56:30Z

Hii @dongjoon-hyun i am unable to simulate this from minikube as there will always be a default service account so i am not sure if i could write any test case for this. if you have any suggestions please let me know.
thanks

skonto · 2019-06-07T19:34:17Z

do we need to document anything here?

Udbhav30 · 2019-06-10T04:47:19Z

do we need to document anything here?

It is already covered as part of tokens and certificate passed by users (eg: kubernetesAuthConfPrefix.OAUTH_TOKEN_CONF_SUFFIX) and also during SPARK-25887 that it can be auto configured in Fabric client using kube-config file. Please let me know if any suggestions thanks.

dongjoon-hyun · 2019-06-16T04:18:30Z

ok to test

SparkQA · 2019-06-16T04:30:23Z

Test build #106550 has finished for PR 24601 at commit f1e8207.

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

SparkQA · 2019-06-16T04:34:28Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/11791/

SparkQA · 2019-06-16T04:51:47Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/11791/

Udbhav30 · 2019-07-17T04:53:38Z

Hii @dongjoon-hyun can you please check if we can merge this.

dongjoon-hyun · 2019-08-12T04:24:34Z

Sorry for being later, @Udbhav30 . If then, could you describe the test procedure in the How was this patch tested? section of PR description? For example, how to create new service account and how to use that?

i am unable to simulate this from minikube as there will always be a default service account so i am not sure if i could write any test case for this.

Udbhav30 · 2019-08-12T07:08:37Z

could you describe the test procedure in the How was this patch tested? section of PR description? For example, how to create new service account and how to use that?

Sure! thanks @dongjoon-hyun
Yes i have updated in PR if there is no service account how user can have alternatives to it.

wackxu · 2019-12-27T10:23:24Z

What is the status of this PR? Can we merge this? in some multi-tenant k8s cluster, we do not use service account for authenticate. so we should allow using some alternatives for service accounts, such as kubeconfig, oauthToken.

wackxu · 2019-12-30T01:25:45Z

@dongjoon-hyun @liyinan926

wackxu · 2020-01-20T09:47:05Z

Could you take a look at this? @dongjoon-hyun @liyinan926

SparkQA · 2020-01-20T09:59:59Z

Test build #117112 has finished for PR 24601 at commit 3c9e2d8.

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

wackxu · 2020-01-20T10:04:44Z

Although we provide some authentication configuration, such as spark.kubernetes.authenticate.driver.mounted.oauthTokenFile, spark.kubernetes.authenticate.driver.mounted.caCertFile, etc. When we use one of them, dirver still use the KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH file, and the error look like bellow:

the KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH serviceAccount not exists

so we need to determine whether KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH exists. If it does not exist, other configurations are allowed to use, such as spark.kubernetes.authenticate.driver.mounted.oauthTokenFile.

Udbhav30 · 2020-01-22T09:25:14Z

Hi @dongjoon-hyun @liyinan926 can you please review this PR

dongjoon-hyun · 2020-01-24T07:39:22Z

...es/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/KubernetesClusterManager.scala

+      val serviceAccountToken =
+        Some(new File(Config.KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH)).filter(_.exists)
+      val serviceAccountCaCrt =
+        Some(new File(Config.KUBERNETES_SERVICE_ACCOUNT_CA_CRT_PATH)).filter(_.exists)


The current logic looks like this.

The difference of this logic is returning None when the file doesn't exist.

However, None is not used when the given oauthTokenFileConf points a valid file because it's called as orElse.

val oauthTokenFile = sparkConf.getOption(oauthTokenFileConf) .map(new File(_)) .orElse(defaultServiceAccountToken)

Is the above correct?

The existing code already tries to use the given file first. Where did you check the following?

If it does not exist, other configurations are allowed to use, such as spark.kubernetes.authenticate.driver.mounted.oauthTokenFile.

The existing code already tries to use the given file first. Where did you check the following?

If it does not exist, other configurations are allowed to use, such as spark.kubernetes.authenticate.driver.mounted.oauthTokenFile.

Yes but still as we are passing 'KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH' to create the kubernetesClient , the kubernetes fabric client will use that service account path and will throw error as @wackxu mentioned.

@Udbhav30 @dongjoon-hyun That is what I means. Let's merge this as soon as possible

dongjoon-hyun · 2020-01-24T08:07:03Z

Retest this please.

SparkQA · 2020-01-24T08:25:37Z

Test build #117346 has finished for PR 24601 at commit 3c9e2d8.

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

SparkQA · 2020-01-24T09:04:55Z

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

SparkQA · 2020-01-24T09:37:46Z

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

wackxu · 2020-02-10T02:08:26Z

@dongjoon-hyun Could you take a look at this? Thank you.

AmplabJenkins · 2020-02-12T22:35:40Z

Can one of the admins verify this patch?

wackxu · 2020-03-02T02:04:29Z

@dongjoon-hyun Hi, Could you take a look at this? This is a bugfix about driver authenticate and we can use other authentication without this.

gengliangwang · 2020-03-10T00:37:27Z

cc @jiangxb1987

wackxu · 2020-03-12T12:27:33Z

Kindly ping @dongjoon-hyun @jiangxb1987 @liyinan926 for take a look, thanks.

github-actions · 2020-06-21T00:24:06Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

dongjoon-hyun · 2020-06-21T01:49:57Z

I removed Stale tag. The community was too busy for Apache Spark 3.0.0. Sorry about that. Now it is released finally, I will review this improvement PR again.

dongjoon-hyun

+1, LGTM. Thank you, @Udbhav30 .
Merged to master for Apache Spark 3.1.0.

Udbhav30 · 2020-06-21T16:59:01Z

I removed Stale tag. The community was too busy for Apache Spark 3.0.0. Sorry about that. Now it is released finally, I will review this improvement PR again.

thanks!!

dongjoon-hyun changed the title ~~[SPARK-27702] Allow using some alternatives for service accounts~~ [SPARK-27702][K8S] Allow using some alternatives for service accounts May 14, 2019

dongjoon-hyun reviewed May 20, 2019

View reviewed changes

dongjoon-hyun added the KUBERNETES label Jun 14, 2019

[SPARK-27702] Allow using some alternatives for service accounts

3c9e2d8

Udbhav30 force-pushed the serviceaccount branch from f1e8207 to 3c9e2d8 Compare January 20, 2020 09:41

dongjoon-hyun reviewed Jan 24, 2020

View reviewed changes

github-actions bot added the Stale label Jun 21, 2020

dongjoon-hyun removed the Stale label Jun 21, 2020

dongjoon-hyun approved these changes Jun 21, 2020

View reviewed changes

dongjoon-hyun closed this in d2a656c Jun 21, 2020

[SPARK-27702][K8S] Allow using some alternatives for service accounts #24601

[SPARK-27702][K8S] Allow using some alternatives for service accounts #24601

Uh oh!

Conversation

Udbhav30 commented May 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

dongjoon-hyun May 20, 2019

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented May 28, 2019

Uh oh!

Udbhav30 commented May 29, 2019

Uh oh!

Udbhav30 commented May 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skonto commented Jun 7, 2019

Uh oh!

Udbhav30 commented Jun 10, 2019

Uh oh!

dongjoon-hyun commented Jun 16, 2019

Uh oh!

SparkQA commented Jun 16, 2019

Uh oh!

SparkQA commented Jun 16, 2019

Uh oh!

SparkQA commented Jun 16, 2019

Uh oh!

Udbhav30 commented Jul 17, 2019

Uh oh!

dongjoon-hyun commented Aug 12, 2019

Uh oh!

Udbhav30 commented Aug 12, 2019

Uh oh!

wackxu commented Dec 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wackxu commented Dec 30, 2019

Uh oh!

wackxu commented Jan 20, 2020

Uh oh!

SparkQA commented Jan 20, 2020

Uh oh!

wackxu commented Jan 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Udbhav30 commented Jan 22, 2020

Uh oh!

dongjoon-hyun Jan 24, 2020

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jan 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Udbhav30 Jan 24, 2020

Choose a reason for hiding this comment

Uh oh!

wackxu Feb 3, 2020

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Jan 24, 2020

Uh oh!

SparkQA commented Jan 24, 2020

Uh oh!

SparkQA commented Jan 24, 2020

Uh oh!

SparkQA commented Jan 24, 2020

Uh oh!

wackxu commented Feb 10, 2020

Uh oh!

AmplabJenkins commented Feb 12, 2020

Uh oh!

wackxu commented Mar 2, 2020

Uh oh!

Udbhav30 commented May 14, 2019 •

edited

Loading

Udbhav30 commented May 31, 2019 •

edited

Loading

wackxu commented Dec 27, 2019 •

edited

Loading

wackxu commented Jan 20, 2020 •

edited

Loading

dongjoon-hyun Jan 24, 2020 •

edited

Loading

dongjoon-hyun commented Jun 21, 2020 •

edited

Loading