From c8ec3dfcf6ddcfb97b9b099b579c2f9bd2c43506 Mon Sep 17 00:00:00 2001
From: Alex Vandiver <alexmv@zulip.com>
Date: Wed, 21 Jun 2023 21:53:00 +0000
Subject: [PATCH] pgroonga: Run upgrade SQL when pgroonga package is updated.

Updating the pgroonga package is not sufficient to upgrade the
extension in PostgreSQL -- an `ALTER EXTENSION pgroonga UPDATE` must
explicitly be run[^1].  Failure to do so can lead to unexpected behavior,
including crashes of PostgreSQL.

Expand on the existing `pgroonga_setup.sql.applied` file, to track
which version of the PostgreSQL extension has been configured.  If the
file exists but is empty, we run `ALTER EXTENSION pgroonga UPDATE`
regardless -- if it is a no-op, it still succeeds with a `NOTICE`:

```
zulip=# ALTER EXTENSION pgroonga UPDATE;
NOTICE:  version "3.0.8" of extension "pgroonga" is already installed
ALTER EXTENSION
```

The simple `ALTER EXTENSION` is sufficient for the
backwards-compatible case[^1] -- which, for our usage, is every
upgrade since 0.9 -> 1.0.  Since version 1.0 was released in 2015,
before pgroonga support was added to Zulip in 2016, we can assume for
the moment that all pgroonga upgrades are backwards-compatible, and
not bother regenerating indexes.

Fixes: #25989.

[^1]: https://pgroonga.github.io/upgrade/
---
 puppet/zulip/manifests/postgresql_base.pp | 22 ++++++++++++----------
 scripts/lib/upgrade-zulip-stage-2         | 13 +++++++++++++
 scripts/setup/pgroonga-config             | 21 +++++++++++++++++++++
 3 files changed, 46 insertions(+), 10 deletions(-)
 create mode 100755 scripts/setup/pgroonga-config

diff --git a/puppet/zulip/manifests/postgresql_base.pp b/puppet/zulip/manifests/postgresql_base.pp
index 12b78455da..246a4a208f 100644
--- a/puppet/zulip/manifests/postgresql_base.pp
+++ b/puppet/zulip/manifests/postgresql_base.pp
@@ -83,18 +83,20 @@ class zulip::postgresql_base {
     }
 
     package{"${postgresql}-pgdg-pgroonga":
-      ensure  => installed,
-      require => [Package[$postgresql],
-                  Exec[$setup_system_deps]],
+      ensure  => latest,
+      require => [
+        Package[$postgresql],
+        Exec[$setup_system_deps]
+      ],
     }
-
-    $dbname = zulipconf('postgresql', 'database_name', 'zulip')
-    exec{'create_pgroonga_extension':
+    exec { 'pgroonga-config':
       require => Package["${postgresql}-pgdg-pgroonga"],
-      # lint:ignore:140chars
-      command => "bash -c 'echo \"CREATE EXTENSION PGROONGA\" | su postgres -c \"psql -v ON_ERROR_STOP=1 ${dbname}\" && touch ${pgroonga_setup_sql_path}.applied'",
-      # lint:endignore
-      creates => "${pgroonga_setup_sql_path}.applied",
+      unless  => @("EOT"/$),
+          test -f ${pgroonga_setup_sql_path}.applied &&
+          test "$(dpkg-query --show --showformat='\${Version}' "${postgresql}-pgdg-pgroonga")" \
+             = "$(cat ${pgroonga_setup_sql_path}.applied)"
+          | EOT
+      command => "${::zulip_scripts_path}/setup/pgroonga-config ${postgresql_sharedir}",
     }
   }
 
diff --git a/scripts/lib/upgrade-zulip-stage-2 b/scripts/lib/upgrade-zulip-stage-2
index 3fabfcab23..88743c95b6 100755
--- a/scripts/lib/upgrade-zulip-stage-2
+++ b/scripts/lib/upgrade-zulip-stage-2
@@ -28,6 +28,7 @@ from scripts.lib.zulip_tools import (
     DEPLOYMENTS_DIR,
     assert_running_as_root,
     get_config,
+    get_config_bool,
     get_config_file,
     get_zulip_pwent,
     listening_publicly,
@@ -205,6 +206,7 @@ def shutdown_server() -> None:
 
 # postgresql.version is required for database servers, but wasn't
 # previously; fill it in based on what the OS provides.
+postgresql_version = None
 if os.path.exists("/etc/init.d/postgresql"):
     postgresql_version = get_config(config_file, "postgresql", "version")
     django_pg_version = subprocess.check_output(
@@ -269,9 +271,20 @@ if glob.glob("/usr/share/postgresql/*/extension/tsearch_extras.control"):
     subprocess.check_call(["apt-get", "remove", "-y", "postgresql-*-tsearch-extras"])
 
 if not (minimal_change or args.skip_puppet):
+    # We need to temporarily hold pgroonga, if installed -- upgrading
+    # it without running the appropriate upgrade SQL can cause
+    # PostgreSQL to crash
+    if postgresql_version is not None and get_config_bool(config_file, "machine", "pgroonga"):
+        subprocess.check_call(
+            ["apt-mark", "hold", f"postgresql-{postgresql_version}-pgdg-pgroonga"]
+        )
     logging.info("Upgrading system packages...")
     subprocess.check_call(["apt-get", "update"])
     subprocess.check_call(["apt-get", "-y", "--with-new-pkgs", "upgrade"])
+    if postgresql_version is not None and get_config_bool(config_file, "machine", "pgroonga"):
+        subprocess.check_call(
+            ["apt-mark", "unhold", f"postgresql-{postgresql_version}-pgdg-pgroonga"]
+        )
 
 # To bootstrap zulip-puppet-apply, we need to install the system yaml
 # package; new installs get this, but old installs may not have it.
diff --git a/scripts/setup/pgroonga-config b/scripts/setup/pgroonga-config
new file mode 100755
index 0000000000..1e95f1ebf8
--- /dev/null
+++ b/scripts/setup/pgroonga-config
@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+set -eux
+
+dbversion=$(crudini --get /etc/zulip/zulip.conf postgresql version)
+dbname=$(crudini --get /etc/zulip/zulip.conf postgresql database_name 2>/dev/null || echo zulip)
+dbuser=$(crudini --get /etc/zulip/zulip.conf postgresql database_user 2>/dev/null || echo zulip)
+
+sharedir="${1:-/usr/share/postgresql/$dbversion}"
+applied_file="$sharedir/pgroonga_setup.sql.applied"
+
+installed_version=$(dpkg-query --show --showformat='${Version}' "postgresql-$dbversion-pgdg-pgroonga")
+
+if [ ! -f "$applied_file" ]; then
+    sql="CREATE EXTENSION PGROONGA; GRANT USAGE ON SCHEMA pgroonga TO $dbuser"
+else
+    sql="ALTER EXTENSION pgroonga UPDATE"
+fi
+
+echo "$sql" | su postgres -c "psql -v ON_ERROR_STOP=1 $dbname"
+
+echo "$installed_version" >"$applied_file"