avoid ipptool errors when Cups is slow to start

Problem statement: on some OSes (most notably RHEL7 and derivatives), the
Cups service may have started and not listen on the IPP port yet. This causes
a problem when this module (re)starts the service and expects to be able to
add/remove printers with ipptool right after.

Thus, modify the standard cups.socket service on RHEL7 through a drop-in
to also manage the IPP port. This way, systemd will accept the incoming
connections and hand them off to Cups once it is really started. This is
mostly a kludge for a 3rd party bug, but still required to make the
module behave reliably.

Fix some cases of leoarnold#35.

Breaking changes:
- Bump the Puppet minimum version to 4.10.0 for Hiera 5.
- Depend on puppetlabs/stdlib and camptocamp/systemd (and its dependency
  puppetlabs/inifile).

.fixtures.yml

@@ -2,4 +2,7 @@
 fixtures:
   symlinks:
     cups: "#{source_dir}"
-
+  forge_modules:
+    systemd: camptocamp/systemd
+    stdlib: puppetlabs/stdlib
+    inifile: puppetlabs/inifile

README.md.erb

@@ -514,6 +514,11 @@ Installs, configures, and manages the CUPS service.
 
 * `service_names`: A name or an array of names of all CUPS services to be managed. Defaults to `cups`.
 
+* `socket_manage`: Workaround a CUPS/OS bug (see `cups::server::socket`).
+   Enabled by default on RHEL7, disabled elsewhere.
+
+* `socket_ensure`: Should match `service_ensure`, the default.
+
 * `web_interface`:  Boolean value to enable or disable the server's web interface.
 
 #### Type: `cups_queue`

data/os/RedHat-7.yaml

@@ -0,0 +1,2 @@
+---
+cups::socket_manage: true

hiera.yaml

@@ -0,0 +1,5 @@
+---
+version: 5
+hierarchy:
+  - name: OS configuration
+    path: "os/%{facts.os.family}-%{facts.os.release.major}.yaml"

manifests/init.pp

@@ -43,6 +43,9 @@
 # @param service_ensure Whether the CUPS services should be `running` or `stopped`.
 # @param service_manage Whether to manage services at all.
 # @param service_names A name or an array of names of all CUPS services to be managed.
+# @param socket_manage Workaround a CUPS/OS bug (see `cups::server::socket`).
+#   Enabled by default on RHEL7, disabled elsewhere.
+# @param socket_ensure Should match `service_ensure`, the default.
 # @param web_interface Boolean value to enable or disable the server's web interface.
 #
 class cups (
@@ -58,6 +61,8 @@
   String                         $service_ensure         = 'running',
   Boolean                        $service_manage         = true,
   Variant[String, Array[String]] $service_names          = 'cups',
+  Boolean                        $socket_manage          = false,
+  String[1]                      $socket_ensure          = $service_ensure,
   Optional[Boolean]              $web_interface          = undef,
 ) inherits cups::params {
 

manifests/server.pp

@@ -11,9 +11,13 @@
 class cups::server inherits cups {
 
   contain cups::server::config
+  contain cups::server::socket
   contain cups::server::services
 
   Class[cups::server::config]
   ~> Class[cups::server::services]
 
+  Class[cups::server::config]
+  -> Class[cups::server::socket]
+  -> Class[cups::server::services]
 }

manifests/server/socket.pp

@@ -0,0 +1,54 @@
+# @private
+#
+# @summary Avoid ipptool errors when Cups is slow to start
+#
+# Problem statement: on some OSes (most notably RHEL7 and derivatives), the
+# Cups service may have started and not listen on the IPP port yet. This causes
+# a problem when this module (re)starts the service and expects to be able to
+# add/remove printers with ipptool right after.
+#
+# If `$cups::socket_manage` is enabled, this class modifies the standard
+# `cups.socket` service to also manage the IPP port. This way, systemd will
+# accept the incoming connections and hand them off to Cups once it is really
+# started. This is mostly a kludge for a 3rd party bug, but still required to
+# make the module behave reliably.
+#
+# If `$cups::socket_manage` is disabled, this workaround is removed.
+#
+# The `$cups::socket_ensure` argument may optionnally be set to `stopped`, and
+# this setting should probably match `$cups::service_ensure`.
+#
+# The `$cups::service_names` argument is used to generate the socket services,
+# ie. each X.service will have its X.socket managed as described above.
+#
+# @author Thomas Equeter
+# @since 3.0.0
+#
+class cups::server::socket inherits cups::server {
+
+  $_dropin_file = systemd::dropin_file { 'workaround-puppet-cups-35.conf':
+    ensure  => if $cups::socket_manage { 'present' } else { 'absent' },
+    unit    => 'cups.socket',
+    content => @(END),
+      [Socket]
+      ListenStream=[::1]:631
+      |END
+  }
+
+  if $cups::socket_manage {
+    Array($cups::service_names, true).each |$_service_name| {
+      $_safe_name = shell_escape($_service_name)
+
+      service { "${_service_name}.socket":
+        ensure    => $cups::socket_ensure,
+        # Both units listen to port 631, however cups.service will play nice if
+        # cups.socket is started first. See sd_listen_fds(3).
+        start     => "systemctl stop ${_safe_name}.service && systemctl start ${_safe_name}.socket",
+        restart   => "systemctl stop ${_safe_name}.service && systemctl restart ${_safe_name}.socket",
+        require   => Class['systemd::systemctl::daemon_reload'],
+        subscribe => $_dropin_file,
+      }
+    }
+  }
+}
+

metadata.json

@@ -13,7 +13,18 @@
     "printing"
   ],
   "dependencies": [
-
+    {
+      "name": "camptocamp/systemd",
+      "version_requirement": ">= 1.1.0 < 3.0.0"
+    },
+    {
+      "name": "puppetlabs/stdlib",
+      "version_requirement": ">= 4.13.1 < 6.0.0"
+    },
+    {
+      "name": "puppetlabs/inifile",
+      "version_requirement": ">= 1.6.0 < 3.0.0"
+    }
   ],
   "operatingsystem_support": [
     {
@@ -92,7 +103,7 @@
   "requirements": [
     {
       "name": "puppet",
-      "version_requirement": ">= 4.0.0 < 7.0.0"
+      "version_requirement": ">= 4.10.0 < 7.0.0"
     }
   ],
   "data_provider": null

spec/acceptance/puppet/cups_queue_after_start.rb

@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'spec_helper_acceptance'
+
+RSpec.describe 'Custom type `cups_queue`' do
+  describe 'when the Cups service is starting' do
+    before(:each) do
+      ensure_cups_is_running
+      # Cups takes time to listen to IPP after service start, depending on the
+      # amount of printers defined. Just a few printers are enough to cause the
+      # cups_queue ipptool calls to fail right after a restart, but adding 50
+      # here makes it future-proof for the ever-faster testing environments.
+      printer_names = (1..50).map { |n| "Office#{n}" }
+      printer_names.each do |name|
+        shell("lpadmin -E -p #{name} -m drv:///sample.drv/generic.ppd -o printer-is-shared=false", silent: true)
+      end
+      remove_queues('Office')
+
+      # Cleanup
+      if fact('systemd') then
+        shell('rm -f /etc/systemd/system/cups.socket.d/workaround-puppet-cups-35.conf')
+        shell('systemctl daemon-reload', accept_all_exit_codes: true)
+        shell('systemctl stop cups.socket cups.service', accept_all_exit_codes: true)
+      else
+        # Not needed, but for coherence with the systemd version
+        shell('service cups stop')
+      end
+    end
+
+    manifest = <<-MANIFEST
+      class { 'cups':
+        service_ensure => 'running',
+      }
+      cups_queue { 'Office':
+        ensure => 'printer',
+      }
+    MANIFEST
+
+    it 'can refresh the service and create a queue' do
+      # Do NOT use ensure_cups_is_running here, it calls the cups class and
+      # installs the dropin file, which is exactly what we want to test below.
+      shell('service cups start')
+      # Cause a rewrite of the configuration and trigger a service refresh.
+      shell('rm /etc/cups/cupsd.conf')
+      apply_manifest(manifest, expect_changes: true)
+    end
+
+    it 'can start the service and create a queue' do
+      apply_manifest(manifest, expect_changes: true)
+    end
+  end
+end

spec/classes/init_spec.rb

@@ -345,6 +345,36 @@
       end
     end
 
+    describe 'socket_manage' do
+      let(:facts) { any_supported_os }
+
+      context 'when set to false' do
+        let(:params) do
+          {
+            socket_manage: false
+          }
+        end
+
+        it { is_expected.to_not contain_service('cups.socket') }
+        it { is_expected.to contain_systemd__dropin_file('workaround-puppet-cups-35.conf').with_ensure('absent') }
+      end
+
+      %w[stopped running].each do |socket_ensure|
+        context "when set to true and ensure => #{socket_ensure}" do
+          let(:params) do
+            {
+              socket_manage: true,
+              socket_ensure: socket_ensure
+            }
+          end
+
+          it { is_expected.to contain_systemd__dropin_file('workaround-puppet-cups-35.conf').with_ensure('present') }
+          it { is_expected.to contain_systemd__dropin_file('workaround-puppet-cups-35.conf').that_notifies('Service[cups.socket]') }
+          it { is_expected.to contain_service('cups.socket').with_ensure(socket_ensure) }
+        end
+      end
+    end
+
     describe 'web_interface' do
       let(:facts) { any_supported_os }
 

spec/classes/server_spec.rb

@@ -7,7 +7,10 @@
     let(:facts) { any_supported_os }
 
     it { is_expected.to contain_class('cups::server::config').that_notifies('Class[cups::server::services]') }
+    it { is_expected.to contain_class('cups::server::config').that_comes_before('Class[cups::server::socket]') }
 
     it { is_expected.to contain_class('cups::server::services') }
+
+    it { is_expected.to contain_class('cups::server::socket').that_comes_before('Class[cups::server::services]') }
   end
 end

spec/spec_helper_acceptance.rb

@@ -16,6 +16,7 @@
     hosts.each do |host|
       run_puppet_install_helper_on(host) unless ENV['BEAKER_provision'] == 'no'
       install_module_on(host)
+      install_module_dependencies_on(host)
       scp_to(host, File.join(project_root, 'spec/fixtures/ppd/textonly.ppd'), '/tmp/')
     end
   end