User templates for hadoop / spark #202
Conversation
|
Hi @ncherel and thank you for this contribution to Flintrock! Sorry about the delay in reviewing it. I will try to take a detailed look through and give feedback by the end of this week.
I would prefer not to add new commands and instead enhance I'll have to think about this more, but that's my initial reaction. |
There was a problem hiding this comment.
This is a solid first crack at implementing this feature, and several people have asked me about it in the past.
There are 2 critical things that need to be addressed to move this PR forward:
- Resolve all merge conflicts.
- Address the issue with the cluster manifest.
There is one additional thing we can address after those 2 are taken care of:
- Have
flintrock configurecopy the built-in templates to Flintrock's config dir and use those as the default paths in Click.
But we can address this later.
| @@ -9,12 +9,22 @@ services: | |||
| # - must be a tar.gz file | |||
| # download-source: "https://www.example.com/files/spark/{v}/spark-{v}.tar.gz" | |||
| # executor-instances: 1 | |||
| # template-dir: # folder containing spark configuration files: | |||
There was a problem hiding this comment.
To make this consistent with the other examples, I'd put the comment above the template-dir line and instead put an example path as template-dir's value.
| hdfs: | ||
| version: 2.7.3 | ||
| # optional; defaults to download from a dynamically selected Apache mirror | ||
| # - must contain a {v} template corresponding to the version | ||
| # - must be a .tar.gz file | ||
| # download-source: "https://www.example.com/files/hadoop/{v}/hadoop-{v}.tar.gz" | ||
| # template-dir: # path to the folder containing the hadoop configuration files |
There was a problem hiding this comment.
Same comment here as on the Spark template dir.
| @@ -218,6 +218,7 @@ def cli(cli_context, config, provider, debug): | |||
| help="URL to download Hadoop from.", | |||
| default='http://www.apache.org/dyn/closer.lua/hadoop/common/hadoop-{v}/hadoop-{v}.tar.gz?as_json', | |||
| show_default=True) | |||
| @click.option('--hdfs-template-dir') | |||
There was a problem hiding this comment.
The default for this should be under Flintrock's configuration dir.
| @@ -239,6 +240,7 @@ def cli(cli_context, config, provider, debug): | |||
| help="Git repository to clone Spark from.", | |||
| default='https://github.com/apache/spark', | |||
| show_default=True) | |||
| @click.option('--spark-template-dir') | |||
There was a problem hiding this comment.
The default for this should be under Flintrock's configuration dir.
| self.manifest = { | ||
| 'version': version, | ||
| 'download_source': download_source, | ||
| 'template_dir': template_dir} |
There was a problem hiding this comment.
This raises an important issue: The point of the manifest is to be a self-contained representation of how the cluster is configured. Any instance of Flintrock should be able to manage a cluster solely using the information in the manifest and from EC2.
With this change, however, Flintrock needs access to the specified local directory to get the correct templates. So if one instance of Flintrock launches a cluster, another instance of Flintrock on a different machine won't be able to expand or restart it correctly because the manifest only has the paths to the templates on some non-cluster machine and not the templates themselves.
Realistically, I would guess that most people using Flintrock do so from a single machine. But I'm not comfortable breaking the implicit guarantee that any instance of Flintrock at version X on any machine can fully manage clusters launched by Flintrock at version X. And I know there are some teams using Flintrock where this would matter.
To address this issue, we need to put the full contents of all the templates in the manifest (or somehow otherwise on the cluster) and use that in the add-slaves, remove-slaves, and start commands as necessary.
There was a problem hiding this comment.
A different approach we could take here is to note the template directory and some kind of hash of the contents in the manifest. If someone wants to expand an existing Flintrock cluster without having the same template files that were used during launch, Flintrock will display a warning but try to continue anyway.
This kind of forgiving behavior is technically dangerous, but in practice it should work well. And the warning should make it clear to users that they are responsible for any borked operations since they are not using the same files that were used during launch.
We should probably use this kind of approach for the other places where Flintrock references local files during launch, like --ec2-user-data.
|
Hi @ncherel! Are you still interested in moving this PR forward? I think it's solid feature add to Flintrock, and if we resolve the issue with the manifest I'd be happy to merge it in. What do you think? |
|
Closing this PR due to inactivity. I might pick this idea up later because I think it's really good! |
This PR makes the following changes:
I manually tested this PR.
I started to run
pytestbut encountered errors not related to my changes. test_static are okayI will run the full test set once the general direction of this PR is approved.
I was thinking of adding a
flintrock generate-templates /path/to/foldercommand to provide the base templates (those bundled with Flintrock). What do you think?Fixes #200