diff --git a/changelog.d/20241209_114200_mb_FC_42163_fc_postgresql_extension_support.md b/changelog.d/20241209_114200_mb_FC_42163_fc_postgresql_extension_support.md new file mode 100644 index 000000000..eeb2f639f --- /dev/null +++ b/changelog.d/20241209_114200_mb_FC_42163_fc_postgresql_extension_support.md @@ -0,0 +1,22 @@ + + +### Impact + +- None. + +### NixOS XX.XX platform + +- The `fc-postgresql` command now supports upgrades of databases with preinstalled extensions: + - When upgrading manually with `fc-postgresql`, add `--extension-names ext1 --extension-names ext2` to the command line. `ext1`/`ext2` must be the package names of the extensions without the `postgresqlPackages.`-prefix. Usually it's the packages in [`services.postgresql.extraPlugins`](https://search.flyingcircus.io/search/options?q=services.postgresql.extraPlugins&channel=fc-24.05-dev#services.postgresql.extraPlugins). + - When using automatic upgrades ([`flyingcircus.services.postgresql.autoUpgrade.enable `](https://search.flyingcircus.io/search/options?q=+flyingcircus.services.postgresql.autoUpgrade.enable+&channel=fc-24.11-dev&page=1#flyingcircus.services.postgresql.autoUpgrade.enable)), existing extensions will be discovered automatically. You don't have to do anything in this case. diff --git a/doc/src/postgresql.md b/doc/src/postgresql.md index 64b806ce0..e92d59498 100644 --- a/doc/src/postgresql.md +++ b/doc/src/postgresql.md @@ -152,6 +152,21 @@ Note that this is done while the old role is still active. It's safe to run the command while PostgreSQL is running as it does not have an impact on the current cluster and downtime is not required. +If custom extensions are enabled with `services.postgresql.extraPlugins`, make sure +to add those to the `fc-postgresql` invocation. I.e. for + +```nix +{ + services.postgresql.extraPlugins = plugins: [ plugins.anonymizer plugins.pgvector ]; +} +``` + +you'll need + +``` +sudo -u postgres fc-postgresql upgrade --new-version 14 --expected mydb --expected otherdb --extension-names anonymizer --extension-names pgvector +``` + The command should automatically find the old data directory for 13, create the new data directory for 14, set it up, and succeed if no problems with the old cluster were found. Problems may occur if the old cluster has been @@ -170,6 +185,9 @@ To actually run the upgrade, use: sudo -u postgres fc-postgresql upgrade --new-version 14 --expected mydb --expected otherdb --upgrade-now ``` +Please note that you'll also need the `--extension-names` parameters as described +above. + This will stop the postgresql service, prevent it from starting during the upgrade, migrate data and mark the old data directory as migrated. This data directory cannot be used by the postgresql service anymore after this point. diff --git a/nixos/services/postgresql/default.nix b/nixos/services/postgresql/default.nix index f0f336aef..2975e4b27 100644 --- a/nixos/services/postgresql/default.nix +++ b/nixos/services/postgresql/default.nix @@ -64,6 +64,16 @@ let collationVerifierScript = pkgs.writers.writePython3Bin "verify-collations" {} (builtins.readFile ./verify-collations.py); + mkExtensionNamesForAutoUpgrade = postgresqlMajor: + let + availableExtensions = lib.mapAttrs' + (attrName: package: lib.nameValuePair (lib.getName package) attrName) + (builtins.removeAttrs pkgs."postgresql${toString postgresqlMajor}Packages" [ "recurseForDerivations" ]); + pluginByPname = package: lib.optionals (availableExtensions?${lib.getName package}) [ + availableExtensions.${lib.getName package} + ]; + in + lib.concatMap pluginByPname (config.services.postgresql.extraPlugins config.services.postgresql.package.pkgs); in { options = with lib; { @@ -159,7 +169,7 @@ in { ''; systemd.services.postgresql.postStart = '' - ln -sfT ${postgresqlPkg} ${upstreamCfg.dataDir}/package + ln -sfT ${config.services.postgresql.package.withPackages config.services.postgresql.extraPlugins} ${upstreamCfg.dataDir}/package ln -sfT ${upstreamCfg.dataDir}/package /nix/var/nix/gcroots/per-user/postgres/package_${cfg.majorVersion} '' + (lib.optionalString (lib.versionAtLeast cfg.majorVersion "15") '' ${collationVerifierScript}/bin/verify-collations ${upstreamCfg.dataDir} @@ -421,12 +431,13 @@ in { "${config.flyingcircus.agent.package}/bin/fc-postgresql upgrade" "--new-version ${cfg.majorVersion}" "--new-data-dir ${upstreamCfg.dataDir}" - "--new-bin-dir ${upstreamCfg.package}/bin" + "--new-bin-dir ${upstreamCfg.package.withPackages (ps: attrValues (lib.getAttrs (mkExtensionNamesForAutoUpgrade cfg.majorVersion) ps))}/bin" "--no-stop" "--nothing-to-do-is-ok" "--upgrade-now" - ] ++ lib.optional cfg.autoUpgrade.checkExpectedDatabases - "--existing-db-check ${expectedDatabaseStr}"; + ] ++ lib.optionals cfg.autoUpgrade.checkExpectedDatabases [ + "--existing-db-check ${expectedDatabaseStr}" + ] ++ map (extName: "--extension-names ${extName}") (mkExtensionNamesForAutoUpgrade cfg.majorVersion); in concatStringsSep " \\\n " upgradeCmd; diff --git a/pkgs/fc/agent/fc/manage/postgresql.py b/pkgs/fc/agent/fc/manage/postgresql.py index e40c3db7c..ce5f7f4b1 100644 --- a/pkgs/fc/agent/fc/manage/postgresql.py +++ b/pkgs/fc/agent/fc/manage/postgresql.py @@ -150,6 +150,10 @@ def upgrade( ), default=None, ), + extension_names: Optional[List[str]] = Option( + default=[], + help="Extensions that are used in the databases. Must be the package name from https://search.nixos.org without the `postgresqlXXPackages.`-prefix.", + ), stop: Optional[bool] = Option(default=None), nothing_to_do_is_ok: Optional[bool] = False, ): @@ -166,7 +170,7 @@ def upgrade( raise Exit(2) new_bin_dir = fc.util.postgresql.build_new_bin_dir( - log, context.pg_data_root, new_version + log, context.pg_data_root, new_version, extension_names ) new_data_dir = context.pg_data_root / new_version.value @@ -345,6 +349,10 @@ def prepare_autoupgrade( show_choices=True, help="PostgreSQL version to upgrade to.", ), + extension_names: Optional[List[str]] = Option( + default=[], + help="Extensions that are used in the databases. Must be the package name from https://search.nixos.org without the `postgresqlXXPackages.`-prefix.", + ), nothing_to_do_is_ok: Optional[bool] = False, ): with open(config) as f: @@ -354,7 +362,7 @@ def prepare_autoupgrade( log = structlog.get_logger() new_bin_dir = fc.util.postgresql.build_new_bin_dir( - log, context.pg_data_root, new_version + log, context.pg_data_root, new_version, extension_names ) new_data_dir = context.pg_data_root / new_version.value diff --git a/pkgs/fc/agent/fc/manage/tests/test_postgresql.py b/pkgs/fc/agent/fc/manage/tests/test_postgresql.py index a923ab425..5f39e1617 100644 --- a/pkgs/fc/agent/fc/manage/tests/test_postgresql.py +++ b/pkgs/fc/agent/fc/manage/tests/test_postgresql.py @@ -1,5 +1,6 @@ import json import traceback +from typing import List from unittest.mock import Mock, patch import fc.manage.postgresql diff --git a/pkgs/fc/agent/fc/util/postgresql.py b/pkgs/fc/agent/fc/util/postgresql.py index 7cafa0d41..b3c8ab571 100644 --- a/pkgs/fc/agent/fc/util/postgresql.py +++ b/pkgs/fc/agent/fc/util/postgresql.py @@ -7,6 +7,7 @@ from enum import Enum from pathlib import Path from subprocess import CalledProcessError, run +from typing import List MIGRATED_TO_TEMPLATE = """\ WARNING: This data directory should not be used anymore! @@ -79,12 +80,17 @@ def is_service_running(): return proc.returncode == 0 -def build_new_bin_dir(log, pg_data_root: Path, new_version: PGVersion): +def build_new_bin_dir( + log, + pg_data_root: Path, + new_version: PGVersion, + extension_names: List[str] = [], +): + extension_names_str = " ".join(extension_names) nix_build_new_pg_cmd = [ "nix-build", - "", - "-A", - "postgresql_" + new_version.value, + "-E", + f"with import {{}}; postgresql_{new_version.value}.withPackages (ps: with ps; [ {extension_names_str} ])", "--out-link", pg_data_root / "pg_upgrade-package", ] @@ -472,6 +478,22 @@ def run_pg_upgrade_check( raise +def retrieve_shared_preload_libraries_setting( + old_data_dir: Path, +) -> str | None: + """ + The setting `shared_preload_libraries` must be correctly set for upgrades with + e.g. `pg_anon` installed. This function assumes that `old_data_dir` has a `postgresql.conf` + managed by NixOS, i.e. a trivial key-value text with quoted strings. + """ + postgresql_conf = old_data_dir / "postgresql.conf" + if postgresql_conf.exists(): + with open(postgresql_conf) as f: + for line in f.readlines(): + if line.startswith("shared_preload_libraries"): + return line.split("=")[1].strip() + + def run_pg_upgrade( log, new_bin_dir: Path, @@ -490,6 +512,17 @@ def run_pg_upgrade( new_bin_dir, ] + if shared_preload_libraries := retrieve_shared_preload_libraries_setting( + old_data_dir + ): + setting = f"-c shared_preload_libraries={shared_preload_libraries}" + upgrade_cmd += [ + "-o", + setting, + "-O", + setting, + ] + if pg_upgrade_clone_available( log, new_bin_dir=new_bin_dir, diff --git a/pkgs/fc/agent/fc/util/tests/test_postgresql.py b/pkgs/fc/agent/fc/util/tests/test_postgresql.py index bed066b58..9c1af5e23 100644 --- a/pkgs/fc/agent/fc/util/tests/test_postgresql.py +++ b/pkgs/fc/agent/fc/util/tests/test_postgresql.py @@ -1,4 +1,5 @@ import json +from typing import List from unittest.mock import Mock import fc.manage.postgresql diff --git a/tests/default.nix b/tests/default.nix index 2577581d7..1a6f9327b 100644 --- a/tests/default.nix +++ b/tests/default.nix @@ -99,6 +99,7 @@ in { postgresql15 = callTest ./postgresql { version = "15"; }; postgresql16 = callTest ./postgresql { version = "16"; }; postgresql-autoupgrade = callSubTests ./postgresql/upgrade.nix {}; + postgresql-autoupgrade-exts = callSubTests ./postgresql/upgrade-with-extension.nix {}; prometheus = callTest ./prometheus.nix {}; rabbitmq = callTest ./rabbitmq.nix {}; redis = callTest ./redis.nix {}; diff --git a/tests/postgresql/upgrade-with-extension.nix b/tests/postgresql/upgrade-with-extension.nix new file mode 100644 index 000000000..a26723d71 --- /dev/null +++ b/tests/postgresql/upgrade-with-extension.nix @@ -0,0 +1,280 @@ +import ../make-test-python.nix ({lib, testlib, pkgs, ... }: +let + release = import ../../release {}; + channel = release.release.src; + + insertSql = pkgs.writeText "insert.sql" '' + create extension anon cascade; + select anon.init(); + create table player(id serial, name text, points int); + insert into player(id,name,points) values (1,'Foo', 23); + insert into player(id,name,points) values (2,'Bar',42); + security label for anon on column player.name is 'MASKED WITH FUNCTION anon.fake_last_name();'; + security label for anon on column player.points is 'MASKED WITH VALUE NULL'; + ''; + + dataTest = pkgs.writeScript "postgresql-tests" '' + set -e + createdb anonymized + psql -v ON_ERROR_STOP=1 --echo-all -d anonymized < ${insertSql} + ''; + + fc-postgresql = "sudo -u postgres -- fc-postgresql"; + + testSetup = '' + # Make nix-build work inside the VM + machine.execute("mkdir -p /nix/var/nix/profiles/per-user/root/") + machine.execute("ln -s ${channel} /nix/var/nix/profiles/per-user/root/channels") + + # Taken from upstream acme.nix + def switch_to(node, name, expect="succeed"): + # On first switch, this will create a symlink to the current system so that we can + # quickly switch between derivations + root_specs = "/tmp/specialisation" + node.execute( + f"test -e {root_specs}" + f" || ln -s $(readlink /run/current-system)/specialisation {root_specs}" + ) + + switcher_path = f"/run/current-system/specialisation/{name}/bin/switch-to-configuration" + rc, _ = node.execute(f"test -e '{switcher_path}'") + if rc > 0: + switcher_path = f"/tmp/specialisation/{name}/bin/switch-to-configuration" + + if expect == "fail": + node.fail(f"{switcher_path} test") + else: + node.succeed(f"{switcher_path} test") + + # helpers from the pg_anonymizer test in nixpkgs + def get_player_table_contents(): + return [ + x.split(',') for x in machine.succeed("sudo -u postgres psql -d anonymized --csv --command 'select * from player'").splitlines()[1:] + ] + + def check_anonymized_row(row, id, original_name): + assert row[0] == id, f"Expected first row to have ID {id}, but got {row[0]}" + assert row[1] != original_name, f"Expected first row to have a name other than {original_name}" + assert not bool(row[2]), "Expected points to be NULL in first row" + + def check_original_data(output): + assert output[0] == ['1','Foo','23'], f"Expected first row from player table to be 1,Foo,23; got {output[0]}" + assert output[1] == ['2','Bar','42'], f"Expected first row from player table to be 2,Bar,42; got {output[1]}" + + def check_anonymized_rows(output): + check_anonymized_row(output[0], '1', 'Foo') + check_anonymized_row(output[1], '2', 'Bar') + + machine.wait_for_unit("postgresql.service") + machine.wait_for_open_port(5432) + + machine.succeed('sudo -u postgres -- sh ${dataTest}') + + with subtest("Anonymize DB"): + check_original_data(get_player_table_contents()) + machine.succeed("sudo -u postgres psql -d anonymized --command 'select anon.anonymize_database();'") + ''; + +in { + name = "postgresql-upgrade-with-extensions"; + testCases = { + manual = { + name = "manual"; + nodes = { + machine = { ... }: { + imports = [ + (testlib.fcConfig { net.fe = false; }) + ]; + + flyingcircus.roles.postgresql12.enable = lib.mkDefault true; + services.postgresql = { + extraPlugins = ps: with ps; [ anonymizer ]; + settings.shared_preload_libraries = lib.mkForce "auto_explain, pg_stat_statements, anon"; + }; + + specialisation = { + pg13.configuration = { + flyingcircus.roles.postgresql12.enable = false; + flyingcircus.roles.postgresql13.enable = true; + }; + pg14.configuration = { + flyingcircus.roles.postgresql12.enable = false; + flyingcircus.roles.postgresql14.enable = true; + }; + pg15.configuration = { + flyingcircus.roles.postgresql12.enable = false; + flyingcircus.roles.postgresql15.enable = true; + }; + }; + + system.extraDependencies = with pkgs; [ + (postgresql_12.withPackages (ps: with ps; [ anonymizer ])) + (postgresql_13.withPackages (ps: with ps; [ anonymizer ])) + (postgresql_14.withPackages (ps: with ps; [ anonymizer ])) + (postgresql_15.withPackages (ps: with ps; [ anonymizer ])) + (postgresql_12.withPackages (ps: [ ])) + (postgresql_13.withPackages (ps: [ ])) + (postgresql_14.withPackages (ps: [ ])) + (postgresql_15.withPackages (ps: [ ])) + ]; + }; + }; + + testScript = '' + ${testSetup} + + with subtest("prepare-autoupgrade should fail when the option is not enabled"): + machine.fail("${fc-postgresql} prepare-autoupgrade --new-version 13") + + with subtest("prepare should fail with unexpected database anonymized"): + machine.fail('${fc-postgresql} upgrade --new-version 13') + + print(machine.succeed("${fc-postgresql} list-versions")) + + with subtest("prepare upgrade 12 -> 13"): + machine.fail('${fc-postgresql} upgrade --new-version 13 --expected anonymized') + machine.succeed("rm -f /srv/postgresql/12/fcio_stopper && systemctl start postgresql") + machine.succeed('${fc-postgresql} upgrade --new-version 13 --expected anonymized --extension-names anonymizer') + machine.succeed("stat /srv/postgresql/13/fcio_upgrade_prepared") + # postgresql should still run + machine.succeed("systemctl status postgresql") + print(machine.succeed("${fc-postgresql} list-versions")) + + with subtest("upgrade 12 -> 13 from prepared state"): + machine.succeed("systemctl status postgresql") + machine.succeed("rm -f /srv/postgresql/13/pg_upgrade_server.log") + print(machine.succeed('${fc-postgresql} upgrade --expected anonymized --new-version 13 --stop --upgrade-now --extension-names anonymizer || cat /srv/postgresql/13/pg_upgrade_server.log')) + machine.succeed("stat /srv/postgresql/12/fcio_migrated_to") + machine.succeed("stat /srv/postgresql/13/fcio_migrated_from") + # postgresql should be stopped + machine.fail("systemctl status postgresql") + print(machine.succeed("${fc-postgresql} list-versions")) + + # Clean up migration and start postgresql12 again for the next round. + machine.execute("rm -rf /srv/postgresql/13") + machine.execute("rm -rf /srv/postgresql/12/fcio_migrated_to") + machine.systemctl("start postgresql") + + with subtest("upgrade 12 -> 14 in one step"): + machine.succeed('${fc-postgresql} upgrade --expected anonymized --new-version 14 --stop --upgrade-now --extension-names anonymizer') + machine.succeed("stat /srv/postgresql/12/fcio_migrated_to") + machine.succeed("stat /srv/postgresql/14/fcio_migrated_from") + # postgresql should be stopped + machine.fail("systemctl status postgresql") + # move to pg14 role and wait for postgresql to start + switch_to(machine, "pg14") + machine.wait_for_unit("postgresql") + print(machine.succeed("${fc-postgresql} list-versions")) + check_anonymized_rows(get_player_table_contents()) + + with subtest("upgrade 14 -> 15 in one step"): + machine.succeed('${fc-postgresql} upgrade --expected anonymized --new-version 15 --stop --upgrade-now --extension-names anonymizer') + machine.succeed("stat /srv/postgresql/14/fcio_migrated_to") + machine.succeed("stat /srv/postgresql/15/fcio_migrated_from") + switch_to(machine, "pg15") + machine.wait_for_unit("postgresql") + print(machine.succeed("${fc-postgresql} list-versions")) + check_anonymized_rows(get_player_table_contents()) + ''; + }; + automatic = { + name = "automatic"; + nodes = { + machine = { ... }: { + imports = [ + (testlib.fcConfig { net.fe = false; }) + ]; + + flyingcircus.roles.postgresql12.enable = lib.mkDefault true; + flyingcircus.services.postgresql.autoUpgrade = { + enable = true; + expectedDatabases = [ "anonymized" ]; + }; + services.postgresql = { + extraPlugins = ps: with ps; [ anonymizer ]; + settings.shared_preload_libraries = lib.mkForce "auto_explain, pg_stat_statements, anon"; + }; + + specialisation = { + pg13UnexpectedDb.configuration = { + flyingcircus.roles.postgresql12.enable = false; + flyingcircus.roles.postgresql13.enable = true; + flyingcircus.services.postgresql.autoUpgrade.expectedDatabases = lib.mkForce []; + }; + pg13.configuration = { + flyingcircus.roles.postgresql12.enable = false; + flyingcircus.roles.postgresql13.enable = true; + }; + pg14.configuration = { + flyingcircus.roles.postgresql12.enable = false; + flyingcircus.roles.postgresql14.enable = true; + }; + pg15.configuration = { + flyingcircus.roles.postgresql12.enable = false; + flyingcircus.roles.postgresql15.enable = true; + }; + }; + + system.extraDependencies = with pkgs; [ + (postgresql_12.withPackages (ps: with ps; [ anonymizer ])) + (postgresql_13.withPackages (ps: with ps; [ anonymizer ])) + (postgresql_14.withPackages (ps: with ps; [ anonymizer ])) + (postgresql_15.withPackages (ps: with ps; [ anonymizer ])) + (postgresql_12.withPackages (ps: [ ])) + (postgresql_13.withPackages (ps: [ ])) + (postgresql_14.withPackages (ps: [ ])) + (postgresql_15.withPackages (ps: [ ])) + ]; + }; + }; + + testScript = '' + ${testSetup} + print(machine.succeed("${fc-postgresql} list-versions")) + + with subtest("autoupgrade should refuse when unexpected DB is present"): + switch_to(machine, "pg13UnexpectedDb", expect="fail") + machine.fail("systemctl status postgresql") + print(machine.succeed("${fc-postgresql} list-versions")) + + with subtest("prepare autoupgrade should fail when unexpected DB is present"): + machine.fail('${fc-postgresql} prepare-autoupgrade --new-version 13') + print(machine.succeed("${fc-postgresql} list-versions")) + + with subtest("autoupgrade 12 -> 13"): + # move to new role and wait for postgresql to start + switch_to(machine, "pg13") + machine.wait_for_unit("postgresql") + machine.succeed("stat /srv/postgresql/12/fcio_migrated_to") + machine.succeed("stat /srv/postgresql/13/fcio_migrated_from") + print(machine.succeed("${fc-postgresql} list-versions")) + + with subtest("prepare autoupgrade 13 -> 14"): + machine.succeed('${fc-postgresql} prepare-autoupgrade --new-version 14 --extension-names anonymizer') + machine.succeed("stat /srv/postgresql/14/fcio_upgrade_prepared") + # postgresql should still run + machine.succeed("systemctl status postgresql") + print(machine.succeed("${fc-postgresql} list-versions")) + + + with subtest("autoupgrade 13 -> 14"): + # move to new role and wait for postgresql to start + switch_to(machine, "pg14") + machine.wait_for_unit("postgresql") + machine.succeed("stat /srv/postgresql/13/fcio_migrated_to") + machine.succeed("stat /srv/postgresql/14/fcio_migrated_from") + print(machine.succeed("${fc-postgresql} list-versions")) + check_anonymized_rows(get_player_table_contents()) + + with subtest("autoupgrade 14 -> 15"): + # move to new role and wait for postgresql to start + switch_to(machine, "pg15") + machine.wait_for_unit("postgresql") + machine.succeed("stat /srv/postgresql/14/fcio_migrated_to") + machine.succeed("stat /srv/postgresql/15/fcio_migrated_from") + print(machine.succeed("${fc-postgresql} list-versions")) + check_anonymized_rows(get_player_table_contents()) + ''; + }; + }; +}) diff --git a/tests/postgresql/upgrade.nix b/tests/postgresql/upgrade.nix index f4c85b55f..2f11a560b 100644 --- a/tests/postgresql/upgrade.nix +++ b/tests/postgresql/upgrade.nix @@ -80,10 +80,10 @@ in { }; system.extraDependencies = with pkgs; [ - postgresql_12 - postgresql_13 - postgresql_14 - postgresql_15 + (postgresql_12.withPackages (ps: [ ])) + (postgresql_13.withPackages (ps: [ ])) + (postgresql_14.withPackages (ps: [ ])) + (postgresql_15.withPackages (ps: [ ])) ]; }; }; @@ -173,10 +173,10 @@ in { }; system.extraDependencies = with pkgs; [ - postgresql_12 - postgresql_13 - postgresql_14 - postgresql_15 + (postgresql_12.withPackages (ps: [ ])) + (postgresql_13.withPackages (ps: [ ])) + (postgresql_14.withPackages (ps: [ ])) + (postgresql_15.withPackages (ps: [ ])) ]; }; };