CLAassistant
commented
1 year ago
All committers have signed the CLA.
Closed JonasVerhofste closed 1 year ago
CLAassistant
commented
1 year ago
All committers have signed the CLA.
puppet-community-rangefinder[bot]
commented
1 year ago Puppetfiles.These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.
Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.
JonasVerhofste
commented
1 year ago Ran a rubocop:auto_correct, should be good now.
Also, this is a non-breaking change. Default behaviour is how it was before the param existed.
JonasVerhofste
commented
1 year ago @LukasAud Seems like the two failing jobs are due to a provisioning error?
LukasAud
commented
1 year ago Waiting for acceptance to finish before merging.
elofu17
commented
1 year ago Did this change introduce a regression?
I had puppetlabs/concat v7.3.0 and saz/ssh v10.0.0 installed on my puppet7 server. Today I upgraded to concat v8.0.0 and ssh v10.1.0 and get the following error:
Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: no parameter named 'create_empty_file' (file: /etc/puppetlabs/code/environments/production/thirdparty_modules/concat/manifests/init.pp, line: 126) on Concat_file[/etc/ssh/sshd_config] (file: /etc/puppetlabs/code/environments/production/thirdparty_modules/concat/manifests/init.pp, line: 126) on node foobar
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping runI immediately reverted back to concat v7.3.0 (I let saz/ssh v10.1.0 remain) and everything started working fine again.
I see in the changelog for puppetlabs/concat v7.4.0, that changes were made for "create_empty_file" stuff, so this PR seems relevant. :)
Unfortunately I needed to stress my revert to get all my puppet nodes running, so I don't have any more debug information.
The code calling Concat_file[/etc/ssh/sshd_config] is Steffen Siegler's module https://forge.puppet.com/modules/saz/ssh/readme v10.1.0.
JonasVerhofste
commented
1 year ago @elofu17 This sounds very much alike #772 (see the mention above your comment).
The module you mention (saz/ssh) has the same "outdated" version requirement (i.e. lower than 8).
Could you test with puppetlabs/concat v7.4.0? That version should have the new param as well but should work, given the metadata of saz/ssh v10.1.0. If that works, then metadata is here the issue as well. :)
elofu17
commented
1 year ago I will upgrade concat from 7.3.0 to 7.4.0 now and see.
elofu17
commented
1 year ago Identical error with 7.4.0 as with 8.0.0. ...so I'm reverting back to 7.3.0 again.
siebrand
commented
1 year ago Same issue here with puppet-nginx 4.4.0 and puppetlabs-concat 8.0.0, as well as with puppetlabs-concat 7.4.0.
Server Error: no parameter named 'create_empty_file' (file: /etc/puppetlabs/code/environments/production/modules/concat/manifests/init.pp, line: 126) on Concat_file[/etc/nginx/sites-available/00-http-php-80.conf] (file: /etc/puppetlabs/code/environments/production/modules/concat/manifests/init.pp, line: 126)
Highest version that works is concat 7.3.3.
JonasVerhofste
commented
1 year ago @siebrand @elofu17 Just to confirm, you did run type generation (puppet generate types --environment ${env}) after bumping the module, right? Because this looks a lot like the manifest being updated, but puppet not correctly knowing about the new param in the type.
elofu17
commented
1 year ago Nope. :-) I don't even know what you are saying to be honest. :)
I've always just bumped the module version number in my Puppetfile. On the next r10k run on the puppet-server, the environment(s) get the new 3rd party modules downloaded and populated.
Then a client node performs a puppet run, and the error is triggered.
I.e. I don't restart the puppet server, I don't clear its caches, I don't generate types or anything. Is this wrong?
elofu17
commented
1 year ago I just tested upgrading to v8.0.0 again, and then restarted the puppet server. Now it works :-)
It sounds like I was using some cached version instead of the new. Interesting that this is the first time I notice any problems. :) Sure, some of the times when I bump the 3rd party modules I also upgrade the puppet server itself, and hence restart it == those times I wouldn't have this kind of problem. But I know that I have also bumped modules without a restart lots of times during the years, and this is the first time I have noticed a problem.
I'll remember to restart the server (or clear its cache) upon bumping modules in the future.
Sorry to have wasted your time, and thanks for your comments.
JonasVerhofste
commented
1 year ago @elofu17 no worries, most important part is that it's fixed! A colleague pointed out that r10k probably takes care of the type generation, so I think you're good. (We're not using r10k, which is why we need to run the type generation manually)
@siebrand could you test bumping and then restarting puppetserver as well?
ekohl
commented
1 year ago FYI: https://www.puppet.com/docs/puppet/7/man/generate.html is the manpage for the command. It is important when using multiple environments because Puppetserver sometimes has a hard time isolating them. It's even the first item in the r10k FAQ: https://github.com/puppetlabs/r10k/blob/main/doc/faq.mkd#how-can-run-i-puppet-generate-types-for-each-changed-environment-during-deployment
siebrand
commented
1 year ago @JonasVerhofste I can confirm that the issue with 7.4.0/8.0.0 does not surface after a restart of puppetserver. Thank you for the help.
By default concat would always create an empty file if no fragment was defined. Sometimes we don't want a file unless it has content.