-
Notifications
You must be signed in to change notification settings - Fork 594
[Heron-3723] Add support for Empty Dir, Host Path, and NFS via CLI #3747
[Heron-3723] Add support for Empty Dir, Host Path, and NFS via CLI #3747
Conversation
aae2f0f
to
c74318e
Compare
heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/V1Controller.java
Show resolved
Hide resolved
heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/V1Controller.java
Outdated
Show resolved
Hide resolved
heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/V1Controller.java
Outdated
Show resolved
Hide resolved
heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/V1Controller.java
Outdated
Show resolved
Hide resolved
heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/V1Controller.java
Outdated
Show resolved
Hide resolved
heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/V1Controller.java
Show resolved
Hide resolved
heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/KubernetesContext.java
Show resolved
Hide resolved
heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/KubernetesContext.java
Outdated
Show resolved
Hide resolved
heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/KubernetesContext.java
Outdated
Show resolved
Hide resolved
heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/KubernetesContext.java
Show resolved
Hide resolved
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. @windhamwong / @nicknezis / @nwangtw can you check this PR?
Ping @nwangtw @nicknezis @windhamwong @nlu90 @huijunw . Please review if you can. |
@nicknezis when you have some free time, please review. I think this might have been some functionality you needed. |
The most recent merge does not change any functionality. It only updates logging in the constructor of the |
Refactored <VolumeConfigKeys> and added keys required for hostPath, nfs, and emptyPath support. Cleaned up references in other routines. cleanup
Generic <getVolumeConfigs> to collect all Volume configs from CLI.
Generic test cases and errors for <getVolumeConfigs>.
<getVolumeClaimTemplates> refactored to only check for required PVC parameters. <getVolumeConfigs> collects configs from CLI.
Parameter validation added/moved to <getVolumeClaimTemplates>.
Added passing test cases for <getVolumeClaimTemplates>.
Added keys for: - emptyDir - hostPath - nfs
Added <getVolumeEmptyDir> to collect emptyDir configs.
<getVolumeEmptyDir> tests for errors and valid input.
Moved required path check to <getVolumeConfigs>.
emptyDir valid options check added.
<getVolumeHostPath> collects and validates Host Path CLI options.
<getVolumeHostPath> tests for errors and valid input.
<getVolumeNFS> collects and validates NFS from CLI options.
<getVolumeNFS> tests for errors and valid input.
Added paths for hostPath and NFS.
<getVolumeHostPath> will check for a required Path on the Host.
Bug fixes in error test case loop.
<getVolumeNFS> will check for a required Path on the NFS server.
Generic Volume Mount creation in <createVolumeMountsCLI>.
Generating Volumes and Mounts: - <createVolumeAndMountsEmptyDirCLI> - <createVolumeAndMountsHostPathCLI> - <createVolumeAndMountsNFSCLI>
<createVolumeAndMountsPVCCLI> will append to volumes and mounts. TODO: replace <createPersistentVolumeClaimVolumesAndMounts>
Updated to support disabling of Volume configs from CLI. Flag switched to <heron.kubernetes.volume.from.cli.disabled>.
Creating Volumes and Volume Mounts in from CLI configurations in <createStatefulSet>.
<configurePodWithVolumesAndMountsFromCLI> will merge Volumes and Mounts from CLI configurations.
Updated <createPersistentVolumeClaims> to set a Storage Class name of "" to disable dynamic allocation.
Added option for "readOnly" Volume Mounts in <createVolumeMountsCLI>.
Added instructions for: - Empty Dir - Host Path - NFS
Moved <createVolumeAndMountsPersistentVolumeClaimCLI> up.
Reviewer(s): @joshfischer1108
Reviewer(s): @joshfischer1108
<getVolumeConfigs> needs to check for disabled functionality after trying to collect parameters. It should only error if they are parameters supplied and functionality is disabled.
Removed in document index as Docusaurus already provides an index.
Updated warning messages for Pod Template and Volume configuration in constructor.
f969120
to
5d27280
Compare
Tested both EmptyDir and HostPath. I didn't test NFS, but the logic is all similar so I'm very confident that it will work similarly to the other two new options. Thank you for these awesome additions! |
You are most welcome and thank you for taking the time to test and review these @nicknezis and @joshfischer1108. I shall wait till tomorrow evening to merge. I do not think there is a need for me to rebase this atop #3749, the merges should interleave the changes from the two PRs. I am going to bring in a documentation notice update into this PR with warning information on the |
Feature #3723: Add support for Empty Dir, Host Path, and NFS via CLI
Closes #3723, Resolves #3723. The duplicate PR is closed #3744 due to tracking the wrong branch (
-dev
).This PR adds support for specifying
Empty Dir
,Host Path
, andNFS
via CLI commands.For further details please see the documentation and the relevant section. Please note that the Helm charts and the commands to disable Volumes configuration specification via the CLI has been updated as well.
Commands
Manager StatefulSet YAML
Executor StatefulSet YAML