search

puppetlabs / r10k

Smarter Puppet deployment
Other
802 stars 355 forks source link

(PE-34196, PE-34917) Make loading metadata in `--incremental` deploys more robust #1374

Closed justinstoller closed 3 months ago

justinstoller commented 3 months ago

Previously, if loading module metadata raised an uncaught exception a user with automation that always used --incremental (like the PE default configuration) could not recover from a failed deployment.

This PR addresses two different tickets that call out specific exceptions that can be raised during metadata parsing. It then goes slightly farther in making metadata loading more robust to avoid further whack-a-mole exception handling.

justinstoller commented 3 months ago

@Sharpie , how do you feel about widening the exceptions we catch this much? I couldn't find an example of an exception we wouldn't want to catch here (save for ^C).

justinstoller commented 3 months ago

FWIW, I manually verified that this fixes PE-35197.

However, I was not able to reproduce the issue described in PE-35196. I see the exception that will be raised from the code that checks for duplicates and I added a clause that will catch it (and similar) if it is raised (what I noted when triaging the ticket). However, it looks like that validation step is only ever performed when actually deploying an environment (after we've loaded the metadata - I missed this fact when triaging the ticket) and should be resolved by updating the control-repo and redeploying. I have a feeling the customer and/or support (and myself) saw the error and over eagerly assigned blame to metadata loading. Regardless, if it is raised somehow during metadata loading (if I'm wrong now) with this PR it will still be caught and handled appropriately.

This change is deployed to stale-mink.delivery.puppetlabs.net for those that want to test it out themselves.