From 13ddbd9c48c2e2f654d00b12735879a101204fcd Mon Sep 17 00:00:00 2001 From: Michael Harp Date: Tue, 7 Jul 2026 08:32:21 -0400 Subject: [PATCH] Use VoxPupuli's puppet-chrony as the reference module bgtm and the style guide used puppetlabs-ntp as their running "real-world best practices" example. ntp is a puppetlabs module, and its parameter-defaults approach (every default in common.yaml, none inline) contradicts the guidance these same docs give. VoxPupuli's puppet-chrony is a community-maintained module with the identical init.pp/install/config/service + contain structure that keeps static defaults inline and overrides only OS-specific values in per-OS Hiera data. - bgtm.md: convert the full walkthrough (intro, main class signature, install/config/service classes, the naming example, and the containment block) to chrony, with every snippet based on real module source. The main class signature now shows inline defaults, the naming example uses chrony::service's service_manage toggle to illustrate the thing_manage convention, and a note explains the assert_private() calls that mark the subordinate classes private. - style_guide.markdown: convert the ntp references where ntp stood in as the reference module (the comment example, the validation-example link, and the parameter-documentation example). The "Bad" parameter-ordering example is left as ntp so chrony is not cast as the bad example. Closes #395 Signed-off-by: Michael Harp --- docs/_openvox_8x/bgtm.md | 147 ++++++++++++-------------- docs/_openvox_8x/style_guide.markdown | 16 +-- 2 files changed, 75 insertions(+), 88 deletions(-) diff --git a/docs/_openvox_8x/bgtm.md b/docs/_openvox_8x/bgtm.md index 95720ade0..90370030f 100644 --- a/docs/_openvox_8x/bgtm.md +++ b/docs/_openvox_8x/bgtm.md @@ -42,7 +42,7 @@ This section covers: * [How best to order your classes (rather than resources)](#ordering). * [How to leverage and utilize dependencies](#dependencies). -To demonstrate a real-world best practices standard module, we will walk through the structure of the [puppetlabs-ntp](https://forge.puppet.com/puppetlabs/ntp) module. +To demonstrate a real-world best practices standard module, we will walk through the structure of VoxPupuli's [puppet-chrony](https://github.com/voxpupuli/puppet-chrony) module. ### Class design @@ -59,81 +59,68 @@ In terms of class structure we recommend the following (more detail below): The main class of any module must share the name of the module and be located in the `init.pp` file. The name and location of the main module class is extremely important, as it guides the [autoloader](./lang_namespaces.html#autoloader-behavior) behavior. The main class of a module is its interface point and ought to be the only parameterized class if possible. Limiting the parameterized classes to just the main class allows you to control usage of the entire module with the inclusion of a single class. This class should provide sensible defaults so that a user can get going with `include module`. -For instance, the main `ntp` class in the `ntp` module looks like this: +For instance, the main `chrony` class in the `chrony` module looks like this: ```puppet -class ntp ( - Boolean $broadcastclient, - Stdlib::Absolutepath $config, - Optional[Stdlib::Absolutepath] $config_dir, - String $config_file_mode, - Optional[String] $config_epp, - Optional[String] $config_template, - Boolean $disable_auth, - Boolean $disable_dhclient, - Boolean $disable_kernel, - Boolean $disable_monitor, - Optional[Array[String]] $fudge, - Stdlib::Absolutepath $driftfile, +class chrony ( + Array[Stdlib::IP::Address] $bindaddress = [], + Array[String] $bindcmdaddress = ['127.0.0.1', '::1'], + Optional[String] $initstepslew = undef, + Array[String] $cmdacl = [], + NotUndef $commandkey = 0, + Stdlib::Unixpath $config = '/etc/chrony/chrony.conf', + Stdlib::Filemode $config_mode = '0644', + Boolean $config_keys_manage = true, + Array[String[1]] $keys = [], + Stdlib::Unixpath $driftfile = '/var/lib/chrony/drift', ... ``` +Note that each static default is declared inline, right on the parameter, so the value is visible in the class signature and in generated reference documentation. + #### `module::install` The install class must be located in the `install.pp` file. It should contain all of the resources related to getting the software that the module manages onto the node. -The install class must be named `module::install`, as in the `ntp` module: +The install class must be named `module::install`, as in the `chrony` module: ```puppet -class ntp::install { - - if $ntp::package_manage { - package { $ntp::package_name: - ensure => $ntp::package_ensure, - } - +class chrony::install { + assert_private() + + package { 'chrony': + ensure => $chrony::package_ensure, + name => $chrony::package_name, + source => $chrony::package_source, + provider => $chrony::package_provider, } - } ``` +The `install`, `config`, and `service` classes are private: they are declared only by the main `chrony` class, never directly by users. Each one calls `assert_private()`, which fails compilation with a clear message if the class is declared from outside its own module. See [public and private classes](./style_guide.html#public-and-private) for more on this distinction. + #### `module::config` The resources related to configuring the installed software should be placed in a config class. The config class must be named `module::config` and must be located in the `config.pp` file. -For example, see the `module::config` class in the `ntp` module: +For example, see the `module::config` class in the `chrony` module: ```puppet -class ntp::config { - - #The servers-netconfig file overrides NTP config on SLES 12, interfering with our configuration. - if $facts['operatingsystem'] == 'SLES' and $facts['operatingsystemmajrelease'] == '12' { - file { '/var/run/ntp/servers-netconfig': - ensure => 'absent' - } - } - - if $ntp::keys_enable { - case $ntp::config_dir { - '/', '/etc', undef: {} - default: { - file { $ntp::config_dir: - ensure => directory, - owner => 0, - group => 0, - mode => '0775', - recurse => false, - } +class chrony::config { + assert_private() + + file { $chrony::config: + ensure => file, + owner => 0, + group => 0, + mode => $chrony::config_mode, + content => epp($chrony::config_template, + { + servers => chrony::server_array_to_hash($chrony::servers, ['iburst']), + pools => chrony::server_array_to_hash($chrony::pools, ['iburst']), + peers => chrony::server_array_to_hash($chrony::peers), } - } - - file { $ntp::keys_file: - ensure => file, - owner => 0, - group => 0, - mode => '0644', - content => epp('ntp/keys.epp'), - } + ), } ... ``` @@ -145,23 +132,22 @@ The remaining service resources, and anything else related to the running state For example: ```puppet -class ntp::service { +class chrony::service { + assert_private() - if ! ($ntp::service_ensure in [ 'running', 'stopped' ]) { - fail('service_ensure parameter must be running or stopped') + if $chrony::service_manage { + service { $chrony::service_name: + ensure => $chrony::service_ensure, + enable => $chrony::service_enable, + } } - if $ntp::service_manage == true { - service { 'ntp': - ensure => $ntp::service_ensure, - enable => $ntp::service_enable, - name => $ntp::service_name, - provider => $ntp::service_provider, - hasstatus => true, - hasrestart => true, + if $chrony::wait_manage { + service { $chrony::wait_name: + ensure => $chrony::wait_ensure, + enable => $chrony::wait_enable, } } - } ``` @@ -177,22 +163,22 @@ Naming consistency is imperative for community comprehension and assists in trou Best practices recommend the pattern of `thing_property` for naming parameters. -For example, in the `ntp` module +For example, in the `chrony` module the service resource uses `service_name`, `service_ensure`, and `service_enable`: ```puppet -class ntp::install { +class chrony::service { - if $ntp::package_manage { - package { $ntp::package_name: - ensure => $ntp::package_ensure, + if $chrony::service_manage { + service { $chrony::service_name: + ensure => $chrony::service_ensure, + enable => $chrony::service_enable, } - } } ``` -If you have a parameter that toggles an entire function on and off, the naming convention can be amended to `thing_manage`. This applies, in particular, to Boolean toggles, such as when the module manages the installation altogether. The `thing_manage` convention allows you to wrap all of the resources in an `if $package_manage {}` test, as shown in the `ntp` example above. +If you have a parameter that toggles an entire function on and off, the naming convention can be amended to `thing_manage`. This applies, in particular, to Boolean toggles, such as when the module manages the service altogether. The `thing_manage` convention allows you to wrap all of the resources in an `if $service_manage {}` test, as shown in the `chrony` example above. Consistent naming across modules helps with the readability and usability of your code. @@ -228,15 +214,16 @@ To allow other modules to form ordering relationships with your module, ensure t Classes do not _automatically_ contain the classes they declare. This is because classes can be declared in several places via `include` and similar functions. To contain classes, use [the `contain` function](./function.html#contain). For more information and context about containment, see [the containment docs](./lang_containment.html). -For example, the `ntp` module uses containment in the main `ntp` class: +For example, the `chrony` module uses containment in the main `chrony` class: ```puppet -contain ntp::install -contain ntp::config -contain ntp::service -Class['::ntp::install'] -> -Class['::ntp::config'] ~> -Class['::ntp::service'] +contain chrony::install +contain chrony::config +contain chrony::service + +Class['chrony::install'] +-> Class['chrony::config'] +~> Class['chrony::service'] ``` ### Dependencies diff --git a/docs/_openvox_8x/style_guide.markdown b/docs/_openvox_8x/style_guide.markdown index 83b86a0af..8b99eea8c 100644 --- a/docs/_openvox_8x/style_guide.markdown +++ b/docs/_openvox_8x/style_guide.markdown @@ -191,15 +191,15 @@ Do not use `/* */` comments in Puppet code. **Good:** ```puppet -# Configures NTP -file { '/etc/ntp.conf': ... } +# Configures chrony +file { '/etc/chrony.conf': ... } ``` **Bad:** ```puppet -/* Creates file /etc/ntp.conf */ -file { '/etc/ntp.conf': ... } +/* Creates file /etc/chrony.conf */ +file { '/etc/chrony.conf': ... } ``` #### Documentation comments @@ -712,7 +712,7 @@ Documentation [comments](#documentation-comments) for OpenVox Strings should be 1. Following lines, if applicable: Define parameters. Parameters should be [typed](./lang_data_type.html). 1. Next lines: Includes and validation come after parameters are defined. Includes may come before or after validation, but should be grouped separately, with all includes and requires in one group and all validations in another. * Validations should validate any parameters and fail catalog compilation if any - parameters are invalid. See [ntp](https://github.com/puppetlabs/puppetlabs-ntp/blob/master/manifests/init.pp#L212:L274) for an example. + parameters are invalid. See [chrony](https://github.com/voxpupuli/puppet-chrony/blob/master/manifests/init.pp#L371-L373) for an example. 1. Next lines, if applicable: Should declare local variables and perform variable munging. 1. Next lines: Should declare resource defaults. 1. Next lines: Should override resources if necessary. @@ -1262,20 +1262,20 @@ For example: ```puppet # @param config_epp Specifies a file to act as a EPP template for the config file. # Valid options: a path (absolute, or relative to the module path). Example value: -# 'ntp/ntp.conf.epp'. A validation error is thrown if you supply both this param **and** +# 'chrony/chrony.conf.epp'. A validation error is thrown if you supply both this param **and** # the `config_template` param. ``` If you use Strings to document your module, include information about Strings in the Reference section of your README so that your users will know how to generate the documentation. See [OpenVox Strings](https://github.com/OpenVoxProject/openvox-strings) documentation for details on usage, installation, and correctly writing documentation comments. -If you do not include Strings code comments, you should include a Reference section in your README with a complete list of all classes, types, providers, defined types, and parameters that the user can configure. Include a brief description, the valid options, and the default values (if any). For example, this is a parameter for the `ntp` module's `ntp` class: +If you do not include Strings code comments, you should include a Reference section in your README with a complete list of all classes, types, providers, defined types, and parameters that the user can configure. Include a brief description, the valid options, and the default values (if any). For example, this is a parameter for the `chrony` module's `chrony` class: ```markdown #### `package_ensure` Data type: String. -Whether to install the NTP package, and what version to install. Values: 'present', 'latest', or a specific version number. +Whether to install the chrony package, and what version to install. Values: 'present', 'latest', or a specific version number. Default value: 'present'. ```