Skip to content
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

Enhance Storm-YRAN for exeution on secure YARN Grid #31

Merged
merged 10 commits into from
Aug 9, 2013

Conversation

anfeng
Copy link
Contributor

@anfeng anfeng commented Aug 1, 2013

This pull request is created so that Storm-YARN could be executed on secure YARN grid. In such a Grid, we need to:

  • support Kerberos authentication w/ RM and NM, and HDFS
  • respect JAVA_HOME environment
  • make nimbus/supervisor/worker/UI logs accessible by YARN

Here is the basic steps to apply storm-yarn in secure grid:

  • kinit <user>@
  • storm-yarn launch <master.yaml path> -appname <app name> --stormZip lib/storm-0.9.0-wip21.zip --output appId.txt --stormConfOutput .storm/storm.yaml
  • storm jar <Storm JAR file> <Storm Class> <...>
  • storm kill <Storm Topology Name>
  • storm-yarn shutdown master.yaml -appId <AppID>
  • yarn logs -applicationId <AppID>

@anfeng
Copy link
Contributor Author

anfeng commented Aug 4, 2013

@liningsun tried out the latest code on his CDH 4.3 cluster. It worked nice. Storm master was launched with Nimbus, UI and supervisors. Then Storm topology was deployed successfully in the newly launched cluster.

String storm_zip_path = (String)storm_conf.get("storm.zip.path");
Path zip = new Path(storm_zip_path);
FileSystem fs = FileSystem.get(this.hadoopConf);
localResources.put("storm", Util.newYarnAppResource(fs, zip,
LocalResourceType.ARCHIVE, LocalResourceVisibility.PUBLIC));
LocalResourceType.ARCHIVE, LocalResourceVisibility.PRIVATE));
Copy link
Collaborator

Choose a reason for hiding this comment

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

The reason behind the PUBLIC was to make it so that the distributed cache would share instances of the .zip file between different users and storm clusters on the same box. The issue is that if it is not world readable it cannot be public. We should add in a piece of code to automatically check to see if the zip file and all of the parent directories all the way up are world readable too. If so we mark it as public if not we fall back to PRIVATE. This should probably be part of Util.newYarnAppResource. We may want to do it as a separate issue/pull request unless you can do it fast.

@revans2
Copy link
Collaborator

revans2 commented Aug 5, 2013

I just had the one comment. The code looks fine as is, but I think we want to create an issue for the public storm.zip. I also am a bit nervous about hadoop-0.23, hadoop-2.1 and master diverging, but that may be a separate issue to look into.

@anfeng
Copy link
Contributor Author

anfeng commented Aug 5, 2013

We make 2 changes to address Bobby's point on storm.zip visibility:

  • storm.zip will be deployed either in APPLICATION visibility or PUBLIC visibility. We remember that visibility in storm configuration
  • supervisors will apply the visibility per storm configuration for storm.zip

//download storm.yaml file
String storm_yaml_output = cl.getOptionValue("stormConfOutput");
if (storm_yaml_output!=null && storm_yaml_output.length()>0) {
//try to download stirm.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor typo s/stirm/storm/

@revans2
Copy link
Collaborator

revans2 commented Aug 9, 2013

Sorry I looked more closely at a few things. Most of what I found is minor, but the stopAllSupervisors I think is important to fix.

WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License. See accompanying LICENSE file.
-->
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to restore the formatting of the copyright notice in pom-0.23.xml. I think the rest of the XML has been reviewed with a non-white-space diff.

@anfeng
Copy link
Contributor Author

anfeng commented Aug 9, 2013

All issues have been fixed. I created a new issue for visibility detection, and will work on it later

@revans2
Copy link
Collaborator

revans2 commented Aug 9, 2013

Looks good to me.

revans2 added a commit that referenced this pull request Aug 9, 2013
Enhance Storm-YARN for execution on secure YARN Grid
@revans2 revans2 merged commit 2f14793 into yahoo:hadoop-0.23 Aug 9, 2013
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.

3 participants