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

add support for Debian #131

Closed
wants to merge 2 commits into from
Closed

add support for Debian #131

wants to merge 2 commits into from

Conversation

rj667
Copy link

@rj667 rj667 commented Aug 17, 2018

Pull Request (PR) description

Tests $facts['operatingsystem'] to tell Ubuntu and Debian apart.
Ubuntu needs 'su root syslog' in /etc/logrotate.conf
Debian does not.

This Pull Request (PR) fixes the following issues

Fixes #114

@@ -22,12 +22,12 @@
}
}
'Debian': {
$default_su_user = versioncmp($facts['operatingsystemmajrelease'], '14.00') ? {
1 => 'root',
$default_su_user = $facts['operatingsystem'] ? {
Copy link
Member

Choose a reason for hiding this comment

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

just as a note in case somebody checks this in the future: This change is fine because we do not support Ubuntu older than 14.04 (versioncmp() used 14.00, not 14.04)

@bastelfreak bastelfreak changed the title added support for Debian add support for Debian Aug 18, 2018
@bastelfreak
Copy link
Member

@rj667 thanks for the PR. Can you please take a look at the used email address in the commit? It isn't associated with your github account. On which Debian version is this now supposed to work? Can you add them to https://github.com/voxpupuli/puppet-logrotate/blob/master/metadata.json please?

@bastelfreak bastelfreak added the needs-work not ready to merge just yet label Aug 18, 2018
@rj667
Copy link
Author

rj667 commented Aug 21, 2018

The test assumes that Debian "should contain Logrotate::Conf[/etc/logrotate.conf] with su_user defined and su_group defined" which is not true.
On (all) Debian /etc/logrotate.conf does not contain su
On (all) Ubuntu /etc/logrotate.conf does contain su root syslog
This is the unmodified file straight from the logrotate package.
This module should not change that.

@bastelfreak
Copy link
Member

@rj667 can you adjust the tests so it matches the reality?

@rj667
Copy link
Author

rj667 commented Sep 4, 2018

Where are the tests? I am not familiar with your CI setup.

@bastelfreak
Copy link
Member

@bastelfreak
Copy link
Member

Obsoleted by #134

@bastelfreak bastelfreak closed this Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-work not ready to merge just yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check debian support
2 participants