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

Remove syslog service validator in GCU #2991

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

wen587
Copy link
Contributor

@wen587 wen587 commented Sep 21, 2023

What I did

ADO:25060558
Remove rsyslog related service validator in GCU because it has been supported in latest image

How I did it

Remove related code

How to verify it

Unit test

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

@wen587 wen587 marked this pull request as ready for review September 22, 2023 01:15
@qiluo-msft qiluo-msft merged commit 6ba6ddf into sonic-net:master Sep 26, 2023
5 checks passed
JunhongMao pushed a commit to JunhongMao/sonic-utilities that referenced this pull request Oct 4, 2023
### What I did
Remove rsyslog related service validator in GCU because it has been supported in latest image
#### How I did it
Remove related code
#### How to verify it
Unit test
StormLiangMS pushed a commit that referenced this pull request Oct 7, 2023
### What I did
Remove rsyslog related service validator in GCU because it has been supported in latest image
#### How I did it
Remove related code
#### How to verify it
Unit test
wen587 added a commit that referenced this pull request Oct 18, 2023
wen587 added a commit that referenced this pull request Oct 20, 2023
StormLiangMS pushed a commit that referenced this pull request Oct 25, 2023
wen587 added a commit that referenced this pull request Nov 13, 2023
What I did
In the privous PR #2991, we removed the rsyslog validator because the background daemon can now react(reset-failed & restart) the rsyslog service when the configuration changes.

However, this change introduced an issue found in nightly tests. The problem is that there is a chance that the rsyslog service is not refreshed promptly after modification.

We have two potential solutions: we can either modify the sonic-mgmt syslog test to wait more time for the completion of the rsyslog update or reintroduce the syslog validator with enhancements. As the update for rsyslog happens after the completion of the GCU apply-patch, I choose to modify in GCU to ensure that it accurately reflects the completion of the rsyslog change when the apply-patch process is finished.

Now the behavior in this pull request aligns with the SONiC CLI when updating rsyslog settings.

How I did it
Add back and update GCU rsyslog validator and align it with syslog CLI.

How to verify it
Unit test and E2E test
StormLiangMS pushed a commit that referenced this pull request Nov 19, 2023
What I did
In the privous PR #2991, we removed the rsyslog validator because the background daemon can now react(reset-failed & restart) the rsyslog service when the configuration changes.

However, this change introduced an issue found in nightly tests. The problem is that there is a chance that the rsyslog service is not refreshed promptly after modification.

We have two potential solutions: we can either modify the sonic-mgmt syslog test to wait more time for the completion of the rsyslog update or reintroduce the syslog validator with enhancements. As the update for rsyslog happens after the completion of the GCU apply-patch, I choose to modify in GCU to ensure that it accurately reflects the completion of the rsyslog change when the apply-patch process is finished.

Now the behavior in this pull request aligns with the SONiC CLI when updating rsyslog settings.

How I did it
Add back and update GCU rsyslog validator and align it with syslog CLI.

How to verify it
Unit test and E2E test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants