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

[FLOC-3521] Support mapping public IP addresses in API client #2230

Closed
wants to merge 1 commit into from

Conversation

jongiddy
Copy link
Contributor

The public_address field of Nodes returned form the flocker.apiclient.FlockerClient actually contains the internal cluster IP address, which can be a private IP address.

Public IP addresses are very useful for clients using the API client module, but a node has no way of knowing which of its IP addresses is truly public, and in some cases, may not even know about a public IP address that routes to it (e.g. NAT addresses).

However, the code using apiclient may know the mapping, especially if it was somehow involved in starting the cluster. This is the case when a cluster is started using run-acceptance-tests where the mapping from internal cluster hostnames to public IP addresses is exported as an environment variable.

This PR allows the apiclient user, when creating a FlockerClient, to pass a mapping of internal hostname IP_ addresses to public IP addresses. Expand the Node class to include both the internal address, now as an additional field called hostname (it's usual name in Flocker), and to set public_address to the public IP address provided by a dictionary mapping hostname to public address.

For Rackspace, the mapping provided by run-acceptance-tests is {} which makes the hostname and public_address fields the same. This is appropriate since Rackspace IP addresses are public.

For AWS, run-acceptance-tests returns a JSON structure {"10.0.0.1": "172.12.12.12"} which can be parsed into a Python dictionary for passing to FlockerClient.

This will be used by the benchmarking code to SSH to individual cluster nodes to measure benchmark metrics.

@itamarst
Copy link
Contributor

Why does this have to be done in the client at all? Why can't the caller do this? (The long term solution is completely ditching IPs from this API, I think).

@jongiddy
Copy link
Contributor Author

Because apiclient promised to provide the public IP address, and then failed to deliver. Also, the client tends to get passed around a fair bit, and the IP address mapping would need to get passed around with it. In the benchmarking code, that forces lots of changes to the function signatures. That's it - no killer reason.

@wallrj
Copy link
Contributor

wallrj commented Nov 26, 2015

@jongiddy #2059 is an alternative option which would also be really useful for CoreOS deployments where the agents run in containers that have different IP addresses. (https://clusterhq.atlassian.net/browse/FLOC-2137)

That's the way I was going to tackle this problem when I embarked on the benchmarking epic.
What do you think?

If you still think this is the best way forward put this back up for review.

@jongiddy
Copy link
Contributor Author

@wallrj Yes, adding public IP address to agent.yml and publishing it, either as a replacement for the current IP, or as additional data, seems the best solution.

I am planning some additional changes to the benchmarking code to make Itamar's suggestion more ergonomic, so I will close this PR. Thanks for your time.

@jongiddy jongiddy closed this Nov 26, 2015
@jongiddy jongiddy deleted the public-addresses-FLOC-3521 branch November 26, 2015 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants