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

[yang-models] Importing other modules sometimes break libyang backlinks #9312

Open
ghooo opened this issue Nov 18, 2021 · 10 comments
Open

[yang-models] Importing other modules sometimes break libyang backlinks #9312

ghooo opened this issue Nov 18, 2021 · 10 comments
Labels
Help Wanted 🆘 Triaged this issue has been triaged YANG YANG model related changes

Comments

@ghooo
Copy link
Contributor

ghooo commented Nov 18, 2021

Description

Importing other modules sometimes break libyang backlinks, which results in break DPB and generic-config-updater unit-tests in sonic-utilities

The build fails and the author of YANG model end up commenting out the the import and also removing the corresponding YANG validation.

Example from sonic-bgp-neighbor.yang

import commented

    // Comment sonic-vlan import here until libyang back-links issue is resolved for VLAN leaf reference.
    //import sonic-vlan {
    //    prefix vlan;
    //}

validation commented

                leaf neighbor {
                    type union {
                        type inet:ip-address;
                        type leafref {
                            path "/port:sonic-port/port:PORT/port:PORT_LIST/port:name";
                        }
                        type leafref {
                            path "/lag:sonic-portchannel/lag:PORTCHANNEL/lag:PORTCHANNEL_LIST/lag:name";
                        }
                       // Comment VLAN leaf reference here until libyang back-links issue is resolved and use VLAN string pattern
                       // type leafref {
                       //     path "/vlan:sonic-vlan/vlan:VLAN/vlan:VLAN_LIST/vlan:name";
                       // }
                        type string {
                            pattern 'Vlan([0-9]{1,3}|[1-3][0-9]{3}|[4][0][0-8][0-9]|[4][0][9][0-4])';
                        }
                    }
                    description "BGP Neighbor, it will be neighbor address or interface name";
                }

The issue can be found in multiple YANG files, retrieve them by: https://github.com/Azure/sonic-buildimage/search?q=back-links+issue

I believe existing backlinks might be broken for some models, as the unit-tests are not covering all cases

Steps to reproduce the issue:

  1. edit sonic-bgp-neighbor.yang
  2. uncomment the import and the validation
  3. submit a PR, and wait for build

Describe the results you received:

Unit-tests failure in DPB and generic-config-updater unit-tests from sonic-utilities repo

Describe the results you expected:

Unit-tests to pass

@ghooo ghooo added the YANG YANG model related changes label Nov 18, 2021
@venkatmahalingam
Copy link
Collaborator

@praveen-li We commented few YANG fields to avoid the build failures in the past as we had the issue with the libyang back-links code, please update the latest on this, do we have any solution to the libyang backlinks issue and we can uncomment the YANG fields now?

// Comment sonic-vlan import here until libyang back-links issue is resolved for VLAN leaf reference.

@zhangyanzhao
Copy link
Collaborator

not blocking issue

@praveen-li
Copy link
Collaborator

qiluo-msft pushed a commit that referenced this issue Feb 21, 2022
#### Why I did it

1. Fix Build exception [example](https://dev.azure.com/mssonic/build/_build/results?buildId=73911&view=logs&jobId=88ce9a53-729c-5fa9-7b6e-3d98f2488e3f&j=cef3d8a9-152e-5193-620b-567dc18af272&t=ac3bce9f-b126-5a26-3fee-28ce0ec1679d)

```
2022-02-19T01:54:23.4200556Z ImportError: cannot import name 'soft_unicode' from 'markupsafe' (/usr/local/lib/python3.8/dist-packages/markupsafe/__init__.py)
```

This is because Jinja2 uses MarkupSafe without specifying an upper limit to the version, MarkupSafe version that was released today removed 'soft_unicode'. So now Jinja2 is complaining.

Related issues:
pallets/jinja#1591
aws/aws-sam-cli#3661


2. Reverts #9136

Fixing build failures in SONiC utils [example](https://dev.azure.com/mssonic/build/_build/results?buildId=73784&view=logs&jobId=83516c17-6666-5250-abde-63983ce72a49&j=83516c17-6666-5250-abde-63983ce72a49&t=6177235f-d4f1-5f72-835a-90ebb93a1784)

One of the errors:
```
 TestPathAddressing.test_find_ref_paths__ref_is_the_whole_key__returns_ref_paths 

self = <tests.generic_config_updater.gu_common_test.TestPathAddressing testMethod=test_find_ref_paths__ref_is_the_whole_key__returns_ref_paths>

    def test_find_ref_paths__ref_is_the_whole_key__returns_ref_paths(self):
        # Arrange
        path = "/PORT/Ethernet0"
        expected = [
            "/ACL_TABLE/NO-NSW-PACL-V4/ports/0",
            "/VLAN_MEMBER/Vlan1000|Ethernet0",
        ]
    
        # Act
        actual = self.path_addressing.find_ref_paths(path, Files.CROPPED_CONFIG_DB_AS_JSON)
    
        # Assert
>       self.assertEqual(expected, actual)
E       AssertionError: Lists differ: ['/ACL_TABLE/NO-NSW-PACL-V4/ports/0', '/VLAN_MEMBER/Vlan1000|Ethernet0'] != ['/ACL_TABLE/NO-NSW-PACL-V4/ports/0']
E       
E       First list contains 1 additional elements.
E       First extra element 1:
E       '/VLAN_MEMBER/Vlan1000|Ethernet0'
E       
E       - ['/ACL_TABLE/NO-NSW-PACL-V4/ports/0', '/VLAN_MEMBER/Vlan1000|Ethernet0']
E       + ['/ACL_TABLE/NO-NSW-PACL-V4/ports/0']
```

The VLAN_MEMBER backlink (can be called referrer link or ref link) is not found.

Issue introduced by #9136
I don't know how this PR passed the build system, it should have failed.

Known YANG issue #9312

#### How I did it
The import to `sonic-vlan` is breaking the build
```
    import sonic-vlan {
        prefix vlan;
    }
```

I am not sure if that's the only issue, so I think reverting the whole PR should be the safer option.

#### How to verify it
Ran sonic-utils tests locally.
@zhangyanzhao
Copy link
Collaborator

Need @li-pingmao help to fix #9136

@zhangyanzhao zhangyanzhao added the Triaged this issue has been triaged label Mar 31, 2022
@zhangyanzhao
Copy link
Collaborator

Praveen will talk to Ping internally.

@zhangyanzhao
Copy link
Collaborator

@li-pingmao would you please join next YANG meeting to provide an update? Thanks.

@ghooo
Copy link
Contributor Author

ghooo commented Jul 25, 2022

Related PR: #9545

@li-pingmao
Copy link
Contributor

@li-pingmao would you please join next YANG meeting to provide an update? Thanks.

Yes, I will join this Thursday's meeting

@zhangyanzhao
Copy link
Collaborator

@li-pingmao need your help to update on this. Thanks.

@Blueve
Copy link
Contributor

Blueve commented Aug 31, 2023

@zhangyanzhao @qiluo-msft @ganglyu this issue has lasting for long time.
We need triage this issue, otherwise we cannot have a solid yang validation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help Wanted 🆘 Triaged this issue has been triaged YANG YANG model related changes
Projects
None yet
Development

No branches or pull requests

6 participants