Skip to content

Handle the pluginsync setting deprecation#683

Merged
mmoll merged 1 commit intotheforeman:masterfrom
ekohl:deprecate-pluginsync
Mar 11, 2019
Merged

Handle the pluginsync setting deprecation#683
mmoll merged 1 commit intotheforeman:masterfrom
ekohl:deprecate-pluginsync

Conversation

@ekohl
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

ekohl commented Mar 1, 2019

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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