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

[GCU] Update RDMA Platform Validator #2913

Merged
merged 12 commits into from
Jul 20, 2023

Conversation

isabelmsft
Copy link
Contributor

@isabelmsft isabelmsft commented Jul 14, 2023

What I did

ADO 24509483
-Fix RDMA platform validator PR to consider two edge cases:

  1. Allow empty string value "" when the operation is remove in jsonpatch. Previously, empty string was not an allowed value, so these remove operations were incorrectly blocked. This is fixed by adding the empty string "" in the platform validator conf file for RDMA tables where remove operation is supported (only PFC_WD).
  2. Ensure proper string parsing for an edge case jsonpatch path and value combination. The edge case combination is: the path includes contains a variable digit (such as /PFC_WD/Ethernet64 where 64 is the variable digit) and value is a dict. In this scenario, we need to clean the path and value combination differently so that the relevant value is properly extracted without an extra "/".

-add support for Mellanox spc2, spc3 asic

How I did it

Update GCU x RDMA platform validator functionality

How to verify it

E2E testing, test_add_rack passes

============================= test session starts ==============================
platform linux2 -- Python 2.7.18, pytest-4.6.11, py-1.11.0, pluggy-0.13.1
ansible: 2.8.12
rootdir: /data/sonic-mgmt-int/tests, inifile: pytest.ini
plugins: metadata-1.11.0, allure-pytest-2.8.22, html-1.22.1, xdist-1.28.0, forked-1.3.0, celery-4.4.7, repeat-0.9.1, ansible-2.2.4
['conf-name', 'group-name', 'topo', 'ptf_image_name', 'ptf', 'ptf_ip', 'ptf_ipv6', 'server', 'vm_base', 'dut', 'inv_name', 'auto_recover', 'comment']
Finished testbed info generating.
collected 1 item

configlet/test_add_rack.py::test_add_rack PASSED                         [100%]

test_pfcwd_status

============================= test session starts ==============================
platform linux2 -- Python 2.7.18, pytest-4.6.11, py-1.11.0, pluggy-0.13.1
ansible: 2.8.12
rootdir: /data/sonic-mgmt-int/tests, inifile: pytest.ini
plugins: metadata-1.11.0, allure-pytest-2.8.22, html-1.22.1, xdist-1.28.0, forked-1.3.0, celery-4.4.7, repeat-0.9.1, ansible-2.2.4
['conf-name', 'group-name', 'topo', 'ptf_image_name', 'ptf', 'ptf_ip', 'ptf_ipv6', 'server', 'vm_base', 'dut', 'inv_name', 'auto_recover', 'comment']
Finished testbed info generating.
collected 4 items

generic_config_updater/test_pfcwd_status.py::test_stop_pfcwd[single] PASSED [ 25%]
generic_config_updater/test_pfcwd_status.py::test_stop_pfcwd[all] PASSED [ 50%]
generic_config_updater/test_pfcwd_status.py::test_start_pfcwd[single] PASSED [ 75%]
generic_config_updater/test_pfcwd_status.py::test_start_pfcwd[all] PASSED [100%]

---------- generated xml file: /data/sonic-mgmt-int/tests/logs/tr.xml ----------

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

@isabelmsft isabelmsft changed the title Fix platform validator [GCU] Update RDMA Platform Validator Jul 14, 2023
@@ -75,7 +79,7 @@ def _get_fields_in_patch():

if 'value' in patch_element.keys() and isinstance(patch_element['value'], dict):
for key in patch_element['value']:
cleaned_fields.append(cleaned_field+ '/' + key)
cleaned_fields.append(cleaned_field + '/' + key) if len(cleaned_field) > 0 else cleaned_fields.append(key)
Copy link
Contributor

@qiluo-msft qiluo-msft Jul 18, 2023

Choose a reason for hiding this comment

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

if

It's better to use if-else block. Single line is too long. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to if-else block

qiluo-msft
qiluo-msft previously approved these changes Jul 19, 2023
wen587
wen587 previously approved these changes Jul 19, 2023
@jarias-lfx
Copy link

/easycla

@isabelmsft isabelmsft dismissed stale reviews from wen587 and qiluo-msft via ef0e685 July 20, 2023 00:00
@isabelmsft isabelmsft merged commit e49fd91 into sonic-net:master Jul 20, 2023
4 checks passed
rajkumar38 pushed a commit to rajkumar38/sonic-utilities that referenced this pull request Jul 25, 2023
isabelmsft added a commit to isabelmsft/sonic-utilities that referenced this pull request Aug 10, 2023
@StormLiangMS
Copy link
Contributor

@isabelmsft ADO?

pdhruv-marvell pushed a commit to pdhruv-marvell/sonic-utilities that referenced this pull request Aug 23, 2023
isabelmsft added a commit to isabelmsft/sonic-utilities that referenced this pull request Nov 22, 2023
yxieca pushed a commit that referenced this pull request Nov 29, 2023
Cherry-pick PR #2692 (set up test infrastructure used in later PR 2857), #2857 (platform validator PR), #2913 (edge case fix), #2951, #3018 (add support for new Mellanox HwSKU in conf file)
This change stops GCU from modifying a protected RDMA field.

Signed-off-by: Stephen Sun <stephens@nvidia.com>
Co-authored-by: Stephen Sun <5379172+stephenxs@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants