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

[LIBCLOUD-766] Unique node.public_ips returned by CloudStack ex_get_nodes and list_nodes #626

Closed
wants to merge 1 commit into
base: trunk
from

Conversation

Projects
None yet
2 participants
@atsaki
Contributor

atsaki commented Nov 7, 2015

This fixes the issue node's public_ips have duplicate values.

https://issues.apache.org/jira/browse/LIBCLOUD-766

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Nov 7, 2015

Member

Thanks.

The change looks good to me, but I will wait on @runseb or someone else with more CloudStack experience :)

Member

Kami commented Nov 7, 2015

Thanks.

The change looks good to me, but I will wait on @runseb or someone else with more CloudStack experience :)

@@ -4652,7 +4652,8 @@ def _to_node(self, data, public_ips=None):
extra['tags'] = self._get_resource_tags(data['tags'])
node = CloudStackNode(id=id, name=name, state=state,
public_ips=public_ips, private_ips=private_ips,
public_ips=list(set(public_ips)),

This comment has been minimized.

@Kami

Kami Nov 7, 2015

Member

Just curious - so the issue only exists for public, but not private ips?

@Kami

Kami Nov 7, 2015

Member

Just curious - so the issue only exists for public, but not private ips?

This comment has been minimized.

@atsaki

atsaki Nov 8, 2015

Contributor

The issue only exists for public ips because private ips are not passed with argument.
The issue occurs when an ip is included in both data['nic'] and argument public_ips.

@atsaki

atsaki Nov 8, 2015

Contributor

The issue only exists for public ips because private ips are not passed with argument.
The issue occurs when an ip is included in both data['nic'] and argument public_ips.

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Nov 8, 2015

Member

Would appreciate if you can sync this branch with latest trunk so I can apply patch directly (I've merged 767 first and now there are conflicts).

Member

Kami commented Nov 8, 2015

Would appreciate if you can sync this branch with latest trunk so I can apply patch directly (I've merged 767 first and now there are conflicts).

@atsaki

This comment has been minimized.

Show comment
Hide comment
@atsaki

atsaki Nov 10, 2015

Contributor

I rebased the branch to the latest trunk.

Contributor

atsaki commented Nov 10, 2015

I rebased the branch to the latest trunk.

@asfgit asfgit closed this in 455a279 Nov 10, 2015

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Nov 10, 2015

Member

Merged, thanks!

Member

Kami commented Nov 10, 2015

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment