Skip to content

Conversation

@LucaCanali
Copy link
Contributor

What changes were proposed in this pull request?

Spark-submit usage message should be put in sync with recent changes in particular regarding K8S support. These are the proposed changes to the usage message:

--executor-cores NUM -> can be useed for Spark on YARN and K8S

--principal PRINCIPAL and --keytab KEYTAB -> can be used for Spark on YARN and K8S

--total-executor-cores NUM> can be used for Spark standalone, YARN and K8S

In addition this PR proposes to remove certain implementation details from the --keytab argument description as the implementation details vary between YARN and K8S, for example.

How was this patch tested?

Manually tested

| Spark on YARN and K8S only:
| --principal PRINCIPAL Principal to be used to login to KDC.
| --keytab KEYTAB The full path to the file that contains the keytab for the
| principal specified above.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens to the rest of the original text?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing. The text that I cut is mostly about the implementation of --keytab (i.e. spark.kerberos.keytab) and refers to the YARN implementation. In K8S the implementation is different. Also on YARN this has evolved recently BTW. I think it is safe and shorter just to omit the implementation details in this scope.

@SparkQA
Copy link

SparkQA commented Jan 14, 2019

Test build #4511 has started for PR 23518 at commit 930f242.

@SparkQA
Copy link

SparkQA commented Jan 15, 2019

Test build #4515 has finished for PR 23518 at commit 930f242.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 15, 2019

Test build #4516 has started for PR 23518 at commit 930f242.

@SparkQA
Copy link

SparkQA commented Jan 15, 2019

Test build #4518 has started for PR 23518 at commit 930f242.

Copy link
Contributor

@jerryshao jerryshao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Is this the only place needs to be changed?

@LucaCanali
Copy link
Contributor Author

I would say so, as I am now aware of other places in the code or doc listing the options for spark-submit with the same details provided in spark-submit --help message. As for the point of clarifying the use of "--keytab" option for Kubernetes, I see that SPARK-26595 takes care of it with an update in docs/security.md

@SparkQA
Copy link

SparkQA commented Jan 16, 2019

Test build #4519 has finished for PR 23518 at commit 930f242.

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

@srowen
Copy link
Member

srowen commented Jan 17, 2019

Merged to master

@srowen srowen closed this in 272428d Jan 17, 2019
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
## What changes were proposed in this pull request?

Spark-submit usage message should be put in sync with recent changes in particular regarding K8S support. These are the proposed changes to the usage message:

--executor-cores NUM -> can be useed for Spark on YARN and K8S

--principal PRINCIPAL  and --keytab KEYTAB -> can be used for Spark on YARN and K8S

--total-executor-cores NUM> can be used for Spark standalone, YARN and K8S

In addition this PR proposes to remove certain implementation details from the --keytab argument description as the implementation details vary between YARN and K8S, for example.

## How was this patch tested?

Manually tested

Closes apache#23518 from LucaCanali/updateSparkSubmitArguments.

Authored-by: Luca Canali <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants