correct bug when validating mfsbsd images

[cruwe]

Hi,

I am so sorry, I fat-fingered a bug into your module. When switching my envs, I noticed a rather important char missing, which I fixed.

The regex /^mfsbsd(-se|-mini)$/ will not match on "pure" mfsbsd images (those without -se and without -mini), which is not the intent here. /^mfsbsd(-se|-mini)?$/ will match correctly on the three options applicable.

modified:   manifests/images.pp

I am so sorry to have messed this up ... Thanks again for your work

[cruwe]

Hi again,

I am just diving deeper into the module just to find out that the creates-clause for the wget command does not prevent spurious image downloads and that it is not possible to boot the newly deployed image because of

LABEL Installer: mfsbsd 10.3-RELEASE amd64 KERNEL images/mfsbsd/10.3-RELEASE/amd64/linux APPEND initrd=images/mfsbsd/10.3-RELEASE/amd64/initrd.gz text

which is obviously for linux.

I suggest reverting the merge until I have sufficient depth to do it correctly, apparently,I have been a tad over-optimistic in my understanding of your module. Sorry again and I'll be back with a correct solution in a couple of days. Cheers!

[cruwe]

Hi,

sorry for all that noise. I think I finally got it and force-pushed my changes into the request. Could you please double-check my adaption of the download-exec?

In addition, in my personal frozen zone holiday stupor, it took me unusually long to finally understand the concept of the permute-resource you sketched in your example - especially kernel and append params being specified in the common section. With your consent/permission, I would like to propose a differently worded example for the documentation in a separate branch and pull-request.

Anyhow, sorry again for the hassle and the noise, Cheers!

[zachfi]

Oh no, permute is terrible. It was written in a time before the future parser existed, and should completely die now that the puppet language has constructs for iteration.

Really no worries about sending more PRs, PRs are cool. :)

I'm open to any improvements to this module, including removing whatever references, examples, etc that mention permute. This module doesn't get much attention these days, but I think a few people are using it as its been relatively stable (read, unchanged :)) and has worked for the use cases it was designed for. That said, relevance wanes if updates aren't made. This module needs tests also, but my attention is on other modules under my maintenance at the moment. I also try to version semantically so upgraders know when to be careful, and I'm keeping a CHANGELOG.

This change looks reasonable. The Exec could likely be replaced with a use of the staging module, but that is elsewhere too.

[cruwe]

Apparently, my code lacked clarity, so thank you for your question.

The difficulty was that because of my change to the $path due to the changed directory structure upstream, the creates points to a file

"${tftp_root}/images/${os}/${path}" which is "${tftp_root}/images/${os}/${maj}/${os}-${ver}-${arch}.img" for 11 and "${tftp_root}/images/${os}/${maj}/${arch}/${os}-${ver}-${arch}.img" for 10.

Differently put, when $path does not include $arch as with mfsbsd 11 (+?), the creates will not match and fail.

Another option would be to follow the upstream directory structure with the cwd expression as in $cwd = $maj ? { /11./ => "${tftp_root}/images/${os}/${ver}/", /default/ =>"${tftp_root}/images/${os}/${ver}/${arch}", }

This is a fact I missed with my original change and which will result in hundreds of mfsbsd images $imgname.{1...\infty}, which is the bug which I introduced.

The issue you rightfully stumbled over is that the local image file name is hidden in $path abnd just missed when reading code and so, at first sight, creates does not match the command. I have reworked the naming to better reflect the structure of remote and local names.

[zachfi]

This looks good. Thank you for the contribution.