From b3b7c133c513dd2c30b00a81e64b2bb33ca92397 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Wed, 22 Jul 2020 18:12:09 +0200
Subject: [PATCH] Drop pid files and fix working dirs

When using systemd, the pid files are actually not important so they can
be dropped.

The the WorkingDirectory is set to ~ which will resolve to the user's
home directory. That means systemd will ensure it is mounted before
starting services. Pulp itself doesn't use it so it's a cheap way for
a more reliable service start.

Subscribing to pulpcore-worker@.service is another bugfix to ensure all
workers use the correct service definition.

Explicitly setting the type and group is more for consistency and
doesn't actually change anything.
---
 manifests/service.pp                            |  8 ++++----
 templates/pulpcore-api.service.erb              | 10 ++--------
 templates/pulpcore-content.service.erb          |  3 ++-
 templates/pulpcore-resource-manager.service.erb |  5 +++--
 templates/pulpcore-worker@.service.erb          |  4 ++--
 5 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/manifests/service.pp b/manifests/service.pp
index 08c8a95e..0f7ba268 100644
--- a/manifests/service.pp
+++ b/manifests/service.pp
@@ -1,7 +1,6 @@
 # configure, enable, and start pulpcore services
 # @api private
 class pulpcore::service {
-
   systemd::unit_file { 'pulpcore-api.service':
     content => template('pulpcore/pulpcore-api.service.erb'),
     active  => true,
@@ -26,9 +25,10 @@
 
   Integer[1, $pulpcore::worker_count].each |$n| {
     service { "pulpcore-worker@${n}.service":
-      ensure  => running,
-      enable  => true,
-      require => [Systemd::Unit_file['pulpcore-worker@.service'], Class['systemd::systemctl::daemon_reload']],
+      ensure    => running,
+      enable    => true,
+      require   => Class['systemd::systemctl::daemon_reload'],
+      subscribe => Systemd::Unit_file['pulpcore-worker@.service'],
     }
   }
 
diff --git a/templates/pulpcore-api.service.erb b/templates/pulpcore-api.service.erb
index e3bed6e3..80e36a97 100644
--- a/templates/pulpcore-api.service.erb
+++ b/templates/pulpcore-api.service.erb
@@ -7,7 +7,8 @@ Wants=network-online.target
 Environment="DJANGO_SETTINGS_MODULE=pulpcore.app.settings"
 Environment="PULP_SETTINGS=<%= scope['pulpcore::settings_file'] %>"
 User=<%= scope['pulpcore::user'] %>
-PIDFile=/run/pulpcore-api.pid
+Group=<%= scope['pulpcore::group'] %>
+WorkingDirectory=~
 RuntimeDirectory=pulpcore-api
 ExecStart=/usr/bin/gunicorn pulpcore.app.wsgi:application \
           --bind '<%= scope['pulpcore::api_host'] %>:<%= scope['pulpcore::api_port'] %>' \
@@ -23,13 +24,6 @@ SyslogIdentifier=pulpcore-api
 Restart=always
 RestartSec=3
 
-# This directive is set to an absolute path in other Pulp units. Using an
-# absolute path is an abuse of the directive, as it should be a relative path,
-# not an absolute path. PIDFile is now used to ensure that PID files are laid
-# out in a standard way. If this directive had any other effects, it is better
-# to use the correct directive than to uncomment this.
-# WorkingDirectory=/var/run/pulpcore-api/
-
 [Install]
 WantedBy=multi-user.target
 
diff --git a/templates/pulpcore-content.service.erb b/templates/pulpcore-content.service.erb
index 79449d40..41e4ca17 100644
--- a/templates/pulpcore-content.service.erb
+++ b/templates/pulpcore-content.service.erb
@@ -7,7 +7,8 @@ Wants=network-online.target
 Environment="DJANGO_SETTINGS_MODULE=pulpcore.app.settings"
 Environment="PULP_SETTINGS=<%= scope['pulpcore::settings_file'] %>"
 User=<%= scope['pulpcore::user'] %>
-WorkingDirectory=/var/run/pulpcore-content/
+Group=<%= scope['pulpcore::group'] %>
+WorkingDirectory=~
 RuntimeDirectory=pulpcore-content
 ExecStart=/usr/bin/gunicorn pulpcore.content:server \
           --bind '<%= scope['pulpcore::content_host'] %>:<%= scope['pulpcore::content_port'] %>' \
diff --git a/templates/pulpcore-resource-manager.service.erb b/templates/pulpcore-resource-manager.service.erb
index ab494b11..f817b712 100644
--- a/templates/pulpcore-resource-manager.service.erb
+++ b/templates/pulpcore-resource-manager.service.erb
@@ -4,14 +4,15 @@ After=network-online.target
 Wants=network-online.target
 
 [Service]
+Type=simple
 Environment="DJANGO_SETTINGS_MODULE=pulpcore.app.settings"
 Environment="PULP_SETTINGS=<%= scope['pulpcore::settings_file'] %>"
 User=<%= scope['pulpcore::user'] %>
-WorkingDirectory=/var/run/pulpcore-resource-manager/
+Group=<%= scope['pulpcore::group'] %>
+WorkingDirectory=~
 RuntimeDirectory=pulpcore-resource-manager
 ExecStart=/usr/bin/rq worker \
           -w pulpcore.tasking.worker.PulpWorker -n resource-manager \
-          --pid=/var/run/pulpcore-resource-manager/resource-manager.pid \
           -c 'pulpcore.rqconfig' \
           --disable-job-desc-logging
 
diff --git a/templates/pulpcore-worker@.service.erb b/templates/pulpcore-worker@.service.erb
index 54a3a564..b1b9a23a 100644
--- a/templates/pulpcore-worker@.service.erb
+++ b/templates/pulpcore-worker@.service.erb
@@ -4,17 +4,17 @@ After=network-online.target
 Wants=network-online.target
 
 [Service]
+Type=simple
 EnvironmentFile=-/etc/default/pulp-workers
 EnvironmentFile=-/etc/default/pulp-workers-%i
 Environment="DJANGO_SETTINGS_MODULE=pulpcore.app.settings"
 Environment="PULP_SETTINGS=<%= scope['pulpcore::settings_file'] %>"
 User=<%= scope['pulpcore::user'] %>
 Group=<%= scope['pulpcore::group'] %>
-WorkingDirectory=/var/run/pulpcore-worker-%i/
+WorkingDirectory=~
 RuntimeDirectory=pulpcore-worker-%i
 ExecStart=/usr/bin/rq worker \
           -w pulpcore.tasking.worker.PulpWorker \
-          --pid=/var/run/pulpcore-worker-%i/reserved-resource-worker-%i.pid \
           -c 'pulpcore.rqconfig' \
           --disable-job-desc-logging