Conversation
library/infoblox_host_record.py
Outdated
| try: | ||
| from infoblox_client import objects, connector, exceptions | ||
| except ImportError: | ||
| raise Exception('infoblox-client is not installed. Please see details here: https://github.com/infobloxopen/infoblox-client') |
There was a problem hiding this comment.
I need to fix the other module, but the preferred method in ansible core at least is to assign a value here, and exit with fail_json
library/infoblox_host_record.py
Outdated
| argument_spec = dict( | ||
| host = dict(type='str' ,required=True), | ||
| name = dict(type='str', required=True), | ||
| mac = dict(type='str', default=None), |
There was a problem hiding this comment.
Can you be explicit in arguement_spec whether or not a parameter is required?
library/infoblox_host_record.py
Outdated
|
|
||
| DOCUMENTATION = """ | ||
| module: infoblox_host_record | ||
| description: manage infoblox host records with infoblox-client |
There was a problem hiding this comment.
Can you add a doc string?
library/infoblox_host_record.py
Outdated
| try: | ||
| #If host record exists check if there is a difference between what is specified in ansible | ||
| #update_if_exists=True seems to no actually do anything and just errors with (record already exists) | ||
| #using a delete/create method to "update" |
There was a problem hiding this comment.
Seems unsafe, if I remember correctly the library has ability to update as approriate. We should be able to make a generic helper class within the module to do something like check what is present and diff, and only push that.
There was a problem hiding this comment.
I agree, I was trying to use the infoblox-client's "update_if_exists" in an earlier iteration but it kept throwing "record already exists". I'll give that another try, I was probably doing something wrong.
There was a problem hiding this comment.
I can confirm that "update_if_exists=True" does not seem to work as intended for Host Records. Maybe this is because I'm on an older Infoblox version (WAPI version 1.7.1)?
I've confirmed it is hitting it for an update:
There was a problem hiding this comment.
Can you open an issue there?
There was a problem hiding this comment.
I was going to open an issue but found this infobloxopen/infoblox-client#68 (Not sure how I didn't see it before). I tested just a comment change and that worked fine.
I'm not really sure how to handle that in the module, just ignore any IP changes?
Edit: I think the best option is to fail the module if ip addresses differ and let the user know they can't update in place the ip address of a host record.
Edit 2: after re-reading, it's not the exact same issue (but I suspect based off of my testing that it is the same root cause) so I am going to open an issue there for clarification.
|
Good stuff overall. In the other module I created I had the state's in I would like to keep extrattrs and comments if they are applicable in all modules, as these are helpful in automation to find existing attributes. |
|
@brampling I heard rumblings that infoblox was developing these modules for core, is this being worked on? Would try and keep a way from duplicating efforts if it is. |
…ses not being updateable
Host record fixes
|
I updated with the changes requested, the things I'm still not sure about are the docstring (Ansible's docs refers to "DOCUMENTATION" as a docstring ) and the logic that decides if a change has occurred (Even with things like comment set in Infoblox most attributes returned "None" via the API). As I'm writing this I see I missed the states in "main()", I'll fix that as well. |
states handled in main
|
@bandit145 hi, I think this will all be a moot point shortly. They are building modules and lookups for infoblox in core: https://github.com/ansible/ansible/tree/devel/lib/ansible/modules/net_tools/nios Will be going into 2.5 shortly. Going to try and test and update anything i can before it gets released. |
|
Excellent, thanks for taking the time to look at this anyway! |
|
Sorry for the slow reply, I've been swamped. Yes, there are Infoblox modules that will be going into Ansible core 2.5. There had been some discussion about it but I wasn't sure of the status until I was pleasantly surprised to find out that there was completed code about two weeks ago. It would be great if you could both have a look at those modules in ansible-devel and let us know what you think. You can open issues on that branch for bugs or let us know directly for feature requests. |
|
Seems this repo is now the code the new collection, and this would not be relevant |
…#152) * Small change to the git ignor as we get set up * Added support for member assignment to network * bump for ci to run * fox for pep issues * pylint fixes * pep fixes * fix: coverage updated to 6.4.4 in devel workflow * fix: coverage update to 6.5.0 for devel workflow * Consilidate changes from #151 and 145 * pep fixes * Added example playbook for GMC * bug fixes (#2) * bug fixes * pep and lint * removed py2.7 * git ignore for vscode * Update range (#3) * fixes for issues 1 and 2 * bump * Add ability to update range * sanity fixes * fix for range in alt view * author update * fix member Idempotence
3 participants
I've been working on a Host record module for Infoblox and I think it's ready to have a PR opened.
I've done limited testing and so far it works. A unit-test playbook is under "tests" for this module, if you modify the arguments you can use this to test the module against your Infoblox instances.
Looking forward to your feedback.
Notes:
I need to complete "DOCUMENTATION" and "EXAMPLES" and add test cases for all parameters.