fix(restore): roll back to the previous data when the restore fails

openldap-ctl restore --force deleted existing config and data before
loading the backup: a failing slapadd left OpenLDAP stopped with empty or
partial databases. Existing data is now moved to a hidden
.restore-previous directory in the same volume, removed when the restore
succeeds, and put back otherwise. OpenLDAP is restarted in both cases if
it was running. A leftover .restore-previous directory from an
interrupted restore blocks the next restore.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

AGENTS.md

@@ -68,7 +68,7 @@ Adding a feature: create `services/openldap-bootstrap/assets/scripts/NN-feature.
 `/usr/bin/openldap-ctl` → `services/openldap/assets/openldap-ctl/openldap-ctl.sh`, which dispatches to `command/<name>.sh` (adding a file there adds a command).
 
 - `backup <prefix> [--config] [--data] [--clean N] [--clean-prefix P]`: `slapcat | gzip` into `OPENLDAP_BACKUP_DIR`, written to a temp file then moved (atomic), umask 077. `--clean` only removes backup files (`*-config.gz`, `*-data.gz`) older than N days, starting with P if set; the cron uses `openldap-cron-` so manual and upgrade backups are kept.
-- `restore <prefix> [--config] [--data] [--force]`: first checks the backup files exist and are valid gzip (before stopping or deleting anything), then refuses if slapd runs or dirs are not empty unless `--force` (stops/restarts slapd, wipes dirs).
+- `restore <prefix> [--config] [--data] [--force]`: first checks the backup files exist and are valid gzip (before stopping or deleting anything), then refuses if slapd runs or dirs are not empty unless `--force` (stops/restarts slapd). Existing data is moved to `<dir>/.restore-previous` (same volume) and put back if the restore fails; a leftover `.restore-previous` (interrupted restore) blocks the next restore.
 - `password generate|hash`, `schema2ldif <file.schema> [deps]`.
 
 ## Coding conventions

CHANGELOG.md

@@ -17,6 +17,7 @@ Project-specific changes only. See upstream [OpenLDAP release notes](https://www
 - Fail bootstrap with an explicit error when `OPENLDAP_BOOTSTRAP_SUFFIX` does not start with `dc=`, instead of creating a root entry with an empty `dc`.
 - Fail bootstrap with an explicit error when `OPENLDAP_BOOTSTRAP_REPLICATION_HOSTS` contains more than 9 hosts.
 - Check that backup files exist and are valid gzip archives before `openldap-ctl restore` stops OpenLDAP or deletes existing data.
+- Roll back to the previous config and data, and restart OpenLDAP if it was running, when `openldap-ctl restore` fails. Existing data is moved to a hidden `.restore-previous` directory in the same volume until the restore succeeds.
 - Print usage when `openldap-ctl schema2ldif` is called without arguments.
 - `openldap-ctl backup --clean` only removes backup files, and the new `--clean-prefix` option limits it to backups starting with a prefix. Cron backups are now named `openldap-cron-<date>-<version>` and only clean these files, manual and upgrade backups are kept. Cron backups named with the previous format are no longer removed automatically.
 

README.md

@@ -149,7 +149,7 @@ Restore a backup:
 docker exec openldap container run -- openldap-ctl restore my-backup --force
 ```
 
-`--force` stops the OpenLDAP process if needed, deletes existing data in the target directories, restores the selected databases, then starts OpenLDAP again. Backup files are checked first: if one is missing or corrupted, nothing is stopped or deleted.
+`--force` stops the OpenLDAP process if needed, replaces existing data in the target directories with the selected databases, then starts OpenLDAP again. Backup files are checked first: if one is missing or corrupted, nothing is stopped or deleted. If the restore fails, the previous data is put back.
 
 The cron service is disabled by default. To schedule regular backups, run it in a separate container alongside OpenLDAP, sharing its configuration, data, and backup volumes:
 

build/tests/upgrade.go

@@ -149,6 +149,29 @@ func BackupRestore(ctx context.Context, client *dagger.Client, image *dagger.Con
 		return fmt.Errorf("data after the restore of a corrupted backup: %w", err)
 	}
 
+	// a backup that fails to load is rolled back to the previous data
+	invalid := withVolumes(image, v).
+		WithExec([]string{"cp", backupDir + "/test-backup-config.gz", backupDir + "/invalid-config.gz"}).
+		WithExec([]string{"sh", "-c", "printf 'dn: not a valid entry\\nfoo\\n' | gzip > " + backupDir + "/invalid-data.gz"})
+
+	r, err = expect(ctx, invalid, false, []string{"Restore failed, rolling back to the previous data"}, "container", "run", "--", "openldap-ctl", "restore", "invalid", "--force")
+	if err != nil {
+		return fmt.Errorf("restore of an invalid backup: %w", err)
+	}
+	rolledBack := readVolumes(r.container)
+	if err := expectPeople(ctx, image, rolledBack); err != nil {
+		return fmt.Errorf("data after the rollback: %w", err)
+	}
+	if _, err := expect(ctx, withVolumes(image, rolledBack), false, nil, "ls", "-d", confDir+"/.restore-previous", dataDir+"/.restore-previous"); err != nil {
+		return fmt.Errorf("previous data directories after the rollback: %w", err)
+	}
+
+	// an interrupted restore is detected
+	interrupted := withVolumes(image, v).WithExec([]string{"mkdir", dataDir + "/.restore-previous"})
+	if _, err := expect(ctx, interrupted, false, []string{"a previous restore was interrupted"}, "container", "run", "--", "openldap-ctl", "restore", "test-backup", "--force"); err != nil {
+		return fmt.Errorf("restore after an interrupted restore: %w", err)
+	}
+
 	return nil
 }
 

services/openldap/assets/openldap-ctl/command/restore.sh

@@ -10,8 +10,14 @@ CONFIG=false
 DATA=false
 FORCE=false
 RESTART=false
+RESTORED=false
 FILE_PREFIX=""
 
+# existing config and data are moved to this hidden directory, in the same volume, until
+# the restore succeeds
+PREVIOUS_DIR_NAME=".restore-previous"
+STASHED_DIRS=()
+
 usage() {
     echo "Usage: openldap-ctl restore file-prefix [--config] [--data] [--force]"
 }
@@ -43,6 +49,50 @@ restore_database() {
     fi
 }
 
+# move the content of a directory to its previous directory
+stash() {
+    local dir="$1"
+    local previous_dir="${dir}/${PREVIOUS_DIR_NAME}"
+
+    container log info "Moving existing data in ${dir} to ${previous_dir} ..."
+    mkdir "${previous_dir}"
+    STASHED_DIRS+=("${dir}")
+
+    find "${dir}" -mindepth 1 -maxdepth 1 ! -name "lost+found" ! -name ".*" \
+        -exec mv -v {} "${previous_dir}/" \; 2>&1 | container log debug
+}
+
+# replace the content of a directory by its previous directory content
+unstash() {
+    local dir="$1"
+    local previous_dir="${dir}/${PREVIOUS_DIR_NAME}"
+
+    container log warning "Restoring previous data in ${dir} ..."
+
+    find "${dir}" -mindepth 1 -maxdepth 1 ! -name "lost+found" ! -name ".*" -exec rm -fr {} +
+    find "${previous_dir}" -mindepth 1 -maxdepth 1 -exec mv -v {} "${dir}/" \; 2>&1 | container log debug
+    rmdir "${previous_dir}"
+}
+
+finish() {
+    local dir
+
+    if [ "${RESTORED}" = true ]; then
+        for dir in "${STASHED_DIRS[@]}"; do
+            rm -fr "${dir:?}/${PREVIOUS_DIR_NAME}"
+        done
+    elif [ "${#STASHED_DIRS[@]}" -gt 0 ]; then
+        container log error "Restore failed, rolling back to the previous data ..."
+        for dir in "${STASHED_DIRS[@]}"; do
+            unstash "${dir}"
+        done
+    fi
+
+    if [ "${RESTART}" = true ]; then
+        container processes start openldap
+    fi
+}
+
 while [ $# -gt 0 ]; do
     case "$1" in
         --config)
@@ -94,6 +144,12 @@ if [ "${DATA}" = true ]; then
     check_backup_file "${DATA_BACKUP_FILE}"
 fi
 
+for dir in "${OPENLDAP_CONF_DIR}" "${OPENLDAP_DATA_DIR}"; do
+    if [ -e "${dir}/${PREVIOUS_DIR_NAME}" ]; then
+        container log fatal "${dir}/${PREVIOUS_DIR_NAME} exists: a previous restore was interrupted. Check the data, then move its content back to ${dir} or delete it."
+    fi
+done
+
 if [ -e "/run/container/processes/openldap.pid" ]; then
 
     if [ "${FORCE}" != true ]; then
@@ -115,28 +171,20 @@ if [ -n "$(find "${OPENLDAP_CONF_DIR}" -mindepth 1 -maxdepth 1 ! -name "lost+fou
 
 fi
 
-if [ "${CONFIG}" = true ]; then
+trap finish EXIT
 
-    if [ -n "$(find "${OPENLDAP_CONF_DIR}" -mindepth 1 -maxdepth 1 ! -name "lost+found" ! -name ".*")" ]; then
-        container log warning "Deleting data in ${OPENLDAP_CONF_DIR} ..."
-        rm -fr "${OPENLDAP_CONF_DIR:?}"/*
-    fi
+if [ "${CONFIG}" = true ]; then
+    stash "${OPENLDAP_CONF_DIR}"
 
     container log info "Restoring OpenLDAP config ..."
     restore_database "${CONFIG_BACKUP_FILE}" 0
 fi
 
 if [ "${DATA}" = true ]; then
-
-    if [ -n "$(find "${OPENLDAP_DATA_DIR}" -mindepth 1 -maxdepth 1 ! -name "lost+found" ! -name ".*")" ]; then
-        container log warning "Deleting data in ${OPENLDAP_DATA_DIR} ..."
-        rm -fr "${OPENLDAP_DATA_DIR:?}"/*
-    fi
+    stash "${OPENLDAP_DATA_DIR}"
 
     container log info "Restoring OpenLDAP data ..."
     restore_database "${DATA_BACKUP_FILE}" 1
 fi
 
-if [ "${RESTART}" = true ]; then
-    container processes start openldap
-fi
+RESTORED=true