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

Allow user to modify loglevel of apt-get update Exec resource #690

Merged
merged 1 commit into from
Aug 4, 2017
Merged

Allow user to modify loglevel of apt-get update Exec resource #690

merged 1 commit into from
Aug 4, 2017

Conversation

tpdownes
Copy link

The goal of this PR is to optionally eliminate unnecessary Notices in the syslog and in agent reports. When the apt_update Exec resource runs successfully the Notice is interpreted by Foreman as a "host modification performed without error." While this is, in some sense, true, it's not something I care to know about at the default level of logging and it's reasonable to optionally reduce it.

Confirmed to be a no-op with these hiera data:

apt::update:
  frequency: daily

Confirmed to eliminate logging at the default agent level of notice when these data are used:

apt::update:
  frequency: daily
  loglevel: debug

@tphoney
Copy link

tphoney commented Aug 1, 2017

@tpdownes, Having this as a configurable option totally makes sense. Great change. However The loglevel variable you are referencing is not set anywhere. It will need to be documented in the readme and a test for the new attribute would be preferable.

@tpdownes
Copy link
Author

tpdownes commented Aug 1, 2017

@tphoney I'm happy to add a README section and to explicitly set it to undef in apt::params:: update_defaults. What you probably won't find useful from me is a check since I don't really know ruby and that's what I think I see being done in Travis. I can learn or try. Is there an easy example to follow?

@tphoney
Copy link

tphoney commented Aug 1, 2017

Any attempt is very welcome, get the attribute in the readme and update the params. That will will be enough.

Many thanks !!!!

@tpdownes
Copy link
Author

tpdownes commented Aug 1, 2017

Thanks for the positive feedback! Should be good to go with revised commit once it builds.

FYI: the TOC in README.md begins at 2. Didn't bother fixing. I didn't think the numbers mattered but I tend to just repeat use 1. for every item in my ordered lists and have found that works in the flavor of markdown used by GitLab (rather than GitHub).

@tphoney
Copy link

tphoney commented Aug 2, 2017

Many thanks @tpdownes!!

@jbondpdx are you happy with the readme changes ?

Copy link

@jbondpdx jbondpdx left a comment

Choose a reason for hiding this comment

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

looks good to me!

@tphoney tphoney merged commit dadbfa6 into puppetlabs:master Aug 4, 2017
@tphoney
Copy link

tphoney commented Aug 4, 2017

Many thanks @tpdownes

@legal90
Copy link

legal90 commented Nov 28, 2017

Hi, @tpdownes

Could you please elaborate how did that solution work for you?
I've defined apt::update with loglevel set to debug and I actually don't see any notes from this resource in the report. But Foreman still shows the status of such Puppet runs as "updated" / "changed", so it's always blue, not green.
I guess that is an expected behavior because an underlying exec resource is always executing. But I just wondering how did you solve it :)

P.s. My Foreman version is 1.8.1.

@tpdownes
Copy link
Author

@legal90 yeah, I had thought it would properly suppress that in Foreman but it doesn't. I left the MR in place because, well, it's nice not to have it at stdout.

In Debian 9, a daily systemd timer is automatically installed, so I might think of having puppetlabs-apt switch to that model even for Debian 8 (and Ubuntu variants) while it remains LTS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants