-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add emptyDir sizeLimit support for local dirs #1993
Add emptyDir sizeLimit support for local dirs #1993
Conversation
fc1c352
to
113a298
Compare
Signed-off-by: Jacob Salway <[email protected]>
113a298
to
73d7469
Compare
@jacobsalway, thanks for the PR! It would be great if you could also update the documentation to reflect this new feature. Additionally, please address the failing pre-commit checks. |
Signed-off-by: Jacob Salway <[email protected]>
a5395c9
to
7c6b770
Compare
@vara-bonthu have bumped the app version (thought it only wanted me to bump the chart version but I understand the check now) and added a sizeLimit field to the volume example on the user guide. As an operator user I usually consult the that page and expected this to work by default given that it's supported in the SparkApplication CRD, so I think the example is good enough but happy to add additional docs if you prefer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jacobsalway, vara-bonthu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* Add emptyDir sizeLimit support Signed-off-by: Jacob Salway <[email protected]> * Bump appVersion and add sizeLimit example Signed-off-by: Jacob Salway <[email protected]> --------- Signed-off-by: Jacob Salway <[email protected]>
* Add emptyDir sizeLimit support Signed-off-by: Jacob Salway <[email protected]> * Bump appVersion and add sizeLimit example Signed-off-by: Jacob Salway <[email protected]> --------- Signed-off-by: Jacob Salway <[email protected]>
Purpose of this PR
sizeLimit
field foremptyDir
volume types that are prefixed withspark-local-dir-...
Change Category
Indicate the type of change by marking the applicable boxes:
Rationale
emptyDir
volume without aspark-local-dir-...
prefix, the webhook will patch the appropriate volume onto the pod spec, but Spark will still create anemptyDir
volume anyway https://github.com/apache/spark/blob/fd3f70c74ba63a0209a4a48a387048ef40380e0f/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/LocalDirsFeatureStep.scala#L38-L40spark-local-dir-...
, which the operator doesn't consider in the webhook and instead supplies at submission time, the sizeLimit is not carried through to the resulting volume in K8semptyDir
volume e.g. if on an NVME node to prevent a node disk pressure conditionChecklist
Before submitting your PR, please review the following:
Additional Notes