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 shared gem path for Puppetserver >= 5.3 #631

Merged
merged 1 commit into from
Oct 4, 2018

Conversation

baurmatt
Copy link
Contributor

Since 5.3.0 Puppet Server can use some gems shipped by puppet-agent. For
this the gem-path settings of puppetserver.conf needs to be extend by
/opt/puppetlabs/puppet/lib/ruby/vendor_gems. This is the default as
shipped by Puppetlabs.

Fixes GH-630.

ekohl
ekohl previously approved these changes Sep 19, 2018
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Haven't verified this, but the code looks correct.

@baurmatt
Copy link
Contributor Author

Tests failures aren't related. Seems like master is broken as well.

@mmoll
Copy link
Contributor

mmoll commented Sep 20, 2018

The hard-coded path will probably break on e.g. FreeBSD, so either a variable for the whole path should get introduced or (if that's possible) it should get deduced from a fact...

@baurmatt
Copy link
Contributor Author

Good point! :) But to be honest, I have no idea what the correct path would be for FreeBSD nor do I have a FreeBSD around to take a look. Could it be /usr/local/share/puppet/lib/ruby/vendor_gems?

@ekohl ekohl dismissed their stale review September 26, 2018 11:08

Using a parameter would be nice

@mmoll
Copy link
Contributor

mmoll commented Sep 27, 2018

puppetserver6 came into the FreeBSD ports collection and that dir is simply not included:

-    gem-path: [${jruby-puppet.gem-home}, "/opt/puppetlabs/server/data/puppetserver/vendored-jruby-gems", "/opt/puppetlabs/puppet/lib/ruby/vendor_gems"]
+    gem-path: [${jruby-puppet.gem-home}, "/var/puppet/server/data/puppetserver/vendored-jruby-gems"]

@mmoll
Copy link
Contributor

mmoll commented Sep 27, 2018

Also, I'd rather would use >= 6.0 as version constraint, at least for now.

@baurmatt
Copy link
Contributor Author

baurmatt commented Sep 28, 2018

FreeBSD: I think that's an error of the package maintainer. The puppetserver.conf file is explicitly managed by the package, see https://svnweb.freebsd.org/ports/head/sysutils/puppetserver5/files/patch-ext__config__conf.d__puppetserver.conf?revision=465587&view=markup. I don't see any reason why the (correct) path shouldn't be included for FreeBSD.

Edit: Ok, seems like FreeBSD basically does a Source installation which doesn't include the vendor_gems path. I will fix that.

6.0 >=: Can you explain why? The gem-path setting was updated by Puppetlabs with 5.3, this module should reflect that.

Since 5.3.0 Puppet Server can use some gems shipped by puppet-agent. For
this the gem-path settings of puppetserver.conf needs to be extend by
/opt/puppetlabs/puppet/lib/ruby/vendor_gems. This is the default as
shipped by Puppetlabs.

Fixes theforemanGH-630.
@baurmatt
Copy link
Contributor Author

Updated the PR so that the gem-path setting for FreeBSD doesn't include the path to vendor_gems

@mmoll
Copy link
Contributor

mmoll commented Sep 29, 2018

FreeBSD: I think that's an error of the package maintainer.

While I have no information about the brackgorund, the default for FreeBSD's puppetservers is not to include that path, so we should just mirror that.

6.0 >=: Can you explain why? The gem-path setting was updated by Puppetlabs with 5.3, this module should reflect that.

Sorry, mixed this up with another PR. 5.3 is correct. 👍

Copy link
Contributor

@mmoll mmoll left a comment

Choose a reason for hiding this comment

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

Untested, but looks OK. We can add a specific paramter, later if it should really be needed.

@ekohl ekohl added the Bug label Oct 4, 2018
@ekohl ekohl merged commit 3f486b1 into theforeman:master Oct 4, 2018
@ekohl
Copy link
Member

ekohl commented Oct 4, 2018

Thanks!

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