puppetlabs / control-repo

A control repository template
Apache License 2.0
204 stars 510 forks source link

Rename site directory to site-modules #71

Closed npwalker closed 5 years ago

npwalker commented 5 years ago

site -> site-modules

What

Starting with the control-repo template on Github.com, the "site" directory name will be gradually shifted across the ecosystem to become "site-modules".

<<<<<<< BEFORE

control-repo/
├── modules/         # This directory contains Forge modules from the Puppetfile
│   └── ...
└── site/            # This directory contains site-specific modules and is added to $modulepath.
    └── ...

=======

control-repo/
├── modules/         # This directory contains Forge modules from the Puppetfile
│   └── ...
└── site-modules/    # This directory contains site-specific modules and is added to $modulepath.
    └── ...

>>>>>>> AFTER

Why

The module, as a standard means of organizing and packaging automation content, is positioned to be a value proposition spanning products. We use modules with Puppet to organize desired state manifests. We use modules with Bolt to organize tasks and plans.

The "site" directory design is part of a long-standing pattern that separates externalized modules managed in a Puppetfile from embedded modules committed directly to the control-repo.

To date, only convention has configured this directory. It has not been codified in any tools. This is changing. We are codifying a default modulepath in Bolt that includes an embedded modules directory, separate from the codified directory for installed Forge modules. For ecosystem consistency, we anticipate following this move by codifying the path in Puppet also.

Users typically create and use embedded modules in their education journey first, prior to using a Puppetfile. Only convention tells users that a "site" directory should contain modules.

We are choosing not to injest and codify "site" with no design consideration simply because it exists as-is today. Puppet has done that a number of times in the past and in so doing missed opportunities for improvement (example: the story of code-manager).

We are codifying the directory name "site-modules". We are doing this to raise the prominence and obviousness of the value-term "module"—the design goal—while respecting and building on established precedent—the constraint.

This codification of a default name will not impact existing users. To date, getting "site" to work at all has required explicit configuration of the modulepath. Existing users therefore have their existing configuration defined already, taking precedence over the codified defaults.

History

The term "site" comes from before the control-repo held the full form it does today. Originally, site/ paired not next to a directory called modules/, but rather paired with other directories, e.g. dist/. In some cases site/ and other module dirs were even placed underneath a parent modules/ directory, as all of these directories are "modules".

  control-repo/
  ├── modules/      # A parent directory for all categorizations of Puppet modules
  .   ├── custom/   # in-house modules intended to address some specific functionality or software
  .   ├── dist/     # distribution modules (like modules you install from puppetforge or git)
  .   └── site/     # wrapper modules, which combine the functionality provided by distribution or custom modules

There was no consistency. Many variations on this pattern existed. Several forces contributed to today's structure solidifying in a form that omits the organizational affiliation with the modules/ parent directory. Those forces included Librarian and r10k defaulting to the codified modulepath "modules", inviting directory names from the original directory structure to be moved up a level. Notable too was lack of prescriptive direction from Puppet Labs on how to do this. There is a notable by-and-large omission of what to configure modulepath to in Gary Larizza's 2014-era blog posts, which served as the closest thing to canon recommendation at that time.

Remenants of this more full structuring, with site/ and dist/, can be found in official Puppet docs and in r10k docs to this day. A blog post from 2014 that does a good job of demonstrating both the era's lack of standardization and also the more complete form site/ came from can be found here.

The decision to codify site-modules/ is intended to restore the clear association with the term "modules" in the codified path while deviating only minimally from the solidified organic pattern.

Puppet has a history of adopting and codifying without question organic patterns. This change tries to balance the value of aligning with those patterns, while not constraining to them dogmatically.


Prior to this commit, we placed modules local to a users installation in the site directory. This was just a convention and the name site doesn't clearly convey what it is for.

After this commit, we place modules local to a users installation in the site-modules directory. This makes it more clear to users that this is a directory that modules go i. When users start with bolt they won't even know what a control-repo is and renaming site to site-modules gives them a better idea of why they should put their modules with tasks in them. Also see:

https://tickets.puppetlabs.com/browse/BOLT-1108

gabe-sky commented 5 years ago

I'm kind of not in favor of this, as tons of deployed customers, lots of blog posts, and numerous talks use the "site" directory. I appreciate the impetus, but I think it's too late to pick a different name. Bolt has very little existing documentation, and they have the luxury; I don't think we do, and we'll ultimately make things worse for new users if they can't copy and paste out of all the existing examples that use "site."

reidmv commented 5 years ago

For clarity, this PR and BOLT-1108 are both results of a discussion originating with CS. This PR is CS-originated, not Bolt-originated. The impetus came out of trying to map the user experience starting out knowing nothing about Bolt or Puppet all the way to understanding and using a Puppet control-repo.

The root problem is needing to be intuitively clear to new users about what a module is without undue "required reading" or required configuration. The directory name site does not communicate in any way that it contains modules, and so when designing education we're forced to either A) require some reading to know that the contents of this directory are modules; B) associate the word site with the word module via a required configuration file; or C) not educate users about what a module is until later, which has the side-effect of leaving the directory name site either confusing or a mystery.

This change only affects net-new users (who else is going to clone this repo?) and is as-nearly backwards compatible with the old name as it is possible to be. The assumption further is that site-modules is intuitively clear about what it contains, which is really what we need to accomplish.

Will write more later but hopefully this sets some context for further commentary.

ahpook commented 5 years ago

i'm 👍 on this generally, as long as people who currently use site won't be forced to change it.

i (inevitably) do have one syntactic nitpick, which is that the hyphen - isn't valid in module, class, or variable names in Puppet, so its use in the directory name is inconsistent; i'd prefer an underbar _.

reidmv commented 5 years ago

@ahpook I'd wondered if anyone was going to bring that up... 🙂

We're proposing a hyphen as a word separator so as to be aligned with the general prescriptions set out in the puppet-nogui design patterns repo. Specifically, the commentary given on the subject in the CLI commands doc guidance and in the API doc.

Boiling it down, the general nogui pattern guidance is "use hyphens as a word separators, NOT underscores, except when there's a technical constraint that dictates otherwise."

This guidance is specifically written for CLI flags (--host-key-check) and API endpoints (/api/v1/word-connector). The nogui repo doesn't explicitly map the guidance to filenames, but we've definitely been applying it there. Examples in the ecosystem include app directories under /etc/puppetlabs (console-services, bolt-server, code-staging), client-tools config files(puppet-access.conf), and more. We have underscore-containing filenames too, but they tend to be the older ones that predate the nogui guidance.

For site-modules, we are separating words. As Gary's blog put it in 2014, this is "a special directory of the Control Repo (usually called ‘site’ which is short for ‘site-specific modules’)". Following nogui guidance, we're joining the two words "site" and "modules" with a hyphen.

You're right that the Puppet language wouldn't allow hyphens for variable, module or class names. I don't think we need to worry about that in choosing a directory name though. Due to the ecosystem lack of universality (especially in filenames specifically), I wouldn't consider the DSL's preference for underscores a strong reason to use one here. Adhering to nogui guidance seems to offer a stronger benefit.

ahpook commented 5 years ago

I don't think either of those examples are actually relevant to directory naming, but I'm open to the argument that neither are mine. So 👍 go for it 👍

hpcprofessional commented 5 years ago

Piggy-backing on @ahpook Mature IaC sites may manage this directory in code. This could cause confusion when working with a local forge, or pulling from a local repo.

From CLI

puppet module install --module_repository http://dev-forge.example.com example-site-modules #breaks

In Puppetfile

forge 'dev-forge.example.com' mod 'example/site-modules', '1.0.1' #breaks

Obviously this isn't a show stopper, but we probably should provide examples of this use case, as well as managing it via Puppetfile as it's no longer trivial. This assumes r10k and code manager workflows of course; I don't anticipate any impact to CD4PE-based workflows.