Github user JoshRosen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2872#discussion_r21873371
  
    --- Diff: ec2/spark_ec2.py ---
    @@ -303,12 +307,17 @@ def launch_cluster(conn, opts, cluster_name):
                 user_data_content = user_data_file.read()
     
         print "Setting up security groups..."
    -    master_group = get_or_make_group(conn, cluster_name + "-master")
    -    slave_group = get_or_make_group(conn, cluster_name + "-slaves")
    +    master_group = get_or_make_group(conn, cluster_name + "-master", 
opts.vpc_id)
    +    slave_group = get_or_make_group(conn, cluster_name + "-slaves", 
opts.vpc_id)
         authorized_address = opts.authorized_address
         if master_group.rules == []:  # Group was just now created
    -        master_group.authorize(src_group=master_group)
    -        master_group.authorize(src_group=slave_group)
    +        if opts.vpc_id == None:
    +            master_group.authorize(src_group=master_group)
    +            master_group.authorize(src_group=slave_group)
    +        else:
    +            master_group.authorize(ip_protocol='icmp', from_port=-1, 
to_port=-1, src_group=slave_group)
    --- End diff --
    
    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](http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_SecurityGroups.html#VPC_Security_Group_Differences)
 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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to