puppetlabs / puppet-specifications

Specification of the Puppet Language, Catalog, Extension points
Other
99 stars 66 forks source link

(PUP-4380) Describe plan for moving non-core modules out of puppet #106

Closed joshcooper closed 6 years ago

joshcooper commented 6 years ago

Describes the user stories, file paths, and technical issues around extracting non-core types and providers from puppet.

hlindberg commented 6 years ago

A thought - loading things at runtime requires quite a lot of IO - if we could generate an index at "some time" that is not runtime we can load that instead of scanning the file system. Vendored modules included in a package could have this as it is not expected that users hack on those when they are on the file system. We could perhaps use the same when installing via module-tool. Reason for me posting this is that puppet prepopulates loaders with known resource types - when they move out to a module, they are not really known and needs to be loaded the same way as everything else. We also do not know if different versions of vendored modules are used in different environments since you can override the vendored - that opens up for problems with environment isolation - and it would be beneficial to also include pre-generated "generate types" to make the types env-safe.

From a use case perspective - "I don't want to worry about environment isolation when I need to override a vendored module", "I don't want to see performance degradation in my compiles"

joshcooper commented 6 years ago

Good points @hlindberg, yes, removing T&P into modules combined with installing different versions of those modules in different environments would make env isolation worse for anyone not using puppet generate types workflow. I will update the use case section.

I agree precomputing that information would be good. I think that would be best handled in pdk build (which will supersede PMT) or in r10k/codemanager. Now that you can HUP opensource puppetserver when new code has been deployed (and PE already has that capability via HUP or codemanager), it should be easier to move in an "everything is pregenerated" mode of operation.

hlindberg commented 6 years ago

For this to work well, we may need to also make use of pregenerated things per module (if not generate for the entire env is fast enough). The generate types currently handles the entire environment and stores the information in a central location. Irrespective of which tool it is building the precomputed information, it needs to be using indexing that is appropriate for a particular version of puppet as I don't think we will stop innovating in the "things we need to load" domain. When we do have these indexes, it is will mean a lot performance wise as as we learned recently - a scan per module to get candidate files is quite expensive.

joshcooper commented 6 years ago

@ahpook @hlindberg I added some revisions in a second commit to make review easier.

joshcooper commented 6 years ago

There are already forge modules with the same module names as what I'm proposing:

https://forge.puppet.com/camptocamp/augeas https://forge.puppet.com/ericsson/cron https://forge.puppet.com/lgbarn/mount https://forge.puppet.com/puppet/selinux

Although the modules are namespaced on the forge, you can't install 2 modules of the same module name from different authors, e.g. if you've already installed camptocamp-augeas then trying to install another augeas module fails:

$ bx puppet module install svarrette-augeas
Notice: Preparing to install into /Users/josh/.puppetlabs/etc/code/environments/production/modules ...
Notice: Downloading from https://forgeapi.puppet.com ...
Error: Could not install module 'svarrette-augeas' (latest)
  Installation would overwrite /Users/josh/.puppetlabs/etc/code/environments/production/modules/augeas
    Currently, 'camptocamp-augeas' (v1.6.0) is installed to that directory
    Use `puppet module install --force` to install this module anyway

So I think we will need to create unique names for all extracted modules, maybe something like puppet-augeas_core, puppet-cron_core, puppet-mount_core?

mciurcio commented 6 years ago

I've being using Puppet since 0.25 and I do training, consulting, deployment (PE and OSS) for a living since then.

I think this proposal is adding extra complexity for little to no actual again.

Puppet never had a "batteries included" experience since forever, IMHO. It is unfeasible to do anything efficiently without at last close to 20 modules from the Forge from the get go:

So Puppet IMHO is already pretty much close to bare bones compared to other solutions, no batteries included at all. A couple of more modules to install as a base to start actually doing something is completely reasonable, in exchange for not adding all this complexity.

Honestly all User Stories written on this PR are a nightmare from a workflow and maintenance perspective and I would not hesitate for a second to tell people/students/clients to do not do it.

Also, aren't we supposed to stop mixing Agent and Server stuff? I see only more scenarios here were this could explode on my face, as has happened in the past.

joshcooper commented 6 years ago

@mciurcio what are you proposing as an alternative? Don't vendor any modules in puppet-agent, and leave it up to users to install modules they care about?

mciurcio commented 6 years ago

@joshcooper Yep. Don't vendor, just publish the modules on the Forge. Sorry if my previous comment sounded harsh, it is not my intention.

trevor-vaughan commented 6 years ago

The only way that I can see this working well is if Puppet could incorporate the author name as part of the module namespace automatically and allow modules to co-exist with the same base names. It probably is far too much complexity for daily maintenance. I wouldn't mind loading speed gains if those are a possibility though.

trevor-vaughan commented 6 years ago

@joshcooper I'd like to add one more: As an experienced Puppet user, I would like to be able to use the puppet-build-tools to build the component releases so that I can test the 'as expected' changes.

joshcooper commented 6 years ago

To watchers, the latest commit reflects our current thinking. Vendored modules will effectively be part of the ruby load path like they are today, and they will not be pluginsynced. See https://github.com/puppetlabs/puppet/pull/6917

New users will get batteries included. Advanced users wanting to manage all module versions can set vendormoduledir = '' and puppet will ignore its vendored modules.