[config] Add ConfigMgmt class for config validation, delete ports, add ports #765
Conversation
|
Gentle Reminder, Kindly Review. |
|
@renukamanavalan |
praveen-li
force-pushed
the
config_mgmt_class_msft
branch
from
ac93377
to
796704d
Compare
|
@samaity: Please fix conflicts. |
praveen-li
force-pushed
the
config_mgmt_class_msft
branch
from
796704d
to
412d162
Compare
|
This pull request introduces 8 alerts when merging 412d162d6005bb89f68d6639e6fbf7e3a36b404e into 7e52ef9 - view on LGTM.com new alerts:
|
|
@jleveque : This PR now has only new file config_mgmt.py which will be used only from config/main.py. main.py changes will be part of PR 766. Thx. |
praveen-li
force-pushed
the
config_mgmt_class_msft
branch
from
74c32dc
to
779e9d0
Compare
Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
…lowed servers by Azure. Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
|
This pull request introduces 3 alerts when merging 2a8d16c into 90efd62 - view on LGTM.com new alerts:
|
|
Build failure: |
… breakOutPort API. Changes: -- Remove addPorts Arg from breakOutPort API. -- Change Test Code accordingly. Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
this pkg will come from sonic-net/sonic-buildimage#4740. Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
@renukamanavalan : yeah I am replying on sonic-net/sonic-buildimage#4740 for this. |
|
retest this please |
|
E ImportError: No module named jsondiff - required module not found |
|
retest this please |
|
libyang[0]: Unable to use search directory "./../../../sonic-yang-models/yang-models/" (No such file or directory) |
@jleveque |
|
@praveen-li: We copy artifacts from the last successful build of buildimage-vs-all. I haven't done a lot of work with the new Jenkins infrastructure. @daall or @lguohan can comment further. |
…ic_yang_models now we do not need relative path to yang models. Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
There was a problem hiding this comment.
Just two concerns only.
-
What if we abort after port delete, before add, which has higher probability as we wait for asynchronous sync of delete up to ASIC-DB -- For this we have filed an active issue.
-
The time point of collection of diffs for delete & add and the application of the same, are not atomic. Just concerned, any in-the-middle change , that could affect
At the least, can we get a advisory lock, to the least that no two port-break commands can run concurrently. May be not for this PR. But to give some attention,
Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
@renukamanavalan -- for 2., we have config lock, but from Azure branch it is removed, so we will come up with approach which caters all. |
|
Check build failure appears to be unrelated to this PR. Merging in spite of it. |
jleveque
changed the title
…d ports (sonic-net#765) Provided a new ConfigMgmt class for - Config Validation - Adding ports - Deleting ports Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
…d ports (sonic-net#765) Provided a new ConfigMgmt class for - Config Validation - Adding ports - Deleting ports Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
- What I did
Provided a class in sonic_util config for
-- Config Validation,
-- BreakOut Ports
Note: It depends on sonic-net/sonic-buildimage#3861
Test cases are added in this PR but described in a dummy PR #947
- How I did it
--ConfigMgmt Class uploads config data from config_Db.json or from config db using SONiC-py-swsssdk APIs .
--Port, vlan and acl related config is cropped out of entire config, this is because yang models are available only for Vlan, Acl and Port as of now.
--Cropped portion of config DB will be given as input to Python Yang LIB while loading config data.
--ConfigMgmt Class provides APIs to breakout, delete and add Ports.
--Using Python yang LIB APIs, this class finds dependencies for each of the port which must be deleted and will delete dependencies.
-- At last after successful validation, new config is written in config DB.
-- While addition, relevant default config is found for ports which are being added. Then entire new config [default + port] is added to current config in data tree.
-- At last after successful validation, new config is written in config DB.
- How to verify it
On Sonic Switch Ran below Tests
Note: Below tests are done with PR 766 and PR 857 changes.
Description for Test cases:
- What I did
Write Unit tests for Config_mgmt library introduced in PR #765.
- How I did it
Key Decisions:
-- Config_mgmt interaction with Redis DB:
These tests will be executed at build time, the entire interaction with Redis DB is mocked using:
import mock_tables.dbconnector
This mocks below classes used in config_mgmt
from swsssdk import ConfigDBConnector, SonicV2Connector, port_util
-- Mock sonic_yang or not:
Most of the functions in config_mgmt calls APIs from sonic_yang_mgmt (https://github.com/Azure/sonic-buildimage/tree/master/src/sonic-yang-mgmt) library. It is important to catch any compatibility issues during build time rather than run time. So it is decided not to mock sonic_yang but use real instance. Due to this sonic_utilities will have a build time dependency on sonic_yang_mgmt.
-- Unit Test each function or the functions exposed to caller:
1. To unit test each function, we need to mock sonic_yang, which is against the previous decision.
2. Also calling the functions which are exposed to the caller is sufficient to unit test all functions. Because functions like breakOutPort internally call every function in the library, and the expected result is not produced if internal functions are not working properly.
3. In the real scenario, the caller will use this library for config validation and to break out the port in a continuous manner. To emulate a real scenario it is decided to exercise only the functions exposed to the caller.
Check Result:
-- How to check expected result for breakout Command:
1. A successful call to breakOutPort API results in a call to writeConfigDB API twice.
One time with config to be deleted and second time with config to be added. Both calls are necessary and should be done in order. This is tested by mocking writeConfigDB API as below:
Then we can check result as below:
Note: it is important to check that both calls are done, rather than just checking final config.
Tests:
-- Create the configMgmt Class object and validate config.
-- Have a table without yang model in config, Check if this table is returned by tablesWithoutYang()
-- Test configWithKeys() to search ports in the config.
-- Important Test case, Test below sequence for port breakout:
Note: this is important to do this sequence in one test case, because it is good to test with continuous change in Config. Also this way, a new configuration for each test is not needed.
Expected Result:
All automated tests should pass.
Each breakout test is checked for expected result as below:
Have a hard coded value of expected dConfig and aConfig, where
dConfig: Config to be deleted.
aConfig: Config to be deleted.
Example:
Actual Result:
- How to verify it
Actual Result:
- 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)