From 4974490c7f2b167ccbddd2d348b9e80afea17ac4 Mon Sep 17 00:00:00 2001 From: Victor Herrero Otal Date: Thu, 7 Sep 2023 13:30:35 +0200 Subject: [PATCH 1/4] add vpc and elastic ips to ecs node response As per documentation [1], the InnerIpAddress field is only relevant when the node network type is set to classic. If the node is within a VPC, the field used is PrivateIpAddress within VpcAttributes. In addition, elastic ips in field EipAddress are also public ips. They differ from other public ips in that they are static and stay the same once allocated. Fields EipAddress and VpcAttributes are already returned in the response as part of the "extra" field of the node object. This is however not consistent with the other drivers as these fields are also public and private ip addresses, respectively. [1] https://www.alibabacloud.com/help/en/ecs/developer-reference/api-describeinstances --- libcloud/compute/drivers/ecs.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/libcloud/compute/drivers/ecs.py b/libcloud/compute/drivers/ecs.py index 250968aacb..f62d7417c8 100644 --- a/libcloud/compute/drivers/ecs.py +++ b/libcloud/compute/drivers/ecs.py @@ -1258,18 +1258,25 @@ def _to_node(self, instance): state = self.NODE_STATE_MAPPING.get(instance_status, NodeState.UNKNOWN) def _get_ips(ip_address_els): - return [each.text for each in ip_address_els] - - public_ip_els = findall( - element=instance, - xpath="PublicIpAddress/IpAddress", - namespace=self.namespace, - ) - public_ips = _get_ips(public_ip_els) - private_ip_els = findall( - element=instance, xpath="InnerIpAddress/IpAddress", namespace=self.namespace - ) - private_ips = _get_ips(private_ip_els) + return [each.text for each in ip_address_els if each.text] + + public_ips = [] + for xpath in ("PublicIpAddress/IpAddress", "EipAddress/IpAddress"): + public_ip_els = findall( + element=instance, + xpath=xpath, + namespace=self.namespace, + ) + public_ips.extend(_get_ips(public_ip_els)) + + private_ips = [] + for xpath in ("InnerIpAddress/IpAddress", "VpcAttributes/PrivateIpAddress/IpAddress"): + private_ip_els = findall( + element=instance, + xpath=xpath, + namespace=self.namespace + ) + private_ips.extend(_get_ips(private_ip_els)) # Extra properties extra = self._get_extra_dict(instance, RESOURCE_EXTRA_ATTRIBUTES_MAP["node"]) From a5e23773ed55956049594ecfc24e8be07908220e Mon Sep 17 00:00:00 2001 From: vicwicker Date: Sun, 17 Sep 2023 11:56:46 +0200 Subject: [PATCH 2/4] extend unit test --- .../test/compute/fixtures/ecs/describe_instances.xml | 6 ++++-- libcloud/test/compute/test_ecs.py | 10 ++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/libcloud/test/compute/fixtures/ecs/describe_instances.xml b/libcloud/test/compute/fixtures/ecs/describe_instances.xml index f1c7713601..05c2422950 100644 --- a/libcloud/test/compute/fixtures/ecs/describe_instances.xml +++ b/libcloud/test/compute/fixtures/ecs/describe_instances.xml @@ -14,7 +14,7 @@ i-28n7dkvov - + 42.112.1.2 @@ -27,7 +27,9 @@ 1 - + + 172.17.1.1 + diff --git a/libcloud/test/compute/test_ecs.py b/libcloud/test/compute/test_ecs.py index f1ca077962..31bf8cdd30 100644 --- a/libcloud/test/compute/test_ecs.py +++ b/libcloud/test/compute/test_ecs.py @@ -79,10 +79,12 @@ def test_list_nodes(self): self.assertEqual("iZ28n7dkvovZ", node.name) self.assertEqual("i-28n7dkvov", node.id) self.assertEqual(NodeState.PENDING, node.state) - self.assertEqual(1, len(node.public_ips)) + self.assertEqual(2, len(node.public_ips)) self.assertEqual("114.215.124.73", node.public_ips[0]) - self.assertEqual(1, len(node.private_ips)) + self.assertEqual("42.112.1.2", node.public_ips[1]) + self.assertEqual(2, len(node.private_ips)) self.assertEqual("10.163.197.74", node.private_ips[0]) + self.assertEqual("172.17.1.1", node.private_ips[1]) expected_extra = { "image_id": "ubuntu1404_64_20G_aliaegis_20150325.vhd", "description": "", @@ -103,13 +105,13 @@ def test_list_nodes(self): vpc = { "vpc_id": "", "vswitch_id": "", - "private_ip_address": None, + "private_ip_address": "172.17.1.1", "nat_ip_address": "", } self._validate_extras(vpc, node.extra["vpc_attributes"]) eip_address = { "allocation_id": "", - "ip_address": "", + "ip_address": "42.112.1.2", "internet_charge_type": "", "bandwidth": None, } From bdecbe3edd9fed8dc10e9f5c9475f4eba959be7b Mon Sep 17 00:00:00 2001 From: vicwicker Date: Sun, 17 Sep 2023 12:09:59 +0200 Subject: [PATCH 3/4] improve test by having two cases --- .../fixtures/ecs/describe_instances.xml | 51 ++++++++++- libcloud/test/compute/test_ecs.py | 89 ++++++++++--------- 2 files changed, 93 insertions(+), 47 deletions(-) diff --git a/libcloud/test/compute/fixtures/ecs/describe_instances.xml b/libcloud/test/compute/fixtures/ecs/describe_instances.xml index 05c2422950..5bef7a46f8 100644 --- a/libcloud/test/compute/fixtures/ecs/describe_instances.xml +++ b/libcloud/test/compute/fixtures/ecs/describe_instances.xml @@ -14,7 +14,7 @@ i-28n7dkvov - 42.112.1.2 + @@ -27,9 +27,7 @@ 1 - - 172.17.1.1 - + @@ -54,5 +52,50 @@ PostPaid 2999-09-08T16:00Z + + ubuntu1404_64_20G_aliaegis_20150325.vhd + + ecs.t1 + + i-28n7dkvov + + 114.215.124.73 + + + + -1 + cn-qingdao-b + PayByTraffic + ca0122d9-374d-4fce-9fc0-71f7c3eaf1c3 + false + 1024 + 1 + + + + 10.163.197.74 + + + + + 1 + true + + sg-28ou0f3xa + + iZ28n7dkvovZ + + classic + + iZ28n7dkvovZ + ecs.t1.small + 2015-12-27T07:35Z + Starting + + cn-qingdao + + PostPaid + 2999-09-08T16:00Z + diff --git a/libcloud/test/compute/test_ecs.py b/libcloud/test/compute/test_ecs.py index 31bf8cdd30..6de84ec783 100644 --- a/libcloud/test/compute/test_ecs.py +++ b/libcloud/test/compute/test_ecs.py @@ -72,51 +72,54 @@ def setUp(self): self.fake_security_group_id = "fake_security_group_id" def test_list_nodes(self): + # the test describes two nodes: + # the first on a classic network and with public ip attached + # the second on a vpc with an elastic ip attached + vpc_ips = [None, "10.163.197.74"] + eips = ["", "114.215.124.73"] nodes = self.driver.list_nodes() self.assertIsNotNone(nodes) - self.assertEqual(1, len(nodes)) - node = nodes[0] - self.assertEqual("iZ28n7dkvovZ", node.name) - self.assertEqual("i-28n7dkvov", node.id) - self.assertEqual(NodeState.PENDING, node.state) - self.assertEqual(2, len(node.public_ips)) - self.assertEqual("114.215.124.73", node.public_ips[0]) - self.assertEqual("42.112.1.2", node.public_ips[1]) - self.assertEqual(2, len(node.private_ips)) - self.assertEqual("10.163.197.74", node.private_ips[0]) - self.assertEqual("172.17.1.1", node.private_ips[1]) - expected_extra = { - "image_id": "ubuntu1404_64_20G_aliaegis_20150325.vhd", - "description": "", - "instance_type_family": "ecs.t1", - "zone_id": "cn-qingdao-b", - "internet_charge_type": "PayByTraffic", - "serial_number": "ca0122d9-374d-4fce-9fc0-71f7c3eaf1c3", - "io_optimized": "false", - "device_available": "true", - "instance_network_type": "classic", - "hostname": "iZ28n7dkvovZ", - "instance_type": "ecs.t1.small", - "creation_time": "2015-12-27T07:35Z", - "instance_charge_type": "PostPaid", - "expired_time": "2999-09-08T16:00Z", - } - self._validate_extras(expected_extra, node.extra) - vpc = { - "vpc_id": "", - "vswitch_id": "", - "private_ip_address": "172.17.1.1", - "nat_ip_address": "", - } - self._validate_extras(vpc, node.extra["vpc_attributes"]) - eip_address = { - "allocation_id": "", - "ip_address": "42.112.1.2", - "internet_charge_type": "", - "bandwidth": None, - } - self._validate_extras(eip_address, node.extra["eip_address"]) - self.assertIsNone(node.extra["operation_locks"]["lock_reason"]) + self.assertEqual(2, len(nodes)) + for node, vpc_ip, eip in zip(nodes, vpc_ips, eips): + self.assertEqual("iZ28n7dkvovZ", node.name) + self.assertEqual("i-28n7dkvov", node.id) + self.assertEqual(NodeState.PENDING, node.state) + self.assertEqual(1, len(node.public_ips)) + self.assertEqual("114.215.124.73", node.public_ips[0]) + self.assertEqual(1, len(node.private_ips)) + self.assertEqual("10.163.197.74", node.private_ips[0]) + expected_extra = { + "image_id": "ubuntu1404_64_20G_aliaegis_20150325.vhd", + "description": "", + "instance_type_family": "ecs.t1", + "zone_id": "cn-qingdao-b", + "internet_charge_type": "PayByTraffic", + "serial_number": "ca0122d9-374d-4fce-9fc0-71f7c3eaf1c3", + "io_optimized": "false", + "device_available": "true", + "instance_network_type": "classic", + "hostname": "iZ28n7dkvovZ", + "instance_type": "ecs.t1.small", + "creation_time": "2015-12-27T07:35Z", + "instance_charge_type": "PostPaid", + "expired_time": "2999-09-08T16:00Z", + } + self._validate_extras(expected_extra, node.extra) + vpc = { + "vpc_id": "", + "vswitch_id": "", + "private_ip_address": vpc_ip, + "nat_ip_address": "", + } + self._validate_extras(vpc, node.extra["vpc_attributes"]) + eip_address = { + "allocation_id": "", + "ip_address": eip, + "internet_charge_type": "", + "bandwidth": None, + } + self._validate_extras(eip_address, node.extra["eip_address"]) + self.assertIsNone(node.extra["operation_locks"]["lock_reason"]) def test_list_nodes_with_ex_node_ids(self): ECSMockHttp.type = "list_nodes_ex_node_ids" From 2c1ea1d2a17fdda8653b09d6f8156d4ddd75319d Mon Sep 17 00:00:00 2001 From: vicwicker Date: Sun, 17 Sep 2023 12:14:43 +0200 Subject: [PATCH 4/4] run black for formatting --- libcloud/compute/drivers/ecs.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/libcloud/compute/drivers/ecs.py b/libcloud/compute/drivers/ecs.py index f62d7417c8..a1bf742e74 100644 --- a/libcloud/compute/drivers/ecs.py +++ b/libcloud/compute/drivers/ecs.py @@ -1271,11 +1271,7 @@ def _get_ips(ip_address_els): private_ips = [] for xpath in ("InnerIpAddress/IpAddress", "VpcAttributes/PrivateIpAddress/IpAddress"): - private_ip_els = findall( - element=instance, - xpath=xpath, - namespace=self.namespace - ) + private_ip_els = findall(element=instance, xpath=xpath, namespace=self.namespace) private_ips.extend(_get_ips(private_ip_els)) # Extra properties