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

Issue: LIBCLOUD-477 Add list/delete/create network (VPC) calls within EC... #203

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
125 changes: 125 additions & 0 deletions libcloud/compute/drivers/ec2.py
Expand Up @@ -1236,6 +1236,131 @@ def ex_describe_keypair(self, name):
'keyFingerprint': fingerprint
}

def ex_list_networks(self):
"""
Return all private virtual private cloud (VPC) networks

:return: list of network dicts
:rtype: ``list``
Copy link
Member

Choose a reason for hiding this comment

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

list of dict

"""
params = {'Action': 'DescribeVpcs'}

result = self.connection.request(self.path,
params=params.copy()).object
Copy link
Member

Choose a reason for hiding this comment

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

Is .copy call necessary here?

request method shouldn't mutate params so the copy call here should be unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly isn't, just came over with a copy paste from a different call :).


# The list which we return
networks = []
for element in findall(element=result,
xpath='vpcSet/item',
namespace=NAMESPACE):

# Get the network id
vpc_id = findtext(element=element,
xpath='vpcId',
namespace=NAMESPACE)

# Get tags
tags = dict((findtext(element=item,
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a regular for loop would make this couple of lines a bit more readable.

xpath='key',
namespace=NAMESPACE),
findtext(element=item,
xpath='value',
namespace=NAMESPACE))
for item in findall(element=element,
xpath='tagSet/item',
namespace=NAMESPACE))

# Set our name is the Name key/value if available
# If we don't get anything back then use the vpc_id
name = tags.get('Name', vpc_id)

networks.append({'vpc_id': vpc_id,
'name': name,
'state': findtext(element=element,
xpath='state',
namespace=NAMESPACE),
'cidr_block': findtext(element=element,
xpath='cidrBlock',
namespace=NAMESPACE),
'dhcp_options_id': findtext(element=element,
xpath='dhcpOptionsId',
namespace=NAMESPACE),
'tags': tags,
'instance_tenancy': findtext(element=element,
xpath=
'instance_tenancy',
namespace=NAMESPACE),
'is_default': findtext(element=element,
xpath='isDefault',
namespace=NAMESPACE)})

return networks

def ex_create_network(self, cidr_block, name=None,
instance_tenancy='default'):
"""
Create a network/VPC

:param cidr_block: The CIDR block assigned to the network
:type cidr_block: ``str``

:param name: An optional name for the network
:type name: ``str``

:param instance_tenancy: The allowed tenancy of instances launched
into the VPC.
Valid values: default/dedicated
:type instance_tenancy: ``str``

:return: Dictionary of network properties
:rtype: ``dict``
"""
params = {'Action': 'CreateVpc',
'CidrBlock': cidr_block,
'InstanceTenancy': instance_tenancy}

result = self.connection.request(self.path, params=params).object

# Get our properties
response = {'vpc_id': findtext(element=result,
Copy link
Member

Choose a reason for hiding this comment

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

This method should now also return EC2Network instance.

Something like network = self._to_network(elem=respose.object should do (you might need to select a correct element first though).

You can then also pass this instance to self.ex_create_tags method.

Copy link
Member

Choose a reason for hiding this comment

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

I would also appreciate if you could synchronize your branch with trunk after you make those changes.

Otherwise I will have a bad time merging this patch since there will be a bunch of merge conflicts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup my fault on that. I'll make sure to synchronize my branch before submitting future PRs.

Copy link
Member

Choose a reason for hiding this comment

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

No problem and thanks for all those PRs :)

xpath='vpc/vpcId',
namespace=NAMESPACE),
'state': findtext(element=result,
xpath='vpc/state',
namespace=NAMESPACE),
'cidr_block': findtext(element=result,
xpath='vpc/cidrBlock',
namespace=NAMESPACE)}

# Attempt to tag our network if the name was provided
if name is not None:
# Build a resource object
Copy link
Member

Choose a reason for hiding this comment

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

If you define a new class which represents a Network, you could get rid of this code.

In cases like this, defining a class is also generally better than using a dict. Dict enforces no structure which means it's fragile and it also makes it harder for the end user to know what to expect.

Then you can also modify ex_list_networks to return a list of instances of this class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No objection here. I'll go ahead and make that change now.

class Resource:
pass

resource = Resource()
resource.id = response['vpc_id']
self.ex_create_tags(resource, {'Name': name})

return response

def ex_destroy_network(self, vpc_id):
"""
Deletes a network/VPC.

:param vpc_id: The ID of the VPC
:type vpc_id: ``str``

:rtype: ``bool``
"""
params = {'Action': 'DeleteVpc', 'VpcId': vpc_id}

result = self.connection.request(self.path, params=params).object
element = findtext(element=result, xpath='return',
namespace=NAMESPACE)

return element == 'true'

def ex_list_security_groups(self):
"""
List existing Security Groups.
Expand Down
10 changes: 10 additions & 0 deletions libcloud/test/compute/fixtures/ec2/create_vpc.xml
@@ -0,0 +1,10 @@
<CreateVpcResponse xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
<requestId>7a662fe5-1f34-4e17-9ee9-69a28a8ac0be</requestId>
<vpc>
<vpcId>vpc-ad3527cf</vpcId>
<state>pending</state>
<cidrBlock>192.168.55.0/24</cidrBlock>
<dhcpOptionsId>dopt-7eded312</dhcpOptionsId>
<instanceTenancy>default</instanceTenancy>
</vpc>
</CreateVpcResponse>
4 changes: 4 additions & 0 deletions libcloud/test/compute/fixtures/ec2/delete_vpc.xml
@@ -0,0 +1,4 @@
<DeleteVpcResponse xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
<requestId>85793fa6-2ece-480c-855f-0f82c3257e50</requestId>
<return>true</return>
</DeleteVpcResponse>
28 changes: 28 additions & 0 deletions libcloud/test/compute/fixtures/ec2/describe_vpcs.xml
@@ -0,0 +1,28 @@
<DescribeVpcsResponse xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
<requestId>be8cfa34-0710-4895-941f-961c5738f8f8</requestId>
<vpcSet>
<item>
<vpcId>vpc-532335e1</vpcId>
<state>available</state>
<cidrBlock>192.168.51.0/24</cidrBlock>
<dhcpOptionsId>dopt-7eded312</dhcpOptionsId>
<tagSet></tagSet>
<instanceTenancy>default</instanceTenancy>
<isDefault>false</isDefault>
</item>
<item>
<vpcId>vpc-62ded30e</vpcId>
<state>available</state>
<cidrBlock>192.168.51.1/24</cidrBlock>
<dhcpOptionsId>dopt-7eded312</dhcpOptionsId>
<tagSet>
<item>
<key>Name</key>
<value>Test VPC</value>
</item>
</tagSet>
<instanceTenancy>default</instanceTenancy>
<isDefault>false</isDefault>
</item>
</vpcSet>
</DescribeVpcsResponse>
43 changes: 43 additions & 0 deletions libcloud/test/compute/test_ec2.py
Expand Up @@ -708,6 +708,37 @@ def test_ex_get_limits(self):
'max-elastic-ips': 5}
self.assertEqual(limits['resource'], expected)

def test_ex_list_networks(self):
vpcs = self.driver.ex_list_networks()

self.assertEqual(len(vpcs), 2)

self.assertEqual('vpc-532335e1', vpcs[0]['vpc_id'])
self.assertEqual('available', vpcs[0]['state'])
self.assertEqual('dopt-7eded312', vpcs[0]['dhcp_options_id'])
self.assertEqual('vpc-532335e1', vpcs[0]['name'])

self.assertEqual('vpc-62ded30e', vpcs[1]['vpc_id'])
self.assertEqual('available', vpcs[1]['state'])
self.assertEqual('dopt-7eded312', vpcs[1]['dhcp_options_id'])
self.assertEqual('Test VPC', vpcs[1]['name'])

def test_ex_create_network(self):
vpc = self.driver.ex_create_network('192.168.55.0/24',
name='Test VPC',
instance_tenancy='default')

self.assertEqual('vpc-ad3527cf', vpc['vpc_id'])
self.assertEqual('pending', vpc['state'])
self.assertEqual('192.168.55.0/24', vpc['cidr_block'])

def test_ex_destroy_network(self):
vpcs = self.driver.ex_list_networks()
vpc = vpcs[0]

resp = self.driver.ex_destroy_network(vpc['vpc_id'])
self.assertTrue(resp)


class EC2USWest1Tests(EC2Tests):
region = 'us-west-1'
Expand Down Expand Up @@ -979,6 +1010,18 @@ def _DescribeAccountAttributes(self, method, url, body, headers):
body = self.fixtures.load('describe_account_attributes.xml')
return (httplib.OK, body, {}, httplib.responses[httplib.OK])

def _DescribeVpcs(self, method, url, body, headers):
body = self.fixtures.load('describe_vpcs.xml')
return (httplib.OK, body, {}, httplib.responses[httplib.OK])

def _CreateVpc(self, method, url, body, headers):
body = self.fixtures.load('create_vpc.xml')
return (httplib.OK, body, {}, httplib.responses[httplib.OK])

def _DeleteVpc(self, method, url, body, headers):
body = self.fixtures.load('delete_vpc.xml')
return (httplib.OK, body, {}, httplib.responses[httplib.OK])


class EucMockHttp(EC2MockHttp):
fixtures = ComputeFileFixtures('ec2')
Expand Down