fixes #8162 - scan classes directories recursively

[shlomizadok]

No description provided.

[domcleal]

I don't really see the relation to the original problem. The ticket indicates the modulepath from the API is ["/etc/puppet/environments/live/modules", "/etc/puppet/environments/$environment/modules/forge", "/etc/puppet/environments/$environment/modules/local"], so scanning any of those recursively isn't going to find the classes any better.

My understanding of the problem description is that $environment doesn't get interpolated at any stage, so those directories are never searched.

[shlomizadok]

I thought so too, but apparently $environment does get interpolated.
I have tested it locally with puppet configured as:

environmentpath  = /etc/puppet/environments
basemodulepath   = /etc/puppet/environments/common:/etc/puppet/modules:/usr/share/puppet/modules:/etc/puppet/environments/$environment/modules/forge

I have the following environments in that puppet: http://192.168.100.2:8000/puppet/environments

[
  "development",
  "example_env",
  "production",
  "common"
]

In example_env, I have created a module under /etc/environments/example_env/modules/forge/modules/motd (with a default motd module).
Now when I query the smart proxy, I get this module: http://192.168.100.2:8000/puppet/environments/example_env/classes
response:

[
  {
    motd: {
      params: {
        template: "motd/motd.erb"
      },
     module: null,
     name: "motd"
    }
  }
]

I have also made sure that the previously saved modules (under production) are parsed correctly.

[domcleal]

What directory is passed into scan_directory in that case, when it isn't working?

[shlomizadok]

I see your point. The directory that was passed was $environment and it was not scanned recursively. Now, it is scanned recursively. Not sure what will happen if I put it outside of environments/$environment path (not sure it's a use case anyhow).

[domcleal]

That sounds incorrect then, it should be /etc/puppet/environments/$environment/modules/forge according to your configuration.

Edit: except interpolated, naturally.

[shlomizadok]

Well, dug around it a bit.
It seems that puppet does not allow to use $environment variable when using directory environments.
The puppet log shows the following error:

Warning: You cannot interpolate $environment within 'modulepath' when using directory environments.  Its value will remain /etc/puppet/environments/development/modules:/etc/puppet/environments/common:/etc/puppet/modules:/usr/share/puppet/modules:/etc/puppet/environments/$environment/modules/forge.

(The error is generated here: https://github.com/puppetlabs/puppet/blob/master/lib/puppet/settings.rb#L1357)

Now, when I look at my smart proxy puppet environment (at: http://localhost:8000/puppet/environments/example_env) I get the following:

{
  name: "example_env",
  paths: [
    "/etc/puppet/environments/example_env/modules",
    "/etc/puppet/environments/common",
    "/etc/puppet/modules",
    "/usr/share/puppet/modules",
    "/etc/puppet/environments/$environment/modules/forge"
  ]
}

When trying to scan directories, we'll get the $environment directory as one of the directories to scan (which will return nothing as it is not found).

My question: Shall I bypass puppet and replace the $environment with the actual name, before scanning it?

[domcleal]

Good find! I'd suggest replying on the ticket and stating that this doesn't appear to be supported in Puppet itself (the commit has an interesting explanation). I think this can be rejected and dropped.

I wouldn't suggest ignoring Puppet and interpolating it, because users will simply find that the classes then appear in Foreman and don't work in Puppet - we should mirror the behaviour as much as possible.

[shlomizadok]

Agreed. I'm closing this PR, but will refer to it in my reply.

[RobertMortimer]

These environment paths do work in puppet for the UAT environment puppet will scan the following (as returned by the curl request):-

"/etc/puppet/environments/uat/modules",
"/etc/puppet/environments/uat/modules/forge",
"/etc/puppet/environments/uat/modules/local"

Foreman on the other hand only scans "/etc/puppet/environments/uat/modules" and modules in the other two locations are ignored

[domcleal]

@RobertMortimer in your bug report and e-mail, the "uat" in the modulepath list is not there - it says $environment. Can you confirm which it is?

If it's $environment then we're saying that Puppet doesn't support this - it might have worked for a couple of minor releases, but certainly in the latest it's been removed again as it wasn't intended.

[RobertMortimer]

The curl request returns a path set for each environment. Like so:-

"uat": {"settings":
{"manifest":"/etc/puppet/environments/uat/manifests",
"environment_timeout":180,
"config_version":"",
"modulepath":["/etc/puppet/environments/uat/modules",
"/etc/puppet/environments/$environment/modules/forge",
"/etc/puppet/environments/$environment/modules/local"]}},

I believe for the given configuration puppet internally expands this to the following module search path:-

"/etc/puppet/environments/uat/modules",
"/etc/puppet/environments/uat/modules/forge",
"/etc/puppet/environments/uat/modules/local"

Foreman on the other hand does not appear to do the expansion resulting in a discrepancy between the Foreman detected modules and the modules available to puppet. This may however be seen as a bug in puppet as if it is expanding this search path for internal use it should also return the absolute path via the curl result without requiring the substitution to be done by the client.

I will double check my old configuration for typos and confirm behavior.

Thanks for looking at this I will get back with more detail
Robert

[domcleal]

Right, so if you're adding the forge/local paths in your basemodulepath, you should know that Puppet 3.7.1 added a warning and stopped this behaviour in this commit: puppetlabs/puppet@65b85b8 (ticket https://tickets.puppetlabs.com/browse/PUP-3162) as it wasn't meant to be supported.

I guess your environment might be working if you're between Puppet 3.5.0 and 3.7.0, but since it's now explicitly not supported by Puppet we decided not to emulate this removed behaviour.

[RobertMortimer]

That is a pain. We have some modified forge module that we wanted to separate to make it obvious that they were not stock modules. Go puppet! not quite time to re-review ansible and salt but puppet does not seem to be helping it's self at the moment. Puppet DB is a pain to configure and the puppet dashboard although paid for does not give us the functions Foreman does (especially in the use of environments where puppet labs increasingly seems to be pushing you to install multiple puppet servers). Thanks for the help and a great product.

[RobertMortimer]

P.S. we dropped the configuration once as it was not supported by Foreman so missed the issues when we upgraded.

[domcleal]

Ah okay. I think the intention is that you deploy environment.conf into your environment directories and set modulepath inside it: https://docs.puppetlabs.com/puppet/latest/reference/config_file_environment.html

Something like this:

modulepath = modules:modules/forge:modules/local

[RobertMortimer]

Will Foreman pick that up?

[domcleal]

Yes, as the Puppet API should then return the modulepath list with absolute paths.

[RobertMortimer]

Thanks