Skip to content

Commit 13ddbd9

Browse files
committed
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 <mike@mikeharp.com>
1 parent d170803 commit 13ddbd9

2 files changed

Lines changed: 75 additions & 88 deletions

File tree

docs/_openvox_8x/bgtm.md

Lines changed: 67 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ This section covers:
4242
* [How best to order your classes (rather than resources)](#ordering).
4343
* [How to leverage and utilize dependencies](#dependencies).
4444

45-
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.
45+
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.
4646

4747
### Class design
4848

@@ -59,81 +59,68 @@ In terms of class structure we recommend the following (more detail below):
5959
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.
6060
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`.
6161

62-
For instance, the main `ntp` class in the `ntp` module looks like this:
62+
For instance, the main `chrony` class in the `chrony` module looks like this:
6363

6464
```puppet
65-
class ntp (
66-
Boolean $broadcastclient,
67-
Stdlib::Absolutepath $config,
68-
Optional[Stdlib::Absolutepath] $config_dir,
69-
String $config_file_mode,
70-
Optional[String] $config_epp,
71-
Optional[String] $config_template,
72-
Boolean $disable_auth,
73-
Boolean $disable_dhclient,
74-
Boolean $disable_kernel,
75-
Boolean $disable_monitor,
76-
Optional[Array[String]] $fudge,
77-
Stdlib::Absolutepath $driftfile,
65+
class chrony (
66+
Array[Stdlib::IP::Address] $bindaddress = [],
67+
Array[String] $bindcmdaddress = ['127.0.0.1', '::1'],
68+
Optional[String] $initstepslew = undef,
69+
Array[String] $cmdacl = [],
70+
NotUndef $commandkey = 0,
71+
Stdlib::Unixpath $config = '/etc/chrony/chrony.conf',
72+
Stdlib::Filemode $config_mode = '0644',
73+
Boolean $config_keys_manage = true,
74+
Array[String[1]] $keys = [],
75+
Stdlib::Unixpath $driftfile = '/var/lib/chrony/drift',
7876
...
7977
```
8078

79+
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.
80+
8181
#### `module::install`
8282

8383
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.
8484

85-
The install class must be named `module::install`, as in the `ntp` module:
85+
The install class must be named `module::install`, as in the `chrony` module:
8686

8787
```puppet
88-
class ntp::install {
89-
90-
if $ntp::package_manage {
91-
package { $ntp::package_name:
92-
ensure => $ntp::package_ensure,
93-
}
94-
88+
class chrony::install {
89+
assert_private()
90+
91+
package { 'chrony':
92+
ensure => $chrony::package_ensure,
93+
name => $chrony::package_name,
94+
source => $chrony::package_source,
95+
provider => $chrony::package_provider,
9596
}
96-
9797
}
9898
```
9999

100+
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.
101+
100102
#### `module::config`
101103

102104
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.
103105

104-
For example, see the `module::config` class in the `ntp` module:
106+
For example, see the `module::config` class in the `chrony` module:
105107

106108
```puppet
107-
class ntp::config {
108-
109-
#The servers-netconfig file overrides NTP config on SLES 12, interfering with our configuration.
110-
if $facts['operatingsystem'] == 'SLES' and $facts['operatingsystemmajrelease'] == '12' {
111-
file { '/var/run/ntp/servers-netconfig':
112-
ensure => 'absent'
113-
}
114-
}
115-
116-
if $ntp::keys_enable {
117-
case $ntp::config_dir {
118-
'/', '/etc', undef: {}
119-
default: {
120-
file { $ntp::config_dir:
121-
ensure => directory,
122-
owner => 0,
123-
group => 0,
124-
mode => '0775',
125-
recurse => false,
126-
}
109+
class chrony::config {
110+
assert_private()
111+
112+
file { $chrony::config:
113+
ensure => file,
114+
owner => 0,
115+
group => 0,
116+
mode => $chrony::config_mode,
117+
content => epp($chrony::config_template,
118+
{
119+
servers => chrony::server_array_to_hash($chrony::servers, ['iburst']),
120+
pools => chrony::server_array_to_hash($chrony::pools, ['iburst']),
121+
peers => chrony::server_array_to_hash($chrony::peers),
127122
}
128-
}
129-
130-
file { $ntp::keys_file:
131-
ensure => file,
132-
owner => 0,
133-
group => 0,
134-
mode => '0644',
135-
content => epp('ntp/keys.epp'),
136-
}
123+
),
137124
}
138125
...
139126
```
@@ -145,23 +132,22 @@ The remaining service resources, and anything else related to the running state
145132
For example:
146133

147134
```puppet
148-
class ntp::service {
135+
class chrony::service {
136+
assert_private()
149137
150-
if ! ($ntp::service_ensure in [ 'running', 'stopped' ]) {
151-
fail('service_ensure parameter must be running or stopped')
138+
if $chrony::service_manage {
139+
service { $chrony::service_name:
140+
ensure => $chrony::service_ensure,
141+
enable => $chrony::service_enable,
142+
}
152143
}
153144
154-
if $ntp::service_manage == true {
155-
service { 'ntp':
156-
ensure => $ntp::service_ensure,
157-
enable => $ntp::service_enable,
158-
name => $ntp::service_name,
159-
provider => $ntp::service_provider,
160-
hasstatus => true,
161-
hasrestart => true,
145+
if $chrony::wait_manage {
146+
service { $chrony::wait_name:
147+
ensure => $chrony::wait_ensure,
148+
enable => $chrony::wait_enable,
162149
}
163150
}
164-
165151
}
166152
```
167153

@@ -177,22 +163,22 @@ Naming consistency is imperative for community comprehension and assists in trou
177163

178164
Best practices recommend the pattern of `thing_property` for naming parameters.
179165

180-
For example, in the `ntp` module
166+
For example, in the `chrony` module the service resource uses `service_name`, `service_ensure`, and `service_enable`:
181167

182168
```puppet
183-
class ntp::install {
169+
class chrony::service {
184170
185-
if $ntp::package_manage {
186-
package { $ntp::package_name:
187-
ensure => $ntp::package_ensure,
171+
if $chrony::service_manage {
172+
service { $chrony::service_name:
173+
ensure => $chrony::service_ensure,
174+
enable => $chrony::service_enable,
188175
}
189-
190176
}
191177
192178
}
193179
```
194180

195-
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.
181+
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.
196182

197183
Consistent naming across modules helps with the readability and usability of your code.
198184

@@ -228,15 +214,16 @@ To allow other modules to form ordering relationships with your module, ensure t
228214

229215
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).
230216

231-
For example, the `ntp` module uses containment in the main `ntp` class:
217+
For example, the `chrony` module uses containment in the main `chrony` class:
232218

233219
```puppet
234-
contain ntp::install
235-
contain ntp::config
236-
contain ntp::service
237-
Class['::ntp::install'] ->
238-
Class['::ntp::config'] ~>
239-
Class['::ntp::service']
220+
contain chrony::install
221+
contain chrony::config
222+
contain chrony::service
223+
224+
Class['chrony::install']
225+
-> Class['chrony::config']
226+
~> Class['chrony::service']
240227
```
241228

242229
### Dependencies

docs/_openvox_8x/style_guide.markdown

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,15 +191,15 @@ Do not use `/* */` comments in Puppet code.
191191
**Good:**
192192

193193
```puppet
194-
# Configures NTP
195-
file { '/etc/ntp.conf': ... }
194+
# Configures chrony
195+
file { '/etc/chrony.conf': ... }
196196
```
197197

198198
**Bad:**
199199

200200
```puppet
201-
/* Creates file /etc/ntp.conf */
202-
file { '/etc/ntp.conf': ... }
201+
/* Creates file /etc/chrony.conf */
202+
file { '/etc/chrony.conf': ... }
203203
```
204204

205205
#### Documentation comments
@@ -712,7 +712,7 @@ Documentation [comments](#documentation-comments) for OpenVox Strings should be
712712
1. Following lines, if applicable: Define parameters. Parameters should be [typed](./lang_data_type.html).
713713
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.
714714
* Validations should validate any parameters and fail catalog compilation if any
715-
parameters are invalid. See [ntp](https://github.com/puppetlabs/puppetlabs-ntp/blob/master/manifests/init.pp#L212:L274) for an example.
715+
parameters are invalid. See [chrony](https://github.com/voxpupuli/puppet-chrony/blob/master/manifests/init.pp#L371-L373) for an example.
716716
1. Next lines, if applicable: Should declare local variables and perform variable munging.
717717
1. Next lines: Should declare resource defaults.
718718
1. Next lines: Should override resources if necessary.
@@ -1262,20 +1262,20 @@ For example:
12621262
```puppet
12631263
# @param config_epp Specifies a file to act as a EPP template for the config file.
12641264
# Valid options: a path (absolute, or relative to the module path). Example value:
1265-
# 'ntp/ntp.conf.epp'. A validation error is thrown if you supply both this param **and**
1265+
# 'chrony/chrony.conf.epp'. A validation error is thrown if you supply both this param **and**
12661266
# the `config_template` param.
12671267
```
12681268

12691269
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.
12701270

1271-
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:
1271+
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:
12721272

12731273
```markdown
12741274
#### `package_ensure`
12751275

12761276
Data type: String.
12771277

1278-
Whether to install the NTP package, and what version to install. Values: 'present', 'latest', or a specific version number.
1278+
Whether to install the chrony package, and what version to install. Values: 'present', 'latest', or a specific version number.
12791279

12801280
Default value: 'present'.
12811281
```

0 commit comments

Comments
 (0)