-
Notifications
You must be signed in to change notification settings - Fork 661
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
[unit test][CLI][pfcwd] Added pfcwd config tests for single and multi ASIC platform. #1248
Conversation
👋 |
retest this please |
2 similar comments
retest this please |
retest this please |
@arlakshm Thanks for providing inputs. Updated as per discussion. |
retest this please |
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
Also can you fix the description since you are not creating a new multi asic db object but extending the existing one? |
retest this please |
1 similar comment
retest this please |
Hi @smaheshm,
|
can you run with "sudo" option. What command are you using? |
I just tried building the sonic utilities python wheel package. Something like “make target/python-wheels/sonic-utilities-xx”(the full path name of the package). |
"sudo" is required to run the 'pfcwd config' tests. Id suggest to add a sudo check in the test script and skip if not root.
|
I think to skip it if it's not prilivege user is a work around for now. But I think it's better to support testing it without "sudo". |
We should not add any checks to skip 'root' privilege in CLI code, since any user can simulate a test environment on a production switch and bypass the 'root' privilege to make config changes. I'm open for any other suggestions. |
sounds reasonable... |
Is it possible to use something like |
FYI. |
… ASIC platform. (sonic-net#1248) * Update Db object to include multi ASIC db clients. * Updated pfcwd CLI commands to use decorator to pass Db object.
- What I did
Added pfcwd config tests for single and multi ASIC platform.
Changed the pfcwd CLI to access the DB object as a parameter.
**- How I did it **
Updated DB object that creates DB client connections for Config DBs and other redis DBs to include db/config clients for all namespaces on a multi ASIC platform and for default namespace on a single ASIC platform.
'run_on_multi_asic' decorator uses this object to reuse the DB client connections, thus always reading the mock redis DB in the memory, instead of loading config DB from file.
- How to verify it
Unit tests
- 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)