voxpupuli / puppet-augeas

Helper for using augeas with puppet
Apache License 2.0
43 stars 82 forks source link

$lens_dir is not set correctly for systems using the official Debian puppet packages #65

Closed baldurmen closed 3 years ago

baldurmen commented 7 years ago

As the title says, $lens_dir is not set correctly for systems running Puppet 4 on Debian with the official Debian packages.

That is:

The problem lies in manifest/param.pp:

  if versioncmp($::puppetversion, '4.0.0') >= 0 {
    $lens_dir = '/opt/puppetlabs/puppet/share/augeas/lenses'
  } elsif (defined('$is_pe') and str2bool("${::is_pe}")) { # lint:ignore:only_variable_string
    # puppet enterpise has a different lens location
    $lens_dir = '/opt/puppet/share/augeas/lenses'
  } else {
    $lens_dir = '/usr/share/augeas/lenses'
  }

The proper $lens_dir directory should instead be /usr/share/augeas/lenses. I'm not proposing a patch since I don't have any idea how one could test if the package installed comes from official Debian repositories or not.

This problem can be fixed temporarily by passing the right $lens_dir value in Hiera, but I feel this should be fixed properly.

raphink commented 7 years ago

Unfortunately, I'm not sure if there's a way to distinguish between the Debian-packaged Puppet 4 and the Puppet Inc.-packaged version… This AIO packaging thing is a mess…

bzed commented 7 years ago

There is a trick you can take from https://github.com/theforeman/puppet-puppet/blob/master/manifests/params.pp:

  if versioncmp($::puppetversion, '4.0') < 0 {
    $aio_package      = false
  } elsif $::osfamily == 'Windows' or $::rubysitedir =~ /\/opt\/puppetlabs\/puppet/ {
    $aio_package      = true
  } else {
    $aio_package      = false
  }

if ($aio_package) -> use /opt/puppetlabs, if not not.

anarcat commented 4 years ago

this is still a problem on puppet 5 in Debian buster. workaround:

  class { 'augeas':
    # hack around https://github.com/camptocamp/puppet-augeas/issues/65
    lens_dir => '/usr/share/augeas/lenses/',
  }
raphink commented 4 years ago

I'll welcome a PR if you can find a clean way to detect how to set this value properly.

raphink commented 4 years ago

Hint: https://github.com/camptocamp/puppet-augeas/pull/67/files is probaly a good start

anarcat commented 4 years ago

Hint: https://github.com/camptocamp/puppet-augeas/pull/67/files is probaly a good start

yeah, it looks like this might have fixed the bug, but #67 broke unit tests so it needed a little more work...

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

baldurmen commented 3 years ago

Pong, not stale, still relevant (grrr stalebots :((((( )

anarcat commented 3 years ago

Pong, not stale, still relevant

Puppet 4? should we still support this? or maybe the issue title should be changed to 5 at least?

(grrr stalebots :((((( )

i used to agree with that sentiment, but this changed my mind:

https://00f.net/2021/03/26/it-doesnt-work/

i encourage you to give it consideration next time you meet the stale bot. :) i certainly go much more lightly on those kind of responses now, especially if i'm in a position to do something about the bug myself. ;)

raphink commented 3 years ago

I also dislike stale bots in general. However I dislike the state of my octobox even more 😁

So long as no client is paying to implement a feature, many open issues get in the way of the (many) protects I maintain, so stale bots are a lesser harm.

raphink commented 3 years ago

As for this bug still being relevant, i welcome new information regarding Puppet 6 or 7, as support for Puppet 4 and 5 is not relevant anymore.

baldurmen commented 3 years ago

I don't run puppet 6 yet (still on 5.5), so it's hard for me to be 100% certain this still happens, but the code I quoted hasn't changed since this bug was opened and I doubt anything in puppet 6+ itself changes this behavior. I think this bug being opened has some worth, even if I don't currently have the time to work on a patch :(

anarcat commented 3 years ago

i can confirm this still happens in puppet 5, and the workaround i used is therefore still necessary. i have not tested in p6/7

raphink commented 3 years ago

Puppet 5 has been EOL since November 2020, so let's focus on Puppet 6 and 7 and confirm this is still necessary.

moobyfr commented 3 years ago

Sad to see puppet 5 being the default on bullseye (not even released), already EOF :/

anarcat commented 3 years ago

On 2021-05-31 10:18:30, Blindauer Emmanuel wrote:

Sad to see puppet 5 being the default on bullseye (not even released), already EOF :/

it's complicated. turns out jruby is hard too.

https://veronneau.org/puppetserver-6-a-debian-packaging-post-mortem.html

-- L'homme construit des maisons parce qu'il est vivant, mais il écrit des livres parce qu'il se sait mortel.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

bzed commented 3 years ago

Puppet 5 has been EOL since November 2020, so let's focus on Puppet 6 and 7 and confirm this is still necessary.

Of course it is. Basically you need to figure out of puppet comes from a (Debian) package or from puppetlabs and decide the path based on that.

bzed commented 3 years ago
commit 486933bb96b48b702d14e303c0d5cb3bf371c6c7 (tag: 1.6.1+bzed1, origin/lensdir-location)
Author: Bernd Zeimetz <bernd@bzed.de>
Date:   Thu Jul 6 16:38:20 2017 +0200

    Handle lense_dir based on $::rubysitedir.

    Fixes: #65

diff --git a/manifests/params.pp b/manifests/params.pp
index bf28630..ccb7219 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -41,7 +41,7 @@ class augeas::params {
     default:  { fail("Unsupported OS family: ${::osfamily}") }
   }

-  if versioncmp($::puppetversion, '4.0.0') >= 0 {
+  if (versioncmp($::puppetversion, '4.0.0') >= 0) and ($::rubysitedir =~ /\/opt\/puppetlabs\/puppet/) {
     $lens_dir = '/opt/puppetlabs/puppet/share/augeas/lenses'
   } elsif (defined('$is_pe') and str2bool("${::is_pe}")) { # lint:ignore:only_variable_string
     # puppet enterpise has a different lens location
diff --git a/spec/classes/augeas_spec.rb b/spec/classes/augeas_spec.rb
index 16f76f4..b6721c5 100644
--- a/spec/classes/augeas_spec.rb
+++ b/spec/classes/augeas_spec.rb
@@ -14,7 +14,11 @@ describe 'augeas' do
     end
   end

-  lens_dir = Puppet.version < '4.0.0' ? '/usr/share/augeas/lenses' : '/opt/puppetlabs/puppet/share/augeas/lenses'
+  if Puppet.version >= '4.0.0' and facts[:rubysitedir] =~ /\/opt\/puppetlabs\/puppet/
+    lens_dir = '/opt/puppetlabs/puppet/share/augeas/lenses'
+  else
+    lens_dir = '/usr/share/augeas/lenses'
+  end

   on_supported_os.each do |os, facts|
     context "on #{os}" do
diff --git a/spec/defines/augeas_lens_spec.rb b/spec/defines/augeas_lens_spec.rb
index 0fa1358..c8aec17 100644
--- a/spec/defines/augeas_lens_spec.rb
+++ b/spec/defines/augeas_lens_spec.rb
@@ -3,7 +3,11 @@ require 'spec_helper'
 describe 'augeas::lens' do
   let (:title) { 'foo' }

-  lens_dir = Puppet.version < '4.0.0' ? '/usr/share/augeas/lenses' : '/opt/puppetlabs/puppet/share/augeas/lenses'
+  if Puppet.version >= '4.0.0' and facts[:rubysitedir] =~ /\/opt\/puppetlabs\/puppet/
+    lens_dir = '/opt/puppetlabs/puppet/share/augeas/lenses'
+  else
+    lens_dir = '/usr/share/augeas/lenses'
+  end

   context 'when declaring augeas class first' do
     on_supported_os.each do |os, facts|
bzed commented 3 years ago

Thats my fix (can't push it right now). Works fine since several years now...

anarcat commented 3 years ago

Thats my fix (can't push it right now). Works fine since several years now...

if you made that a "format-patch" output, i could send a formal PR

bzed commented 3 years ago

@anarcat needs review anyway, I'll see if I can update it to the latest version. also it would be nice to know if the approach would be accepted at all...

raphink commented 3 years ago

The rubysitedir condition might be sufficient actually.