search

example42 / puppi

Puppet module to manage applications deployments and servers local management
http://www.example42.com
Other
142 stars 84 forks source link

Make params_lookup work with strict_variables, take 2 #117

Closed sathieu closed 9 years ago

sathieu commented 9 years ago

We use catch for future parser and begin/rescue for current parser.

This is an enhanced version of c9798a08177c3bec513e67744397fd466c9401d9.

sathieu commented 9 years ago

@alvagante, @salderma @Andor: Can you confirm that this new PR replacing #108 works with your old Ruby versions?

alvagante commented 9 years ago

Thx for the new attempt @sathieu

Andor commented 9 years ago

Works for me. CentOS 6, puppet 3.7.4, ruby 1.8.7

sathieu commented 9 years ago

@alvagante Can you merge it?

The previous error (return can't jump across threads) shouldn't occur anymore as the "return" are outside the "catch".

alvagante commented 9 years ago

Yes, I'd definitively merge, but would like confirmation from @salderma that this works for him too , will ping him

salderma commented 9 years ago

I think we're ok to with my testing. I've got 3 RHEL-ish and 5 Windows servers in the environment I put this into. The linux guys aren't reporting errors, but every puppet run has puppi related files changing content and I'm not certain why...

/File[puppize_openssh]/content  content changed '{md5}9b9f59e3514d4ee624e5e2b408b6ac0a' to '{md5}38e3b8176e278bc8462ccd03434d6202'

What's strange is that these changes aren't consistent from run to run. Reports are showing varying combinations of several different puppet managed files. Sometimes it's just one file per run, others it's several. That might be unrelated to this change, but it's certainly different that prior to deploying this branch. What do you think?

alvagante commented 9 years ago

@salderma , you can safely remove the parameter puppi => true (set via params_lookup) from the openssh class (and basically from all nextgen modules). It creates a file which was supposed to be used by puppi2, which was never completed. This parameter has an hash of the scope variables, and hence , on older ruby versions, it may change the order of its keys at every puppet run

Incidentally this behaviour should not have changed with this commit, so it's strange that you see it now (never seen before on the same OS versions?)

salderma commented 9 years ago

I generally don't use puppi. I think we have some globals in the ENC to set puppi = false, but it appears this environment didn't have that set. Two of the servers are picking it up from an ENC hostgroup now, and didn't seem to be before, so I guess I need to look at how changing their puppet environment affected the precedence of setting that global.

Anyway, the blatant issues I had with the first attempt have not shown up. Thanks for the rework!

alvagante commented 9 years ago

Great, thanks everybody for the collaboration!

sathieu commented 9 years ago

Yes, thanks everybody.

Note that the puppi testsuite still fails on strict_variables, but at least other modules that depends on it won't.

alvagante commented 9 years ago

@sathieu I've problems in getting rid of warning with the puppet4 version of params_lookup as done here: https://github.com/example42/puppi/blob/4xcompat/lib/puppet/functions/params_lookup.rb

Discussion here: https://github.com/example42/puppi/issues/123

Do you have any suggestion for that?