launchpad-reviewers team mailing list archive

[Colin Watson <mp+450740@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:charm-db-update into launchpad:master.

Commit message:
charm: Add launchpad-db-update

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/450740

This handles database schema updates, adding `preflight` and `db-update` actions.  It should allow us to replace another function for which we currently rely on non-Juju infrastructure.

On production, we can't do this from `launchpad-admin`, since the distinction between the `master`/`stable` and `db-devel`/`db-stable` branches means that we need to have different commits deployed for ordinary administrative functions and for schema updates.  I considered making the behaviour of this charm be optional behaviour in `launchpad-admin` rather than adding a new charm, but I ended up needing to turn most of `launchpad-admin` off, and if the two modes are going to have mostly disjoint charm code anyway then they might as well have different charms.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:charm-db-update into launchpad:master.

diff --git a/charm/launchpad-db-update/README.md b/charm/launchpad-db-update/README.md
new file mode 100644
index 0000000..1babbdd
--- /dev/null
+++ b/charm/launchpad-db-update/README.md
@@ -0,0 +1,74 @@
+# Launchpad database schema updates
+
+This charm provides tools for managing schema updates of Launchpad
+deployments.
+
+Launchpad has two separate "trunk" branches: the `master` branch (feeding
+`stable` after tests pass) and the `db-devel` branch (feeding `db-stable`
+after tests pass).  On production, database permissions are updated on each
+deployment from `stable`, and full database schema updates are applied
+separately from `db-stable`.
+
+For a simple local deployment, you will need the following relations:
+
+    juju relate launchpad-db-update:db postgresql:db
+    juju relate launchpad-db-update:db-admin postgresql:db-admin
+    juju relate launchpad-db-update rabbitmq-server
+
+An action is available to perform a schema update:
+
+    juju run-action --wait launchpad-db-update/leader db-update
+
+## pgbouncer management
+
+In deployments that use it, this charm can administer the `pgbouncer` load
+balancer to disable connections to the primary database for the duration of
+the update.
+
+To use this mode, you need to use the
+[external-services](https://code.launchpad.net/~ubuntuone-hackers/external-services/+git/external-services)
+proxy charm in place of relating directly to `postgresql`, in order to have
+greater control over connection strings.  `external-services` will need to
+be configured along the lines of the following:
+
+    options:
+      db_connections: |
+        launchpad_db_update:
+          master: "postgresql://user:password@host:port/dbname"
+          standbys: []
+          admin: "postgresql://user:password@host:port/dbname"
+        launchpad_pgbouncer:
+          master: "postgresql://user:password@host:port/dbname"
+
+`launchpad_db_update` and `launchpad_pgbouncer` may have other names if
+needed as long as they match the `databases` option below;
+`launchpad_db_update` must define a direct connection to the primary
+database, bypassing `pgbouncer`, while `launchpad_pgbouncer` must define a
+connection to `pgbouncer` itself.
+
+`launchpad-db-update` will need configuration similar to the following (the
+values of the entries in `databases` serve as keys into the `db_connections`
+option above):
+
+    options:
+      databases: |
+        db:
+          name: "launchpad_db_update"
+        pgbouncer:
+          name: "launchpad_pgbouncer"
+
+You will need the following relations:
+
+    juju relate launchpad-db-update:db external-services:db
+    juju relate launchpad-db-update:db-admin external-services:db-admin
+    juju relate launchpad-db-update:pgbouncer external-services:db
+    juju relate launchpad-db-update rabbitmq-server
+
+In this mode, an additional action is available:
+
+    juju run-action --wait launchpad-db-update/leader preflight
+
+This checks whether the system is ready for a database schema update (i.e.
+that no processes are connected that would have problems if they were
+interrupted).  The operator should ensure that it succeeds before running
+the `db-update` action.
diff --git a/charm/launchpad-db-update/actions.yaml b/charm/launchpad-db-update/actions.yaml
new file mode 100644
index 0000000..6eed53f
--- /dev/null
+++ b/charm/launchpad-db-update/actions.yaml
@@ -0,0 +1,7 @@
+preflight:
+  description: >
+    Confirm that the system is ready for a database schema update.  This
+    checks that no processes are connected that would have problems if they
+    were interrupted.
+db-update:
+  description: Perform a database schema update.
diff --git a/charm/launchpad-db-update/actions/actions.py b/charm/launchpad-db-update/actions/actions.py
new file mode 100755
index 0000000..4239cc3
--- /dev/null
+++ b/charm/launchpad-db-update/actions/actions.py
@@ -0,0 +1,63 @@
+#! /usr/bin/python3
+# Copyright 2023 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+import subprocess
+import sys
+import traceback
+from pathlib import Path
+
+sys.path.append("lib")
+
+from charms.layer import basic  # noqa: E402
+
+basic.bootstrap_charm_deps()
+basic.init_config_states()
+
+from charmhelpers.core import hookenv  # noqa: E402
+from charms.launchpad.payload import home_dir  # noqa: E402
+from ols import base  # noqa: E402
+
+
+def preflight():
+    hookenv.log("Running preflight checks.")
+    script = Path(home_dir(), "bin", "preflight")
+    if script.exists():
+        subprocess.run(
+            ["sudo", "-H", "-u", base.user(), script],
+            check=True,
+        )
+        hookenv.action_set({"result": "Preflight checks passed"})
+    else:
+        message = "Preflight checks not available; missing pgbouncer relation?"
+        hookenv.log(message)
+        hookenv.action_fail(message)
+
+
+def db_update():
+    hookenv.log("Running database schema update.")
+    script = Path(home_dir(), "bin", "db-update")
+    subprocess.run(["sudo", "-H", "-u", base.user(), script], check=True)
+    hookenv.action_set({"result": "Database schema update completed"})
+
+
+def main(argv):
+    action = Path(argv[0]).name
+    try:
+        if action == "preflight":
+            preflight()
+        elif action == "db-update":
+            db_update()
+        else:
+            hookenv.action_fail(f"Action {action} not implemented.")
+    except Exception:
+        hookenv.action_fail("Unhandled exception")
+        tb = traceback.format_exc()
+        hookenv.action_set(dict(traceback=tb))
+        hookenv.log(f"Unhandled exception in action {action}:")
+        for line in tb.splitlines():
+            hookenv.log(line)
+
+
+if __name__ == "__main__":
+    main(sys.argv)
diff --git a/charm/launchpad-db-update/actions/db-update b/charm/launchpad-db-update/actions/db-update
new file mode 120000
index 0000000..405a394
--- /dev/null
+++ b/charm/launchpad-db-update/actions/db-update
@@ -0,0 +1 @@
+actions.py
\ No newline at end of file
diff --git a/charm/launchpad-db-update/actions/preflight b/charm/launchpad-db-update/actions/preflight
new file mode 120000
index 0000000..405a394
--- /dev/null
+++ b/charm/launchpad-db-update/actions/preflight
@@ -0,0 +1 @@
+actions.py
\ No newline at end of file
diff --git a/charm/launchpad-db-update/charmcraft.yaml b/charm/launchpad-db-update/charmcraft.yaml
new file mode 100644
index 0000000..afb3e49
--- /dev/null
+++ b/charm/launchpad-db-update/charmcraft.yaml
@@ -0,0 +1,63 @@
+type: charm
+bases:
+  - build-on:
+    - name: ubuntu
+      channel: "20.04"
+      architectures: [amd64]
+    run-on:
+    - name: ubuntu
+      channel: "20.04"
+      architectures: [amd64]
+parts:
+  charm-wheels:
+    source: https://git.launchpad.net/~ubuntuone-hackers/ols-charm-deps/+git/wheels
+    source-commit: "42c89d9c66dbe137139b047fd54aed49b66d1a5e"
+    source-submodules: []
+    source-type: git
+    plugin: dump
+    organize:
+      "*": charm-wheels/
+    prime:
+      - "-charm-wheels"
+  ols-layers:
+    source: https://git.launchpad.net/ols-charm-deps
+    source-commit: "f63ae0386275bf9089b30c8abae252a0ea523633"
+    source-submodules: []
+    source-type: git
+    plugin: dump
+    organize:
+      "*": layers/
+    stage:
+      - layers
+    prime:
+      - "-layers"
+  launchpad-layers:
+    after:
+      - ols-layers
+    source: https://git.launchpad.net/launchpad-layers
+    source-commit: "6ca1d670f636e1abb8328d88fc5fda80cb75152a"
+    source-submodules: []
+    source-type: git
+    plugin: dump
+    organize:
+      launchpad-base: layers/layer/launchpad-base
+      launchpad-db: layers/layer/launchpad-db
+      launchpad-payload: layers/layer/launchpad-payload
+    stage:
+      - layers
+    prime:
+      - "-layers"
+  charm:
+    after:
+      - charm-wheels
+      - launchpad-layers
+    source: .
+    plugin: reactive
+    build-snaps: [charm]
+    build-packages: [libpq-dev, python3-dev]
+    build-environment:
+      - CHARM_LAYERS_DIR: $CRAFT_STAGE/layers/layer
+      - CHARM_INTERFACES_DIR: $CRAFT_STAGE/layers/interface
+      - PIP_NO_INDEX: "true"
+      - PIP_FIND_LINKS: $CRAFT_STAGE/charm-wheels
+    reactive-charm-build-arguments: [--binary-wheels-from-source]
diff --git a/charm/launchpad-db-update/layer.yaml b/charm/launchpad-db-update/layer.yaml
new file mode 100644
index 0000000..ce7034d
--- /dev/null
+++ b/charm/launchpad-db-update/layer.yaml
@@ -0,0 +1,10 @@
+includes:
+  - layer:launchpad-db
+repo: https://git.launchpad.net/launchpad
+options:
+  ols-pg:
+    databases:
+      db:
+        name: launchpad_dev
+      pgbouncer:
+        name: pgbouncer_dev
diff --git a/charm/launchpad-db-update/metadata.yaml b/charm/launchpad-db-update/metadata.yaml
new file mode 100644
index 0000000..0a18a3a
--- /dev/null
+++ b/charm/launchpad-db-update/metadata.yaml
@@ -0,0 +1,25 @@
+name: launchpad-db-update
+display-name: launchpad-db-update
+summary: Launchpad database schema updates
+maintainer: Launchpad Developers <launchpad-dev@xxxxxxxxxxxxxxxxxxx>
+description: |
+  Launchpad is an open source suite of tools that help people and teams
+  to work together on software projects.
+
+  This charm provides tools for managing schema updates of Launchpad
+  deployments.
+subordinate: false
+requires:
+  db:
+    interface: pgsql
+  # A direct connection to the primary database, bypassing pgbouncer.
+  # (full-update.py disables access via pgbouncer to the primary database
+  # for the duration of the update, so we must have direct access.)
+  db-admin:
+    interface: pgsql
+  # A connection to the pgbouncer load balancer.  The schema update process
+  # uses this to check for long-running connections and to disable access to
+  # the primary database for the duration of the update.
+  pgbouncer:
+    interface: pgsql
+    optional: true
diff --git a/charm/launchpad-db-update/reactive/launchpad-db-update.py b/charm/launchpad-db-update/reactive/launchpad-db-update.py
new file mode 100644
index 0000000..e9c4b06
--- /dev/null
+++ b/charm/launchpad-db-update/reactive/launchpad-db-update.py
@@ -0,0 +1,127 @@
+# Copyright 2023 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+import os.path
+
+from charmhelpers.core import hookenv, host, templating
+from charms.launchpad.base import get_service_config
+from charms.launchpad.db import strip_dsn_authentication, update_pgpass
+from charms.launchpad.payload import configure_lazr, home_dir
+from charms.reactive import (
+    clear_flag,
+    endpoint_from_flag,
+    is_flag_set,
+    set_flag,
+    when,
+    when_any,
+    when_not,
+    when_not_all,
+)
+from ols import base, postgres
+from psycopg2.extensions import make_dsn, parse_dsn
+
+
+def any_dbname(dsn):
+    parsed_dsn = parse_dsn(dsn)
+    parsed_dsn["dbname"] = "*"
+    return make_dsn(**parsed_dsn)
+
+
+@when(
+    "launchpad.db.configured",
+    "db.master.available",
+    "db-admin.master.available",
+)
+@when_not("service.configured")
+def configure():
+    config = get_service_config()
+
+    db = endpoint_from_flag("db.master.available")
+    db_primary, _ = postgres.get_db_uris(db)
+    config["db_primary"] = strip_dsn_authentication(db_primary)
+
+    db_admin = endpoint_from_flag("db-admin.master.available")
+    db_admin_primary, _ = postgres.get_db_uris(db_admin)
+    # We assume that this admin user works for any database on this host,
+    # which seems to be true in practice.
+    update_pgpass(any_dbname(db_admin_primary))
+    config["db_admin_primary"] = strip_dsn_authentication(db_admin_primary)
+
+    if is_flag_set("pgbouncer.master.available"):
+        pgbouncer = endpoint_from_flag("pgbouncer.master.available")
+        pgbouncer_primary, _ = postgres.get_db_uris(pgbouncer)
+        update_pgpass(pgbouncer_primary)
+        config["pgbouncer_primary"] = strip_dsn_authentication(
+            pgbouncer_primary
+        )
+    else:
+        pgbouncer = None
+
+    configure_lazr(
+        config,
+        "launchpad-db-update-lazr.conf",
+        "launchpad-db-update/launchpad-lazr.conf",
+    )
+    bin_dir = os.path.join(home_dir(), "bin")
+    host.mkdir(bin_dir, owner=base.user(), group=base.user(), perms=0o755)
+    scripts = {
+        "db-update": True,
+        "preflight": pgbouncer is not None,
+    }
+    for script, enable in scripts.items():
+        script_path = os.path.join(bin_dir, script)
+        if enable:
+            templating.render(
+                f"{script}.j2",
+                script_path,
+                config,
+                owner=base.user(),
+                group=base.user(),
+                perms=0o755,
+            )
+        elif os.path.exists(script_path):
+            os.unlink(script_path)
+
+    set_flag("service.configured")
+    if pgbouncer is not None:
+        set_flag("service.pgbouncer.configured")
+    hookenv.status_set("active", "Ready")
+
+
+@when("service.configured")
+@when_not_all(
+    "launchpad.db.configured",
+    "db.master.available",
+    "db-admin.master.available",
+)
+def deconfigure():
+    clear_flag("service.configured")
+
+
+@when("service.pgbouncer.configured")
+@when_not("service.configured")
+def deconfigure_optional_services():
+    clear_flag("service.pgbouncer.configured")
+
+
+@when_any(
+    "db-admin.database.changed",
+    "pgbouncer.database.changed",
+)
+@when("service.configured")
+def any_db_changed():
+    clear_flag("service.configured")
+    clear_flag("db-admin.database.changed")
+    clear_flag("pgbouncer.database.changed")
+
+
+@when("pgbouncer.master.available", "service.configured")
+@when_not("service.pgbouncer.configured")
+def pgbouncer_available():
+    clear_flag("service.configured")
+
+
+@when("service.pgbouncer.configured")
+@when_not("pgbouncer.master.available")
+def pgbouncer_unavailable():
+    clear_flag("service.configured")
diff --git a/charm/launchpad-db-update/templates/db-update.j2 b/charm/launchpad-db-update/templates/db-update.j2
new file mode 100755
index 0000000..4ca8e87
--- /dev/null
+++ b/charm/launchpad-db-update/templates/db-update.j2
@@ -0,0 +1,22 @@
+#! /bin/sh
+# Copyright 2023 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# Part of the launchpad-db-update Juju charm.
+
+set -e
+
+export LPCONFIG=launchpad-db-update
+
+{% if pgbouncer_primary -%}
+# Fastdowntime update, managing connections using pgbouncer.
+{{ code_dir }}/database/schema/full-update.py \
+    --pgbouncer='{{ pgbouncer_primary }}'
+{% else -%}
+# We can't manage connections using pgbouncer in this environment.  Attempt
+# a simple schema upgrade, which may fail if anything has an active database
+# connection.
+{{ code_dir }}/database/schema/upgrade.py
+{{ code_dir }}/database/schema/security.py
+{% endif %}
+
diff --git a/charm/launchpad-db-update/templates/launchpad-db-update-lazr.conf b/charm/launchpad-db-update/templates/launchpad-db-update-lazr.conf
new file mode 100644
index 0000000..ad354a9
--- /dev/null
+++ b/charm/launchpad-db-update/templates/launchpad-db-update-lazr.conf
@@ -0,0 +1,17 @@
+# Public configuration data.  The contents of this file may be freely shared
+# with developers if needed for debugging.
+
+# A schema's sections, keys, and values are automatically inherited, except
+# for '.optional' sections.  Update this config to override key values.
+# Values are strings, except for numbers that look like ints.  The tokens
+# true, false, and none are treated as True, False, and None.
+
+[meta]
+extends: ../launchpad-db-lazr.conf
+
+[database]
+rw_main_primary: {{ db_admin_primary }}
+rw_main_standby: None
+db_statement_timeout: None
+soft_request_timeout: None
+
diff --git a/charm/launchpad-db-update/templates/preflight.j2 b/charm/launchpad-db-update/templates/preflight.j2
new file mode 100755
index 0000000..ef2a88f
--- /dev/null
+++ b/charm/launchpad-db-update/templates/preflight.j2
@@ -0,0 +1,11 @@
+#! /bin/sh
+# Copyright 2023 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# Part of the launchpad-db-update Juju charm.
+
+set -e
+
+LPCONFIG=launchpad-db-update {{ code_dir }}/database/schema/preflight.py \
+    --skip-connection-check --pgbouncer='{{ pgbouncer_primary }}'
+