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

[yang] syslog yang model #9265

Merged
merged 6 commits into from
Dec 7, 2021
Merged

[yang] syslog yang model #9265

merged 6 commits into from
Dec 7, 2021

Conversation

arlakshm
Copy link
Contributor

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan arlakshm@microsoft.com

Why I did it

Add yang model for syslog server

How I did it

Add new file sonic-syslog.yang and new files for tests

How to verify it

Compile target/python-wheels/sonic_yang_mgmt-1.0-py3-none-any.whl

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

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

@arlakshm
syslog yang model
e620bcf 
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
@arlakshm arlakshm requested a review from lguohan as a code owner November 15, 2021 20:05
@arlakshm arlakshm added the YANG YANG model related changes label Nov 15, 2021
@wen587 wen587 self-requested a review November 18, 2021 07:35
@wen587
Copy link
Contributor

wen587 commented Nov 18, 2021

Hi, could you add a NEG_TEST for invalid IPv4 address such as 1000.0.0.5
I tested locally but I don't know why it doesn't block invalid IPv4 address.

@arlakshm
address review comments
9c30226 
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
@wen587 wen587 mentioned this pull request Nov 19, 2021
Merged
4 tasks
@arlakshm
address review comments
792dd2c 
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
@arlakshm
Copy link
Contributor Author

Hi, could you add a NEG_TEST for invalid IPv4 address such as 1000.0.0.5 I tested locally but I don't know why it doesn't block invalid IPv4 address.

@wen587, I have updated the constraint and test in the latest commit.

@wen587
Copy link
Contributor

wen587 commented Nov 25, 2021

Hi @arlakshm , thanks for the update.
Is it possible to support ipv6 syslog server in yang? Seems config syslog add does support ipv6 server.

@arlakshm
Copy link
Contributor Author

Seems config syslog add does support ipv6 server.

Hi @wen587, as far as I know we support only ipv4 syslog server. The CLI command might be missing the IP address validation ?
@qiluo-msft , @renukamanavalan can you confirm

@arlakshm
Copy link
Contributor Author

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft
Copy link
Collaborator

qiluo-msft commented Nov 30, 2021 •
edited
Loading

Seems config syslog add does support ipv6 server.

Hi @wen587, as far as I know we support only ipv4 syslog server. The CLI command might be missing the IP address validation ? @qiluo-msft , @renukamanavalan can you confirm

Checking #3754, we should support ipv6 syslog server address. Otherwise it will be a bug.

In reply to: 983057725

@arlakshm
Copy link
Contributor Author

arlakshm commented Dec 1, 2021

Seems config syslog add does support ipv6 server.

Hi @wen587, as far as I know we support only ipv4 syslog server. The CLI command might be missing the IP address validation ? @qiluo-msft , @renukamanavalan can you confirm

Checking #3754, we should support ipv6 syslog server address. Otherwise it will be a bug.

Add support of ipv6 syslog server in the latest commit

@arlakshm arlakshm requested a review from qiluo-msft December 1, 2021 16:00
@arlakshm
add ipv6 syslog server
56dc42e 
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
@arlakshm
fix build issue
3bbd40f 
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
@arlakshm
Copy link
Contributor Author

arlakshm commented Dec 2, 2021

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

qiluo-msft
qiluo-msft approved these changes Dec 2, 2021
View reviewed changes
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 also check with other reviewers for active comments.

wen587
wen587 approved these changes Dec 3, 2021
View reviewed changes
Copy link
Contributor

@wen587 wen587 left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks.

@arlakshm
Copy link
Contributor Author

arlakshm commented Dec 3, 2021

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@arlakshm
Copy link
Contributor Author

arlakshm commented Dec 3, 2021

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

praveen-li
praveen-li approved these changes Dec 7, 2021
View reviewed changes
Copy link
Member

@praveen-li praveen-li left a comment

Choose a reason for hiding this comment

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

LGTM

@arlakshm arlakshm merged commit 1509f43 into sonic-net:master Dec 7, 2021
@qiluo-msft qiluo-msft added the Request for 202111 Branch For PRs being requested for 202111 branch label Jan 5, 2022
judyjoseph pushed a commit that referenced this pull request Jan 9, 2022
@arlakshm @judyjoseph
[yang] syslog yang model (#9265)
a42dc6f 
Why I did it
Add yang model for syslog server

How I did it
Add new file sonic-syslog.yang and new files for tests

How to verify it
Compile target/python-wheels/sonic_yang_mgmt-1.0-py3-none-any.whl

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan arlakshm@microsoft.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@praveen-li praveen-li praveen-li approved these changes

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

@wen587 wen587 wen587 approved these changes

@renukamanavalan renukamanavalan renukamanavalan left review comments

@lguohan lguohan Awaiting requested review from lguohan

@dgsudharsan dgsudharsan Awaiting requested review from dgsudharsan

@rathnasabapathyv rathnasabapathyv Awaiting requested review from rathnasabapathyv

@ganglyu ganglyu Awaiting requested review from ganglyu

Assignees
No one assigned
Labels
Included in 202111 Branch Request for 202111 Branch For PRs being requested for 202111 branch YANG YANG model related changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@arlakshm @wen587 @qiluo-msft @praveen-li @renukamanavalan @judyjoseph