Unknown variable aio-agent breaks multi version puppet env about puppet-archive HOT 14 CLOSED

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

from puppet-archive.

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.

from puppet-archive.

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

from puppet-archive.

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.

from puppet-archive.

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

from puppet-archive.

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.

from puppet-archive.

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
[email protected]

from puppet-archive.

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.

from puppet-archive.

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
[email protected]

from puppet-archive.

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

from puppet-archive.

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

from puppet-archive.

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

from puppet-archive.

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

from puppet-archive.

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

from puppet-archive.