Merge ~barryprice/postgresql-charm/+git/postgresql-charm:master into postgresql-charm:master

[Barry Price]

Add new scripts and config options to warn the operator about any backlog in WAL-E backups - see LP:1814523

[Stuart Bishop (stub)]

Coding seems fine.

Only running the cron job on master units is arguable. On one hand, running it everywhere is wasteful. On the other, it is only a quick script and disk write every minute or two.

Some int() casts are missing, which need to be fixed because this isn't Perl.

review: Needs Fixing

[Stuart Bishop (stub)]

Code changes are good

review: Approve

[Stuart Bishop (stub)]

Unit tests look fine.

The best way of testing stuff under reactive/ is to move the code elsewhere. This needs to improve, but isn't your problem. Best way for now is to move testable chunks of code elsewhere.

The main() can be testable, but needs a mock for the print function. Mocking print and dealing with sys.exit are not obvious, so it might be better to avoid using them (using 'return' instead of sys.exit, leaving it to the __main__ section to sys.exit(main(sys.argv)) instead of just main(sys.argv), and defining an easily mocked wrapper around print and using it instead.

Start by changing the main() signature into main(args=sys.argv), which allows you to pass in alternative arguments giving you control.

Next, move the 'max_age_filename' constant definition from inside main() to module scope (and in CAPS per PEP8 naming conventions). This allows your test to override it (using a try: finally: block to put back the original value, or setUp/tearDown methods to do the same). Alternatively, make it a function def max_age_filename(): return '/var/whatsit'. This function can then be mocked by the test to return a different result. A third alternative is pass it in as yet another parameter to main(), but that trick gets old fast.

Now your test can look something like:

import check_latest_ready_wal

@patch('check_latest_ready_wal.print')
def test_main(self, mock_print):
    with tempfile.TemporaryFile() as max_age_f:
        max_age_f.write(b'42\n')
        max_age_f.flush()
        org_max_age_path = check_latest_ready_wal.MAX_AGE_FILENAME
        try:
            check_latest_ready_wal.MAX_AGE_FILENAME = max_age_f.name
            check_latest_ready_wal([]) # overriding sys.argv
            with self.assertRaises(SystemExit) as ex:
                main([])
            self.assertEqual(ex.code, 0)
            mock_print.assert_called_once_with('OK: Expected message')
        finally:
            check_latest_ready_wal.MAX_AGE_FILENAME = org_max_age_path

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 8be5b9ae4eab4d43a90a4f036e3a040120929ca5

diff --git a/config.yaml b/config.yaml
index b70b53f..94daff9 100644
--- a/config.yaml
+++ b/config.yaml
@@ -170,6 +170,18 @@ options:
       You need enough space for this many backups plus one more, as
       an old backup will only be removed after a new one has been
       successfully made to replace it.
+  wal_e_backup_stale_warn_threshold:
+    type: int
+    default: 300
+    description: >
+      How many seconds the oldest un-uploaded WAL database backup can be
+      before the Nagios check will issue a warning.
+  wal_e_backup_stale_warn_threshold:
+    type: int
+    default: 600
+    description: >
+      How many seconds the oldest un-uploaded WAL database backup can be
+      before the Nagios check will issue a critical error.
   streaming_replication:
     type: boolean
     default: true
@@ -308,7 +320,7 @@ options:
     type: float
     description: DEPRECATED. Use extra_pg_conf.
   autovacuum_analyze_scale_factor:
-    default: 0.1  
+    default: 0.1
     type: float
     description: DEPRECATED. Use extra_pg_conf.
   autovacuum_vacuum_cost_delay:
@@ -366,7 +378,7 @@ options:
   kernel_shmall:
     default: 0
     type: int
-    description: DEPRECATED and ignored. 
+    description: DEPRECATED and ignored.
   kernel_shmmax:
     default: 0
     type: int
@@ -384,7 +396,7 @@ options:
     type: int
     description: DEPRECATED. Use extra_pg_conf.
   collapse_limit:
-    default: -1 
+    default: -1
     type: int
     description: DEPRECATED. Use extra_pg_conf.
   temp_buffers:
@@ -396,7 +408,7 @@ options:
     type: string
     description: DEPRECATED. Use extra_pg_conf.
   checkpoint_segments:
-    default: 10 
+    default: 10
     type: int
     description: DEPRECATED. Use extra_pg_conf.
   checkpoint_completion_target:
diff --git a/reactive/postgresql/nagios.py b/reactive/postgresql/nagios.py
index 23c4626..8893460 100644
--- a/reactive/postgresql/nagios.py
+++ b/reactive/postgresql/nagios.py
@@ -110,8 +110,57 @@ def update_nrpe_config():
                    description='Check pgsql',
                    check_cmd='check_pgsql -P {} -l {}'.format(port, user))
 
+    # copy the check script which will run cronned as postgres user
+    with open('scripts/find_latest_ready_wal.py.py') as fh:
+        check_script = fh.read()
+
+    check_script_path = '{}/{}'.format(
+        helpers.scripts_dir(),
+        'find_latest_ready_wal.py.py'
+    )
+    helpers.write(check_script_path, check_script, mode=0o755)
+
+    # create an (empty) file with appropriate permissions for the above
+    check_output_path = '/var/lib/nagios/postgres-wal-e-max-age.txt'
+    helpers.write(check_output_path, '', mode=0o644,
+                  user='postgres', group='postgres')
+
+    # retrieve the threshold values from the charm config
+    check_warn_threshold = helpers.config_yaml.get(
+        'wal_e_backup_stale_warn_threshold'
+    )
+    check_crit_threshold = helpers.config_yaml.get(
+        'wal_e_backup_stale_crit_threshold'
+    )
+
+    # create the cron job to run the above
+    check_cron = "*/2 * * * * postgres {}".format(check_script_path)
+    check_cron_path = '/etc/cron.d/postgres-stale-wal-e-check'
+    helpers.write(check_cron_path, check_cron, mode=0o644,
+                  user='root', group='root')
+
+    # copy the nagios plugin which will check the cronned output
+    with open('scripts/check_latest_ready_wal.py') as fh:
+        check_script = fh.read()
+
+    check_script_path = '{}/{}'.format(
+        '/usr/local/lib/nagios/plugins',
+        'check_latest_ready_wal.py'
+    )
+    helpers.write(check_script_path, check_script, mode=0o755,
+                  user='nagios', group='nagios')
+
+    # write the nagios check definition
+    nrpe.add_check(shortname='pgsql_stale_wal',
+                   description='Check for stale WAL backups',
+                   check_cmd='{} {} {}'.format(
+                       check_script_path,
+                       check_warn_threshold,
+                       check_crit_threshold,
+                   ))
+
     if reactive.is_state('postgresql.replication.is_master'):
-        # TODO: These should be calcualted from the backup schedule,
+        # TODO: These should be calculated from the backup schedule,
         # which is difficult since that is specified in crontab format.
         warn_age = 172800
         crit_age = 194400
diff --git a/scripts/check_latest_ready_wal.py b/scripts/check_latest_ready_wal.py
 new file mode 100755
index 0000000..87f4b30
--- /dev/null
+++ b/scripts/check_latest_ready_wal.py
@@ -0,0 +1,54 @@
+#!/usr/bin/python3
+
+import sys
+
+MAX_AGE_FILENAME = '/var/lib/nagios/postgres-wal-e-max-age.txt'
+
+
+def get_val_from_file(filename):
+    with open(filename) as the_file:
+        content = the_file.read().strip()
+    return int(content)
+
+
+def make_nice_age(seconds):
+    minutes, seconds = divmod(int(seconds), 60)
+    hours, minutes = divmod(minutes, 60)
+    days, hours = divmod(hours, 24)
+    return "{} days, {} hours, {} minutes and {} seconds".format(days, hours, minutes, seconds)
+
+
+def main(args=sys.argv):
+    NAGIOS_OK = 0
+    NAGIOS_WARN = 1
+    NAGIOS_FAIL = 2
+
+    try:
+        warn_threshold = int(args[1])
+    except IndexError:
+        print("Syntax error: not enough arguments given")
+        return NAGIOS_FAIL
+
+    try:
+        crit_threshold = int(args[2])
+    except IndexError:
+        print("Syntax error: not enough arguments given")
+        return NAGIOS_FAIL
+
+    max_age = get_val_from_file(MAX_AGE_FILENAME)
+    nice_age = make_nice_age(max_age)
+    ret_val = NAGIOS_OK
+
+    if max_age > crit_threshold:
+        print("CRITICAL: Last WAL-E backup was {} ago".format(nice_age))
+        ret_val = NAGIOS_FAIL
+    elif max_age > warn_threshold:
+        print("WARNING: Last WAL-E backup was {} ago".format(nice_age))
+        ret_val = NAGIOS_WARN
+    else:
+        print("OK: No stale WAL-E backups found")
+    return ret_val
+
+
+if __name__ == '__main__':
+    sys.exit(main(sys.argv))
diff --git a/scripts/find_latest_ready_wal.py b/scripts/find_latest_ready_wal.py
 new file mode 100755
index 0000000..6727be1
--- /dev/null
+++ b/scripts/find_latest_ready_wal.py
@@ -0,0 +1,27 @@
+#!/usr/bin/python3
+
+import glob
+import os
+import time
+
+
+def file_age(filepath):
+    return time.time() - os.path.getmtime(filepath)
+
+
+def main():
+    max_seen_age = 0
+    ready_files = glob.glob('/var/lib/postgresql/*/*/pg_{xlog,wal}/archive_status/*.ready')
+    max_age_filename = '/var/lib/nagios/postgres-wal-e-max-age.txt'
+
+    for ready_file in ready_files:
+        this_age = file_age(ready_file)
+        if this_age > max_seen_age:
+            max_seen_age = this_age
+
+    with open(max_age_filename, 'w') as max_age_file:
+        max_age_file.write('{}\n'.format(max_seen_age))
+
+
+if __name__ == '__main__':
+    main()
diff --git a/unit_tests/test_check_latest_ready_wal.py b/unit_tests/test_check_latest_ready_wal.py
 new file mode 100644
index 0000000..7acb4ba
--- /dev/null
+++ b/unit_tests/test_check_latest_ready_wal.py
@@ -0,0 +1,49 @@
+# Copyright 2019 Canonical Ltd.
+#
+# This file is part of the PostgreSQL Charm for Juju.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3, as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import os.path
+import sys
+import tempfile
+import unittest
+
+ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir))
+sys.path.insert(1, os.path.join(ROOT, 'scripts'))
+
+from check_latest_ready_wal import get_val_from_file, make_nice_age
+
+
+class TestCheckLatestReadyWal(unittest.TestCase):
+
+    def test_get_val_from_file(self):
+        fp = tempfile.NamedTemporaryFile()
+        fp.write(b'42\n')
+        fp.flush()
+        self.assertEqual(
+            get_val_from_file(fp.name),
+            42
+        )
+        fp.close()
+
+    def test_make_nice_age(self):
+        seconds = 123456
+        self.assertEqual(
+            make_nice_age(seconds),
+            '1 days, 10 hours, 17 minutes and 36 seconds'
+        )
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/unit_tests/test_find_latest_ready_wal.py b/unit_tests/test_find_latest_ready_wal.py
 new file mode 100644
index 0000000..07eea66
--- /dev/null
+++ b/unit_tests/test_find_latest_ready_wal.py
@@ -0,0 +1,39 @@
+# Copyright 2019 Canonical Ltd.
+#
+# This file is part of the PostgreSQL Charm for Juju.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3, as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import os.path
+import sys
+import tempfile
+import unittest
+
+ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir))
+sys.path.insert(1, os.path.join(ROOT, 'scripts'))
+
+from find_latest_ready_wal import file_age
+
+
+class TestFileAge(unittest.TestCase):
+    def test_file_age(self):
+        fp = tempfile.NamedTemporaryFile()
+        self.assertEqual(
+            int(file_age(fp.name)),
+            0
+        )
+        fp.close()
+
+
+if __name__ == '__main__':
+    unittest.main()