Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename site directory to site-modules #71

Merged
merged 4 commits into from
Feb 26, 2019

Conversation

npwalker
Copy link
Contributor

@npwalker npwalker commented Feb 1, 2019

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

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
@npwalker npwalker requested review from reidmv and adreyer February 1, 2019 01:03
@gabe-sky
Copy link
Contributor

gabe-sky commented Feb 1, 2019

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
Copy link
Contributor

reidmv commented Feb 1, 2019

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.

@@ -1,2 +1,2 @@
modulepath = site:modules:$basemodulepath
modulepath = site-modules:modules:$basemodulepath
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is opposite of what bolt does.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in bolt places modules before site-modules?

This change just renamed site to site-modules. idk if we care much about the ordering. I think it would only matter in rare scenarios

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah my assumption is that this order isn't super important in most cases. Bolt uses the first entry on the modulepath to install to the puppetfile from so it has to be modules and it seems like it would be nice if the standard control repo behaved the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It used to be that way: #44

The symlink served for a good discussion point around change impact, but
in the end having it makes for a more confusing experience overall both
to new users cloning the control-repo to get started and also to anyone
accustomed to "site". A new user won't miss "site". A symlink will muddy
the waters over the change for long-time users. Better for clarity to be
all-in and not include a symlink.
Copy link
Contributor

@binford2k binford2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure there are more descriptive names out there, but this seems the best bridge between the past and the future. Even people reading old blog posts referencing site will almost certainly intuitively guess that site-modules is the evolution of that.

@ahpook
Copy link

ahpook commented Feb 26, 2019

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
Copy link
Contributor

reidmv commented Feb 26, 2019

@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
Copy link

ahpook commented Feb 26, 2019

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 👍

@reidmv reidmv merged commit 306107b into puppetlabs:production Feb 26, 2019
reidmv added a commit to reidmv/seteam-control-repo that referenced this pull request Mar 6, 2019
To match the template puppetlabs/control-repo.

See puppetlabs/control-repo#71
@hpcprofessional
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants