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

[rsyslog]Setting log file size to 16Mb #9504

Merged
merged 13 commits into from
Jan 14, 2022
Merged

Conversation

dgsudharsan
Copy link
Collaborator

@dgsudharsan dgsudharsan commented Dec 10, 2021

Signed-off-by: Sudharsan Dhamal Gopalarathnam sudharsand@nvidia.com

Why I did it

The existing log file size in sonic is 1 Mb. Over a period of time this leads to huge number of log files which becomes difficult for monitoring applications to handle.
Instead of large number of small files, the size of the log file is not set to 16 Mb which reduces the number of files over a period of time.

How I did it

Changed the size parameter and related macros in logrotate config for rsyslog

How to verify it

Execute logrotate manually and verify the limit when the file gets rotated.

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

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

Description for the changelog

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

Signed-off-by: Sudharsan Dhamal Gopalarathnam <sudharsand@nvidia.com>
@dgsudharsan dgsudharsan requested a review from lguohan as a code owner December 10, 2021 20:56
liat-grozovik
liat-grozovik previously approved these changes Dec 11, 2021
@liat-grozovik liat-grozovik added the Request for 202111 Branch For PRs being requested for 202111 branch label Dec 11, 2021
@liat-grozovik
Copy link
Collaborator

@lguohan could you please approve this fix?

@lguohan
Copy link
Collaborator

lguohan commented Dec 15, 2021

i think we will have issue when disk size is small, can this be a template? keep 1M for disk size /var/log partition less than 200MB.

@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yxieca
Copy link
Contributor

yxieca commented Jan 3, 2022

@dgsudharsan Do we need any 'default' configuration before the logrotate config was rendered? Have you confirmed that the next logrotate will pick up the new config after rendering? Can you also add a sonic-mgmt test for this?

@dgsudharsan
Copy link
Collaborator Author

@dgsudharsan Do we need any 'default' configuration before the logrotate config was rendered? Have you confirmed that the next logrotate will pick up the new config after rendering? Can you also add a sonic-mgmt test for this?

Yes I tested this scenario logrotate picks up after rendered. Its only few seconds difference between cron and rendering.
There are multiple scenarios

  1. Onie install - there will be no rsyslog config in logrotate and so it will not be considered for logrotate. However the next time logrotate runs after 10 minutes, there will be rsyslog logrotate config which will be used.
  2. Upgrade - I believe the conf files under /etc would be copied during upgrade and old conf file would be used before rendering and after will pick up the new one
  3. reboot - The conf file will already be rendered before reboot and hence that will be used.

Regarding sonic-mgmt, I don't think there are any test cases although the suite covers logrotate as a utility for all existing tests.
Do you recommend to write a test case for covering logrotate functionality? If yes I believe that needs to be considered as a separate effort.

@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yxieca
Copy link
Contributor

yxieca commented Jan 6, 2022

Regarding sonic-mgmt, I don't think there are any test cases although the suite covers logrotate as a utility for all existing tests. Do you recommend to write a test case for covering logrotate functionality? If yes I believe that needs to be considered as a separate effort.

@dgsudharsan thanks for the details. Yes please track a different effort and add a sonic-mgmt test.

yxieca
yxieca previously approved these changes Jan 6, 2022
@yxieca
Copy link
Contributor

yxieca commented Jan 6, 2022

/azp 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.

Signed-off-by: Sudharsan Dhamal Gopalarathnam <sudharsand@nvidia.com>
@dgsudharsan
Copy link
Collaborator Author

/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.

@dgsudharsan
Copy link
Collaborator Author

/azpw run Azure.sonic-buildimage (Test kvmtest-t0)

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage (Test kvmtest-t0)

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@dgsudharsan
Copy link
Collaborator Author

/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).

@yxieca yxieca merged commit bd0a19a into sonic-net:master Jan 14, 2022
judyjoseph pushed a commit that referenced this pull request Jan 17, 2022
Why I did it
The existing log file size in sonic is 1 Mb. Over a period of time this leads to huge number of log files which becomes difficult for monitoring applications to handle.
Instead of large number of small files, the size of the log file is not set to 16 Mb which reduces the number of files over a period of time.

How I did it
Changed the size parameter and related macros in logrotate config for rsyslog

How to verify it
Execute logrotate manually and verify the limit when the file gets rotated.

Signed-off-by: Sudharsan Dhamal Gopalarathnam <sudharsand@nvidia.com>
liat-grozovik pushed a commit that referenced this pull request Nov 8, 2022
…12599)

- Why I did it
Fix logrotate firstaction script to reflect correct size. The size was modified to change dynamically based on disk size. However this variable was not updated
#9504

- How I did it
Updated the variable based on disk size

- How to verify it
Verify in the generated rsyslog file if the variable is correctly generated from jinja template
yxieca pushed a commit that referenced this pull request Nov 10, 2022
…12599)

- Why I did it
Fix logrotate firstaction script to reflect correct size. The size was modified to change dynamically based on disk size. However this variable was not updated
#9504

- How I did it
Updated the variable based on disk size

- How to verify it
Verify in the generated rsyslog file if the variable is correctly generated from jinja template
@dgsudharsan dgsudharsan deleted the log_resize branch March 9, 2023 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Included in 202111 Branch Request for 202111 Branch For PRs being requested for 202111 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants