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.

Sign up for GitHub

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

Jump to bottom

[ssip]: Add YANG model #10992

Merged
merged 6 commits into from
Jun 23, 2022
Merged

[ssip]: Add YANG model #10992

merged 6 commits into from
Jun 23, 2022
+202 −9

Conversation

nazariig
Copy link
Collaborator

@nazariig nazariig commented May 31, 2022 •
edited by liat-grozovik
Loading

HLD: sonic-net/SONiC#1002

Why I did it

  • To implement Syslog Source IP feature

How I did it

  • Added the relevant yang model

How to verify it

  • Added unit test

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

  • N/A

Link to config_db schema for YANG module changes

  • N/A

A picture of a cute animal (not mandatory but encouraged)

      .---.        .-----------
     /     \  __  /    ------
    / /     \(  )/    -----
   //////   ' \/ `   ---
  //// / // :    : ---
 // /   /  /`    '--
//          //..\\
       ====UU====UU====
           '//||\\`
             ''``

@nazariig nazariig requested a review from qiluo-msft as a code owner May 31, 2022 18:13
@nazariig nazariig mentioned this pull request May 31, 2022
Merged
@nazariig nazariig added the YANG YANG model related changes label May 31, 2022
@nazariig
Copy link
Collaborator Author

nazariig commented Jun 1, 2022

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@nazariig
Copy link
Collaborator Author

nazariig commented Jun 1, 2022

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

venkatmahalingam
venkatmahalingam reviewed Jun 14, 2022
View reviewed changes
src/sonic-yang-models/yang-models/sonic-syslog.yang

} /* end of container SYSLOG_SERVER */
leaf source {
description "Syslog source IP address";
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens when the source IP is not configured within a VRF (say. mgmt or data VRF)? rsyslog takes available IP from the kernel and send the log messages?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@venkatmahalingam the behavior is according to the HLD: CLI will check IP existence and error will be raised in case it missing

Copy link
Collaborator

Choose a reason for hiding this comment

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

SONIC YANG model will be used for validating the configs present in json file for config load/reload, so YANG validation is important.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@venkatmahalingam such kind of validation is not possible with the current DB schema and YANG approach since we don't reference a configured IPs. Also not sure if it's even possible to add such a constraint in current YANG model to validate whether interface IP is configured across the specific VRF. This brings a lot of complexity. Any suggestions are appreciated.

dgsudharsan
dgsudharsan suggested changes Jun 14, 2022
View reviewed changes
Copy link
Collaborator

@dgsudharsan dgsudharsan left a comment

Choose a reason for hiding this comment

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

Can you please update https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md as well?

src/sonic-yang-models/yang-models/sonic-syslog.yang Show resolved Hide resolved
@ganglyu
Copy link
Contributor

ganglyu commented Jun 15, 2022

would you please add the link for HLD?

@nazariig
Copy link
Collaborator Author

would you please add the link for HLD?

@ganglyu done

@nazariig
[ssip]: Update YANG model.
d625213 
Signed-off-by: Nazarii Hnydyn <[email protected]>
@nazariig
[ssip]: YANG model rev01.
3f1446e 
Signed-off-by: Nazarii Hnydyn <[email protected]>
@nazariig
[ssip]: YANG model rev02.
3582940 
Signed-off-by: Nazarii Hnydyn <[email protected]>
@nazariig
[ssip]: YANG model rev03.
1e4781c 
Signed-off-by: Nazarii Hnydyn <[email protected]>
@nazariig
[ssip]: Add YANG UTs.
956301a 
Signed-off-by: Nazarii Hnydyn <[email protected]>
@nazariig
[ssip]: YANG UTs rev01.
1a7eb8f 
Signed-off-by: Nazarii Hnydyn <[email protected]>
@nazariig
Copy link
Collaborator Author

Can you please update https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md as well?

@dgsudharsan will do

@qiluo-msft qiluo-msft requested a review from wen587 June 20, 2022 06:10
@liat-grozovik liat-grozovik merged commit 9b59ee0 into sonic-net:master Jun 23, 2022
@nazariig
Copy link
Collaborator Author

nazariig commented Jul 6, 2022

Can you please update https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md as well?

@dgsudharsan done: #11363

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@venkatmahalingam venkatmahalingam venkatmahalingam left review comments

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@dgsudharsan dgsudharsan dgsudharsan approved these changes

@zhangyanzhao zhangyanzhao Awaiting requested review from zhangyanzhao

@liat-grozovik liat-grozovik Awaiting requested review from liat-grozovik

@volodymyrsamotiy volodymyrsamotiy Awaiting requested review from volodymyrsamotiy

@ganglyu ganglyu Awaiting requested review from ganglyu

@wen587 wen587 Awaiting requested review from wen587

Assignees
No one assigned
Labels
YANG YANG model related changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@nazariig @mssonicbld @ganglyu @qiluo-msft @dgsudharsan @venkatmahalingam @liat-grozovik