Skip to content

enhancement for node_mgmt_epg_to_contract (DCNE-87)#658

Merged
lhercot merged 17 commits intoCiscoDevNet:masterfrom
Ziaf007:node_mgmt_epg_to_contract
Jul 16, 2025
Merged

enhancement for node_mgmt_epg_to_contract (DCNE-87)#658
lhercot merged 17 commits intoCiscoDevNet:masterfrom
Ziaf007:node_mgmt_epg_to_contract

Conversation

@Ziaf007
Copy link
Copy Markdown
Contributor

@Ziaf007 Ziaf007 commented May 21, 2024

-) Added node_mgmt_epg_to_contract module
-) Test.yml still pending, working on it.

DCNE-87

@github-actions github-actions Bot changed the title enhancement for node_mgmt_epg_to_contract enhancement for node_mgmt_epg_to_contract (DCNE-87) Oct 18, 2024
@Ziaf007 Ziaf007 force-pushed the node_mgmt_epg_to_contract branch 2 times, most recently from 820f4cc to 2873077 Compare October 24, 2024 14:15
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
@akinross akinross requested a review from samiib December 16, 2024 13:52
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread tests/integration/targets/aci_node_mgmt_epg_to_contract/tasks/main.yml Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
@shrsr shrsr requested a review from akinross March 20, 2025 13:33
Comment thread plugins/module_utils/constants.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py Outdated
Comment thread plugins/modules/aci_node_mgmt_epg_to_contract.py
@shrsr shrsr self-requested a review March 23, 2025 00:49
shrsr
shrsr previously approved these changes Mar 23, 2025
Copy link
Copy Markdown
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

akinross
akinross previously approved these changes Mar 25, 2025
Copy link
Copy Markdown
Collaborator

@akinross akinross 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
Copy Markdown
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.

Black formatting and Sanity tests failed, can you please fix it?

@shrsr
Copy link
Copy Markdown
Collaborator

shrsr commented Mar 25, 2025

@Ziaf007 To add to @gmicol's comment, can you please use the following commands? ->

pip3 install black
black <path_to_module> -l 159

@Ziaf007 Ziaf007 dismissed stale reviews from akinross and shrsr via 4c45f4f March 25, 2025 22:03
@shrsr shrsr requested review from akinross, gmicol and shrsr March 25, 2025 22:28
shrsr
shrsr previously approved these changes Mar 25, 2025
Copy link
Copy Markdown
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

Comment on lines +103 to +104
- inb_provide_present_check_mode.sent.fvRsProv.attributes.dn == "uni/tn-mgmt/mgmtp-default/inb-anstest_inb/rsprov-aci_inb_http"
- inb_provide_present_check_mode.sent.fvRsProv.attributes.tnVzBrCPName == 'aci_inb_http'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

for checkmode we should check proposed in instead of sent

Suggested change
- inb_provide_present_check_mode.sent.fvRsProv.attributes.dn == "uni/tn-mgmt/mgmtp-default/inb-anstest_inb/rsprov-aci_inb_http"
- inb_provide_present_check_mode.sent.fvRsProv.attributes.tnVzBrCPName == 'aci_inb_http'
- inb_provide_present_check_mode.proposed.fvRsProv.attributes.dn == "uni/tn-mgmt/mgmtp-default/inb-anstest_inb/rsprov-aci_inb_http"
- inb_provide_present_check_mode.proposed.fvRsProv.attributes.tnVzBrCPName == 'aci_inb_http'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done everywhere.

ansible.builtin.assert:
that:
- consume_absent_check_mode is changed
- consume_absent_check_mode.previous.0.fvRsCons is defined
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you also assert the attributes of previous?

description:
- Bind Node Management EPGs to Contracts on Cisco ACI fabrics.
notes:
- The C(epg) and C(contract) used must exist before using this module in your playbook.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- The C(epg) and C(contract) used must exist before using this module in your playbook.
- The O(epg) and O(contract) used must exist before using this module in your playbook.

Ziaf007 added 17 commits July 10, 2025 15:56
…e PR. Shall test to merge Root and SubClass-1 into a single Root class now
2) fixed some linting
3) modified the query assertations accordingly in test file
All checks passed in test file.
PLAY RECAP *********************************************************************
azure_cloud                : ok=8    changed=0    unreachable=0    failed=0    skipped=51   rescued=0    ignored=0
cn-dmz-apic-m1-02-v42      : ok=7    changed=0    unreachable=0    failed=0    skipped=52   rescued=0    ignored=0
cn-dmz-apic-m1-03-v52      : ok=55   changed=24   unreachable=0    failed=0    skipped=4    rescued=0    ignored=8
cn-dmz-apic-m1-04-v602h    : ok=55   changed=24   unreachable=0    failed=0    skipped=4    rescued=0    ignored=8
Module ready for review
…seem to fail. Pushing to branch so one can pull to their local and test
@Ziaf007 Ziaf007 force-pushed the node_mgmt_epg_to_contract branch from 4039642 to c00a359 Compare July 10, 2025 10:41
@akinross akinross requested review from akinross and sajagana July 10, 2025 15:12
Copy link
Copy Markdown
Collaborator

@akinross akinross 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
Copy Markdown
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
Copy Markdown
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

Copy link
Copy Markdown
Collaborator

@sajagana sajagana 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
Copy Markdown
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
Copy Markdown
Member

@lhercot lhercot left a comment

Choose a reason for hiding this comment

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

LGTM

@lhercot lhercot merged commit e5e6898 into CiscoDevNet:master Jul 16, 2025
22 of 23 checks passed
@lhercot
Copy link
Copy Markdown
Member

lhercot commented Jul 16, 2025

Thank you for the contribution

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.

aci_epg_to_contract module does not work for associating contracts to Node mgmt EPGs (DCNE-87)

7 participants