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.

Sign up for GitHub

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

Jump to bottom

[minor_change] add support for configuration of system endpoint contr… #507

Merged
merged 4 commits into from
Oct 26, 2023
Merged

[minor_change] add support for configuration of system endpoint contr… #507

merged 4 commits into from
Oct 26, 2023

Conversation

akinross
Copy link
Collaborator

…ols, ip aging, ep loop protection and roque endpoint control with aci_system_endpoint_controls module

This is a single module (aci_system_endpoint_controls) from a larger PR of tim cragg #419

Splitting up the module for easier review

@akinross akinross added the enhancement New feature or request label Oct 25, 2023
@akinross akinross self-assigned this Oct 25, 2023
@akinross
[minor_change] add support for configuration of system endpoint contr…
083ee41 
…ols, ip aging, ep loop protection and roque endpoint control with aci_system_endpoint_controls module
@codecov
Copy link

codecov bot commented Oct 25, 2023 •
edited
Loading

Codecov Report

Attention: 22 lines in your changes are missing coverage. Please review.

Comparison is base (0cb9b2a) 96.56% compared to head (6ec61ca) 35.39%.

❗ Current head 6ec61ca differs from pull request most recent head fc19eb6. Consider uploading reports for the commit fc19eb6 to get more accurate results

Additional details and impacted files 
@@             Coverage Diff             @@
##           master     #507       +/-   ##
===========================================
- Coverage   96.56%   35.39%   -61.18%     
===========================================
  Files         186      187        +1     
  Lines        8591     8631       +40     
  Branches     1276     1283        +7     
===========================================
- Hits         8296     3055     -5241     
- Misses        222     5576     +5354     
+ Partials       73        0       -73
Flag Coverage Δ
integration ?
sanity 35.39% <45.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
plugins/module_utils/constants.py 100.00% <100.00%> (ø)
plugins/modules/aci_system_endpoint_controls.py 43.58% <43.58%> (ø)

... and 181 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sajagana
sajagana requested changes Oct 25, 2023
View reviewed changes
tests/integration/targets/aci_system_endpoint_controls/tasks/main.yml Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should add a task without passing ep_loop_protection - action value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://github.com/CiscoDevNet/ansible-aci/pull/419/files#diff-ece9ab568910565c93cb16b9042c34f6857ecf51ee313f21135e47f812d8ba56

is this task not doing so already?

@akinross akinross requested a review from sajagana October 25, 2023 16:12
shrsr
shrsr requested changes Oct 25, 2023
View reviewed changes
plugins/modules/aci_system_endpoint_controls.py Outdated
module: aci_system_endpoint_controls
short_description: Manage System Enpoint Controls (ep:IpAgingP, ep:ControlP, and ep:LoopProtectP)
description:
- Manage System Enpoint Controls on Cisco ACI fabrics.
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • Manage System Endpoint Controls on Cisco ACI fabrics.

plugins/modules/aci_system_endpoint_controls.py Outdated
DOCUMENTATION = r"""
---
module: aci_system_endpoint_controls
short_description: Manage System Enpoint Controls (ep:IpAgingP, ep:ControlP, and ep:LoopProtectP)
Copy link
Collaborator

Choose a reason for hiding this comment

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

short_description: Manage System Endpoint Controls (ep:IpAgingP, ep:ControlP, and ep:LoopProtectP)

shrsr
shrsr previously approved these changes Oct 25, 2023
View reviewed changes
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

@akinross akinross requested a review from shrsr October 25, 2023 17:43
shrsr
shrsr previously approved these changes Oct 25, 2023
View reviewed changes
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

samiib
samiib previously approved these changes Oct 26, 2023
View reviewed changes
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 👍

sajagana
sajagana previously approved these changes Oct 26, 2023
View reviewed changes
Copy link
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!

lhercot
lhercot requested changes Oct 26, 2023
View reviewed changes
plugins/modules/aci_system_endpoint_controls.py Outdated
suboptions:
admin_state:
description:
- Whether to enable IP Aging Controls on the fabric.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Whether to enable IP Aging Controls on the fabric.
- Whether to enable EP Loop Protection on the fabric.

@akinross akinross dismissed stale reviews from sajagana, samiib, and shrsr via d79561d October 26, 2023 17:53
shrsr
shrsr previously approved these changes Oct 26, 2023
View reviewed changes
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

lhercot
lhercot approved these changes Oct 26, 2023
View reviewed changes
Copy link
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 46899b0 into CiscoDevNet:master Oct 26, 2023
17 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lhercot lhercot lhercot approved these changes

@samiib samiib samiib left review comments

@sajagana sajagana sajagana left review comments

@shrsr shrsr shrsr left review comments

@anvitha-jain anvitha-jain Awaiting requested review from anvitha-jain anvitha-jain is a code owner

@gmicol gmicol Awaiting requested review from gmicol gmicol is a code owner

Assignees

@akinross akinross

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@akinross @lhercot @shrsr @sajagana @samiib