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

[sonic-yang-mgmt]: sonic-yang-mgmt package for configuration validation. #3861

Merged
merged 18 commits into from
May 21, 2020

Conversation

praveen-li
Copy link
Collaborator

@praveen-li praveen-li commented Dec 9, 2019

This package includes python yang libraries which will be used with sonic utilities
package to validate the config. This python libraries are written on top of libyang
and also provides functionality to translate the config from SONiC ConfigDB to SONiC
YANG and vice-versa.

NOTE: This PR Depends on PR 3730

This PR has changes done under PR 3874 and 3891 as well. So those can be closed once this is merged.

Signed-off-by: Praveen Chaudhary [email protected]
Signed-off-by: Ping Mao [email protected]

- What I did

wheel package Makefiles

  • wheel package Makefiles for sonic-yang-mgmt package.

libyang Python APIs:

  • python APIs based on libyang
  • functions to load/merge yang models and Yang data files
  • function to validate data trees based on Yang models
  • functions to merge yang data files/trees
  • add/set/delete node in schema and data trees
  • find data/schema nodes from xpath from the Yang data/schema tree in memory
  • find dependencies
  • dump the data tree in json/xml

Extension of libyang Python APIs:

-- Cropping input config based on Yang Model.
-- Translate input config based on Yang Model.
-- rev Translate input config based on Yang Model.
-- Find xpath of port, portleaf and a yang list.
-- Find if node is key of a list while deletion if yes, then delete the parent.

- How I did it

wheel package Makefiles

  • Added steps to build sonic-yang-mgmt python wheel package with sonic build.
  • Also to install it by default on sonic image.

libyang Python APIs:

  • based on libyang python2 modules which is built by libyang
  • add functions to load/validate Yang models and Yang data trees
  • Add functions to

Extension of libyang Python APIs:

From the json format of yang models, a map is created from config DB tables
to container in yang model. Input Config is cropped on based of this map.
Input Config is also translated based of this map.
Similarly from yang data tree, output is reverse translated to config DB.

This PR also includes:
-- Find xpath of port, portleaf and a yang list.
-- Find if node is key of a list while deletion if yes, then delete the parent.

Yang model can be represented in JSON as below:

{
  "module": {
    "@name": "sonic-vlan", 
    "@xmlns": "urn:ietf:params:xml:ns:yang:yin:1", 
    "@xmlns:vlan": "http://github.com/Azure/sonic-vlan", 
    "@xmlns:yang": "urn:ietf:params:xml:ns:yang:ietf-yang-types", 
    "@xmlns:inet": "urn:ietf:params:xml:ns:yang:ietf-inet-types", 
    "@xmlns:head": "http://sonic-head", 
    "@xmlns:port": "http://github.com/Azure/sonic-port", 
    "namespace": {
      "@uri": "http://github.com/Azure/sonic-vlan"
    }, 
    "prefix": {
      "@value": "vlan"
    }, 
    "import": [
      {
        "@module": "ietf-yang-types", 
        "prefix": {
          "@value": "yang"
        }
      }, 
      {
        "@module": "ietf-inet-types", 
        "prefix": {
          "@value": "inet"
        }
      }, 
      {
        "@module": "sonic-head", 
        "prefix": {
          "@value": "head"
        }, 
        "revision-date": {
          "@date": "2019-07-01"
        }
      }, 
      {
        "@module": "sonic-port", 
        "prefix": {
          "@value": "port"
        }, 
        "revision-date": {
          "@date": "2019-07-01"
        }
      }
    ], 
    "organization": {
      "text": "Linkedin Corporation"
    }, 
    "contact": {
      "text": "[email protected]"
    }, 
    "description": {
      "text": "VLAN yang Module for SONiC OS"
    }, 
    "revision": {
      "@date": "2019-07-01", 
      "description": {
        "text": "First Revision"
      }
    },
    "container": {
      "@name": "sonic-vlan", 
      "container": [
        {
          "@name": "VLAN_INTERFACE", 
          "description": {
            "text": "VLAN_INTERFACE part of config_db.json"
          }, 
          "list": {
            "@name": "VLAN_INTERFACE_LIST", 
            "key": {
              "@value": "vlanid ip-prefix"
            }, 
            "leaf": [
              {
                "@name": "vlanid", 
                "type": {
                  "@name": "leafref", 
                  "path": {
                    "@value": "../../../VLAN/VLAN_LIST/vlanid"
                  }
                }
              }, 
              {
                "@name": "ip-prefix", 
                "type": {
                  "@name": "inet:ip-prefix"
                }, 
                "mandatory": {
                  "@value": "true"
                }
              }, 

This is used to create a map between Config DB Table and YANG Model Container.
Note: As per guideline https://github.com/Azure/SONiC/blob/master/doc/mgmt/SONiC_YANG_Model_Guidelines.md. Table maps to only container in YANG.

- How to verify it

wheel package Makefiles

Built this package separately.

pchaudha@be1e6e58be56:/sonic$ make -f slave.mk target/python-wheels/sonic_yang_mgmt-1.0-py-none-any.whl
SONiC Build System

Build Configuration
"CONFIGURED_PLATFORM"             : "broadcom"
"CONFIGURED_ARCH"                 : "amd64"
"SONIC_CONFIG_PRINT_DEPENDENCIES" : ""
"SONIC_BUILD_JOBS"                : "1"
"SONIC_CONFIG_MAKE_JOBS"          : "24"
"SONIC_USE_DOCKER_BUILDKIT"       : ""
"USERNAME"                        : "admin"
"PASSWORD"                        : "YourPaSsWoRd"
"ENABLE_DHCP_GRAPH_SERVICE"       : ""
"SHUTDOWN_BGP_ON_START"           : ""
"ENABLE_PFCWD_ON_START"           : ""
"INSTALL_DEBUG_TOOLS"             : ""
"ROUTING_STACK"                   : "frr"
"FRR_USER_UID"                    : "300"
"FRR_USER_GID"                    : "300"
"ENABLE_SYNCD_RPC"                : ""
"ENABLE_ORGANIZATION_EXTENSIONS"  : "y"
"HTTP_PROXY"                      : ""
"HTTPS_PROXY"                     : ""
"ENABLE_SYSTEM_TELEMETRY"         : "y"
"ENABLE_RESTAPI"                  : "n"
"ENABLE_ZTP"                      : ""
"SONIC_DEBUGGING_ON"              : ""
"SONIC_PROFILING_ON"              : ""
"KERNEL_PROCURE_METHOD"           : "build"
"BUILD_TIMESTAMP"                 : ""
"BLDENV"                          : ""
"VS_PREPARE_MEM"                  : "yes"
"ENABLE_SFLOW"                    : "y"


pchaudha@be1e6e58be56:/sonic$ ls target/python-wheels/sonic_yang_mgmt-1.0-py-none-any.whl
target/python-wheels/sonic_yang_mgmt-1.0-py-none-any.whl
pchaudha@be1e6e58be56:/sonic$

libyang Python APIs:

  • pytests while build sonic-yang-mgmt python wheel package
  • create sample Yang models for sonic ACL/interface/port/clan
  • sample Yang data files
  • test file with test cases, it includes categories to test, xpath and expected results for testing Yang data/schema tree wrapper functions
pytests while build target/python-wheels/sonic_yang_mgmt-1.0-py-none-any.whl:

============================= test session starts ==============================
platform linux2 -- Python 2.7.13, pytest-3.0.6, py-1.4.32, pluggy-0.4.0
rootdir: /sonic/src/sonic-yang-mgmt, inifile:
plugins: cov-2.4.0
collected 18 items

tests/test_sonic_yang_mgmt.py .
tests/libyang-python-tests/test_sonic_yang.py .................

========================== 18 passed in 0.21 seconds ===========================

Extension of libyang Python APIs:

Added PyTest:

    def test_xlate_rev_xlate(self):
        # In this test, xlation and revXlation is tested with latest Sonic
        # YANG model.

        yang_dir = "/sonic/src/sonic-yang-models/yang-models/"
        yang_test_file = "/sonic/src/sonic-yang-models/tests/yang_model_tests/yangTest.json"
        jIn = self.readIjsonInput(yang_test_file, 'SAMPLE_CONFIG_DB_JSON')
        # load yang models
        syc = sy.sonic_yang(yang_dir)

        syc.loadYangModel()

        syc.load_data(json.loads(jIn))

        syc.get_data()

        if syc.jIn and syc.jIn == syc.revXlateJson:
            print("Xlate and Rev Xlate Passed")
        else:
            print("Xlate and Rev Xlate failed")
            # make it fail
            assert False == True

        return

- Description for the changelog
This package includes python yang libraries which will be used with sonic utilities
pacakge to validate the config. This python libraries are written on top of libyang
and also provides functionality to translate the config from SONiC ConfigDB to SONiC
YANG and vice-versa.

- A picture of a cute animal (not mandatory but encouraged)
I can put Microsoft logo :). Thx

@praveen-li
Copy link
Collaborator Author

retest this please

@praveen-li praveen-li force-pushed the build_sonic_yang_mgmt branch 3 times, most recently from 3267568 to 9c3dc09 Compare December 11, 2019 06:09
@li-pingmao li-pingmao mentioned this pull request Dec 11, 2019
@renukamanavalan renukamanavalan self-assigned this Dec 17, 2019
Copy link
Contributor

@renukamanavalan renukamanavalan left a comment

Choose a reason for hiding this comment

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

This is where the SONiC Yang files are to be checked in.
Curious, why there is none,

rules/sonic-yang-mgmt-py2.mk Outdated Show resolved Hide resolved
src/sonic-yang-mgmt/setup.py Outdated Show resolved Hide resolved
src/sonic-yang-mgmt/setup.py Outdated Show resolved Hide resolved
@praveen-li
Copy link
Collaborator Author

This is where the SONiC Yang files are to be checked in.
Curious, why there is none,

@renuka

This PR has basic build steps, Code and YANG files comes with PR 3730, 3784 & 3891.

@svc-acs
Copy link
Collaborator

svc-acs commented Jan 22, 2020

Build started for merge commit.

@praveen-li praveen-li force-pushed the build_sonic_yang_mgmt branch 2 times, most recently from ff9e266 to bf6f677 Compare March 20, 2020 22:31
@praveen-li praveen-li changed the title [sonic-yang-mgmt]: Basic build infra for sonic-yang-mgmt package. [sonic-yang-mgmt]: a.)Dev code, b.) build time test code and c.) wheel package Makefiles for sonic-yang-mgmt package. Mar 20, 2020
@praveen-li
Copy link
Collaborator Author

praveen-li commented Apr 3, 2020 via email

@lgtm-com
Copy link

lgtm-com bot commented Apr 7, 2020

This pull request introduces 17 alerts when merging 2eaba1c8fd2fd70e93f5dccac20572cc12d43acf into 711445c - view on LGTM.com

new alerts:

  • 7 for Unused local variable
  • 5 for Unused import
  • 2 for Illegal raise
  • 1 for Testing equality to None
  • 1 for 'import *' may pollute namespace
  • 1 for Module is imported with 'import' and 'import from'

@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2020

This pull request introduces 17 alerts when merging 1246ce4e7d24a070b8b7a9c7df7ff34753d9fb25 into de5a04a - view on LGTM.com

new alerts:

  • 7 for Unused local variable
  • 5 for Unused import
  • 2 for Illegal raise
  • 1 for Testing equality to None
  • 1 for 'import *' may pollute namespace
  • 1 for Module is imported with 'import' and 'import from'

@lgtm-com
Copy link

lgtm-com bot commented Apr 9, 2020

This pull request introduces 1 alert when merging a065d2a6c5a05557a79f049ce5737909bd4a47cb into 7405f8c - view on LGTM.com

new alerts:

  • 1 for Illegal raise

@lgtm-com
Copy link

lgtm-com bot commented Apr 9, 2020

This pull request introduces 1 alert when merging 1b8977d910f60ff561179b511e8a5a62cb3e71b6 into 7405f8c - view on LGTM.com

new alerts:

  • 1 for Illegal raise

@praveen-li praveen-li force-pushed the build_sonic_yang_mgmt branch 3 times, most recently from 67f0028 to fb26bce Compare April 15, 2020 18:54
li-pingmao and others added 3 commits May 8, 2020 12:14
Changes:
1.) prefix test_ for sample yang models.
2.) name properly with data_node or schema_node.
3.) Update function calls after libyang PLY APIs updates.
4.) Fix test cases after sample yang files name changes.
…ig for the test.

Changes:
1.) All the YANG models are not merged yet, so test config has tables with no YANG,
allow this scenario for tests as of now.
…oving reference to sonic.

Changes:
1.) To avoid confusion, Changing yang models to generic and removing reference to sonic
2.) Additing Relative path whenever needed.
@praveen-li
Copy link
Collaborator Author

retest vsimage please

1 similar comment
@praveen-li
Copy link
Collaborator Author

retest vsimage please

@praveen-li
Copy link
Collaborator Author

praveen-li commented May 11, 2020

05/11/2020 Review:
AI:Make the function private and only the function which are called from outside should be public.

Praveen Chaudhary added 2 commits May 13, 2020 11:46
…user warning.

Changes:
1.) Store Tables with YANG models seperately, so that a warning to user
can be issued with enough information.
2.) print the tables without YANG models properly in cropConfigDB().

Signed-off-by: Praveen Chaudhary [email protected]
…ly for user warning.

Changes:
1.) Added config for extra tables in yangTest.json.
2.) Added new test case to find tables without yang models.
3.) Move common part in a helper function.

Signed-off-by: Praveen Chaudhary [email protected]
@praveen-li
Copy link
Collaborator Author

praveen-li commented May 13, 2020

@renukamanavalan, @lguohan, @zhenggen-xu

Latest commit to address below review comments:

    [sonic_yang.py]: Categories public and private functions.

    Changes:

    1.) Categories public and private functions.
    2.) Minor spaces related changes.
    3.) Defining class SonicYangException and using it in all public functions.
    4.) Changing Class names to PascalCase.
    5.) Keeping functions, which deals with Libyang, in lower_underscore_case, rest functions names are in camelCase.

Kindly let me know if any other concerns. Thanks a lot for the review.

@lgtm-com
Copy link

lgtm-com bot commented May 13, 2020

This pull request introduces 1 alert when merging d450aec234ab6f47b30b3b4e61b23e3448f1d9e8 into 0542afb - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented May 13, 2020

This pull request introduces 1 alert when merging 0466e8272b0a3ab853e4b8d8484499a8dc8e4f5c into 0542afb - view on LGTM.com

new alerts:

  • 1 for Unused import

Changes:

1.) Categories public and private functions.
2.) Minor spaces related changes.
3.) Defining class SonicYangException and using it in all public functions.
4.) Changing Class names to PascalCase.
5.) Keeping functions, which deals with Libyang, in lower_underscore_case, rest functions names are in camelCase.
@praveen-li
Copy link
Collaborator Author

retest vsimage please

@praveen-li
Copy link
Collaborator Author

@renukamanavalan, @lguohan, @zhenggen-xu

All tests passed, Kindly review. Thanks a lot.

@renukamanavalan
Copy link
Contributor

renukamanavalan commented May 14, 2020

Overall concern -- based on our discussion on this PR.

This PR adds additional validation for load & reload. With neither update, nor save guarded by the same validation, there is a good probable window to get a saved .JSON, which can't be loaded back.

This can lead to inconsistent state and difficulties ahead.

As we have SWSS sdk, as the center point for all updates, we have a single point for validation.

Request: Please make this as a priority to address in future PRs, ASAP.

@praveen-li
Copy link
Collaborator Author

Overall concern -- based on our discussion on this PR.

This PR adds additional validation for load & reload. With neither update, nor save guarded by the same validation, there is a good probable window to get a saved .JSON, which can't be loaded back.

This can lead to inconsistent state and difficulties ahead.

As we have SWSS sdk, as the center point for all updates, we have a single point for validation.

Request: Please make this as a priority to address in future PRs, ASAP.

@renukamanavalan @lguohan @zhenggen-xu
Sure, That will be my next target. At SWSS SDK level, I need to find whether the user is trying to update the config or trying to add an entire new config. The rest should be doable. And The testing effort will be huge.

Note: As of now, I had to remove config validation while config load\reload, because I notice multiple ASIC files change. So I would like to know first, whether we need validation for each file or not ?, Please let me know if you can help me here.

@lguohan lguohan changed the title [sonic-yang-mgmt]: a.)Dev code, b.) build time test code and c.) wheel package Makefiles for sonic-yang-mgmt package. [sonic-yang-mgmt]: sonic-yang-mgmt package for configuration validation. May 21, 2020
@lguohan lguohan merged commit 0ccdd70 into sonic-net:master May 21, 2020
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.

6 participants