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

create controller_managed_device_groups #432

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from

Conversation

pugnacity
Copy link
Contributor

@pugnacity pugnacity commented Oct 1, 2024

This should fix #423

@pugnacity pugnacity marked this pull request as ready for review October 2, 2024 10:02
@@ -0,0 +1,36 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

You will need to add your test file to the main.yml file for it to be picked up and run with the rest of the integration tests.

that:
- test_two['changed']
- test_two['controller_managed_device_group']['name'] == "Test Controller Group One"

Copy link
Contributor

Choose a reason for hiding this comment

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

For the integration tests, we try and stick to a pattern like this:

  • Create with minimal required fields
  • Try and create again to verify idempotency
  • Create a second object with all fields
  • Try and create again to verify idempotency
  • Update one of the previous objects
  • Try and update again to verify idempotency
  • Delete one of the previous objects
  • Try and delete again to verify idempotency

Please refactor your tests to follow this pattern.

parent_location: "{{ lookup('networktocode.nautobot.lookup', 'locations', api_endpoint=nautobot_url, token=nautobot_token, api_filter='name=\"Parent Test Location\"') }}"

- name: "CONTROLLER 1: Necessary info creation"
networktocode.nautobot.controller:
Copy link
Contributor

Choose a reason for hiding this comment

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

To narrow the scope of the tests in this file, please move all creation of prerequisite items to the nautobot-populate.py file. You will find many examples in there to mimic.

plugins/modules/controller_managed_device_group.py Outdated Show resolved Hide resolved
plugins/modules/controller_managed_device_group.py Outdated Show resolved Hide resolved
plugins/modules/controller_managed_device_group.py Outdated Show resolved Hide resolved
plugins/modules/controller_managed_device_group.py Outdated Show resolved Hide resolved
plugins/modules/device.py Show resolved Hide resolved
@@ -290,6 +296,7 @@ def main():
vc_priority=dict(required=False, type="int"),
comments=dict(required=False, type="str"),
local_config_context_data=dict(required=False, type="dict"),
controller_managed_device_group=dict(required=False, type="raw"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add this to one of the existing integration tests for device? You can follow this pattern to omit the field for the unsupported versions:

nestable: "{{ true if nautobot_version is version('1.5', '>=') else omit }}"

tasks.py Outdated Show resolved Hide resolved
pugnacity and others added 8 commits October 2, 2024 19:34
Co-authored-by: Joe Wesch <10467633+joewesch@users.noreply.github.com>
Co-authored-by: Joe Wesch <10467633+joewesch@users.noreply.github.com>
Co-authored-by: Joe Wesch <10467633+joewesch@users.noreply.github.com>
Co-authored-by: Joe Wesch <10467633+joewesch@users.noreply.github.com>
Co-authored-by: Joe Wesch <10467633+joewesch@users.noreply.github.com>
Comment on lines 637 to 638
{"name": "controller_one", "location": "Child Test Location", "status": {"name": "Active"}},
{"name": "controller_two", "location": "Child Test Location", "status": {"name": "Active"}},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joewesch please give me a hint why this is not working.

Copy link
Contributor

Choose a reason for hiding this comment

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

Your problem is here:

created_controller = make_nautobot_calls(nb.extras.controller, controller)

Controllers aren't in the extras app, but rather the dcim app. Plus, the endpoint is plural (controllers):

created_controller = make_nautobot_calls(nb.dcim.controllers, controller)

Comment on lines 32 to 37
- name: "CONTROLLER 2: ASSERT - Create duplicate"
assert:
that:
- not test_two['changed']
- test_two['controller_managed_device_group']['name'] == "Test Controller Group One"
- test_two['msg'] == "Test Controller Group One already exists"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fails because of

    "diff": {
        "after": {
            "controller": "controller_one"
        },
        "before": {
            "controller": "ebe13ee7-b713-4d1b-b839-2ae0159996da"
        }
    }

Did I something wrong, or why is the change now with the ID?

Copy link
Contributor

Choose a reason for hiding this comment

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

If the GET call returns the ID of the controller instead of the name you probably need to add controller to the CONVERT_TO_ID dictionary so the name gets converted for proper comparison.

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.

New Model: Managed Device Group
2 participants