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

Restructure clc_public_ip #22

Closed

Conversation

bjfelton
Copy link
Contributor

This pull request represents a breaking change to the existing clc_public_ip module.

Now that we've got that out of the way, let me explain what this pull request seeks to accomplish.

  1. Restructures module to follow idiomatic, single-resource design (read: it only works on a single server)
  2. Adds support for mapping public ip to existing private ip
  3. Adds support for specifying port range
  4. Addresses issue clc_publicip module Should Support Mixed Protocol Ports #19 by collapsing previous protocol and ports attributes into a single list of dictionaries (ports) that includes protocol (optional, default 'TCP', valid values ['TCP','UDP']), port (required), and port_to (optional) options
  5. Removes 'ICMP' protocol since a) it is always enabled and b) providing it without a port to the python-sdk induces an error
  6. Adds full test coverage for new code and additional coverage for existing code
  7. Updates all related docs and examples

NOTES:

  • I have only tested this on a VM's primary network -- more testing should be done when the secondary NIC functionality is finished
  • I have confirmed that multiple public ips can be created as long as a private ip is provided on the second (both?) request(s). I did not adjust the docs stating that only a single public ip was possible.

 - Update to tests to accommodate single server instead of server list
 - Update to core module to remove list dependencies
 - Appropriate plumbing to retrieve single server and wait on single request
 - Updates to internal docs/examples to reflect single server
@bjfelton
Copy link
Contributor Author

I'm going to be adding functionality to this in the next day or two to automatically enable ICMP. It turns out that this is not automatically enabled, so I'm planning to enable the option automatically, as per the current Control experience.

 - Mimics current Control behavior
 - The request includes a port because the python sdk currently requires it
 - Ensures users aren't confused if/when viewing the public ip in Control,
   as it will show Ping enabled even if it's missing
@jwcolbert
Copy link
Contributor

Closing, please submit this PR against the develop_2.0 branch.

@jwcolbert jwcolbert closed this May 5, 2016
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.

2 participants