upstream snmp sonic-yang files #10828

suresh-rupanagudi · 2022-05-13T15:01:09Z

Fix #10549
Fix #10550

Why I did it

Create sonic yang model for SNMP
Tables:SNMP, SNMP_COMMUNITY

How I did it

Defined yang models based for SNMP based on snmp.yml

How to verify it

Added test cases to verify

ganglyu · 2022-05-14T00:01:13Z

src/sonic-yang-models/tests/yang_model_tests/tests_config/snmp.json

+        "sonic-snmp:sonic-snmp": {
+            "sonic-snmp:SNMP_COMMUNITY": {
+                "SNMP_COMMUNITY_LIST": [
+                {


Please check indentation

taken care. Please see new changes

ganglyu · 2022-05-14T00:05:19Z

src/sonic-yang-models/yang-models/sonic-snmp.yang

+          description
+            "SNMP System Contact.";
+        }
+	  }


Please replace tab

Taken care.

ganglyu · 2022-05-14T00:09:52Z

src/sonic-yang-models/yang-models/sonic-snmp.yang

+      container CONTACT {
+        leaf Contact {
+          type string {
+             length "1..255";


not to allow configuration of empty string.

liushilongbuaa · 2022-05-17T11:06:06Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2022-05-17T11:06:19Z

Azure Pipelines successfully started running 1 pipeline(s).

qiluo-msft · 2022-05-17T21:52:54Z

src/sonic-yang-models/tests/files/sample_config_db.json

+                "Contact": "[email protected]"
+            },
+            "LOCATION": {
+                "Location": "Sonic Location"


Sonic

It is not invented by SONiC. Let's call it "SNMP Server Location" #Closed

qiluo-msft · 2022-05-17T21:55:04Z

src/sonic-yang-models/tests/files/sample_config_db.json

+
+        "SNMP": {
+            "CONTACT": {
+                "Contact": "[email protected]"


[email protected]

Let's use [email protected] which is more safe. #Closed

suresh-rupanagudi · 2022-05-19T00:04:27Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2022-05-19T00:04:32Z

Commenter does not have sufficient privileges for PR 10828 in repo Azure/sonic-buildimage

suresh-rupanagudi · 2022-05-19T00:06:09Z

/azpw run Azure.sonic-buildimage

mssonicbld · 2022-05-19T00:06:11Z

/AzurePipelines run Azure.sonic-buildimage

azure-pipelines · 2022-05-19T00:06:21Z

Azure Pipelines successfully started running 1 pipeline(s).

qiluo-msft · 2022-06-17T21:17:06Z

Could you help resolve the conflict, and merge latest master if not following it?

…uildimage into snmp_yang_upstream1

qiluo-msft · 2022-06-23T18:30:32Z

Could you merge master again and resolve the conflicts?

suresh-rupanagudi · 2022-07-18T04:30:26Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2022-07-18T04:30:31Z

Commenter does not have sufficient privileges for PR 10828 in repo Azure/sonic-buildimage

suresh-rupanagudi · 2022-07-18T04:31:32Z

/azpw run Azure.sonic-buildimage

mssonicbld · 2022-07-18T04:31:34Z

/AzurePipelines run Azure.sonic-buildimage

azure-pipelines · 2022-07-18T04:31:46Z

Azure Pipelines successfully started running 1 pipeline(s).

wen587 · 2022-07-28T08:10:44Z

Hi @suresh-rupanagudi , I saw the test always fail on GCU test.

Error: Given patch will produce invalid config. Error: Data Loading Failed\nAll Keys are not parsed in SNMP_COMMUNITY\ndict_keys(['public'])\nexceptionList:[\"'TYPE'\"]",

Here is the reason:
In your defined YANG MODEL, the leaf type is lowercase. https://github.com/Azure/sonic-buildimage/blob/8bd08d2658bf28524cec08da7138ba630c22f4dd/src/sonic-yang-models/yang-models/sonic-snmp.yang#L63

But in SONiC default config, the type is by default uppercase. Below is the example of my SONiC config.

    "SNMP_COMMUNITY": {
        "public": {
            "TYPE": "RO"                    <=uppercase `TYPE`
        }
    }

I think the default SONiC SNMP config need to be updated.

suresh-rupanagudi · 2022-07-28T10:27:54Z

Thanks JingWeng, Gang for looking into this.
1)why are we not getting this error when we build "sonic-broadcom.bin" locally?.
What needs to be done to run this locally?.

Now solution is
1)to change "type" to "TYPE" in yang file. OR
2)change "TYPE" to "type" in snmp_yml_to_configdb.py file. We might also need to take care all the tests where community is used.
Yang standard allows defining identifier with capital letters.
But I could not see any yang module using this.
Please let me know

From rfc8407.

"Identifiers SHOULD follow a consistent naming pattern throughout the
module. Only lowercase letters, numbers, and dashes SHOULD be used
in identifier names. Uppercase characters, the period character, and
the underscore character MAY be used if the identifier represents a
well-known value that uses these characters. YANG does not permit
any other characters in YANG identifiers."

ganglyu · 2022-07-28T10:37:39Z

Pipeline will run basic test, so we can detect errors before merging PR. Please follow sonic-mgmt/docs at master * sonic-net/sonic-mgmt (github.com)<https://github.com/sonic-net/sonic-mgmt/tree/master/docs> to setup testbed and run tests. I think modify yang file will be easier, and sonic-acl.yang is using capital letters. BR Gang From: suresh-rupanagudi ***@***.***> Sent: Thursday, July 28, 2022 6:28 PM To: Azure/sonic-buildimage ***@***.***> Cc: Gang Lv ***@***.***>; Review requested ***@***.***> Subject: Re: [Azure/sonic-buildimage] upstream snmp sonic-yang files (PR #10828) Thanks JingWeng, Gang for looking into this. 1)why are we not getting this error when we build "sonic-broadcom.bin" locally?. What needs to be done to run this locally?. Now solution is 1)to change "type" to "TYPE" in yang file. OR 2)change "TYPE" to "type" in snmp_yml_to_configdb.py file. We might also need to take care all the tests where community is used. Yang standard allows defining identifier with capital letters. But I could not see any yang module using this. Please let me know From rfc8407. "Identifiers SHOULD follow a consistent naming pattern throughout the module. Only lowercase letters, numbers, and dashes SHOULD be used in identifier names. Uppercase characters, the period character, and the underscore character MAY be used if the identifier represents a well-known value that uses these characters. YANG does not permit any other characters in YANG identifiers." - Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzure%2Fsonic-buildimage%2Fpull%2F10828%23issuecomment-1197959503&data=05%7C01%7Cganglv%40microsoft.com%7C29e097283f4a48313a4308da7083dc86%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637946008927605692%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HmqqTHo4pGYHDzBlShkH1ybkTNQXtmVlEbZlNmrWBwA%3D&reserved=0>, or unsubscribe<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAVG7POSUY65BVMXHPVSSQXTVWJODNANCNFSM5V3UZPDQ&data=05%7C01%7Cganglv%40microsoft.com%7C29e097283f4a48313a4308da7083dc86%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637946008927761915%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=TTTA263%2FIHuNbLBmciSx2Xh1yiQeh46x1rQFEW%2Bz9jc%3D&reserved=0>. You are receiving this because your review was requested.Message ID: ***@***.***>

wen587 · 2022-08-11T02:35:14Z

Thanks JingWeng, Gang for looking into this. 1)why are we not getting this error when we build "sonic-broadcom.bin" locally?. What needs to be done to run this locally?.

1)Because it doesn't have any test failures during the image build.
It failed during the sonic-mgmt test, because the actual config is not as that as we defined.

For example, you define the SNMP_COMMUNITY['public']['type'] with RO or RW, so you just test the config here with

        "SNMP_COMMUNITY": {
            "public": {
                "type": "RO"           <===== lowercase 'type'
            },

However it tested real machine during sonic-mgmt test and found that the real config doesn't validate SNMP_COMMUNITY's YANG. Because the real config is

    "SNMP_COMMUNITY": {
        "public": {
            "TYPE": "RO"             <===== uppercase 'TYPE'
        }
    },

What needs to be done to run this locally?.

I think you can modify the config here and change type to TYPE. It also need related YANG model update.

suresh-rupanagudi · 2022-08-16T06:25:37Z

/azpw run Azure.sonic-buildimage

mssonicbld · 2022-08-16T06:25:38Z

/AzurePipelines run Azure.sonic-buildimage

azure-pipelines · 2022-08-16T06:25:48Z

Azure Pipelines successfully started running 1 pipeline(s).

suresh-rupanagudi · 2022-08-17T04:42:48Z

Hi jingwenxie,
Can you please look at failures.

yxieca · 2022-08-26T21:02:25Z

@suresh-rupanagudi this change cannot be cherrypicked to 202205 cleanly. Please create new PR for 202205 branch.

yxieca · 2023-01-19T17:19:59Z

@suresh-rupanagudi, @zhangyanzhao, can you help create separate PR for 202205 branch? If the change has been done. Please link the 202205 PR to this PR so that I can close the cherry-picking request.

suresh-rupanagudi · 2023-01-23T14:28:52Z

ETA for PR on 202205 branch is 2/20/2023

yxieca · 2023-02-02T19:31:43Z

@suresh-rupanagudi is this PR not needed for 202211 branch?

suresh-rupanagudi · 2023-02-21T10:38:45Z

Hi,
Created new PR on 202205
#13896

Not sure if this PR required for 202211 branch. Please cherry pick changes if required.

suresh-rupanagudi requested a review from qiluo-msft as a code owner May 13, 2022 15:01

zhangyanzhao mentioned this pull request May 13, 2022

[yang] Need Yang for SNMP table #10549

Closed

qiluo-msft requested review from ganglyu and tsvanduyn May 13, 2022 16:42

adyeung mentioned this pull request May 13, 2022

[yang] Need Yang for SNMP_COMMUNITY table #10550

Closed

ganglyu reviewed May 14, 2022

View reviewed changes

ganglyu previously approved these changes May 16, 2022

View reviewed changes

ganglyu self-requested a review May 16, 2022 07:27

upstream snmp sonic-yang files

f89ac89

suresh-rupanagudi dismissed ganglyu’s stale review via f89ac89 May 17, 2022 06:00

suresh-rupanagudi force-pushed the snmp_yang_upstream1 branch from eff7abf to f89ac89 Compare May 17, 2022 06:00

qiluo-msft reviewed May 17, 2022

View reviewed changes

ganglyu previously approved these changes May 18, 2022

View reviewed changes

Changed SNMP Contact and Location values as per review comments.

cdca94b

suresh-rupanagudi dismissed ganglyu’s stale review via cdca94b May 19, 2022 00:02

ganglyu approved these changes May 19, 2022

View reviewed changes

qiluo-msft approved these changes May 19, 2022

View reviewed changes

Merge branch 'master' of https://github.com/suresh-rupanagudi/sonic-b…

e870e19

…uildimage into snmp_yang_upstream1

Changed type to TYPE.

a603db4

suresh-rupanagudi mentioned this pull request Aug 16, 2022

[YANG] Add missing SNMP, SNMP_COMMUNITY YANG models #11695

Closed

7 tasks

wen587 approved these changes Aug 16, 2022

View reviewed changes

wen587 added the Request for 202205 Branch label Aug 17, 2022

qiluo-msft merged commit 2bb8306 into sonic-net:master Aug 17, 2022

yxieca added the Cherry Pick Conflict_202205 label Aug 26, 2022

yxieca added the Request for 202111 Branch For PRs being requested for 202111 branch label Feb 2, 2023

yxieca added Request for 202205 Branch and removed Request for 202205 Branch Request for 202111 Branch For PRs being requested for 202111 branch labels Feb 2, 2023

yxieca added the Included in 202205 Branch label Feb 21, 2023

upstream snmp sonic-yang files #10828

upstream snmp sonic-yang files #10828

Conversation

suresh-rupanagudi commented May 13, 2022 • edited by ganglyu Loading

Why I did it

How I did it

How to verify it

ganglyu May 14, 2022 • edited Loading

Choose a reason for hiding this comment

suresh-rupanagudi May 17, 2022

Choose a reason for hiding this comment

ganglyu May 14, 2022

Choose a reason for hiding this comment

suresh-rupanagudi May 17, 2022

Choose a reason for hiding this comment

ganglyu May 14, 2022

Choose a reason for hiding this comment

suresh-rupanagudi May 16, 2022

Choose a reason for hiding this comment

liushilongbuaa commented May 17, 2022

azure-pipelines bot commented May 17, 2022

qiluo-msft May 17, 2022 • edited Loading

Choose a reason for hiding this comment

qiluo-msft May 17, 2022 • edited Loading

Choose a reason for hiding this comment

suresh-rupanagudi commented May 19, 2022

azure-pipelines bot commented May 19, 2022

suresh-rupanagudi commented May 19, 2022

mssonicbld commented May 19, 2022

azure-pipelines bot commented May 19, 2022

qiluo-msft commented Jun 17, 2022

qiluo-msft commented Jun 23, 2022

suresh-rupanagudi commented Jul 18, 2022

azure-pipelines bot commented Jul 18, 2022

suresh-rupanagudi commented Jul 18, 2022

mssonicbld commented Jul 18, 2022

azure-pipelines bot commented Jul 18, 2022

wen587 commented Jul 28, 2022 • edited Loading

suresh-rupanagudi commented Jul 28, 2022

From rfc8407.

ganglyu commented Jul 28, 2022 via email

wen587 commented Aug 11, 2022

suresh-rupanagudi commented Aug 16, 2022

mssonicbld commented Aug 16, 2022

azure-pipelines bot commented Aug 16, 2022

suresh-rupanagudi commented Aug 17, 2022

yxieca commented Aug 26, 2022

yxieca commented Jan 19, 2023

suresh-rupanagudi commented Jan 23, 2023

yxieca commented Feb 2, 2023

suresh-rupanagudi commented Feb 21, 2023

suresh-rupanagudi commented May 13, 2022 •

edited by ganglyu

Loading

ganglyu May 14, 2022 •

edited

Loading

qiluo-msft May 17, 2022 •

edited

Loading

qiluo-msft May 17, 2022 •

edited

Loading

wen587 commented Jul 28, 2022 •

edited

Loading