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

Handle the pluginsync setting deprecation #683

Merged
merged 1 commit into from
Mar 11, 2019

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Mar 1, 2019

In Puppet 4 the pluginsync setting was deprecated. At least in Puppet 5 it generates a deprecation warning and in Puppet 6 it was removed. Because the default was already true, we only emit it if it's non-default to avoid the deprecation warning. In Puppet 6 we can detect an invalid setting so we hard fail.

In Puppet 4 the pluginsync setting was deprecated. At least in Puppet 5
it generates a deprecation warning and in Puppet 6 it was removed.
Because the default was already true, we only emit it if it's
non-default to avoid the deprecation warning. In Puppet 6 we can detect
an invalid setting so we hard fail.
@mmoll
Copy link
Contributor

mmoll commented Mar 1, 2019

I'm fine with this as-is but asking myself if it would be good to dump a warn on Puppet 5... or is that the same kind of visibility as the one generated by Puppet itself?

@ekohl
Copy link
Member Author

ekohl commented Mar 1, 2019

The reason I noticed this was because I saw this in my logs:

Mar  1 14:02:40 foreman puppet-agent[18582]: Config file /etc/puppetlabs/puppet/puppet.conf changed; triggering re-parse of all config files.
Mar  1 14:02:40 foreman puppet-agent[18582]: Setting 'pluginsync' is deprecated.
Mar  1 14:02:40 foreman puppet-agent[18582]:   (location: /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/defaults.rb:1879:in `block in <module:Puppet>')
Mar  1 14:02:55 foreman puppet-agent[18582]: Config file /etc/puppetlabs/puppet/puppet.conf changed; triggering re-parse of all config files.

@mmoll
Copy link
Contributor

mmoll commented Mar 1, 2019

@ekohl So this is only visible in the logfile? Or also in the Puppet Report in Foreman (I tihnk a puppet warn would show up there and also change host status)?

@ekohl
Copy link
Member Author

ekohl commented Mar 1, 2019

That's from syslog, not the puppet report. I can see about that.

Copy link
Contributor

@alexjfisher alexjfisher left a comment

Choose a reason for hiding this comment

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

I might even be tempted to rip it out altogether and see if anyone screams. The setting having been deprecated for so long, I'd be surprised, (and interested), if anyone is actively setting it to false.

@ekohl
Copy link
Member Author

ekohl commented Mar 11, 2019

@mmoll do you think including this in a patch release is good enough? In the next major version drop it.

@mmoll
Copy link
Contributor

mmoll commented Mar 11, 2019

@ekohl sure, let's do this... We'll drop Ubuntu/trusty and Puppet 4.x in the next round of module releases anyway, I presume...

@mmoll mmoll merged commit 5ecda93 into theforeman:master Mar 11, 2019
@mmoll
Copy link
Contributor

mmoll commented Mar 11, 2019

merged, bedankt @ekohl!

@ekohl ekohl deleted the deprecate-pluginsync branch March 11, 2019 16:01
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.

4 participants