Modify F5 GTM Pool #554
Modify F5 GTM Pool #554
Conversation
| short_description: "Manages F5 BIG-IP GTM pools" | ||
| description: | ||
| - "Manages F5 BIG-IP GTM pools" | ||
| version_added: "1.9" |
|
@perzizzle is this ready for re-review? |
|
@gregdek I believe I adddressed all of bcoca's concerns. It would be good for mhite to review if possible. He wrote the LTM modules which are very similar. |
|
Unfortunately I don't have GTM to smoke test this with. Can you solicit some testers on the mailing list? |
| bigsuds_found = True | ||
|
|
||
| def bigip_api(bigip, user, password): | ||
| api = bigsuds.BIGIP(hostname=bigip, username=user, password=password) |
There was a problem hiding this comment.
hostname=bigip => hostname=server
for consistency
|
So same here, not tested as no GTM gear, but an offline code review. |
|
Hey @srvg -- have the necessary changes been made to satisfy review, or no? |
|
Playbook to validate functionality https://gist.github.com/perzizzle/5fd772bea18abf5da079 depends on an extra_vars.yml similar to https://gist.github.com/perzizzle/190195da598a0311ce6b run with ansible-playbook main_virtual_server.yml --extra-vars=@extra_vars.yml -vv |
|
@caphrim007 Please review this PR to make sure it adheres to the following guidelines: http://docs.ansible.com/developing_modules.html#module-checklist If it passes these guidelines, and if you believe it’s a good PR otherwise, please add a comment with "shipit" in the text, and we will flag it for inclusion. If it fails these guidelines, please add a comment with "needs_revision" in the text, along with the needed changes, and we will flag it for revision. |
|
@perzizzle in the example playbook you provided, it uses a module called bigip_gtm_facts. Is that in a separate PR? Or something local to your dev environment? I dont see it in the devel branch |
|
It's another module I wrote that I haven't put in pull request for yet. It still has specifics to my environment and needs to be refactored. I can post a gist if you'd like. |
|
Ok, np. When I tested it with the stock bigip_facts, it failed to return the pools which, I think, is only reading LTM pools? Anyways, this might be an issue on our end insofar it seems we allow one to configure GTM related modules if you have GTM licensed but not provisioned :-/ which seems kinda awkward. I will check on this though as it means that you don't receive a failure-ish message if you try to create a pool on a box that doesn't have GTM provisioned. Maybe this doesn't happen in 12.0.0. Will check. |
|
This is indeed expected behavior, so a gtm_facts module would certainly be helpful to read those specific pools. |
|
@gregdek quick before the merge conflicts appear again :) |
| server: | ||
| description: | ||
| - BIG-IP host | ||
| required: true |
There was a problem hiding this comment.
@perzizzle this needs to support server_port (see existing modules)
|
@gundalow @perzizzle needs_review |
| options: | ||
| server: | ||
| description: | ||
| - BIG-IP host |
There was a problem hiding this comment.
Can the descriptions please be sentences (Capital letter + full stop)?
A minor thing, but I'm trying to get all of this consistent and cleaner.
|
needs_revision - Few documentation things and some potential improvements Also if you could get this clean to: |
|
Thanks @perzizzle for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review. [This message brought to you by your friendly Ansibull-bot.] |
|
@perzizzle if you're interested, I can take this PR, finish it off, and get it merged. Lemme know |
|
@perzizzle A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer. [This message brought to you by your friendly Ansibull-bot.] |
|
@caphrim007 go for it. |
|
@perzizzle Another friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open. If you have addressed the issues and believe it's ready for review, please comment with the text "ready_for_review". If we don't hear from you within another 14 days, we will close this pull request. [This message brought to you by your friendly Ansibull-bot.] |
|
@perzizzle Hey, How are you getting on with this, we are getting close to the 2.2. cut off |
|
@gundalow I will own this feature. It wont be ready for 2.2 so we can close this PR in favor of the one I will send in the future. |
|
I'll reference this PR in a future PR |
|
@caphrim007 Thanks for taking this on. As discussed about and in Networking meeting 2016-09-07 a fresh PR will be created by @caphrim007 when ready. This will be post 2.2. |
Supports adding/removing/enabling/disabling F5 GTM pools