Unknown variable aio-agent breaks multi version puppet env

[eperdeme]

Master - 2.2.1-1
Agent - 3.8.2

When using a newer master and an older agent the fact 'aio-agent' does not exist causing agent runs to fail, due to the puppetserver expects the agent to have the fact 'aio-agent' which the older clients do not.

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Unknown variable: '::aio_agent_version'. at /etc/puppetlabs/code/environments/develop/modules/archive/manifests/params.pp:26:11 at /etc/puppetlabs/code/environments/develop/modules/ui_broker/manifests/init.pp:33 on node test.node.com.
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping run

[igalic]

so the codepath under #86 is never exercised in a test?

[daenney]

That should only break with strict variables being on. With strict variables on accessing aio_agent_version would not return the implicit nil/undef but simply balk at the fact that the variable is not declared.

[igalic]

what's the version independent, correct fix for this?

[daenney]

I don't think there's anything wrong with it. I'd like to hear from @eperdeme first and if they have strict variables enabled. If they do, that would explain the error and I'd have an easy fix. If they don't something else is going on which would really need some investigating and help reproducing.

[eperdeme]

Howdi,

Yes we have 'strict_variables = true' it seems to be on by default in puppet.conf for puppetserver-2.2.1-1.el7.noarch

[daenney]

Right, that's the problem. I'm really curious if someone can verify that this is the default as shipped by Puppet Labs. They haven't announced an intent to enforce/required strict variables and as far as I know that was postponed until Puppet 5 to not combine the language migration with the strict variables change as that would significantly increase the workload. Though quite a few modules nowadays do test for strict variables.

So, here's what's happening. When strict_varibles is on in Puppet accessing any variable that is not defined results in a compilation error (whereas previous it would be nil/undef/empty). This means that when we try to if $::aio_agent_version, if aio_agent_version wasn't sent over from Facter that statement blows up.

There's a few ways forward. As of Puppet 4 the $facts variable is available which holds all facts reported by Facter too. However, contrary to the $::some_fact values they can be looked up if they don't exist on the virtue that $facts is a hash and in Puppet, looking up a non-existent key in a hash returns nil/undef.

Unfortunately, if we need to guarantee Puppet 3 compatibility we can't switch to that because in Puppet 3 the $facts hash only becomes available once trusted_node_data is enabled which is a setting people have to manually apply.

In order to work around this we implemented a serious nasty but functional workaround in apt which created a local $xfacts which essentially behaves like $facts but populates it with the $::fact variables that we need or undef/nil when they don't exist: https://github.com/puppetlabs/puppetlabs-apt/blob/b11a45ec690642fd65588ce25957140d49650c0c/manifests/params.pp#L7-L46

Before we move to implement any of these fixes we need to validate that the packages as shipped by Puppet Labs now enable strict variables and if so act accordingly. If this is a distribution package we have a whole different problem to solve.

[rnelson0]

Potential ways to address this:

http://spuder.github.io/jekyll/update/defensive-puppet-with-strict-variables/
https://github.com/maestrodev/puppet-wget/pull/66/files

Two ways to approach depending on the specific need. I'm sure there's more.

Rob Nelson
rnelson0@gmail.com

[daenney]

Yeah, the second one is basically a variation on what we did in apt. I dislike getvar(), it's been buggy before and unless the namespace you want to look up is stored in a string Puppet can do it just fine. It is an easy shortcut though that needs way less lines of code to achieve the same thing if you have to support Puppet 3 and 4 so that might be a good reason to do so. In Puppet 4 you just use $facts['aio_agent_version] and you're good to go.

Regardless, we need to figure out if this is a packaging trip-up first, though we're always happy to accept PRs that enable things like strict variables compatibility.

[rnelson0]

I think it may behoove us to add a strict vars test to Travis CI at the
same time, to prevent regressions and prepare for v5.

Rob Nelson
rnelson0@gmail.com

[logicminds]

Just downloaded PE 2015.03 and the params code that determines which gem provider to use is not working correctly because $::aio_agent_version does not exist. Maybe this should be a case statement comparing all the versions since PE puppet no longer contains 'puppet enterprise' in the newer versions

https://github.com/voxpupuli/puppet-archive/blob/master/manifests/params.pp#L22

[daenney]

@logicminds That's a completely different bug. Please raise a separate issue about that.

[aerostitch]

Hi,
Can you test if it works with #117 please?
Thanks
Joseph

[aerostitch]

Hi @eperdeme ,

The latest changes brought by #117 and #121 should fix this. Can you confirm that you don't have the issue anymore by using the master branch of this module please?
If so I think this can be closed right?

Thanks
Joseph

[jyaworski]

Closing due to recently-merged PRs. Please re-open if this is still an issue.