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 publicip #41

Merged

Conversation

bjfelton
Copy link
Contributor

@bjfelton bjfelton commented May 6, 2016

Resubmitting per request. Copy/pasta from the previous PR:

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
 - 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 jwcolbert merged commit 8ab782d into CenturyLinkCloud:develop_2.0 May 6, 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