testing: Add a system-provided postgres for testing

.github/workflows/ci.yaml

@@ -267,7 +267,7 @@ jobs:
           - NAME: postgres
             CFG: compile-gcc
             COMPILER: gcc
-            TEST_DB_PROVIDER: postgres
+            TEST_DB_PROVIDER: system-postgres
             TEST_NETWORK: regtest
           # And don't forget about elements (like cdecker did when
           # reworking the CI...)
@@ -290,10 +290,46 @@ jobs:
             COMPILER: gcc
             TEST_NETWORK: regtest
             EXPERIMENTAL_SPLICING: 1
+    services:
+      postgres:
+        # Docker Hub image
+        image: postgres
+        # Provide the password for postgres
+        env:
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_INITDB_ARGS: "-c max_connections=2000"
+        ports:
+          - "5432:5432"
+        # Set health checks to wait until postgres has started
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
     steps:
       - name: Checkout
         uses: actions/checkout@v4
 
+      - name: Check disk space pre-cleanup
+        run: df -h
+      - name: Free up disk space
+        # We need more space than GHA usually provides when testing
+        # against the `postgres` service. This step frees some space
+        # on the volume containing tools, in the hope of freeing
+        # enough space for postgres tests to succeed.
+        uses: jlumbroso/free-disk-space@main
+        with:
+          # Enable all cleanup options
+          tool-cache: true
+          android: true
+          dotnet: true
+          haskell: true
+          swift: true
+          golang: true
+          php: true
+      - name: Check disk space pre-cleanup
+        run: df -h
+
       - name: Set up Python 3.10
         uses: actions/setup-python@v5
         with:
@@ -340,6 +376,8 @@ jobs:
           TEST_DB_PROVIDER: ${{ matrix.TEST_DB_PROVIDER }}
           TEST_NETWORK: ${{ matrix.TEST_NETWORK }}
           LIGHTNINGD_POSTGRES_NO_VACUUM: 1
+          # Yes, that DSN is silly :-) It's only used for the TEST_DB_PROVIDER=system-postgres
+          CLN_TEST_POSTGRES_DSN: "postgres://postgres:[email protected]:5432/postgres"
         run: |
           env
           cat config.vars

contrib/pyln-testing/pyln/testing/db.py

@@ -12,6 +12,7 @@
 import subprocess
 import time
 from typing import Dict, List, Optional, Union
+from urllib.parse import urlparse
 
 
 class BaseDb(object):
@@ -25,11 +26,18 @@ def __init__(self, path: str) -> None:
         self.provider = None
 
     def get_dsn(self) -> None:
-        """SQLite3 doesn't provide a DSN, resulting in no CLI-option.
-        """
+        """SQLite3 doesn't provide a DSN, resulting in no CLI-option."""
         return None
 
-    def query(self, query: str) -> Union[List[Dict[str, Union[int, bytes]]], List[Dict[str, Optional[int]]], List[Dict[str, str]], List[Dict[str, Union[str, int]]], List[Dict[str, int]]]:
+    def query(
+        self, query: str
+    ) -> Union[
+        List[Dict[str, Union[int, bytes]]],
+        List[Dict[str, Optional[int]]],
+        List[Dict[str, str]],
+        List[Dict[str, Union[str, int]]],
+        List[Dict[str, int]],
+    ]:
         orig = os.path.join(self.path)
         copy = self.path + ".copy"
         shutil.copyfile(orig, copy)
@@ -68,22 +76,23 @@ def wipe_db(self):
 
 
 class PostgresDb(BaseDb):
-    def __init__(self, dbname, port):
+    def __init__(self, dbname, hostname, port, username, password):
         self.dbname = dbname
         self.port = port
         self.provider = None
+        self.hostname = hostname
+        self.username = username
+        self.password = password
 
-        self.conn = psycopg2.connect("dbname={dbname} user=postgres host=localhost port={port}".format(
-            dbname=dbname, port=port
-        ))
+        self.conn = psycopg2.connect(
+            f"dbname={dbname} user={username} password={password} host={hostname} port={port}"
+        )
         cur = self.conn.cursor()
-        cur.execute('SELECT 1')
+        cur.execute("SELECT 1")
         cur.close()
 
     def get_dsn(self):
-        return "postgres://postgres:password@localhost:{port}/{dbname}".format(
-            port=self.port, dbname=self.dbname
-        )
+        return f"postgres://{self.username}:{self.password}@{self.hostname}:{self.port}/{self.dbname}"
 
     def query(self, query):
         cur = self.conn.cursor()
@@ -105,13 +114,15 @@ def execute(self, query):
             cur.execute(query)
 
     def stop(self):
-        """Clean up the database.
-        """
+        """Clean up the database."""
         self.conn.close()
-        conn = psycopg2.connect("dbname=postgres user=postgres host=localhost port={self.port}")
+        conn = psycopg2.connect(
+            f"dbname=postgres user={self.username} host={self.hostname} password={self.password} port={self.port}"
+        )
         cur = conn.cursor()
         cur.execute("DROP DATABASE {};".format(self.dbname))
         cur.close()
+        conn.close()
 
     def wipe_db(self):
         cur = self.conn.cursor()
@@ -128,10 +139,7 @@ def start(self) -> None:
         pass
 
     def get_db(self, node_directory: str, testname: str, node_id: int) -> Sqlite3Db:
-        path = os.path.join(
-            node_directory,
-            'lightningd.sqlite3'
-        )
+        path = os.path.join(node_directory, "lightningd.sqlite3")
         return Sqlite3Db(path)
 
     def stop(self) -> None:
@@ -143,69 +151,92 @@ def __init__(self, directory):
         self.directory = directory
         self.port = None
         self.proc = None
+        self.hostname = "127.0.0.1"  # We default to localhost, but can be overridden
+        self.username = "postgres"
+        self.password = "postgres"
+
         print("Starting PostgresDbProvider")
 
     def locate_path(self):
         # Use `pg_config` to determine correct PostgreSQL installation
-        pg_config = shutil.which('pg_config')
+        pg_config = shutil.which("pg_config")
         if not pg_config:
-            raise ValueError("Could not find `pg_config` to determine PostgreSQL binaries. Is PostgreSQL installed?")
+            raise ValueError(
+                "Could not find `pg_config` to determine PostgreSQL binaries. Is PostgreSQL installed?"
+            )
 
-        bindir = subprocess.check_output([pg_config, '--bindir']).decode().rstrip()
+        bindir = subprocess.check_output([pg_config, "--bindir"]).decode().rstrip()
         if not os.path.isdir(bindir):
-            raise ValueError("Error: `pg_config --bindir` didn't return a proper path: {}".format(bindir))
-
-        initdb = os.path.join(bindir, 'initdb')
-        postgres = os.path.join(bindir, 'postgres')
+            raise ValueError(
+                "Error: `pg_config --bindir` didn't return a proper path: {}".format(
+                    bindir
+                )
+            )
+
+        initdb = os.path.join(bindir, "initdb")
+        postgres = os.path.join(bindir, "postgres")
         if os.path.isfile(initdb) and os.path.isfile(postgres):
             if os.access(initdb, os.X_OK) and os.access(postgres, os.X_OK):
                 logging.info("Found `postgres` and `initdb` in {}".format(bindir))
                 return initdb, postgres
 
-        raise ValueError("Could not find `postgres` and `initdb` binaries in {}".format(bindir))
+        raise ValueError(
+            "Could not find `postgres` and `initdb` binaries in {}".format(bindir)
+        )
 
     def start(self):
         passfile = os.path.join(self.directory, "pgpass.txt")
         # Need to write a tiny file containing the password so `initdb` can
         # pick it up
-        with open(passfile, 'w') as f:
-            f.write('cltest\n')
+        with open(passfile, "w") as f:
+            f.write("cltest\n")
 
         # Look for a postgres directory that isn't taken yet. Not locking
         # since this is run in a single-threaded context, at the start of each
         # test. Multiple workers have separate directories, so they can't
         # trample each other either.
         for i in itertools.count():
-            self.pgdir = os.path.join(self.directory, 'pgsql-{}'.format(i))
+            self.pgdir = os.path.join(self.directory, "pgsql-{}".format(i))
             if not os.path.exists(self.pgdir):
                 break
 
         initdb, postgres = self.locate_path()
-        subprocess.check_call([
-            initdb,
-            '--pwfile={}'.format(passfile),
-            '--pgdata={}'.format(self.pgdir),
-            '--auth=trust',
-            '--username=postgres',
-        ])
-        conffile = os.path.join(self.pgdir, 'postgresql.conf')
-        with open(conffile, 'a') as f:
-            f.write('max_connections = 1000\nshared_buffers = 240MB\n')
+        subprocess.check_call(
+            [
+                initdb,
+                "--pwfile={}".format(passfile),
+                "--pgdata={}".format(self.pgdir),
+                "--auth=trust",
+                "--username=postgres",
+            ]
+        )
+        conffile = os.path.join(self.pgdir, "postgresql.conf")
+        with open(conffile, "a") as f:
+            f.write("max_connections = 1000\nshared_buffers = 240MB\n")
 
         self.port = reserve_unused_port()
-        self.proc = subprocess.Popen([
-            postgres,
-            '-k', '/tmp/',  # So we don't use /var/lib/...
-            '-D', self.pgdir,
-            '-p', str(self.port),
-            '-F',
-            '-i',
-        ])
+        self.proc = subprocess.Popen(
+            [
+                postgres,
+                "-k",
+                "/tmp/",  # So we don't use /var/lib/...
+                "-D",
+                self.pgdir,
+                "-p",
+                str(self.port),
+                "-F",
+                "-i",
+            ]
+        )
         # Hacky but seems to work ok (might want to make the postgres proc a
         # TailableProc as well if too flaky).
         for i in range(30):
             try:
-                self.conn = psycopg2.connect("dbname=template1 user=postgres host=localhost port={}".format(self.port))
+                self.conn = psycopg2.connect(
+                    "dbname=template1 user=postgres host=localhost port={}".format(
+                        self.port
+                    )
+                )
                 break
             except Exception:
                 time.sleep(0.5)
@@ -215,13 +246,15 @@ def start(self):
 
     def get_db(self, node_directory, testname, node_id):
         # Random suffix to avoid collisions on repeated tests
-        nonce = ''.join(random.choice(string.ascii_lowercase + string.digits) for _ in range(8))
+        nonce = "".join(
+            random.choice(string.ascii_lowercase + string.digits) for _ in range(8)
+        )
         dbname = "{}_{}_{}".format(testname, node_id, nonce)
 
         cur = self.conn.cursor()
         cur.execute("CREATE DATABASE {};".format(dbname))
         cur.close()
-        db = PostgresDb(dbname, self.port)
+        db = PostgresDb(dbname, self.hostname, self.port, self.username, self.password)
         return db
 
     def stop(self):
@@ -241,3 +274,49 @@ def stop(self):
         self.proc.wait()
         shutil.rmtree(self.pgdir)
         drop_unused_port(self.port)
+
+
+class SystemPostgresDbProvider(PostgresDbProvider):
+    """A DB provider that uses an externally controlled postgres instance.
+
+    Spinning postgres instances up and down is costly. We are keeping
+    tests separate by assigning them random names, so we can share a
+    single DB cluster. This provider does just that: it talks to an
+    externally managed cluster, creates and deletes DBs on demand, but
+    does not manage the cluster's lifecycle.
+
+    The external cluster to talk to can be specified via the
+    `CLN_TEST_POSTGRES_DSN` environment variable.
+
+    Please make sure that the user specified in the DSN has the
+    permission to create new DBs.
+
+    Since tests, may end up interrupted, and may not clean up the
+    databases they created, be aware that over time your cluster may
+    accumulate quite a few databases. This mode is mostly intended for
+    CI where a throwaway postgre cluster can be spun up and tested
+    against.
+
+    """
+
+    def __init__(self, directory):
+        self.dsn = os.environ.get("CLN_TEST_POSTGRES_DSN")
+        self.conn = None
+        parts = urlparse(self.dsn)
+
+        self.hostname = parts.hostname
+        self.username = parts.username
+        self.password = parts.password if parts.password else ""
+        self.port = parts.port if parts.port else 5432
+        self.dbname = parts.path
+
+    def stop(self):
+        pass
+
+    def start(self):
+        self.conn = psycopg2.connect(self.dsn)
+        cur = self.conn.cursor()
+        cur.execute("SELECT 1")
+        cur.close()
+        # Required for CREATE DATABASE to work
+        self.conn.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT)