fix(backup)!: read the instance from one engine-name set, and trust it

Two shapes fell through the inline regex, which knew `database`, `db` and
`postgres` only. A container named exactly after its engine - what a compose
file writes as `container_name: postgres` - carries no separator before the
token, so it resolved to nothing and 6.0.0 stopped dumping it without saying
so. And a swarm task of a central MariaDB reads `mariadb_mariadb.1.<id>`,
where `_mariadb` was no token at all, so that database has never been dumped
under swarm at all.

ENGINE_NAMES states the set once and serves both readings: carried as a
suffix it makes the rest the instance, being one outright makes the container
its own instance.

backup_mariadb_or_postgres stops calling an application container a database.
container_engine recognises an engine by its client tools, which an
application image often ships, so refusing the dump alone would have recorded
the volume as `database: true, dumped: false` - the exact shape a restore
drill reads as a database that was missed. Without an instance there is no
database to record.

BREAKING CHANGE: `mariadb` and `mysql` join the suffix tokens, so a container
named `<app>-mariadb` resolves to the instance `<app>` rather than to its own
name. A databases.csv keyed on the full container name has to move to the
application name, or name the container in --database-containers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-18 05:11:46 +02:00
parent 704481a505
commit f97efb10c4
4 changed files with 56 additions and 13 deletions

View File

@@ -16,13 +16,18 @@ if TYPE_CHECKING:
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
ENGINE_NAMES = ("database", "postgres", "mariadb", "mysql", "db")
_SUFFIX_RE = re.compile(rf"(_|-)({'|'.join(ENGINE_NAMES)})")
def get_instance(container: str, database_containers: list[str]) -> str | None: def get_instance(container: str, database_containers: list[str]) -> str | None:
"""The databases.csv instance a container serves, or None for no database. """The databases.csv instance a container serves, or None for no database.
A declared container is its own instance. Every other name is normalised by A declared container is its own instance. Every other name is read against
stripping a database suffix token, which maps both `<app>-database` from ENGINE_NAMES: carrying one as a suffix makes the rest the instance, which
compose and `<app>_database.1.<task>` from swarm onto the same instance. maps `<app>-database` from compose and `<app>_database.1.<task>` from swarm
onto the same one; being one outright makes the container its own instance,
the shape a compose file writes as `container_name: postgres`.
Args: Args:
container: the running container's name. container: the running container's name.
@@ -30,14 +35,16 @@ def get_instance(container: str, database_containers: list[str]) -> str | None:
declared engines whatever they are called. declared engines whatever they are called.
Returns: Returns:
The instance name, or None when the name carries no database token: an The instance name, or None when the name neither carries nor is an
application container is not an engine, even when it ships the client engine name: an application container is not an engine, even when it
tools that would let a dump command start. ships the client tools that would let a dump command start.
""" """
if container in database_containers: if container in database_containers:
return container return container
parts = re.split(r"(_|-)(database|db|postgres)", container) parts = _SUFFIX_RE.split(container)
return parts[0] if len(parts) > 1 else None if len(parts) > 1:
return parts[0]
return container if container in ENGINE_NAMES else None
def fallback_pg_dumpall( def fallback_pg_dumpall(

View File

@@ -10,7 +10,7 @@ from pandas.errors import EmptyDataError
from baudolo.databases import COLUMNS, DELIMITER from baudolo.databases import COLUMNS, DELIMITER
from .db import backup_database from .db import backup_database, get_instance
from .docker import has_tool, image_id from .docker import has_tool, image_id
DUMP_TOOLS: tuple[tuple[str, str], ...] = ( DUMP_TOOLS: tuple[tuple[str, str], ...] = (
@@ -76,6 +76,8 @@ def backup_mariadb_or_postgres(
engine = container_engine(container) engine = container_engine(container)
if engine is None: if engine is None:
return VolumeOutcome(database=False, dumped=False) return VolumeOutcome(database=False, dumped=False)
if get_instance(container, database_containers) is None:
return VolumeOutcome(database=False, dumped=False)
db_type, dump_tool = engine db_type, dump_tool = engine
dumped = backup_database( dumped = backup_database(
container=container, container=container,

View File

@@ -13,10 +13,11 @@ by its own launcher and ships pg_dumpall, so a dump command starts there and
writes a file that looks like a backup and holds none of the data. writes a file that looks like a backup and holds none of the data.
""" """
import json
import unittest import unittest
from pathlib import Path from pathlib import Path
from baudolo.generation import DUMP_SUFFIX, FILES_DIR, SQL_DIR from baudolo.generation import DUMP_SUFFIX, FILES_DIR, MANIFEST_FILE, SQL_DIR
from .helpers import ( from .helpers import (
POSTGRES_DATA_DIR, POSTGRES_DATA_DIR,
@@ -158,6 +159,26 @@ class TestE2EAppContainerShipsClientTools(unittest.TestCase):
self.assertTrue(marker.is_file(), f"expected a file backup at {marker}") self.assertTrue(marker.is_file(), f"expected a file backup at {marker}")
self.assertIn(MARKER, marker.read_text(encoding="utf-8")) self.assertIn(MARKER, marker.read_text(encoding="utf-8"))
def test_the_manifest_does_not_call_the_application_volume_a_database(self) -> None:
manifest = json.loads(
(self.volume_dir(self.app_volume).parent / MANIFEST_FILE).read_text(
encoding="utf-8"
)
)
entry = manifest["volumes"][self.app_volume]
self.assertFalse(entry["database"], entry)
self.assertFalse(entry["dumped"], entry)
def test_the_manifest_records_the_engine_volume_as_dumped(self) -> None:
manifest = json.loads(
(self.volume_dir(self.engine_volume).parent / MANIFEST_FILE).read_text(
encoding="utf-8"
)
)
entry = manifest["volumes"][self.engine_volume]
self.assertTrue(entry["database"], entry)
self.assertTrue(entry["dumped"], entry)
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()

View File

@@ -24,12 +24,21 @@ class TestDeclaredContainers(unittest.TestCase):
get_instance("shop-database", ["shop-database"]), "shop-database" get_instance("shop-database", ["shop-database"]), "shop-database"
) )
def test_an_undeclared_central_engine_resolves_to_nothing(self) -> None: def test_a_qualified_central_name_still_has_to_be_declared(self) -> None:
"""`postgres-central` has no separator before its token, so nothing is
stripped - a central engine has to be declared to be found."""
self.assertIsNone(get_instance("postgres-central", [])) self.assertIsNone(get_instance("postgres-central", []))
class TestContainersNamedAfterTheirEngine(unittest.TestCase):
def test_a_bare_engine_name_is_its_own_instance(self) -> None:
for name in ("postgres", "mariadb", "mysql", "db", "database"):
with self.subTest(container=name):
self.assertEqual(get_instance(name, []), name)
def test_a_swarm_task_of_such_a_container_keeps_the_instance(self) -> None:
self.assertEqual(get_instance("postgres_postgres.1.k3f9x2", []), "postgres")
self.assertEqual(get_instance("mariadb_mariadb.1.k3f9x2", []), "mariadb")
class TestDedicatedEngines(unittest.TestCase): class TestDedicatedEngines(unittest.TestCase):
def test_compose_names_the_container_with_a_hyphen(self) -> None: def test_compose_names_the_container_with_a_hyphen(self) -> None:
self.assertEqual(get_instance("discourse-database", []), "discourse") self.assertEqual(get_instance("discourse-database", []), "discourse")
@@ -49,6 +58,10 @@ class TestDedicatedEngines(unittest.TestCase):
def test_mariadb_uses_the_same_suffix(self) -> None: def test_mariadb_uses_the_same_suffix(self) -> None:
self.assertEqual(get_instance("matomo-database", []), "matomo") self.assertEqual(get_instance("matomo-database", []), "matomo")
def test_an_engine_named_suffix_is_stripped_too(self) -> None:
self.assertEqual(get_instance("shop-mariadb", []), "shop")
self.assertEqual(get_instance("shop-mysql", []), "shop")
class TestApplicationContainers(unittest.TestCase): class TestApplicationContainers(unittest.TestCase):
def test_a_bare_application_name_is_not_a_database(self) -> None: def test_a_bare_application_name_is_not_a_database(self) -> None: