From 2b223de04b26210cc865c3f6061d8d139859f34c Mon Sep 17 00:00:00 2001
From: Bogdan Dobrelya <bdobreli@redhat.com>
Date: Wed, 3 Oct 2018 09:53:35 +0200
Subject: [PATCH] Copytruncate containerized logrotate configuration

This reimplements commit 67a7dc70f2885b7db2a42bc28c25ece0bbeba3e4.
Copytruncate becomes a default for containerized logrotate. The
solution based on signals processing goes away.

As long as key deployment framework components heat-engine and
mistral-engine do not tolerate SIGHUP copytruncate should be used.

There is more openstack services, like neutron-server, nova-scheduler
that cannot handle SIGHUP nicely yet.

Nor can we fall back to that predates the containerization of services
because of the following reasons:

* We cannot and should not use the restart command in postrotate as it
  was before containerization of services. For that a container needs
  to be privileged and granted a docker socket bind-mount, which is a
  total security antipattern and defeats the very purpose of
  containerization. Things may change with future adoption of Podman
  and/or kubelet control plane though. If/when that happens, we might
  consider an option for postrotate to terminate a process with
  SIGTERM, to have the process instantly respawned via its systemd
  unit/kubelet restart policies.

* Individual services' logrotate configs worth nothing, when still
  being handled by a central logrotation container running crond. And
  it needs to remain centralized as individual containers neither do
  run crond nor contain logrotate, nor lightweight containers following
  12-factors apps recommendations should do anything like that. Nor the
  host logrotate/crond can do rotation of logs for containers as we do/
  should not install required packages on the host, but only in
  containers. See also the spec [0] explaining the reasoning better.

All of that makes copytruncate a global choice for logs rotation of
containerized services as we just cannont be sure, if a service foo
*really* does correct processing of SIGHUP. We leave that option for
future implementation in the hope things get fixed eventually. As well
as the aforementioned systemd/kublet option, or the option to provide
stdout only logging [0] and let the logrotate thing go.

[0] https://review.openstack.org/#/c/462900

Closes-Bug: #1795411
Related-Bug: #1276694
Change-Id: Ibdad7859a389d0ff37bbf7bfd9f4c521a05a5ea1
Signed-off-by: Bogdan Dobrelya <bdobreli@redhat.com>
---
 manifests/profile/base/logging/logrotate.pp          |  5 +++++
 ...grotate-copytruncate-hourly-f0851bec551f5f5f.yaml |  5 +++++
 templates/logrotate/containers_logrotate.conf.erb    | 12 ++++--------
 3 files changed, 14 insertions(+), 8 deletions(-)
 create mode 100644 releasenotes/notes/logrotate-copytruncate-hourly-f0851bec551f5f5f.yaml

diff --git a/manifests/profile/base/logging/logrotate.pp b/manifests/profile/base/logging/logrotate.pp
index 459091d78..d1b6ac560 100644
--- a/manifests/profile/base/logging/logrotate.pp
+++ b/manifests/profile/base/logging/logrotate.pp
@@ -50,6 +50,10 @@
 #  [*user*]
 #    (optional) Defaults to 'root'. Configures cron job for logrotate.
 #
+#  [*copytruncate*]
+#    (optional) Defaults to True.
+#    Configures the logrotate copytruncate parameter.
+#
 #  [*delaycompress*]
 #    (optional) Defaults to True.
 #    Configures the logrotate delaycompress parameter.
@@ -91,6 +95,7 @@ class tripleo::profile::base::logging::logrotate (
   $weekday          = '*',
   Integer $maxdelay = 90,
   $user             = 'root',
+  $copytruncate     = true,
   $delaycompress    = true,
   $compress         = true,
   $rotation         = 'daily',
diff --git a/releasenotes/notes/logrotate-copytruncate-hourly-f0851bec551f5f5f.yaml b/releasenotes/notes/logrotate-copytruncate-hourly-f0851bec551f5f5f.yaml
new file mode 100644
index 000000000..1e9e716a1
--- /dev/null
+++ b/releasenotes/notes/logrotate-copytruncate-hourly-f0851bec551f5f5f.yaml
@@ -0,0 +1,5 @@
+---
+upgrade:
+  - |
+    Logrotate's copytruncate is used by default for containerized services logs
+    rotation.  The default period to keep old logs remains unchanged (14 days).
diff --git a/templates/logrotate/containers_logrotate.conf.erb b/templates/logrotate/containers_logrotate.conf.erb
index 2aa743a28..8bf2b7739 100644
--- a/templates/logrotate/containers_logrotate.conf.erb
+++ b/templates/logrotate/containers_logrotate.conf.erb
@@ -1,4 +1,4 @@
-/var/log/containers/*/*log /var/log/containers/*/*/*log {
+/var/log/containers/*/*log /var/log/containers/*/*/*log /var/log/containers/*/*err {
   <%= @rotation %>
   rotate <%= @rotate %>
   maxage <%= @purge_after_days %>
@@ -10,6 +10,9 @@
   maxsize <%= @maxsize %>
   missingok
   notifempty
+<%- if @copytruncate %>
+  copytruncate
+<%- end %>
 <%- if @delaycompress %>
   delaycompress
 <%- end %>
@@ -22,12 +25,5 @@
     \( -mtime +<%= @purge_after_days %> -or \
     -atime +<%= @purge_after_days %> -or \
     -ctime +<%= @purge_after_days %> \) -exec rm -f {} \;;
-    /sbin/lsof -nPs +L1 +D /var/log/containers 2>/dev/null|\
-    grep -v /var/log/httpd/ |\
-    awk '/\S+\s+[0-9]+\s.*\/var\/log\/.*\(deleted\)/ {print $2}' |\
-    sort -u | /bin/xargs -n1 -r -t kill -HUP;
-    /sbin/lsof -nPs +L1 +D /var/log/containers 2>/dev/null|\
-    awk '/\S+\s+[0-9]+\s.*\/var\/log\/httpd\/.*\(deleted\)/ {print $2}' |\
-    sort -u | /bin/xargs -n1 -r -t kill -USR1
   endscript
 }