Restore behaviour of servers and pools parameters

REFERENCE.md

@@ -17,6 +17,19 @@
 * `chrony::params`: chrony class parameters
 * `chrony::service`: Manages the chrony service
 
+### Functions
+
+#### Public Functions
+
+
+#### Private Functions
+
+* `chrony::server_array_to_hash`: Function to normalise servers/pools/peers
+
+### Data types
+
+* [`Chrony::Servers`](#chronyservers): Type for the `servers`, `pools` and `peers` parameters.
+
 ## Classes
 
 ### `chrony`
@@ -34,14 +47,25 @@ Installs and configures chrony
 include chrony
 ```
 
-##### Use specific servers
+##### Use specific servers (These will be configured with the `iburst` option.)
 
 ```puppet
 class { 'chrony':
   servers => [ 'ntp1.corp.com', 'ntp2.corp.com', ],
 }
 ```
 
+##### Two specific servers without `iburst`
+
+```puppet
+class { 'chrony':
+  servers => {
+    'ntp1.corp.com' => [],
+    'ntp2.corp.com' => [],
+  },
+}
+```
+
 ##### Ensure a secret password is used for chronyc
 
 ```puppet
@@ -297,19 +321,20 @@ Default value: ``undef``
 
 ##### `peers`
 
-Data type: `Any`
+Data type: `Chrony::Servers`
 
 This selects the servers to use for NTP peers (symmetric association).
-It is an array of servers.
+It can be an array of peers or a hash of peers with their respective options.
 
 Default value: `[]`
 
 ##### `servers`
 
-Data type: `Variant[Hash,Array[Stdlib::Host]]`
+Data type: `Chrony::Servers`
 
 This selects the servers to use for NTP servers.  It can be an array of servers
-or a hash of servers to their respective options.
+or a hash of servers to their respective options. If an array is used, `iburst` will be configured for each server.
+If you don't want to use `iburst`, use a hash instead.
 
 Default value: `{
     '0.pool.ntp.org' => ['iburst'],
@@ -320,10 +345,10 @@ Default value: `{
 
 ##### `pools`
 
-Data type: `Variant[Hash,Array[Stdlib::Fqdn]]`
+Data type: `Chrony::Servers`
 
 This is used to specify one or more *pools* of NTP servers to use instead of individual NTP servers.
-Similar to [`server`](#server), it can be an array of pools or a hash of pools to their respective options.
+Similar to [`server`](#server), it can be an array of pools, (using iburst), or a hash of pools to their respective options.
 See [pool](https://chrony.tuxfamily.org/doc/3.4/chrony.conf.html#pool)
 
 Default value: `{}`
@@ -526,3 +551,40 @@ Directory to store measurement history in on exit.
 
 Default value: `$chrony::params::dumpdir`
 
+## Functions
+
+## Data types
+
+### `Chrony::Servers`
+
+This type is for the `servers`, `pools` and `peers` parameters.
+
+#### Examples
+
+##### A hash of servers
+
+```puppet
+{
+  'ntp1.example.com => [
+    'minpoll 3',
+    'maxpoll 6',
+  ],
+  'ntp2.example.com => [
+    'iburst',
+    'minpoll 4',
+    'maxpoll 8',
+  ],
+}
+```
+
+##### An array of servers
+
+```puppet
+[
+  'ntp1.example.com',
+  'ntp2.example.com',
+]
+```
+
+Alias of `Variant[Hash[Stdlib::Host, Optional[Array[String]]], Array[Stdlib::Host]]`
+

functions/server_array_to_hash.pp

@@ -0,0 +1,12 @@
+# @summary Function to normalise servers/pools/peers
+#
+# @api private
+function chrony::server_array_to_hash(Variant[Hash,Array] $servers, $options = []) >> Hash {
+  if $servers.is_a(Hash) {
+    $servers
+  } else {
+    $servers.reduce({}) |$memo, $server| { # lint:ignore:manifest_whitespace_opening_brace_before
+      $memo + { $server => $options }
+    }
+  }
+}

manifests/config.pp

@@ -43,7 +43,13 @@
     owner   => 0,
     group   => 0,
     mode    => '0644',
-    content => epp($config_template),
+    content => epp($config_template,
+      {
+        servers => chrony::server_array_to_hash($servers, ['iburst']),
+        pools   => chrony::server_array_to_hash($pools, ['iburst']),
+        peers   => chrony::server_array_to_hash($peers),
+      }
+    ),
   }
 
   file { $config_keys:

manifests/init.pp

@@ -2,10 +2,17 @@
 #
 # @example Install chrony with default options
 #   include chrony
-# @example Use specific servers
+# @example Use specific servers (These will be configured with the `iburst` option.)
 #   class { 'chrony':
 #     servers => [ 'ntp1.corp.com', 'ntp2.corp.com', ],
 #   }
+# @example Two specific servers without `iburst`
+#   class { 'chrony':
+#     servers => {
+#       'ntp1.corp.com' => [],
+#       'ntp2.corp.com' => [],
+#     },
+#   }
 # @example Ensure a secret password is used for chronyc
 #   class { 'chrony':
 #     servers         => [ 'ntp1.corp.com', 'ntp2.corp.com', ],
@@ -104,13 +111,14 @@
 #   Also see [`package_source`](#package_source).
 # @param peers
 #   This selects the servers to use for NTP peers (symmetric association).
-#   It is an array of servers.
+#   It can be an array of peers or a hash of peers with their respective options.
 # @param servers
 #   This selects the servers to use for NTP servers.  It can be an array of servers
-#   or a hash of servers to their respective options.
+#   or a hash of servers to their respective options. If an array is used, `iburst` will be configured for each server.
+#   If you don't want to use `iburst`, use a hash instead.
 # @param pools
 #   This is used to specify one or more *pools* of NTP servers to use instead of individual NTP servers.
-#   Similar to [`server`](#server), it can be an array of pools or a hash of pools to their respective options.
+#   Similar to [`server`](#server), it can be an array of pools, (using iburst), or a hash of pools to their respective options.
 #   See [pool](https://chrony.tuxfamily.org/doc/3.4/chrony.conf.html#pool)
 # @param refclocks
 #   This should be a Hash of hardware reference clock drivers to use.  They hash
@@ -201,14 +209,14 @@
   Optional[String] $package_source                                 = undef,
   Optional[String] $package_provider                               = undef,
   $refclocks                                                       = [],
-  $peers                                                           = [],
-  Variant[Hash,Array[Stdlib::Host]] $servers                       = {
+  Chrony::Servers $peers                                           = [],
+  Chrony::Servers $servers                                         = {
     '0.pool.ntp.org' => ['iburst'],
     '1.pool.ntp.org' => ['iburst'],
     '2.pool.ntp.org' => ['iburst'],
     '3.pool.ntp.org' => ['iburst'],
   },
-  Variant[Hash,Array[Stdlib::Fqdn]] $pools                         = {},
+  Chrony::Servers $pools                                           = {},
   Numeric $makestep_seconds                                        = 10,
   Integer $makestep_updates                                        = 3,
   Array[String] $queryhosts                                        = [],

spec/classes/chrony_spec.rb

@@ -16,6 +16,25 @@
       let(:facts) do
         facts
       end
+      let(:config_file) do
+        case facts[:osfamily]
+        when 'Archlinux', 'RedHat', 'Suse'
+          '/etc/chrony.conf'
+        else
+          '/etc/chrony/chrony.conf'
+        end
+      end
+      let(:keys_file) do
+        case facts[:osfamily]
+        when 'Archlinux', 'RedHat', 'Suse'
+          '/etc/chrony.keys'
+        else
+          '/etc/chrony/chrony.keys'
+        end
+      end
+      let(:config_file_contents) do
+        catalogue.resource('file', config_file).send(:parameters)[:content]
+      end
 
       context 'with defaults' do
         it { is_expected.to compile.with_all_deps }
@@ -237,6 +256,141 @@
         end
       end
 
+      describe 'servers' do
+        context 'by default' do
+          it do
+            expected_lines = [
+              'server 0.pool.ntp.org iburst',
+              'server 1.pool.ntp.org iburst',
+              'server 2.pool.ntp.org iburst',
+              'server 3.pool.ntp.org iburst'
+            ]
+            expect(config_file_contents.split("\n") & expected_lines).to eq(expected_lines)
+          end
+        end
+        context 'when servers is an array' do
+          let(:params) do
+            {
+              servers: ['ntp1.corp.com', 'ntp2.corp.com'],
+            }
+          end
+
+          it do
+            expected_lines = [
+              'server ntp1.corp.com iburst',
+              'server ntp2.corp.com iburst',
+            ]
+            expect(config_file_contents.split("\n") & expected_lines).to eq(expected_lines)
+          end
+        end
+        context 'when servers is an (unsorted) hash' do
+          let(:params) do
+            {
+              servers: {
+                'ntp3.corp.com' => [],
+                'ntp1.corp.com' => ['key 25', 'iburst'],
+                'ntp4.corp.com' => :undef,
+                'ntp2.corp.com' => ['key 25', 'iburst'],
+              }
+            }
+          end
+
+          it do
+            expected_lines = [
+              'server ntp1.corp.com key 25 iburst',
+              'server ntp2.corp.com key 25 iburst',
+              'server ntp3.corp.com',
+              'server ntp4.corp.com',
+            ]
+            expect(config_file_contents.split("\n") & expected_lines).to eq(expected_lines)
+          end
+        end
+      end
+
+      describe 'pools' do
+        context 'by default' do
+          it { expect(config_file_contents).not_to match(%r{^pool}) }
+        end
+        context 'when pools is an array' do
+          let(:params) do
+            {
+              pools: ['0.pool.ntp.org', '1.pool.ntp.org']
+            }
+          end
+
+          it do
+            expected_lines = [
+              'server 0.pool.ntp.org iburst',
+              'server 1.pool.ntp.org iburst',
+            ]
+            expect(config_file_contents.split("\n") & expected_lines).to eq(expected_lines)
+          end
+        end
+        context 'when pools is a hash' do
+          let(:params) do
+            {
+              pools: {
+                '3.pool.ntp.org' => [],
+                '0.pool.ntp.org' => ['maxsources 4'],
+                '1.pool.ntp.org' => ['maxsources 4'],
+                '2.pool.ntp.org' => ['maxsources 4'],
+              }
+            }
+          end
+
+          it do
+            expected_lines = [
+              'pool 0.pool.ntp.org maxsources 4',
+              'pool 1.pool.ntp.org maxsources 4',
+              'pool 2.pool.ntp.org maxsources 4',
+              'pool 3.pool.ntp.org',
+            ]
+            expect(config_file_contents.split("\n") & expected_lines).to eq(expected_lines)
+          end
+        end
+      end
+
+      describe 'peers' do
+        context 'by default' do
+          it { expect(config_file_contents).not_to match(%r{^peer}) }
+        end
+        context 'when peers is an array' do
+          let(:params) do
+            {
+              peers: ['peer1.example.com', 'peer2.example.com']
+            }
+          end
+
+          it do
+            expected_lines = [
+              'peer peer1.example.com',
+              'peer peer2.example.com',
+            ]
+            expect(config_file_contents.split("\n") & expected_lines).to eq(expected_lines)
+          end
+        end
+        context 'when peers is a hash' do
+          let(:params) do
+            {
+              peers: {
+                'peer1.example.com' => [],
+                'peer2.example.com' => ['maxpoll 6'],
+                'peer3.example.com' => :undef,
+              }
+            }
+          end
+
+          it do
+            expected_lines = [
+              'peer peer1.example.com',
+              'peer peer2.example.com maxpoll 6',
+              'peer peer3.example.com',
+            ]
+            expect(config_file_contents.split("\n") & expected_lines).to eq(expected_lines)
+          end
+        end
+      end
+
       context 'unmanaged chrony.keys file' do
         let(:params) do
           {

spec/type_aliases/servers_spec.rb

@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe 'Chrony::Servers' do
+  [
+    ['ntp1.example.com', 'ntp2.example.com'],
+    {
+      'ntp1.example.com' => [],
+      'ntp2.example.com' => ['maxpoll 6'],
+    },
+    {},
+    [],
+    {
+      'ntp1.example.com' => :undef
+    }
+  ].each do |value|
+    describe value.inspect do
+      it { is_expected.to allow_value(value) }
+    end
+  end
+end