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

Make logrotate.d file not override global settings #749

Merged
merged 4 commits into from
Dec 18, 2018

Conversation

mjuwit
Copy link
Contributor

@mjuwit mjuwit commented Dec 14, 2018

Moved common logrotate parameters inside of the brackets for the various logfiles.

Description

logrotate.d confs are loaded via "dumb" include statement in the logrotate.conf which "Reads the file given as an argument as if it was included inline where the include directive appears." (from logrotate manpage).

Previous version was applying the 'rotate', 'weekly', 'compress', 'missingok', and 'datetext' parameters outside of the mustache brackets for the individual log files. This would override any settings made in the main system 'logrotate.conf' file before the include statement, thus potentially altering global logrotate behavior for the system.

Motivation and Context

xdmod shouldn't alter global logrotate.conf parameters (thus potentially altering logrotate behavior for the entire system without knowledge of the system administrators).

Tests performed

Running this version on our sytsems -- had to change the file as it was altering our global logrotate configuration.

Types of changes

  • [ x] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project as found in the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

jtpalmer and others added 2 commits November 8, 2018 14:12
logrotate.d confs are loaded via "dumb" include statement in the logrotate.conf which "Reads the file given as an argument as if it was included inline where the include directive appears." (from manpage).

Previous version was applying the 'rotate', 'weekly', 'compress', 'missingok', and 'datetext' parameters outside of the mustache brackets for the individual log files.  This would override any settings made in the main system 'logrotate.conf' file before the include statement, thus altering global logrotate behavior.
@jtpalmer jtpalmer added this to the 8.1.0 milestone Dec 14, 2018
@jtpalmer jtpalmer added bug Bugfixes enhancement Enhancement of the functionality of an existing feature labels Dec 14, 2018
@jtpalmer jtpalmer changed the base branch from xdmod8.0 to xdmod8.1 December 14, 2018 19:09
Copy link
Contributor

@jtpalmer jtpalmer left a comment

Choose a reason for hiding this comment

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

Please rebase these changes on the xdmod8.1 branch.

@jtpalmer
Copy link
Contributor

Thank you for bringing this issue to our attention. I didn't realize those directives would override other files.

@chakrabortyr chakrabortyr dismissed jtpalmer’s stale review December 18, 2018 20:05

Issues addressed as per our convo.

Copy link
Contributor

@chakrabortyr chakrabortyr left a comment

Choose a reason for hiding this comment

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

Looks rebased/good now. Thank you again for bringing this to our attention.

Copy link
Contributor

@smgallo smgallo left a comment

Choose a reason for hiding this comment

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

LGTM

@chakrabortyr chakrabortyr merged commit ecbb6f2 into ubccr:xdmod8.1 Dec 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugfixes enhancement Enhancement of the functionality of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants