severity 1056303 important
tags 1056303 +patch
thanks

Hi,

On Mon, Nov 20, 2023 at 09:49:34AM +0100, Max Kellermann wrote:
> When I tried to use "pg_createcluster" to configure my pre-existing
> PostgreSQL data directory with a new Debian install, it deleted the
> whole cluster with all databases instead.  (This serious data loss
> justifies "severity critical" according to
> https://www.debian.org/Bugs/Developer#severities)
 
> Steps to reproduce:
> 
>  pg_createcluster 15 test
>  cp /etc/postgresql/15/test/postgresql.conf /var/lib/postgresql/15/test/

You missed to copy the other configuration files, as cautioned in the
manpage:

| If the data directory already exists, it is integrated into the
| postgresql-common structure by moving the configuration file and
| setting the data_directory option. Please note that this only works
| for data directories which were created directly with initdb, i.e.
| all the configuration files (postgresql.conf etc.) must be present in
| the data directory.

>  rm -r /etc/postgresql/15/test
>  pg_createcluster 15 test
> 
> This creates a new cluster just for the demo, then deletes the
> configuration directory, after copying the postgresql.conf to the data
> directory.
> 
> I expect the second "pg_createcluster" call to find the existing data
> directory and configure it; and it tries to do so indeed:
> 
>  Configuring already existing cluster (configuration:
>  /etc/postgresql/15/test, data: /var/lib/postgresql/15/test, owner:
>  103:111)
> 
> After it finds and moves a "postgresql.conf", it however fails to find
> "pg_hba.conf"

Right, see above.
 
>  Error: move_conffile: required configuration file 
> /var/lib/postgresql/15/test/pg_hba.conf does not exist
 
> This fails the whole operation, and apparently, "pg_createcluster"
> tries to roll back by invoking "pg_dropcluster 15 test 2>/dev/null",
> destroying all pre-existing data.
> 
> If "postgresql.conf" does not exist (or is empty), "pg_dropcluster" is
> not invoked.

This is because pg_creatcluster copies postgresql.conf first, and gets
version and cluster from it - if those are set, the data directory is
removed in the cleanup phase on error.
 
I think it would be sensible to only remove the data directory if it
actually got created by pg_createcluster, so I propose the attached
patch; I have also opened a merge request on Salsa for this:
https://salsa.debian.org/postgresql/postgresql-common/-/merge_requests/19


Michael
>From c1e61705ab9384a005a009ed6327bb40d9027afb Mon Sep 17 00:00:00 2001
From: Michael Banck <michael.ba...@credativ.de>
Date: Tue, 2 Jan 2024 15:54:04 +0100
Subject: [PATCH] pg_createcluster: Do not remove existing data directory on
 failure. (Closes: #105630)

---
 pg_createcluster | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/pg_createcluster b/pg_createcluster
index 66f0b82..0c36561 100755
--- a/pg_createcluster
+++ b/pg_createcluster
@@ -361,6 +361,8 @@ if (-f "$datadir/PG_VERSION") {
     ($owneruid, $ownergid) = (stat "$datadir/PG_VERSION")[4,5];
     if ($existingver == $version) {
         print "Configuring already existing cluster (configuration: $confdir, data: $datadir, owner: $owneruid:$ownergid)\n";
+        # Do not remove already existing data directory on errors
+        $cleanup_cruft = 0;
     } else {
         error "$datadir contains a version $existingver cluster, but $version was requested";
     }
-- 
2.39.2

Reply via email to