Skip to content

Conversation

@vicwicker
Copy link
Contributor

@vicwicker vicwicker commented Sep 17, 2023

Description

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 private IP is returned by the VpcAttributes/PrivateIpAddress.

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 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

Status

  • done, ready for review

Checklist (tick everything that applies)

  • Code linting (required, can be done after the PR checks)
  • Documentation
  • Tests
  • ICLA (required for bigger changes)

@vicwicker vicwicker changed the title Ecs node ips Add VPC private IP and Elastic IP to ECS node response as private and public IP Sep 17, 2023
@vicwicker vicwicker changed the title Add VPC private IP and Elastic IP to ECS node response as private and public IP Add VPC IP and Elastic IP to ECS node response as private and public IP Sep 17, 2023
@vicwicker vicwicker changed the title Add VPC IP and Elastic IP to ECS node response as private and public IP Add VPC IP and Elastic IP to ECS node as private and public IP Sep 17, 2023
vicwicker and others added 4 commits September 17, 2023 13:50
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
@codecov-commenter
Copy link

Codecov Report

Merging #1954 (d71a92e) into trunk (7f52e3c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##            trunk    #1954   +/-   ##
=======================================
  Coverage   83.19%   83.19%           
=======================================
  Files         353      353           
  Lines       81381    81387    +6     
  Branches     8583     8586    +3     
=======================================
+ Hits        67701    67707    +6     
  Misses      10874    10874           
  Partials     2806     2806           
Files Changed Coverage Δ
libcloud/compute/drivers/ecs.py 84.98% <100.00%> (+0.10%) ⬆️
libcloud/test/compute/test_ecs.py 100.00% <100.00%> (ø)

Copy link
Member

@Kami Kami left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@Kami Kami merged commit 8de93f4 into apache:trunk Sep 29, 2023
asfgit pushed a commit that referenced this pull request Oct 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants