Skip to content

kvm: Logrotate should not touch agent.log#3597

Merged
yadvr merged 1 commit into
apache:masterfrom
CLDIN:logrotate
Sep 19, 2019
Merged

kvm: Logrotate should not touch agent.log#3597
yadvr merged 1 commit into
apache:masterfrom
CLDIN:logrotate

Conversation

@wido
Copy link
Copy Markdown
Contributor

@wido wido commented Sep 13, 2019

Logrotate should only touch security_group.log and resizevolume.log
as the agent.log is already rotated by log4j inside the Agent.

Having two systems trying to rotate agent.log leads to all kinds of
issues like having binary (compressed) data in the middle of a plain-text
log file.

Signed-off-by: Wido den Hollander wido@widodh.nl

Logrotate should only touch security_group.log and resizevolume.log
as the agent.log is already rotated by log4j inside the Agent.

Having two systems trying to rotate agent.log leads to all kinds of
issues like having binary (compressed) data in the middle of a plain-text
log file.

In addition we do not have to rotate the logs every day, only when they
grow larger than 10M. On fairly idle hypervisors this should not cause
those logs to rotate every day.

Signed-off-by: Wido den Hollander <wido@widodh.nl>
Copy link
Copy Markdown
Member

@GabrielBrascher GabrielBrascher left a comment

Choose a reason for hiding this comment

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

LGTM

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Sep 19, 2019

(el6 failure expected)
@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-282

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Sep 19, 2019

Config and packaging only change, wont' need smoketests.

@yadvr yadvr merged commit 8170ec5 into apache:master Sep 19, 2019
@onitake
Copy link
Copy Markdown
Contributor

onitake commented Oct 17, 2019

Isn't the commit message a bit misleading?
Aside from removing agent.log from the logrotate config, it also added /var/log/cloudstack/agent/resizevolume.log - was this part of @agentlog@ before?

@ustcweizhou
Copy link
Copy Markdown
Contributor

@onitake no, resizevolume.log is new in logrotate
I think it would be good to add security_group.log to logrotate.

@onitake
Copy link
Copy Markdown
Contributor

onitake commented Oct 17, 2019

Ah, ok.
Just thought it may be good if this is mentioned somewhere.

@ustcweizhou
Copy link
Copy Markdown
Contributor

@rhtyd can you cherry-pick this commit to 4.13 branch ?

DaanHoogland pushed a commit that referenced this pull request Jan 3, 2020
Logrotate should only touch security_group.log and resizevolume.log
as the agent.log is already rotated by log4j inside the Agent.

Having two systems trying to rotate agent.log leads to all kinds of
issues like having binary (compressed) data in the middle of a plain-text
log file.

In addition we do not have to rotate the logs every day, only when they
grow larger than 10M. On fairly idle hypervisors this should not cause
those logs to rotate every day.

Signed-off-by: Wido den Hollander <wido@widodh.nl>
DaanHoogland added a commit that referenced this pull request Jan 3, 2020
* 4.13:
  ui: fix for truncated name for project accounts (#3793)
  kvm: Logrotate should not touch agent.log (#3597)
ustcweizhou pushed a commit to ustcweizhou/cloudstack that referenced this pull request Feb 28, 2020
Logrotate should only touch security_group.log and resizevolume.log
as the agent.log is already rotated by log4j inside the Agent.

Having two systems trying to rotate agent.log leads to all kinds of
issues like having binary (compressed) data in the middle of a plain-text
log file.

In addition we do not have to rotate the logs every day, only when they
grow larger than 10M. On fairly idle hypervisors this should not cause
those logs to rotate every day.

Signed-off-by: Wido den Hollander <wido@widodh.nl>
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.

6 participants