I’ve been working on understanding and fixing client-side resource generation. Below is a summary of what I’ve found and where I think we should go from here.
Keep in mind that while I’ve chatted with other PL engineers about most of this, it’s not yet real roadmap. I’m taking it to the list to get feedback on these changes before we move forward with actually planning this work. In other words, “These are my views and don’t necessarily reflect the views of my employer”. I’m mostly looking for feedback on 1. Does the “semantics and capabilities” section seem sane and correct from your experiences, and is there anything I missed? 2. Do my proposed changes to resource generation make sense? But of course any response is welcome. Background In Puppet 4.2.0 and earlier (I don’t know how far back this behavior applies) there weren’t a lot of differences between generate and eval_generate. Both happened after the catalog had been converted into the resource graph. Neither could participate in autorequire or dependency metaparams. By virtue of happening later in the catalog application, eval_generate could rely on earlier resources in the catalog to set up the system for it. The only advantage to using generate in this setup was that it worked for depthfirst resources. Starting in Puppet 4.3.0, due to PUP-1963 <https://tickets.puppetlabs.com/browse/PUP-1963>, generate and eval_generate are a bit more distinct than they used to be. generate has been moved to happen earlier in the catalog application so that it can participate in autorequires and dependency metaparam expansion. This was mostly done to support the module team’s efforts around the native-type version of the Concat module. Semantics and capabilities of the resource generation functions AFAICT, the capabilities listed here match the current code, although it’s not the most efficient or clean part of puppet’s codebase. Any changes that I think are necessary are listed in the following section. generate: Semantically, this is “I’m adding some resources to the catalog”. Capability-wise, that means: - Generated resources are not contained by the parent - The generated resources participate in metaparam expansion and autorequires - We can place the new resources relatively “before” or “after” the parent in the catalog (this is the depthfirst type option) - Resources are added to the catalog before anything is evaluated eval_generate: Semantically, this is “I require additional instances of resources to do my work”. Capability-wise, that means: - Generated resources are contained by their parent. - The generated resources can have no additional dependency edges added - Resources can only be evaluated immediately after the parent (since they are created as part of the parent evaluation, and are contained) - Dependency resources have already been evaluated by the time these resources are created Proposed Changes to Resource Generation In Puppet 5: - Remove the depthfirst option on types. This is only used by the Tidy type internally, and has no uses on the forge. It makes our code around resource generation more complex, but more importantly it relies on the assumption that the generate function should automatically create edges between the generated resources and their parent. (see below) In Puppet 6+: - Remove the automatic parenting between generated and generating resources. This automatic parenting makes sense for the eval_generate case where the resources are necessarily closely related, but does not necessarily make sense for resources generated before catalog application has started. Additionally, now that relationship metaparams work for generated resources, it’s possible for the generation method to do this directly, instead of relying on the generation mechanisms. To quote the Zen of Python, “Explicit is better than Implicit”. - Rename generate and eval_generate to something that better matches their actual capabilities and use cases. It’s worth noting that the two items in “6+” may become requirements for a future type/provider V2 API, since they’d be fairly large breakages to current types. All of this is far enough out that it is essentially “wishful thinking”. Proposed Changes to Puppet Types As far as I can tell, all core Puppet types should be using eval_generate. They do not require the metaparam expansion that generate affords them, and moving generation as late as possible protects against possible race conditions (The most well known one is requiring a package to be installed before you can purge a custom resource type, but my favorite is using a Mount resource to ruin Tidy’s day). I believe this is a non-breaking change that we could do in Puppet 4.5, as a bugfix for the above race conditions. The exception to that is Tidy, which uses depthfirst so it can provide some fancy reporting on how many files have been tidied. I’m inclined to just rip that reporting out in Puppet 5, along with the depthfirst option. Recommendations for Custom Types My general recommendation is “use eval_generate unless you absolutely need to use generate”. This avoids the race conditions mentioned above, and ensures that the work done by generated resources is “close” (in terms of graph traversal) to the generating resource. It’s unfortunate that eval_generate and generate are named the way they were. If I had my way they’d be generate and early_generate, or something along those lines. We won’t be able to change this for a long time (probably never in the current type/provider API), so we’re going to need to live with it and document best practices for now. -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/CADWDnrmQyW80yaUcvHAqtPcX3-i_S7WuuM9tsQaRJzKC9JhDdQ%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
