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

Add a new module to manage Port Channel Interfaces in Fabric Resource Policies Template. (DCNE-52) #539

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

Conversation

gmicol
Copy link
Collaborator

@gmicol gmicol commented Sep 27, 2024

resolves #466

New Modules Added:

  • ndo_port_channel_interface

@gmicol gmicol added enhancement New feature or request jira-sync Sync this issue to Jira labels Sep 27, 2024
@gmicol gmicol self-assigned this Sep 27, 2024
@gmicol
Copy link
Collaborator Author

gmicol commented Oct 7, 2024

To all reviewers, the test is not complete yet as I am waiting for the ndo_interface_setting module to be merged. Please review accordingly. thanks!

plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
@gmicol gmicol requested a review from akinross October 15, 2024 17:50
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
@gmicol gmicol force-pushed the mso_port_channel_interface branch 2 times, most recently from 027e458 to eb8cf29 Compare October 21, 2024 17:58
plugins/module_utils/mso.py Outdated Show resolved Hide resolved
plugins/modules/ndo_port_channel_interface.py Show resolved Hide resolved
@gmicol gmicol requested a review from samiib October 23, 2024 17:15
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 6.25000% with 15 lines in your changes missing coverage. Please review.

Project coverage is 80.31%. Comparing base (4fa9818) to head (108465f).

Files with missing lines Patch % Lines
plugins/module_utils/mso.py 6.25% 15 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #539      +/-   ##
==========================================
- Coverage   80.85%   80.31%   -0.54%     
==========================================
  Files          63       62       -1     
  Lines        6058     5999      -59     
  Branches     1594     1581      -13     
==========================================
- Hits         4898     4818      -80     
- Misses        806      827      +21     
  Partials      354      354              
Flag Coverage Δ
integration 80.31% <6.25%> (-0.54%) ⬇️

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

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

samiib
samiib previously approved these changes Oct 24, 2024
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

akinross
akinross previously approved these changes Oct 24, 2024
Copy link
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

akinross
akinross previously approved these changes Nov 5, 2024
Copy link
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

@@ -318,6 +318,19 @@ def write_file(module, url, dest, content, resp, tmpsrc=None):
os.remove(tmpsrc)


def format_interface_descriptions(interface_descriptions, node=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we use this function in mso.py? If not, move it to the module.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we will be using it in three different modules, Physical, Port Channel and VPC interfaces. that's why it's currently in mso.py as the default script to place our generic functions.

plugins/modules/ndo_port_channel_interface.py Outdated Show resolved Hide resolved
ops.append(dict(op="replace", path="{0}/{1}/name".format(path, match.index), value=name))
proposed_payload["name"] = name

if description is not None and mso.existing.get("description") != description:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sajagana wrote a function for this, asked him to do a separate PR for it so we can rebase the open PRs on it. Shoudl we do this?
https://github.com/CiscoDevNet/ansible-mso/pull/563/files#diff-f3e3720de5c754cfeb714aa1ed0f9d0a1a043f8c0005944d58565decefe1817bR31

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, let's do it like this. will rebase on this PR once merged.

@gmicol gmicol requested a review from akinross November 19, 2024 18:47
Copy link
Collaborator

@anvitha-jain anvitha-jain left a comment

Choose a reason for hiding this comment

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

LGTM

…rt_channel_interface.py module to be consistent with future modules.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request jira-sync Sync this issue to Jira
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: New module for Port Channel Interface in Fabric Resource Policies Template (DCNE-52)
6 participants