| There's a lot of history with this issue. I think the desired behavior is:
- It should be possible to install a dependency needed by a provider that implements prefetch in a single agent run.
- If prefetch fails:
- the overall report status should be marked as failed.
- the error message should indicate which resource type and provider failed.
- all resources associated with the provider should be marked as failed.
- any resources that depend on the failed resource(s) should be marked as skipped due to the failure.
- unrelated resources should continue to be applied.
Here are my notes about how we got here: Redmine-6907 prefetch was called on all providers before their dependencies (commands, etc) were installed, so you couldn't install a dependency needed by a provider implementing prefetch in the same run. Puppet was changed to lazily prefetch, but in doing so swallowed all StandardErrors. This caused puppet to think no resources were present, and it would try to create them, even though they might already exist. This was common for REST providers managing remote resources. PUP-3656 prefetch was modified to only rescue LoadError and MissingCommand. The thinking was there must be something wrong with the provider if it raises an error other than LoadError or MissingCommand. If a provider raised a different exception, then the entire transaction was aborted. However, none of the resources associated with the provider were marked as failed, and neither was the overall report status. This caused puppet to report success even though there were prefetch errors (eg packages).
PUP-7630 All exceptions were rescued. Exceptions other than LoadError and MissingCommand caused the resource to be marked as failed. But failed resources for other reasons were not logged or marked as failed.
PUP-8962 Modified to only rescue StandardError not Exception. Always log the failed provider before re-raising. future_features flag was added but not enabled prior to Puppet 6.0.
PUP-8288 Due to PUP-7630, puppet could rescue provider exceptions (other than MissingCommand and LoadError) and never mark the resource as failed. So the overall report status was "success". This ticket changed puppet so it assumed the transaction failed, and only marked success if the transaction finished
For this ticket, I think we should rescue LoadError and StandardError, log the error with resource type and provider, record that prefetch failed, mark all directly affected resources as failed, etc. |
