Modified test cases by QOS DB reference format remove #3824

AshokDaparthi · 2021-07-20T18:57:09Z

Depends on sonic-net/sonic-utilities#1626
Depends on sonic-net/sonic-swss#1754
Depends on sonic-net/sonic-buildimage#7752

QOS tables in config db used ABNF format i.e "[TABLE_NAME|name] to refer fieldvalue to other qos tables.

Example:
Config DB:
"Ethernet92|3": {
"scheduler": "[SCHEDULER|scheduler.1]",
"wred_profile": "[WRED_PROFILE|AZURE_LOSSLESS]"
},
"Ethernet0|0": {
"profile": "[BUFFER_PROFILE|ingress_lossy_profile]"
},
"Ethernet0": {
"dscp_to_tc_map": "[DSCP_TO_TC_MAP|AZURE]",
"pfc_enable": "3,4",
"pfc_to_queue_map": "[MAP_PFC_PRIORITY_TO_QUEUE|AZURE]",
"tc_to_pg_map": "[TC_TO_PRIORITY_GROUP_MAP|AZURE]",
"tc_to_queue_map": "[TC_TO_QUEUE_MAP|AZURE]"
},
This format is not consistent with other DB schema followed in sonic.
And also this reference in DB is not required, This is taken care by YANG "leafref".

Removed this format from all platform files to consistent with other sonic db schema.
Example:
"Ethernet92|3": {
"scheduler": "scheduler.1",
"wred_profile": "AZURE_LOSSLESS"
},

Modified test cases to remove DB references.

lgtm-com · 2021-07-20T19:26:11Z

This pull request introduces 1 alert when merging a5e6a86df1ae9ad301312a78d758c858cde3194a into 8f3ae4f - view on LGTM.com

new alerts:

1 for Syntax error

yxieca · 2021-07-20T22:43:06Z

@AshokDaparthi please explain how this change was tested? Why would LGTM catch syntax errors?

AshokDaparthi · 2021-07-20T22:54:28Z

@AshokDaparthi please explain how this change was tested? Why would LGTM catch syntax errors?

This code testing needs others repo changes, At present even i mentioned "Depends on" at present pull request "Checks/regression tests" not using new packages. I am looking at how to run sonic-mgmt tests.

tests/qos/test_buffer.py

stephenxs

Approved for test_buffer (pytest). Please check with other reviewers as well.
Thanks.

stephenxs · 2021-08-26T22:59:53Z

HI @yxieca @lguohan @AshokDaparthi @neethajohn
To which branch will the feature be merged?
I just realized the sonic-mgmt change can affect all the existing branches without this feature.
After the PR is merged, sonic-mgmt will assume the format of qos/buffer references to be [<field-name>], like ingress_lossless_pool.
However, in the branches without this feature, the qos/buffer references are still in the old format, like [BUFFER_POOL|ingress_lossless_pool]. This will fail the qos test in old branches.
I think there are two ways to handle this

Fork a dedicated branch of sonic-mgmt for older branches
Handle the format of qos/buffer references per version.
- in older branches, like 202012, the format should be [<table-name>|<field-name>]
- in the latest branch, the format should be <field-name>.

zhangyanzhao · 2021-09-09T17:39:22Z

Build failure need be handled, this PR blocks sonic-net/sonic-buildimage#7375

Syntax error fix Addressed review comment

lgtm-com · 2021-09-20T05:11:25Z

This pull request introduces 1 alert when merging 3038adf34405243a2f2a612c76a2bccca8b823f5 into 79e5e78 - view on LGTM.com

new alerts:

1 for Variable defined multiple times

AshokDaparthi · 2021-09-20T15:28:59Z

@stephenxs , @neethajohn - Could you please review this, Modified test cases to handle release.

neethajohn · 2021-09-20T20:01:39Z

@AshokDaparthi , there is a lot of code change here. I am unable to tell for sure if all the relevant files have been modified. What is the testing that you have done? Can you share the test results?

AshokDaparthi · 2021-09-21T00:25:22Z

@neethajohn - I depend on "Checks" in pull requests. I tried simulating "present/master" release as new format and also tested "present/master" as old format. Both got passed. We do not have any simulated setup which can run sonic-mgmt qos test cases with h/w or vs. I did manual testing in broadcom platform.

ansible/roles/fanout/templates/sonic_deploy_arista_7060.j2

ansible/vars/configlet/t1-64-lag-clet/clet-add-qos-new-dbfmt.json

neethajohn · 2021-09-20T20:44:13Z

spytest/apis/qos/cos.py

@@ -15,7 +15,7 @@ def config_port_qos_map(dut, obj_name, interface, **kwargs):
            st.log("Please provide obj_name like 'AZURE' and interface like 'Ethernet0,Ethernet1'")
            return False
        else:
-            cos_specific_dict = {"tc_to_queue_map": "[TC_TO_QUEUE_MAP|" + obj_name + "]", "dscp_to_tc_map": "[DSCP_TO_TC_MAP|" + obj_name + "]" }
+            cos_specific_dict = {"tc_to_queue_map": obj_name, "dscp_to_tc_map": obj_name}


I don't see any version check for all the files changed under the spytest folder. Am I missing something?

@neethajohn - spytest are not handled for backward compatibility, i hope spytest are not used actively for sanity or test. I will take this up after all changes merged.

@ramakristipati , please review the spytest changes

tests/common/utilities.py

neethajohn · 2021-09-21T00:48:20Z

@stephenxs , there are some new changes. Can you please review again?

stephenxs

@AshokDaparthi Some minor comments.

tests/qos/test_buffer.py

Description of PR Qos sai failures seen due to changes in #3824 Signed-off-by: Neetha John <nejo@microsoft.com>

AshokDaparthi requested review from lolyu, yxieca and a team as code owners July 20, 2021 18:57

AshokDaparthi mentioned this pull request Jul 20, 2021

SONIC QOS YANG - Remove qos tables field value reference format sonic-net/sonic-buildimage#7752

Merged

yxieca requested a review from neethajohn July 20, 2021 22:42

stephenxs reviewed Jul 30, 2021

View reviewed changes

tests/qos/test_buffer.py Outdated Show resolved Hide resolved

stephenxs approved these changes Aug 11, 2021

View reviewed changes

stephenxs mentioned this pull request Sep 1, 2021

QOS fieldvalue reference ABNF format to string sonic-net/sonic-utilities#1626

Merged

AshokDaparthi force-pushed the sonic-yang-qos-fv-db-ref-remove branch from 83c38f4 to e80769d Compare September 2, 2021 21:54

AshokDaparthi mentioned this pull request Sep 9, 2021

SONiC QOS Scheduler, WRED, Queue Yangs sonic-net/sonic-buildimage#7281

Merged

AshokDaparthi force-pushed the sonic-yang-qos-fv-db-ref-remove branch 2 times, most recently from 7d417cb to 7eeac59 Compare September 20, 2021 04:00

Modified test cases by QOS DB reference format remove

f04c012

Syntax error fix Addressed review comment

AshokDaparthi force-pushed the sonic-yang-qos-fv-db-ref-remove branch from 7eeac59 to 3038adf Compare September 20, 2021 04:44

AshokDaparthi force-pushed the sonic-yang-qos-fv-db-ref-remove branch from 3038adf to 2ef9e81 Compare September 20, 2021 05:17

neethajohn suggested changes Sep 21, 2021

View reviewed changes

AshokDaparthi force-pushed the sonic-yang-qos-fv-db-ref-remove branch from fcd4365 to b935879 Compare September 23, 2021 02:13

Modified to testcase to handle QOS DB old and new fomats

03f4560

AshokDaparthi force-pushed the sonic-yang-qos-fv-db-ref-remove branch from b935879 to 03f4560 Compare September 23, 2021 02:16

stephenxs reviewed Sep 24, 2021

View reviewed changes

tests/qos/test_buffer.py Outdated Show resolved Hide resolved

tests/qos/test_buffer.py Outdated Show resolved Hide resolved

tests/qos/test_buffer.py Outdated Show resolved Hide resolved

Addressed review comments

3f4972b

neethajohn approved these changes Sep 24, 2021

View reviewed changes

stephenxs approved these changes Sep 24, 2021

View reviewed changes

smaheshm merged commit 97d31d4 into sonic-net:master Sep 27, 2021

neethajohn mentioned this pull request Sep 29, 2021

[qos] Specify correct handle to access dut resources #4382

Merged

4 tasks

lolyu pushed a commit that referenced this pull request Sep 29, 2021

Specify the correct dut handle (#4382)

0db5b3a

Description of PR Qos sai failures seen due to changes in #3824 Signed-off-by: Neetha John <nejo@microsoft.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modified test cases by QOS DB reference format remove #3824

Modified test cases by QOS DB reference format remove #3824

AshokDaparthi commented Jul 20, 2021

lgtm-com bot commented Jul 20, 2021

yxieca commented Jul 20, 2021

AshokDaparthi commented Jul 20, 2021

stephenxs left a comment

stephenxs commented Aug 26, 2021

zhangyanzhao commented Sep 9, 2021

lgtm-com bot commented Sep 20, 2021

AshokDaparthi commented Sep 20, 2021

neethajohn commented Sep 20, 2021

AshokDaparthi commented Sep 21, 2021

neethajohn Sep 20, 2021

AshokDaparthi Sep 21, 2021

neethajohn Sep 21, 2021

neethajohn commented Sep 21, 2021

stephenxs left a comment

Modified test cases by QOS DB reference format remove #3824

Modified test cases by QOS DB reference format remove #3824

Conversation

AshokDaparthi commented Jul 20, 2021

lgtm-com bot commented Jul 20, 2021

yxieca commented Jul 20, 2021

AshokDaparthi commented Jul 20, 2021

stephenxs left a comment

Choose a reason for hiding this comment

stephenxs commented Aug 26, 2021

zhangyanzhao commented Sep 9, 2021

lgtm-com bot commented Sep 20, 2021

AshokDaparthi commented Sep 20, 2021

neethajohn commented Sep 20, 2021

AshokDaparthi commented Sep 21, 2021

neethajohn Sep 20, 2021

Choose a reason for hiding this comment

AshokDaparthi Sep 21, 2021

Choose a reason for hiding this comment

neethajohn Sep 21, 2021

Choose a reason for hiding this comment

neethajohn commented Sep 21, 2021

stephenxs left a comment

Choose a reason for hiding this comment