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

Adding Support for Traffic Class in Onyx Switches #55280

Closed
wants to merge 11 commits into from

Conversation

Projects
None yet
3 participants
@anasbadaha
Copy link
Contributor

commented Apr 15, 2019

Signed-off-by: Anas Badaha anasb@mellanox.com

SUMMARY

Adding Support for Traffic Class in Onyx Switches

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION

lib/ansible/modules/network/onyx/onyx_traffic_class.py
test/units/modules/network/onyx/fixtures/onyx_show_dcb_ets_interface.cfg
test/units/modules/network/onyx/fixtures/onyx_show_interface_congestion_control.cfg
test/units/modules/network/onyx/test_onyx_traffic_class.py

ansible 2.9.0.dev0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /.autodirect/mtrswgwork/anasb/ansible_dev8/lib/ansible
  executable location = /.autodirect/mtrswgwork/anasb/ansible_dev8/bin/ansible
  python version = 2.7.5 (default, Jul 13 2018, 13:06:57) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]

Adding Support for Traffic Class in Onyx Switches
Signed-off-by: Anas Badaha <anasb@mellanox.com>
@ansibot

This comment has been minimized.

anasbadaha added some commits Apr 15, 2019

Fix Pep8 Failures in onyx_traffic_class.py
Signed-off-by: Anas Badaha <anasb@mellanox.com>
Fix Pep8 Failures in onyx_traffic_class.py phase 2
Signed-off-by: Anas Badaha <anasb@mellanox.com>

@ansibot ansibot added needs_revision and removed core_review labels Apr 15, 2019

@anasbadaha

This comment has been minimized.

Copy link
Contributor Author

commented Apr 15, 2019

Hi @samerd ,

Please take a look on onyx_traffic_class module and approve it if you don't have any comments.

Thanks

@ansibot ansibot added core_review and removed needs_revision labels Apr 15, 2019

@samerd
Copy link
Contributor

left a comment

Anas,
Great Job,
However, after review, i think that the congestion control spec is complicated.
I would prefer the following:
control
threshold_mode: (absolute, relative)
min_threshold
max_threshold

this would be simpler for the user and for you to validate.
sorry for the late design change

anasbadaha added some commits Apr 21, 2019

Fix Samer's Comments
Signed-off-by: Anas Badaha <anasb@mellanox.com>
@ansibot

This comment has been minimized.

Copy link
Contributor

commented Apr 21, 2019

@anasbadaha this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

anasbadaha added some commits Apr 21, 2019

Fix Samer's comments Phase 2
Signed-off-by: Anas Badaha <anasb@mellanox.com>
Fix Samer's Comments Phase 3
Signed-off-by: Anas Badaha <anasb@mellanox.com>

anasbadaha added some commits Apr 21, 2019

Fix shippable failures in onyx_traffic_class.py
Signed-off-by: Anas Badaha <anasb@mellanox.com>
Fix json response
Signed-off-by: Anas Badaha <anasb@mellanox.com>
Rerun shippable again
Signed-off-by: Anas Badaha <anasb@mellanox.com>
Fix Shippable Failures Phase 3
Signed-off-by: Anas Badaha <anasb@mellanox.com>

@anasbadaha anasbadaha closed this Apr 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.