Module doesn't work with Puppet 4 due to undef variable passing

[sammcj]

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: 4.8.x Enterprise
• Distribution: CentOS 7
• Module version: 2.2.2

How to reproduce (e.g Puppet code you use)

Try using the module under any version of Puppet 4.x (assuming you do have strict_variables enabled which you should as it will be enforced shortly).

What are you seeing

Compilation fails when accessing variables from facts that don't yet exist as the module hasn't created them.

It looks like it's using the old Puppet 2/3 style if param == undef , when it should be using if defined(param).

See: https://docs.puppet.com/puppet/latest/reference/lang_facts_and_builtin_vars.html#compiler-variables

Available and should be used as of Puppet 4.x:

_strictvariables = true (Puppet master/apply only) — This makes uninitialized variables cause parse errors, which can help squash difficult bugs by failing early instead of carrying undef values into places that don’t expect them.

What behaviour did you expect instead

Facts to be loaded, then usable.

Output log

Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Unknown variable: '::gluster_peer_list'. at /etc/puppetlabs/code/environments/samm_gluster_poc/modules/gluster/manifests/peer.pp:59:10 at /etc/puppetlabs/code/environments/samm_gluster_poc/site/profiles/manifests/services/gluster/host.pp:13 on node int-gluster-01.fqdn
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping run

Although the peers are set in hiera and are being called from the profile.

cc/ @ross-w

[tux-o-matic]

Question for the team: should a new TravisCI scenario be added for Puppet 4.0 with STRICT_VARIABLES=YES?

[ross-w]

Hi there, I have put in a PR which seems to work for me with basic testing on PE 2016.4.0 with strict variables enabled

[sammcj]

As mentioned on @ross-w's merge request:

Have tested @ross-w's branch against 4 CentOS 7 machines of various state and it fixes all the puppet 4 variable issues!

++ on merging this

[sammcj]

@tux-o-matic while not on the team, I would suggest that it is definitly a good idea, knowing several people within Puppet and listening to their recommendations as well as the offical Puppet documentation that states this will be enforced in Puppet 5 as well as being something that is very sensible to enable it would, to me at least make sense ensuring this variable is set so that the code quality is not only kept to the upstream standard but also so there is less rework in the future.

[tux-o-matic]

@ross-w you might have been too fast and skipped #83.