[SPARK-3405] add subnet-id and vpc-id options to spark_ec2.py #2872

ghost · 2014-10-21T06:29:49Z

Based on this gist:
https://gist.github.com/amar-analytx/0b62543621e1f246c0a2

We use security group ids instead of security group to get around this issue:
boto/boto#350

AmplabJenkins · 2014-10-21T06:32:10Z

Can one of the admins verify this patch?

ddaws · 2014-10-21T19:53:29Z

Awesome! I am glad to see that this was a priority to someone with the time. 👍

AmplabJenkins · 2014-10-21T23:13:31Z

Can one of the admins verify this patch?

jontg · 2014-11-19T19:45:47Z

+1

Nypias · 2014-11-22T11:48:28Z

I would be interested in merging this patch as well :)

jontg · 2014-11-22T17:24:20Z

We had a couple of issues with this patch, in particular the script depends on instances having a public dns name or ip. I had to modify the script a little to get our cluster started, but of course we didn't invest in making it general. See https://github.com/relateiq/spark/commit/48ab2d1c8cccc00a5d26145b4d19a414c17f62c2 Does boto have a best practice for handling VPC? Feels from the bugs I've read that boto+VPC is something everyone has their own workaround for...

brdw · 2014-12-05T15:02:21Z

I'd love to see this as well. We have a strict vpc policy.

ddaws · 2014-12-05T16:31:44Z

I'll buy anyone willing to take care of this merge coffee via @changetip :)

changetip · 2014-12-05T16:32:16Z

Hi mvj101, dreid93 sent you a Bitcoin tip worth 1 lunch (21,255 bits/$8.00), and I'm here to deliver it ➔ collect your tip at ChangeTip.com.

Learn more about ChangeTip

evanvolgas · 2014-12-05T22:47:24Z

Is VPC support slated for the next maintenance release? Support for VPCs is definitely needed for a lot of us, and it'd be great if we didn't have to patch it ourselves.

Conflicts: ec2/spark_ec2.py

jeffsteinmetz · 2014-12-09T03:23:47Z

The EC2 docs could also be updated to include these new switches.

tylerprete · 2014-12-10T23:47:03Z

We at Radius would really love to see this merged in as well.

tylerprete · 2014-12-11T18:33:28Z

@jontg I'm using this patch with your modifications (private_ip_address), but I'm getting the following errors when the script tries and starts the master:

SHUTDOWN_MSG: Shutting down NameNode at java.net.UnknownHostException: ip-10-0-2-213: ip-10-0-2-213

10.0.2.213 is the master's ip in this case, but it looks like it's picking up ip-10-0-2-213 as the hostname and that isn't resolving. Did you run into anything like this, and if so, how'd you resolve it?

Thanks!

ddaws · 2014-12-12T19:36:51Z

Just a heads up / bump. I am buying everyone a coffee (
in bitcoin ;) ) who contributes to getting this merged in!

jontg · 2014-12-12T21:48:46Z

@tylerprete That might occur if your VPC is not set up to auto-assign DNS records. If you can, that is where I would suggest beginning an investigation.

tylerprete · 2014-12-13T22:55:27Z

@jontg thanks for the help. Turned on dns and now everything is working.

JoshRosen · 2014-12-16T01:51:35Z

ec2/spark_ec2.py

I guess that we need to have separate logic for the VPC / non-VPC security group rule creation, since according to Amazon's Differences Between Security Groups for EC2-Classic and EC2-VPC guide, when using EC2-VPC:

When you add a rule to a security group, you must specify a protocol, and it can be any protocol with a standard protocol number, or all protocols (see Protocol Numbers).

In the non-VPC case, I guess that the

master_group.authorize(src_group=master_group) master_group.authorize(src_group=slave_group)

lines are authorizing all inbound traffic from instances belonging to master group, regardless of the protocol / ports of that traffic.

However, it looks like the VPC case here only authorizes TCP traffic. I don't think that we rely on UDP traffic anywhere, but for consistency's sake it would be good if both the VPC and non-VPC branches here created equivalent rules.

JoshRosen · 2014-12-16T02:04:49Z

Since this is an often-requested feature, we should mention this in the EC2 documentation page: https://github.com/apache/spark/blob/master/docs/ec2-scripts.md

JoshRosen · 2014-12-16T02:07:10Z

ec2/spark_ec2.py

Minor grammar / style nit, but what do you think about "VPC subnet to launch instances in" as the help text?

JoshRosen · 2014-12-16T02:11:38Z

Overall, this looks good to me. I left a couple of nitpicky comments, but besides that + documentation, I'd be happy to merge this.

To address a question asked upthread:

Is VPC support slated for the next maintenance release? Support for VPCs is definitely needed for a lot of us, and it'd be great if we didn't have to patch it ourselves.

This probably won't be merged into Spark 1.2.1/1.1.2 since our policy is to not add new features in maintenance releases. However, newer versions of Spark EC2 are capable of launching clusters with older Spark versions, so you'd be able to use Spark 1.3.0's scripts to launch clusters in your VPC using, say, Spark 1.2.0.

ghost · 2014-12-16T05:50:36Z

Thanks, I believe I've updated the code according to your comments.

Mike

JoshRosen · 2014-12-16T06:37:04Z

Even though we don't have Jenkins tests for the EC2 scripts, I'm just going to have Jenkins run this so that I can avoid an inadvertent build break.

Jenkins, this is ok to test.

(Edit: I guess Jenkins only picks up commands if they're the only content in a comment?)

JoshRosen · 2014-12-16T07:49:18Z

Jenkins, this is ok to test.

SparkQA · 2014-12-16T07:52:27Z

Test build #24486 has started for PR 2872 at commit 4dc6756.

This patch merges cleanly.

SparkQA · 2014-12-16T07:53:20Z

Test build #24486 has finished for PR 2872 at commit 4dc6756.

This patch fails Python style tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-12-16T07:53:21Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24486/
Test FAILed.

ghost · 2014-12-16T07:55:58Z

Fixing style issues now.

SparkQA · 2014-12-16T08:02:31Z

Test build #24488 has started for PR 2872 at commit be9cb43.

This patch merges cleanly.

SparkQA · 2014-12-16T09:24:27Z

Test build #24488 has finished for PR 2872 at commit be9cb43.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-12-16T09:24:31Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24488/
Test PASSed.

JoshRosen · 2014-12-16T20:12:51Z

Thanks for fixing up the style issue. This looks good to me, so I'll merge this into master. Thanks for your patience with the slow review!

ddaws · 2014-12-17T04:43:17Z

@amar-analytx here's a coffee for making the gist that @mvj101 based his initial PR on. @changetip

changetip · 2014-12-17T04:43:41Z

Hi @amar-analytx, @dreid93 sent you a Bitcoin tip worth a coffee (4,526 bits/$1.50), and I'm here to deliver it ➔ collect your tip.

Learn more about ChangeTip

ddaws · 2014-12-17T04:43:50Z

@mvj101 a coffee for you sir @changetip

ddaws · 2014-12-17T04:44:49Z

@jontg may I buy you a coffee for your work helping people with this issue? @changetip

ddaws · 2014-12-17T04:45:43Z

@JoshRosen thanks for merging this in. Here's a coffee @changetip

jontg · 2014-12-17T04:55:14Z

@dreid93 I certainly wouldn't say no... ;-)

ddaws · 2014-12-18T01:27:47Z

@jontg a coffee for you sir @changetip

ddaws · 2014-12-18T01:33:18Z

@changetip does not appear to be picking up my mentions and sending the appropriate tip. :/

JoshRosen · 2014-12-18T01:49:12Z

It looks like this PR may have broken the ability to launch spot clusters:

Traceback (most recent call last):
  File "./spark_ec2.py", line 1147, in <module>
    main()
  File "./spark_ec2.py", line 1139, in main
    real_main()
  File "./spark_ec2.py", line 988, in real_main
    (master_nodes, slave_nodes) = launch_cluster(conn, opts, cluster_name)
  File "./spark_ec2.py", line 437, in launch_cluster
    user_data=user_data_content)
TypeError: request_spot_instances() got an unexpected keyword argument 'security_group_ids'

It looks like the latest version of Boto supports this argument (http://boto.readthedocs.org/en/latest/ref/ec2.html#boto.ec2.connection.EC2Connection.request_spot_instances), but not ours. We're using boto 2.4.1, which was released on May 16, 2012, but this feature was only added August 2012: boto/boto@145a899

I might be able to fix this by just upgrading to a newer version of Boto.

ghost · 2014-12-18T02:32:25Z

Oops, apologies for this breakage. I haven't worked with spot instances. Feel free to revert this pull request and I or someone else can address that corner case as time allows.

Thanks,

Mike

JoshRosen · 2014-12-18T02:51:59Z

I or someone else can address that corner case as time allows.

Let's just update boto. I'll submit a PR for this shortly.

JoshRosen · 2014-12-18T02:58:31Z

Actually, I'm going to revert this for now. Looks like the boto update will take a bit more work than I thought.

JoshRosen · 2014-12-18T03:00:43Z

Ugh, this doesn't revert cleanly due to another patch that I merged. I've got to go, so I'm just going to leave this for now. Someone else can deal with this if it's urgent, otherwise I'll do it tomorrow.

ghost · 2014-12-18T03:12:30Z

Ok, I'll send a PR to revert in a few minutes.

Thanks,
Mike

ghost · 2014-12-18T03:37:01Z

#3728

JoshRosen · 2014-12-18T21:45:53Z

I've opened a PR to upgrade the Boto version, which fixes this issue: #3737

JoshRosen · 2014-12-19T17:10:41Z

Couple of quick questions about this, just to confirm: do I always need to specify the --subnet-id option when using the --vpc-id option? Do I have to run ./spark-ec2 from a node that's inside the VPC?

ghost · 2014-12-19T19:00:17Z

Haven't worked with this in a while and different versions of boto may alter things, but

You might not need to specify a subnet if you have a default VPC configured. In general I think if you don't specify a subnet boto ignores the vpc argument. Since VPC/subnet can be tied to different regions it's a good practice to specify everything as a sanity check.
Should be able to to run ./spark-ec2 from any box connected to the internet - doesn't need to be run within ec2 or within a vpc/subnet.

[SPARK-3405] add subnet-id and vpc-id options to spark_ec2.py

52aaeec

Merge branch 'master' into SPARK-3405

1ebffa1

Conflicts: ec2/spark_ec2.py

JoshRosen reviewed Dec 16, 2014
View reviewed changes

Mike Jennings and others added 2 commits December 15, 2014 19:29

Merge branch 'master' of https://github.com/apache/spark into SPARK-3405

ad90a36

Update for code review.

731d94c

Remove duplicate comment

4dc6756

pep8 spark_ec2.py runs cleanly.

be9cb43

asfgit closed this in d12c071 Dec 16, 2014

[SPARK-3405] add subnet-id and vpc-id options to spark_ec2.py #2872

[SPARK-3405] add subnet-id and vpc-id options to spark_ec2.py #2872

Uh oh!

Conversation

ghost commented Oct 21, 2014

Uh oh!

AmplabJenkins commented Oct 21, 2014

Uh oh!

ddaws commented Oct 21, 2014

Uh oh!

AmplabJenkins commented Oct 21, 2014

Uh oh!

jontg commented Nov 19, 2014

Uh oh!

Nypias commented Nov 22, 2014

Uh oh!

jontg commented Nov 22, 2014

Uh oh!

brdw commented Dec 5, 2014

Uh oh!

ddaws commented Dec 5, 2014

Uh oh!

changetip commented Dec 5, 2014

Uh oh!

evanvolgas commented Dec 5, 2014

Uh oh!

jeffsteinmetz commented Dec 9, 2014

Uh oh!

tylerprete commented Dec 10, 2014

Uh oh!

tylerprete commented Dec 11, 2014

Uh oh!

ddaws commented Dec 12, 2014

Uh oh!

jontg commented Dec 12, 2014

Uh oh!

tylerprete commented Dec 13, 2014

Uh oh!

JoshRosen Dec 16, 2014

Choose a reason for hiding this comment

Uh oh!

JoshRosen commented Dec 16, 2014

Uh oh!

JoshRosen Dec 16, 2014

Choose a reason for hiding this comment

Uh oh!

JoshRosen commented Dec 16, 2014

Uh oh!

ghost commented Dec 16, 2014

Uh oh!

JoshRosen commented Dec 16, 2014

Uh oh!

JoshRosen commented Dec 16, 2014

Uh oh!

SparkQA commented Dec 16, 2014

Uh oh!

SparkQA commented Dec 16, 2014

Uh oh!

AmplabJenkins commented Dec 16, 2014

Uh oh!

ghost commented Dec 16, 2014

Uh oh!

SparkQA commented Dec 16, 2014

Uh oh!

SparkQA commented Dec 16, 2014

Uh oh!

AmplabJenkins commented Dec 16, 2014

Uh oh!

JoshRosen commented Dec 16, 2014

Uh oh!

ddaws commented Dec 17, 2014

Uh oh!

changetip commented Dec 17, 2014

Uh oh!

ddaws commented Dec 17, 2014

Uh oh!

ddaws commented Dec 17, 2014

Uh oh!

ddaws commented Dec 17, 2014

Uh oh!