From 1e43bea1ffc9f8a86166c5730d603be89743d844 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 22 Aug 2013 22:13:17 +0200 Subject: [PATCH] LIBCLOUD-381: Fixed port forwarding handling, added tests for volumes, improved docstrings --- libcloud/compute/drivers/cloudstack.py | 289 ++++++++++++++++----- .../listPortForwardingRules_default.json | 1 + .../fixtures/cloudstack/listVolumes_default.json | 1 + .../ktucloud/listPortForwardingRules_default.json | 1 + libcloud/test/compute/test_cloudstack.py | 55 +++- 5 files changed, 283 insertions(+), 64 deletions(-) create mode 100644 libcloud/test/compute/fixtures/cloudstack/listPortForwardingRules_default.json create mode 100644 libcloud/test/compute/fixtures/cloudstack/listVolumes_default.json create mode 100644 libcloud/test/compute/fixtures/ktucloud/listPortForwardingRules_default.json diff --git a/libcloud/compute/drivers/cloudstack.py b/libcloud/compute/drivers/cloudstack.py index 80a63e1..b9c08db 100644 --- a/libcloud/compute/drivers/cloudstack.py +++ b/libcloud/compute/drivers/cloudstack.py @@ -23,6 +23,7 @@ from libcloud.compute.base import Node, NodeDriver, NodeImage, NodeLocation,\ NodeSize, StorageVolume from libcloud.compute.types import NodeState, LibcloudError + class CloudStackNode(Node): "Subclass of Node so we can expose our extension methods." @@ -56,13 +57,13 @@ class CloudStackNode(Node): class CloudStackAddress(object): "A public IP address." - def __init__(self, node, id, address): - self.node = node + def __init__(self, id, address, driver): self.id = id self.address = address + self.driver = driver def release(self): - self.node.ex_release_public_ip(self) + self.driver.ex_release_public_ip(self) def __str__(self): return self.address @@ -71,7 +72,7 @@ class CloudStackAddress(object): return self.__class__ is other.__class__ and self.id == other.id -class CloudStackForwardingRule(object): +class CloudStackIPForwardingRule(object): "A NAT/firewall forwarding rule." def __init__(self, node, id, address, protocol, start_port, end_port=None): @@ -89,6 +90,25 @@ class CloudStackForwardingRule(object): return self.__class__ is other.__class__ and self.id == other.id +class CloudStackPortForwardingRule(object): + "A Port forwarding rule for Source NAT." + + def __init__(self, node, rule_id, address, protocol, public_port, + private_port): + self.node = node + self.id = rule_id + self.address = address + self.protocol = protocol + self.public_port = public_port + self.private_port = private_port + + def delete(self): + self.node.ex_delete_port_forwarding_rule(self) + + def __eq__(self, other): + return self.__class__ is other.__class__ and self.id == other.id + + class CloudStackDiskOffering(object): """A disk offering within CloudStack.""" @@ -105,16 +125,18 @@ class CloudStackDiskOffering(object): class CloudStackNetwork(object): """Class representing a CloudStack Network""" - def __init__(self, displaytext, name, networkofferingid, id, zoneid): + def __init__(self, displaytext, name, networkofferingid, id, zoneid, + driver): self.displaytext = displaytext self.name = name self.networkofferingid = networkofferingid self.id = id self.zoneid = zoneid + self.driver = driver def __repr__(self): return (('') + 'networkofferingid=%s, zoneid=%s, driver%s>') % (self.id, self.displaytext, self.name, self.networkofferingid, self.zoneid, self.driver.name)) @@ -139,7 +161,8 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): 'Stopped': NodeState.TERMINATED, 'Stopping': NodeState.TERMINATED, 'Destroyed': NodeState.TERMINATED, - 'Expunging': NodeState.TERMINATED + 'Expunging': NodeState.TERMINATED, + 'Error': NodeState.TERMINATED } def __init__(self, key, secret=None, secure=True, host=None, @@ -189,6 +212,9 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): return images def list_locations(self): + """ + @rtype C{list} of L{NodeLocation} + """ locs = self._sync_request('listZones') locations = [] for loc in locs['zone']: @@ -236,26 +262,47 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): extra={'zoneid': vm['zoneid'], } ) - addrs = public_ips_map.get(vm['id'], {}).items() - addrs = [CloudStackAddress(node, v, k) for k, v in addrs] - node.extra['ip_addresses'] = addrs + addresses = public_ips_map.get(vm['id'], {}).items() + addresses = [CloudStackAddress(node, v, k) for k, v in addresses] + node.extra['ip_addresses'] = addresses rules = [] - for addr in addrs: + for addr in addresses: result = self._sync_request('listIpForwardingRules') for r in result.get('ipforwardingrule', []): - rule = CloudStackForwardingRule(node, r['id'], addr, - r['protocol'].upper(), - r['startport'], - r['endport']) - rules.append(rule) + if r['virtualmachineid'] == node.id: + rule = CloudStackIPForwardingRule(node, r['id'], + addr, + r['protocol'] + .upper(), + r['startport'], + r['endport']) + rules.append(rule) node.extra['ip_forwarding_rules'] = rules + rules = [] + + for addr in addrs: + result = self._sync_request('listPortForwardingRules') + for r in result.get('portforwardingrule', []): + if r['virtualmachineid'] == node.id: + rule = CloudStackPortForwardingRule(node, r['id'], + addr, + r['protocol'] + .upper(), + r['publicport'], + r['privateport']) + rules.append(rule) + node.extra['port_forwarding_rules'] = rules + nodes.append(node) return nodes def list_sizes(self, location=None): + """ + @rtype C{list} of L{NodeSize} + """ szs = self._sync_request('listServiceOfferings') sizes = [] for sz in szs['serviceoffering']: @@ -309,7 +356,8 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): driver=self, extra={'zoneid': location.id, 'ip_addresses': [], - 'forwarding_rules': [], + 'ip_forwarding_rules': [], + 'port_forwarding_rules': [] } ) @@ -317,16 +365,20 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): """ @inherits: L{NodeDriver.reboot_node} @type node: L{CloudStackNode} + + @rtype: C{boolean} """ - self._async_request('destroyVirtualMachine', id=node.id) + res = self._async_request('destroyVirtualMachine', id=node.id) return True def reboot_node(self, node): """ @inherits: L{NodeDriver.reboot_node} @type node: L{CloudStackNode} + + @rtype: C{boolean} """ - self._async_request('rebootVirtualMachine', id=node.id) + res = self._async_request('rebootVirtualMachine', id=node.id) return True def ex_start(self, node): @@ -388,7 +440,11 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): return diskOfferings def ex_list_networks(self): - """List the available networks""" + """ + List the available networks + + @rtype C{list} of L{CloudStackNetwork} + """ nets = self._sync_request('listNetworks')['network'] @@ -399,12 +455,16 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): net['name'], net['networkofferingid'], net['id'], - net['zoneid'])) + net['zoneid'], + self)) return networks - def create_volume(self, size, name, location, snapshot=None): - # TODO Add snapshot handling + def create_volume(self, size, name, location=None, snapshot=None): + """ + Creates a data volume + Defaults to the first location + """ for diskOffering in self.ex_list_disk_offerings(): if diskOffering.size == size or diskOffering.customizable: break @@ -416,6 +476,9 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): if diskOffering.customizable: extraParams['size'] = size + if location is None: + location = self.list_locations()[0] + requestResult = self._async_request('createVolume', name=name, diskOfferingId=diskOffering.id, @@ -430,10 +493,19 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): driver=self, extra=dict(name=volumeResponse['name'])) + def destroy_volume(self, volume): + """ + @rtype: C{boolean} + """ + self._sync_request('deleteVolume', id=volume.id) + return True + def attach_volume(self, node, volume, device=None): """ @inherits: L{NodeDriver.attach_volume} @type node: L{CloudStackNode} + + @rtype: C{boolean} """ # TODO Add handling for device name self._async_request('attachVolume', id=volume.id, @@ -441,64 +513,161 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): return True def detach_volume(self, volume): + """ + @rtype: C{boolean} + """ self._async_request('detachVolume', id=volume.id) return True - def destroy_volume(self, volume): - self._sync_request('deleteVolume', id=volume.id) - return True + def list_volumes(self): + """ + List all volumes - def ex_allocate_public_ip(self, node): + @rtype: C{list} of L{StorageVolume} """ - Allocate a public IP and bind it to a node. + list_volumes = [] + volumes = self._sync_request('listVolumes') + for vol in volumes['volume']: + list_volumes.append(StorageVolume(id=vol['id'], + name=vol['name'], + size=vol['size'], + driver=self)) + return list_volumes + + def ex_list_public_ips(self): + """ + Lists all Public IP Addresses. - @param node: Node which should be used - @type node: L{CloudStackNode} + @rtype: C{list} of L{CloudStackAddress} + """ + res = self._sync_request('listPublicIpAddresses') + ips = [] + for ip in res['publicipaddress']: + ips.append(CloudStackAddress(ip['id'], ip['ipaddress'], self)) + return ips + + def ex_allocate_public_ip(self, location=None): + """ + Allocate a public IP. + + @param location: Zone + @type location: L{NodeLocation} @rtype: L{CloudStackAddress} """ + if location is None: + location = self.list_locations()[0] - zoneid = node.extra['zoneid'] - addr = self._async_request('associateIpAddress', zoneid=zoneid) + addr = self._async_request('associateIpAddress', zoneid=location.id) addr = addr['ipaddress'] - result = self._sync_request('enableStaticNat', - virtualmachineid=node.id, - ipaddressid=addr['id']) - if result.get('success', '').lower() != 'true': - return None - - node.public_ips.append(addr['ipaddress']) - addr = CloudStackAddress(node, addr['id'], addr['ipaddress']) - node.extra['ip_addresses'].append(addr) + addr = CloudStackAddress(addr['id'], addr['ipaddress'], self) return addr - def ex_release_public_ip(self, node, address): + def ex_release_public_ip(self, address): """ Release a public IP. - @param node: Node which should be used - @type node: L{CloudStackNode} - @param address: CloudStackAddress which should be used @type address: L{CloudStackAddress} @rtype: C{bool} """ - - node.extra['ip_addresses'].remove(address) - node.public_ips.remove(address.address) - - self._async_request('disableStaticNat', ipaddressid=address.id) self._async_request('disassociateIpAddress', id=address.id) return True + def ex_list_port_forwarding_rules(self): + """ + Lists all Port Forwarding Rules + + @rtype: C{list} of L{CloudStackPortForwardingRule} + """ + rules = [] + result = self._sync_request('listPortForwardingRules') + if result == {}: + pass + else: + nodes = self.list_nodes() + for rule in result['portforwardingrule']: + node = [n for n in nodes + if n.id == rule['virtualmachineid']] + addr = [a for a in self.ex_list_public_ips() + if a.address == rule['ipaddress']] + rules.append(CloudStackPortForwardingRule + (node[0], + rule['id'], + addr[0], + rule['protocol'], + rule['publicport'], + rule['privateport'])) + + return rules + + def ex_create_port_forwarding_rule(self, address, private_port, + public_port, protocol, node): + """ + Creates a Port Forwarding Rule, used for Source NAT + + @param address: IP address of the Source NAT + @type address: L{CloudStackAddress} + + @param private_port: Port of the virtual machine + @type private_port: C{int} + + @param protocol: Protocol of the rule + @type protocol: C{str} + + @param public_port: Public port on the Source NAT address + @type public_port: C{int} + + @param node: The virtual machine + @type node: L{CloudStackNode} + + @rtype: L{CloudStackPortForwardingRule} + """ + args = { + 'ipaddressid': address.id, + 'protocol': protocol, + 'privateport': int(private_port), + 'publicport': int(public_port), + 'virtualmachineid': node.id, + 'openfirewall': True + } + + result = self._async_request('createPortForwardingRule', **args) + rule = CloudStackPortForwardingRule(node, + result['portforwardingrule'] + ['id'], + address, + protocol, + public_port, + private_port) + node.extra['port_forwarding_rules'].append(rule) + return rule + + def ex_delete_port_forwarding_rule(self, node, rule): + """ + Remove a Port forwarding rule. + + @param node: Node used in the rule + @type node: L{CloudStackNode} + + @param rule: Forwarding rule which should be used + @type rule: L{CloudStackPortForwardingRule} + + @rtype: C{bool} + """ + + node.extra['port_forwarding_rules'].remove(rule) + res = self._async_request('deletePortForwardingRule', id=rule.id) + return res['success'] + def ex_add_ip_forwarding_rule(self, node, address, protocol, start_port, end_port=None): """ "Add a NAT/firewall forwarding rule. - @param node: Node which should be used - @type node: L{CloudStackNode} + @param node: Node which should be used + @type node: L{CloudStackNode} @param address: CloudStackAddress which should be used @type address: L{CloudStackAddress} @@ -509,10 +678,10 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): @param start_port: Start port which should be used @type start_port: C{int} - @param end_port: End port which should be used - @type end_port: C{int} + @param end_port: End port which should be used + @type end_port: C{int} - @rtype: L{CloudStackForwardingRule} + @rtype: L{CloudStackForwardingRule} """ protocol = protocol.upper() @@ -529,8 +698,8 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): result = self._async_request('createIpForwardingRule', **args) result = result['ipforwardingrule'] - rule = CloudStackForwardingRule(node, result['id'], address, - protocol, start_port, end_port) + rule = CloudStackIPForwardingRule(node, result['id'], address, + protocol, start_port, end_port) node.extra['ip_forwarding_rules'].append(rule) return rule @@ -542,7 +711,7 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): @type node: L{CloudStackNode} @param rule: Forwarding rule which should be used - @type rule: L{CloudStackForwardingRule} + @type rule: L{CloudStackForwardingRule} @rtype: C{bool} """ @@ -673,8 +842,8 @@ class CloudStackNodeDriver(CloudStackDriverMixIn, NodeDriver): @rtype: C{dict} """ - - res = self._sync_request('registerSSHKeyPair', name=name, publickey=key_material) + res = self._sync_request('registerSSHKeyPair', name=name, + publickey=key_material) return { 'keyName': res['keypair']['name'], 'keyFingerprint': res['keypair']['fingerprint'] diff --git a/libcloud/test/compute/fixtures/cloudstack/listPortForwardingRules_default.json b/libcloud/test/compute/fixtures/cloudstack/listPortForwardingRules_default.json new file mode 100644 index 0000000..460645c --- /dev/null +++ b/libcloud/test/compute/fixtures/cloudstack/listPortForwardingRules_default.json @@ -0,0 +1 @@ +{ "listportforwardingrulesresponse" : {"count": 1, "portforwardingrule": [{"protocol": "tcp", "virtualmachineid": "7d8de712-aa7a-4901-a8b1-fd223f0ca459", "ipaddress": "178.170.71.253", "cidrlist": "", "tags": [], "ipaddressid": "50cd9456-d4db-4a48-8cf5-950dba8d2fdb", "virtualmachinedisplayname": "yoyo", "privateendport": "22", "state": "Active", "publicendport": "22", "privateport": "22", "virtualmachinename": "yoyo", "publicport": "22", "id": "4644652a-7573-4e50-aafb-48a171c9bcb2"}]}} diff --git a/libcloud/test/compute/fixtures/cloudstack/listVolumes_default.json b/libcloud/test/compute/fixtures/cloudstack/listVolumes_default.json new file mode 100644 index 0000000..5fa8fd1 --- /dev/null +++ b/libcloud/test/compute/fixtures/cloudstack/listVolumes_default.json @@ -0,0 +1 @@ +{ "listvolumesresponse" : { "count":1 ,"volume" : [ {"id":"fe1ada16-57a0-40ae-b577-01a153690fb4","name":"ROOT-69942","zoneid":"7dbc4787-ec2f-498d-95f0-848c8c81e5da","zonename":"MTV-Zone1","type":"ROOT","deviceid":0,"virtualmachineid":"3239ade9-fd25-405c-8eda-59f0313a3fb0","vmname":"apb-cent32-bld","vmdisplayname":"apb-cent32-bld","vmstate":"Stopped","size":139264,"created":"2013-04-16T16:25:57-0700","state":"Ready","account":"andrew","domainid":"41a4917b-7952-499d-ba7f-4c57464d3dc8","domain":"ROOT","storagetype":"local","hypervisor":"KVM","storage":"c2422.halxg.cloudera.com","destroyed":false,"serviceofferingid":"7cc4f8c3-7c56-4155-9916-9f42072ea712","serviceofferingname":"Tiny","serviceofferingdisplaytext":"Tiny (1 core, 1GB RAM)","isextractable":false} ] } } diff --git a/libcloud/test/compute/fixtures/ktucloud/listPortForwardingRules_default.json b/libcloud/test/compute/fixtures/ktucloud/listPortForwardingRules_default.json new file mode 100644 index 0000000..460645c --- /dev/null +++ b/libcloud/test/compute/fixtures/ktucloud/listPortForwardingRules_default.json @@ -0,0 +1 @@ +{ "listportforwardingrulesresponse" : {"count": 1, "portforwardingrule": [{"protocol": "tcp", "virtualmachineid": "7d8de712-aa7a-4901-a8b1-fd223f0ca459", "ipaddress": "178.170.71.253", "cidrlist": "", "tags": [], "ipaddressid": "50cd9456-d4db-4a48-8cf5-950dba8d2fdb", "virtualmachinedisplayname": "yoyo", "privateendport": "22", "state": "Active", "publicendport": "22", "privateport": "22", "virtualmachinename": "yoyo", "publicport": "22", "id": "4644652a-7573-4e50-aafb-48a171c9bcb2"}]}} diff --git a/libcloud/test/compute/test_cloudstack.py b/libcloud/test/compute/test_cloudstack.py index 943f7d5..575fe53 100644 --- a/libcloud/test/compute/test_cloudstack.py +++ b/libcloud/test/compute/test_cloudstack.py @@ -61,7 +61,7 @@ class CloudStackNodeDriverTest(unittest.TestCase, TestCaseMixin): try: cls('key', 'secret', True, 'localhost', '/path') except Exception: - self.fail('host and path provided but driver raised an exception') + self.fail('host and path provided but driver raised an exception') def test_create_node_immediate_failure(self): size = self.driver.list_sizes()[0] @@ -189,6 +189,25 @@ class CloudStackNodeDriverTest(unittest.TestCase, TestCaseMixin): self.assertTrue(attachReturnVal) + def test_detach_volume(self): + volumeName = 'gre-test-volume' + location = self.driver.list_locations()[0] + volume = self.driver.create_volume(10, volumeName, location) + res = self.driver.detach_volume(volume) + self.assertTrue(res) + + def test_destroy_volume(self): + volumeName = 'gre-test-volume' + location = self.driver.list_locations()[0] + volume = self.driver.create_volume(10, volumeName, location) + res = self.driver.destroy_volume(volume) + self.assertTrue(res) + + def test_list_volumes(self): + volumes = self.driver.list_volumes() + self.assertEquals(1, len(volumes)) + self.assertEquals('ROOT-69942', volumes[0].name) + def test_list_nodes(self): node = self.driver.list_nodes()[0] self.assertEquals('test', node.name) @@ -197,6 +216,15 @@ class CloudStackNodeDriverTest(unittest.TestCase, TestCaseMixin): location = self.driver.list_locations()[0] self.assertEquals('Sydney', location.name) + def test_list_sizes(self): + sizes = self.driver.list_sizes() + self.assertEquals('Compute Micro PRD', sizes[0].name) + self.assertEquals('105', sizes[0].id) + self.assertEquals(384, sizes[0].ram) + self.assertEquals('Compute Large PRD', sizes[2].name) + self.assertEquals('69', sizes[2].id) + self.assertEquals(6964, sizes[2].ram) + def test_ex_start_node(self): node = self.driver.list_nodes()[0] res = node.ex_start() @@ -207,6 +235,16 @@ class CloudStackNodeDriverTest(unittest.TestCase, TestCaseMixin): res = node.ex_stop() self.assertEquals('Stopped', res) + def test_destroy_node(self): + node = self.driver.list_nodes()[0] + res = node.destroy() + self.assertTrue(res) + + def test_reboot_node(self): + node = self.driver.list_nodes()[0] + res = node.reboot() + self.assertTrue(res) + def test_ex_list_keypairs(self): keypairs = self.driver.ex_list_keypairs() fingerprint = '00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:' + \ @@ -227,7 +265,9 @@ class CloudStackNodeDriverTest(unittest.TestCase, TestCaseMixin): def test_ex_import_keypair(self): fingerprint = 'c4:a1:e5:d4:50:84:a9:4c:6b:22:ee:d6:57:02:b8:15' - path = os.path.join(os.path.dirname(__file__), "fixtures", "cloudstack", "dummy_rsa.pub") + path = os.path.join(os.path.dirname(__file__), "fixtures", + "cloudstack", + "dummy_rsa.pub") res = self.driver.ex_import_keypair('foobar', path) self.assertEqual(res['keyName'], 'foobar') @@ -235,9 +275,12 @@ class CloudStackNodeDriverTest(unittest.TestCase, TestCaseMixin): def test_ex_import_keypair_from_string(self): fingerprint = 'c4:a1:e5:d4:50:84:a9:4c:6b:22:ee:d6:57:02:b8:15' - path = os.path.join(os.path.dirname(__file__), "fixtures", "cloudstack", "dummy_rsa.pub") + path = os.path.join(os.path.dirname(__file__), "fixtures", + "cloudstack", + "dummy_rsa.pub") - res = self.driver.ex_import_keypair_from_string('foobar', open(path).read()) + res = self.driver.ex_import_keypair_from_string('foobar', + open(path).read()) self.assertEqual(res['keyName'], 'foobar') self.assertEqual(res['keyFingerprint'], fingerprint) @@ -261,6 +304,10 @@ class CloudStackNodeDriverTest(unittest.TestCase, TestCaseMixin): '0.0.0.0/0') self.assertTrue(res) + def test_ex_list_public_ips(self): + ips = self.driver.ex_list_public_ips() + self.assertEqual(ips[0].address, '1.1.1.49') + class CloudStackMockHttp(MockHttpTestCase): fixtures = ComputeFileFixtures('cloudstack') -- 1.8.2.3