rhtconsulting / rhc-ose

OpenShift Automation and Utilities by Red Hat Consulting
42 stars 34 forks source link

Add Neutron floating IP support and other tweaks #196

Closed vvaldez closed 8 years ago

vvaldez commented 8 years ago

What does this PR do?

This PR addresses Issue #195 to release/delete Neutron floating IPs on terminated instances.

How should this be manually tested?

With a running environment using Neutron Floating IPs, run this playbook as normal specifying the environment ID or override parameters as such:

ansible-playbook terminate.yml -e "env_id=casl-vvaldez-XYZ"

Optionally this can be run against a non-Neutron environment like OS1 to verify that is still works and will skip the Floating IP task.

Is there a relevant Issue open for this?

None.

Who would you like to review this?

/cc @oybed @etsauer @sabre1041

oybed commented 8 years ago

@vvaldez I would have preferred to have this broken into some smaller PRs, but that's for next time. :-)

I think in general it's fine, but would probably recommend that we consider creating a python module for some of this instead of using the "shell" module with a long command with all kinds of piping to other commands. Having said so, coming Ansible 2.x (we're targeting 2.1 or later), we can probably utilize some of the existing OpenStack modules(?), so if this holds us over till then, I'm fine with it.

Lastly, for the comment I added on the sudo user; We should discuss the "sudo" use a bit more to figure out how we'd like to solve this long term - IMHO we shouldn't have to specify this for every role/play, but let's talk when we get a chance (possibly a bit of a lower priority).

vvaldez commented 8 years ago

I can definitely split this into two smaller PRs. I thought about doing that but was on a roll with changes for the corner cases I found. I think this is an ok work around to get by for now until we move to 2.x. Otherwise URI/API or a custom module would definitely be cleaner than this.

vvaldez commented 8 years ago

@oybed I split out the extra non-Neutron enhancements into PR #197 and updated this commit and description.

etsauer commented 8 years ago

@vvaldez this looks good. going to merge