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 fixed to make config apply-patch work #9517

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

MaratGubaiev
Copy link
Contributor

Why I did it

fixing sonic-net/sonic-utilities#1935 Running "sudo config apply-patch ..." in latest (as of 11/15) master image applied in SONiC switches fails with inability to parse config for given yang models.

Note that the updates described in the issue are not actual any more because of new changes in the yang models.

How I did it

I improved yang models to make them correspond to config.

TODO

"collector_vrf" type is set to "string". It should be more accurate type.

How to verify it

Perform sudo config apply-patch <some_patch>.json-patch
Patch should be applied and there should be no errors*.

Example of a dummy patch
[
    { "op": "replace", "path": "/PORT/Ethernet124/admin_status", "value": "up" }
]

Example of an empty patch
[]

NOTE that errors might be caused by improper config. E.g. I changed
    "ACL_TABLE": {
        "1": {
            "policy_desc": "Mirror Session",
            "ports": "Ethernet16",
            "stage": "INGRESS",
            "type": "MIRROR"
        },
        "2": {
            "policy_desc": "Mirror Session",
            "ports": "Ethernet16,Ethernet20",
            "stage": "INGRESS",
            "type": "MIRROR"
        },
.........................................
.........................................
.........................................

to

    "ACL_TABLE": {
        "1": {
            "policy_desc": "Mirror Session",
            "ports": ["Ethernet16"],
            "stage": "INGRESS",
            "type": "MIRROR"
        },
        "2": {
            "policy_desc": "Mirror Session",
            "ports": ["Ethernet16", "Ethernet20"],
            "stage": "INGRESS",
            "type": "MIRROR"
        },
.........................................
.........................................
.........................................

to avoid libyang error

libyang[0]: Duplicated instance of "ports" leaf-list ("e"). (path: /sonic-acl:sonic-acl/ACL_TABLE/ACL_TABLE_LIST[ACL_TABLE_NAME='1']/ports[.='e'])
sonic_yang(3):Data Loading Failed:Duplicated instance of "ports" leaf-list ("e").

Signed-off-by: Marat Gubaiev <marat_gubaiev@jabil.com>
Copy link
Contributor

@ganglyu ganglyu left a comment

Choose a reason for hiding this comment

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

Please add UT for these changes.

@@ -404,6 +404,46 @@ module sonic-crm {
type threshold;
}

leaf srv6_my_sid_entry_threshold_type{
Copy link
Collaborator

Choose a reason for hiding this comment

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

   [](http://example.com/codeflow?start=0&length=2)

Mixing tabs and spaces in the same file. We prefer 4 spaces than 1 tab.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@qiluo-msft qiluo-msft added the YANG YANG model related changes label Jan 13, 2022
@qiluo-msft qiluo-msft added this to To be reviewed in yang via automation Jan 13, 2022
qiluo-msft
qiluo-msft previously approved these changes Jan 13, 2022
Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

LGTM. Please check with other reviewers.

yang automation moved this from To be reviewed to Reviewer approved Jan 13, 2022
@qiluo-msft qiluo-msft dismissed their stale review January 13, 2022 19:24

Sorry, dismiss. Wrong page.

yang automation moved this from Reviewer approved to Review in progress Jan 13, 2022
@MaratGubaiev
Copy link
Contributor Author

Please add UT for these changes.

done

}

leaf srv6_nexthop_low_threshold {
type threshold;
Copy link
Contributor

Choose a reason for hiding this comment

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

please check indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

yang automation moved this from Review in progress to Reviewer approved May 16, 2022
@zhangyanzhao
Copy link
Collaborator

@MaratGubaiev can you please fix the build failure? So that we can merge. Thanks.

@zhangyanzhao
Copy link
Collaborator

@MaratGubaiev can you please help to sign the EasyCLA and address the conflicts? Thanks.

@zhangyanzhao
Copy link
Collaborator

Long time no response from the submitter, we will duplicate a new PR to track and close this one.

@zhangyanzhao
Copy link
Collaborator

@qiluo-msft will help to create a new PR. Thanks.

@zhangyanzhao zhangyanzhao added the Triaged this issue has been triaged label Mar 7, 2024
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
yang
Reviewer approved
Development

Successfully merging this pull request may close these issues.

None yet

4 participants