Skip to content

Conversation

@Shourai
Copy link
Contributor

@Shourai Shourai commented Oct 5, 2023

Add oob_ip documentation

Related Issue

New Behavior

Add the option to use the out of band IP as ansible_host instead of primary_ip.

Contrast to Current Behavior

There is no option to use the out of band ip currently

Discussion: Benefits and Drawbacks

Allow users to use the new out of band ip option in their playbooks.
Also the query filter has_oob_ip: True is available.

Changes to the Documentation

Proposed Release Note Entry

Add the ability to set out of band IP as ansible host.

Double Check

  • I have read the comments and followed the CONTRIBUTING.md.
  • I have explained my PR according to the information in the comments or in a linked issue.
  • My PR targets the devel branch.

Copy link
Contributor

@sc68cal sc68cal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a test added to our integration tests

@Shourai Shourai reopened this Oct 11, 2023
@Shourai
Copy link
Contributor Author

Shourai commented Oct 11, 2023

@sc68cal, I have rebased this pull request on #1085
However I am unsure how to write tests for this change.
Any help would be appreciated!

@sc68cal
Copy link
Contributor

sc68cal commented Nov 9, 2023

@Shourai you would need to add an example JSON response from Netbox for a device, that has the out of band IP attribute set, and then have an assert or equivalent check that ansible_host for the device gets set as the OOB IP instead of the primary_ip

Take a look at the existing tests in

https://github.com/netbox-community/ansible_modules/tree/devel/tests/integration/targets/inventory-v3.6/files

@Shourai
Copy link
Contributor Author

Shourai commented Dec 14, 2023

I have tried setting up a development environment for netbox.
The netbox enviroment works fine and pre-populating netbox also works fine.

However when I try to run ansible-test integration -v --color --coverage --python 3.11 inventory-v3.6 or any other inventory version for that matter it throws errors.

+ ./compare_inventory_json.py ./files/test-inventory-options.json /home/canary/worksflow_netbox/ansible_collections/netbox/netbox/tests/output/.tmp/output_dir/test-inventory-options.json
Traceback (most recent call last):
  File "/home/<user>/worksflow_netbox/ansible_collections/netbox/netbox/tests/output/.tmp/integration/inventory-v3.6-3ie7asx5-ÅÑŚÌβŁÈ/tests/integration/targets/inventory-v3.6/./compare_inventory_json.py", line 152, in <module>
    main()
  File "/home/<user>/worksflow_netbox/ansible_collections/netbox/netbox/tests/output/.tmp/integration/inventory-v3.6-3ie7asx5-ÅÑŚÌβŁÈ/tests/integration/targets/inventory-v3.6/./compare_inventory_json.py", line 144, in main
    print(json.dumps(result, sort_keys=True, indent=4))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/__init__.py", line 238, in dumps
    **kw).encode(obj)
          ^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 202, in encode
    chunks = list(chunks)
             ^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 432, in _iterencode
    yield from _iterencode_dict(o, _current_indent_level)
  File "/usr/lib/python3.11/json/encoder.py", line 406, in _iterencode_dict
    yield from chunks
  File "/usr/lib/python3.11/json/encoder.py", line 439, in _iterencode
    o = _default(o)
        ^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 180, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type PrettyOrderedSet is not JSON serializable

This just repeats several times.

@sc68cal
Copy link
Contributor

sc68cal commented Dec 18, 2023

Ok, I guess this is straight forward enough where we can just merge

@Shourai
Copy link
Contributor Author

Shourai commented Dec 21, 2023

Thank you!

@Shourai
Copy link
Contributor Author

Shourai commented Apr 3, 2024

Hello, is there anything I can do to get this merged?

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