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

Adds subnet_id to launch method. #169

Merged
merged 2 commits into from Oct 26, 2015
Merged

Conversation

MatthewRalston
Copy link
Contributor

This pull request fixes two issues with boto.

  • Fixes an issue with inter-node communication permissions. It seems that boto's authorize method still requires the ip_protocol, source port (0) and destination port (65535) along with the src_group parameter.
  • Boto has an issue with specifying a subnet and a groupName simultaneously. The solution described by this thread seems to be specifying the subnet_id's instead of the subnet name.

Let me know what you want to see here.

  1. When I pass a subnet_id to boto's run_instances method, I get the following error. This seems to be a incompletely addressed idiosyncracy of the boto library and how it prefers for subnets to be specified. See this stackoverflow post and this github issue related to the run_instances call with a subnet_id. It seems that boto prefers subnet ids over subnet names in this method.
[2015-10-26 10:10:34,542: DEBUG/MainProcess] <?xml version="1.0" encoding="UTF-8"?>
<Response><Errors><Error><Code>InvalidParameterCombination</Code><Message>The parameter groupName cannot be used with the parameter subnet</Message></Error></Errors><RequestID>e72077dd-f31d-4c94-b0ac-970f6c41fe56</RequestID></Response>
[2015-10-26 10:10:34,542: ERROR/MainProcess] 400 Bad Request
[2015-10-26 10:10:34,542: ERROR/MainProcess] <?xml version="1.0" encoding="UTF-8"?>
<Response><Errors><Error><Code>InvalidParameterCombination</Code><Message>The parameter groupName cannot be used with the parameter subnet</Message></Error></Errors><RequestID>e72077dd-f31d-4c94-b0ac-970f6c41fe56</RequestID></Response>
[2015-10-26 10:10:34,543: ERROR/MainProcess] Problem launching an instance: The parameter groupName cannot be used with the parameter subnet (code InvalidParameterCombination; status 400)
Traceback (most recent call last):
  File "/home/ralstonm/Projects/bioblend/bioblend/cloudman/launch.py", line 177, in launch
    placement=placement)
  File "/home/ralstonm/.pyenv/versions/cloudlaunch/lib/python2.7/site-packages/boto/ec2/connection.py", line 973, in run_instances
    verb='POST')
  File "/home/ralstonm/.pyenv/versions/cloudlaunch/lib/python2.7/site-packages/boto/connection.py", line 1208, in get_object
    raise self.ResponseError(response.status, response.reason, body)
EC2ResponseError: EC2ResponseError: 400 Bad Request
<?xml version="1.0" encoding="UTF-8"?>
<Response><Errors><Error><Code>InvalidParameterCombination</Code><Message>The parameter groupName cannot be used with the parameter subnet</Message></Error></Errors><RequestID>e72077dd-f31d-4c94-b0ac-970f6c41fe56</RequestID></Response>

… to pass security group ids instead of security group names to bypass a bug in boto.
@MatthewRalston
Copy link
Contributor Author

The travis build seems to have failed for a reason unrelated to my changes. I can build this locally:

~/Projects/bioblend >pip install -e `pwd`
DEPRECATION: --download-cache has been deprecated and will be removed in the future. Pip now automatically uses and configures its cache.
Obtaining file:///home/ralstonm/Projects/bioblend
Requirement already satisfied (use --upgrade to upgrade): requests>=2.4.3 in /home/ralstonm/.pyenv/versions/cloudlaunch/lib/python2.7/site-packages (from bioblend==0.7.0.dev0)
Requirement already satisfied (use --upgrade to upgrade): requests-toolbelt in /home/ralstonm/.pyenv/versions/cloudlaunch/lib/python2.7/site-packages (from bioblend==0.7.0.dev0)
Requirement already satisfied (use --upgrade to upgrade): boto>=2.9.7 in /home/ralstonm/.pyenv/versions/cloudlaunch/lib/python2.7/site-packages (from bioblend==0.7.0.dev0)
Requirement already satisfied (use --upgrade to upgrade): pyyaml in /home/ralstonm/.pyenv/versions/cloudlaunch/lib/python2.7/site-packages (from bioblend==0.7.0.dev0)
Requirement already satisfied (use --upgrade to upgrade): six in /home/ralstonm/.pyenv/versions/cloudlaunch/lib/python2.7/site-packages (from bioblend==0.7.0.dev0)
Installing collected packages: bioblend
  Running setup.py develop for bioblend
Successfully installed bioblend-0.7.0.dev0

Some of the idiosyncracies of the boto call have been described where I am using them in Cloudlaunch

@@ -253,13 +260,14 @@ def create_cm_security_group(self, sg_name='CloudMan'):
cmsg = self.ec2_conn.create_security_group(sg_name, 'A security '
'group for CloudMan')
except EC2ResponseError as e:
err_msg = "Problem creating security group '{0}': {1} (code {2}; " \
err_msg = "Problem creaInvalid value 'null' for protocol. VPC security group rules must specify protocols explicitlyting security group '{0}': {1} (code {2}; " \
Copy link
Member

Choose a reason for hiding this comment

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

creaInvalid... explicitlyting: I think you pasted in the wrong place!

@nsoranzo
Copy link
Member

The TravisCI failed on Galaxy dev branch due to Galaxy-related problems, but on the other Galaxy branches there are 3 flake8 issues introduced by your PR:

./bioblend/cloudman/launch.py:113:58: E231 missing whitespace after ','
./bioblend/cloudman/launch.py:176:46: E265 block comment should start with '# '
./bioblend/cloudman/launch.py:308:50: E231 missing whitespace after ','

@afgane
Copy link
Contributor

afgane commented Oct 26, 2015

Tested this manually and it worked fine so merged in (with a minor whitespace fix).

@nsoranzo
Copy link
Member

I've fixed the TravisCI failures due to the Galaxy eggs-to-wheels migration, future PRs should be fine.

@MatthewRalston
Copy link
Contributor Author

Thank you all

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.

None yet

3 participants