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

Replaced the ndo modules current value with actual API response value (DCNE-185) #589

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sajagana
Copy link
Collaborator

Note:

Fixed the below NDO modules:

  • ndo_synce_interface_policy
  • ndo_vlan_pool
  • ndo_dhcp_option_policy
  • ndo_dhcp_relay_policy
  • ndo_l3_domain
  • ndo_physical_domain
  • ndo_route_map_policy_multicast

… response value and fixed the output value of the ndo_route_map_policy_multicast and ndo_vlan_pool modules
@sajagana sajagana force-pushed the 538_ndo_modules_current_value_fix branch from 8c27fc9 to 39d30ea Compare December 19, 2024 08:31
@github-actions github-actions bot changed the title Replaced the ndo modules current value with actual API response value Replaced the ndo modules current value with actual API response value (DCNE-185) Dec 19, 2024
plugins/module_utils/mso.py Outdated Show resolved Hide resolved
plugins/modules/ndo_dhcp_option_policy.py Show resolved Hide resolved
plugins/modules/ndo_dhcp_option_policy.py Show resolved Hide resolved
@@ -85,20 +83,20 @@
- cm_create_new_l3_domain is changed
- cm_create_new_l3_domain.previous == {}
- cm_create_new_l3_domain.current.name == "ansible_test_l3_domain"
- cm_create_new_l3_domain.current.pool == "ansible_test_vlan_pool_1"
- cm_create_new_l3_domain.current.pool != ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

so we only return the pool uuid?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, let me insert the name attribute into the output.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But how do we compare this to the input of name? or should we expose both uuid and pool name? in output do we only want to have uuid returned, as this from normal input is not something a user knows unless the use a query on the vlan pool object or create the vlan pool via ansible and refer to the returned output.

@sajagana sajagana force-pushed the 538_ndo_modules_current_value_fix branch from 2240b6b to 5943cf6 Compare December 20, 2024 17:30
@sajagana sajagana requested a review from akinross December 20, 2024 17:33
Copy link
Collaborator

@shrsr shrsr left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@gmicol gmicol left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@samiib samiib left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -186,15 +179,14 @@ def main():

if pool and match.details.get("pool") != pool:
ops.append(dict(op="replace", path="{0}/{1}/pool".format(path, match.index), value=mso_template.get_vlan_pool_uuid(pool)))
match.details["pool"] = pool
match.details["pool"] = mso_template.get_vlan_pool_uuid(pool)
Copy link
Collaborator

Choose a reason for hiding this comment

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

think same issue is happening for physical domain

response = mso.request(mso_template.templates_path, method="POST", data=payload)
payload["templateId"] = response.get("templateId")
mso.existing = mso.request(mso_template.templates_path, method="POST", data=payload)
payload["templateId"] = mso.existing.get("templateId")
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you check the response for tenant and its corresponding id in the response also?

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.

Update the existing NDO modules to return the actual current value based on the API response (DCNE-185)
6 participants