[SPARK-40065][K8S] Mount ConfigMap on executors with non-default profile as well #37504
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can one of the admins verify this patch? |
|
@dongjoon-hyun @ScrapCodes could you review this? This PR is a tiny fix for the executor ConfigMap mount. |
dongjoon-hyun
approved these changes
Aug 19, 2022
dongjoon-hyun
pushed a commit
that referenced
this pull request
Aug 19, 2022
…ile as well ### What changes were proposed in this pull request? This fixes a bug where ConfigMap is not mounted on executors if they are under a non-default resource profile. ### Why are the changes needed? When `spark.kubernetes.executor.disableConfigMap` is `false`, expected behavior is that the ConfigMap is mounted regardless of executor's resource profile. However, it is not mounted if the resource profile is non-default. ### Does this PR introduce _any_ user-facing change? Executors with non-default resource profile will have the ConfigMap mounted that was missing before if `spark.kubernetes.executor.disableConfigMap` is `false` or default. If certain users need to keep that behavior for some reason, they would need to explicitly set `spark.kubernetes.executor.disableConfigMap` to `true`. ### How was this patch tested? A new test case is added just below the existing ConfigMap test case. Closes #37504 from nsuke/SPARK-40065. Authored-by: Aki Sukegawa <nsuke@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 41ca629) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
dongjoon-hyun
pushed a commit
that referenced
this pull request
Aug 19, 2022
…ile as well ### What changes were proposed in this pull request? This fixes a bug where ConfigMap is not mounted on executors if they are under a non-default resource profile. ### Why are the changes needed? When `spark.kubernetes.executor.disableConfigMap` is `false`, expected behavior is that the ConfigMap is mounted regardless of executor's resource profile. However, it is not mounted if the resource profile is non-default. ### Does this PR introduce _any_ user-facing change? Executors with non-default resource profile will have the ConfigMap mounted that was missing before if `spark.kubernetes.executor.disableConfigMap` is `false` or default. If certain users need to keep that behavior for some reason, they would need to explicitly set `spark.kubernetes.executor.disableConfigMap` to `true`. ### How was this patch tested? A new test case is added just below the existing ConfigMap test case. Closes #37504 from nsuke/SPARK-40065. Authored-by: Aki Sukegawa <nsuke@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 41ca629) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
|
Merged to master/3.3/3.2. |
|
Welcome to the Apache Spark community, @nsuke . |
|
thank you! |
sunchao
pushed a commit
to sunchao/spark
that referenced
this pull request
Jun 2, 2023
…ile as well This fixes a bug where ConfigMap is not mounted on executors if they are under a non-default resource profile. When `spark.kubernetes.executor.disableConfigMap` is `false`, expected behavior is that the ConfigMap is mounted regardless of executor's resource profile. However, it is not mounted if the resource profile is non-default. Executors with non-default resource profile will have the ConfigMap mounted that was missing before if `spark.kubernetes.executor.disableConfigMap` is `false` or default. If certain users need to keep that behavior for some reason, they would need to explicitly set `spark.kubernetes.executor.disableConfigMap` to `true`. A new test case is added just below the existing ConfigMap test case. Closes apache#37504 from nsuke/SPARK-40065. Authored-by: Aki Sukegawa <nsuke@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 41ca629) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This fixes a bug where ConfigMap is not mounted on executors if they are under a non-default resource profile.
Why are the changes needed?
When
spark.kubernetes.executor.disableConfigMapisfalse, expected behavior is that the ConfigMap is mounted regardless of executor's resource profile. However, it is not mounted if the resource profile is non-default.Does this PR introduce any user-facing change?
Executors with non-default resource profile will have the ConfigMap mounted that was missing before if
spark.kubernetes.executor.disableConfigMapisfalseor default. If certain users need to keep that behavior for some reason, they would need to explicitly setspark.kubernetes.executor.disableConfigMaptotrue.How was this patch tested?
A new test case is added just below the existing ConfigMap test case.