-
Notifications
You must be signed in to change notification settings - Fork 107
Add auto to link level speed options #577
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
Conversation
akinross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for the contribution, see few comments that I believe would need to be added
|
@akinross i have updated the documentation and added the new integration tests. i tried to run the whole test playbook to verify, but im getting throttled by the Cisco sandbox APIC API, and its not possible to change to certificate-based authentication. I could at least get some snippets from it: |
|
Got the whole tests running. |
akinross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i will run tests locally when you have changed the assert for checkmode
tests/integration/targets/aci_interface_policy_link_level/tasks/main.yml
Show resolved
Hide resolved
tests/integration/targets/aci_interface_policy_link_level/tasks/main.yml
Outdated
Show resolved
Hide resolved
tests/integration/targets/aci_interface_policy_link_level/tasks/main.yml
Show resolved
Hide resolved
tests/integration/targets/aci_interface_policy_link_level/tasks/main.yml
Outdated
Show resolved
Hide resolved
…s/main.yml Co-authored-by: Akini Ross <akinross@cisco.com>
akinross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment response
tests/integration/targets/aci_interface_policy_link_level/tasks/main.yml
Outdated
Show resolved
Hide resolved
akinross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
shrsr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Samita B <98932546+samiib@users.noreply.github.com>
akinross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
shrsr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #577 +/- ##
===========================================
- Coverage 96.26% 35.20% -61.06%
===========================================
Files 231 231
Lines 10647 10663 +16
Branches 1603 1607 +4
===========================================
- Hits 10249 3754 -6495
- Misses 305 6909 +6604
+ Partials 93 0 -93
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
samiib
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Lionel Hercot <lionel.hercot@gmail.com>
akinross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
shrsr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
samiib
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
lhercot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Thanks for your contribution! |
"auto" for the link level policy is missing from the choices in the ansible module. This adds the "auto" option to allow an auto negotiating interface speed to be configured on the APIC controller