10:42 a.m.
This is an automated email from the git hooks/post-receive script.
mreynolds pushed a commit to branch 389-ds-base-1.4.1
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.1 by this push:
new fb67a9a Issue 50689 - Failed db restore task does not report an error
fb67a9a is described below
commit fb67a9a697f1ae1b71e02d1f0eeb660f77f15978
Author: Mark Reynolds <mreynolds(a)redhat.com>
AuthorDate: Mon Nov 4 15:45:55 2019 -0500
Issue 50689 - Failed db restore task does not report an error
Bug Description: If you have a back up that contains a backend that
is not configured the restore fails, but a success
return code is returned to the client. This happens
becuase the return code gets overwritten after the
failure.
Fix Description: Preserve the error code upon failure and properly update
the task exit code.
relates: https://pagure.io/389-ds-base/issue/50689
Reviewed by: tboardaz & lkrispen(Thanks!!)
Never rewrite the orginal error code
---
dirsrvtests/tests/suites/backups/backup_test.py | 73 +++++++++++++++++++++++++
ldap/servers/slapd/back-ldbm/archive.c | 42 +++++++-------
ldap/servers/slapd/back-ldbm/dblayer.c | 23 ++++----
src/lib389/lib389/cli_conf/backup.py | 3 +-
4 files changed, 106 insertions(+), 35 deletions(-)
diff --git a/dirsrvtests/tests/suites/backups/backup_test.py
b/dirsrvtests/tests/suites/backups/backup_test.py
new file mode 100644
index 0000000..e938914
--- /dev/null
+++ b/dirsrvtests/tests/suites/backups/backup_test.py
@@ -0,0 +1,73 @@
+import logging
+import pytest
+import os
+from datetime import datetime
+from lib389._constants import DEFAULT_SUFFIX, INSTALL_LATEST_CONFIG
+from lib389.properties import BACKEND_SAMPLE_ENTRIES, TASK_WAIT
+from lib389.topologies import topology_st as topo
+from lib389.backend import Backend
+from lib389.tasks import BackupTask, RestoreTask
+
+DEBUGGING = os.getenv("DEBUGGING", default=False)
+if DEBUGGING:
+ logging.getLogger(__name__).setLevel(logging.DEBUG)
+else:
+ logging.getLogger(__name__).setLevel(logging.INFO)
+log = logging.getLogger(__name__)
+
+
+def test_missing_backend(topo):
+ """Test that an error is returned when a restore is performed for a
+ backend that is no longer present.
+
+ :id: 889b8028-35cf-41d7-91f6-bc5193683646
+ :setup: Standalone Instance
+ :steps:
+ 1. Create a second backend
+ 2. Perform a back up
+ 3. Remove one of the backends from the config
+ 4. Perform a restore
+ :expectedresults:
+ 1. Success
+ 2. Success
+ 3. Success
+ 4. Failure
+ """
+
+ # Create a new backend
+ BE_NAME = 'backupRoot'
+ BE_SUFFIX = 'dc=back,dc=up'
+ props = {
+ 'cn': BE_NAME,
+ 'nsslapd-suffix': BE_SUFFIX,
+ BACKEND_SAMPLE_ENTRIES: INSTALL_LATEST_CONFIG
+ }
+ be = Backend(topo.standalone)
+ backend_entry = be.create(properties=props)
+
+ # perform backup
+ backup_dir_name = "backup-%s" %
datetime.now().strftime("%Y_%m_%d_%H_%M_%S")
+ archive = os.path.join(topo.standalone.ds_paths.backup_dir, backup_dir_name)
+ backup_task = BackupTask(topo.standalone)
+ task_properties = {'nsArchiveDir': archive}
+ backup_task.create(properties=task_properties)
+ backup_task.wait()
+ assert backup_task.get_exit_code() == 0
+
+ # Remove new backend
+ backend_entry.delete()
+
+ # Restore the backup - it should fail
+ restore_task = RestoreTask(topo.standalone)
+ task_properties = {'nsArchiveDir': archive}
+ restore_task.create(properties=task_properties)
+ restore_task.wait()
+ assert restore_task.get_exit_code() != 0
+
+
+if __name__ == '__main__':
+ # Run isolated
+ # -s for DEBUG mode
+ CURRENT_FILE = os.path.realpath(__file__)
+ pytest.main(["-s", CURRENT_FILE])
+
diff --git a/ldap/servers/slapd/back-ldbm/archive.c
b/ldap/servers/slapd/back-ldbm/archive.c
index 1b06cf9..8ad635a 100644
--- a/ldap/servers/slapd/back-ldbm/archive.c
+++ b/ldap/servers/slapd/back-ldbm/archive.c
@@ -19,18 +19,18 @@ int
ldbm_back_archive2ldbm(Slapi_PBlock *pb)
{
struct ldbminfo *li;
- char *rawdirectory = NULL; /* -a <directory> */
- char *directory = NULL; /* normalized */
- char *backendname = NULL;
- int return_value = -1;
- int task_flags = 0;
- int run_from_cmdline = 0;
Slapi_Task *task;
- int is_old_to_new = 0;
ldbm_instance *inst = NULL;
char *dbversion = NULL;
char *dataversion = NULL;
- int value = 0;
+ int32_t value = 0;
+ char *rawdirectory = NULL; /* -a <directory> */
+ char *directory = NULL; /* normalized */
+ char *backendname = NULL;
+ int32_t return_value = -1;
+ int32_t task_flags = 0;
+ int32_t run_from_cmdline = 0;
+ int32_t is_old_to_new = 0;
slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &li);
slapi_pblock_get(pb, SLAPI_SEQ_VAL, &rawdirectory);
@@ -172,8 +172,7 @@ ldbm_back_archive2ldbm(Slapi_PBlock *pb)
"the backup set. error=%d
(%s)\n",
return_value, dblayer_strerror(return_value));
if (task) {
- slapi_task_log_notice(task, "Failed to read the backup file set "
- "from %s",
+ slapi_task_log_notice(task, "Failed to read the backup file set from
%s",
directory);
}
}
@@ -184,7 +183,7 @@ ldbm_back_archive2ldbm(Slapi_PBlock *pb)
char *p;
char c;
char *bakup_dir = NULL;
- int skipinit = SLAPI_UPGRADEDB_SKIPINIT;
+ int32_t skipinit = SLAPI_UPGRADEDB_SKIPINIT;
p = strrchr(directory, '/');
if (NULL == p) {
@@ -211,25 +210,26 @@ ldbm_back_archive2ldbm(Slapi_PBlock *pb)
return_value = ldbm_back_upgradedb(pb);
}
} else {
- ldbm_instance *inst;
Object *inst_obj;
- int ret;
+ int32_t ret;
if (0 != return_value) {
- /* error case (607331)
- * just to go back to the previous state if possible */
- if ((return_value = dblayer_start(li, DBLAYER_NORMAL_MODE))) {
+ /*
+ * error case (607331)
+ * just to go back to the previous state if possible (preserve
+ * original error for now)
+ */
+ if ((ret = dblayer_start(li, DBLAYER_NORMAL_MODE))) {
slapi_log_err(SLAPI_LOG_ERR,
"ldbm_back_archive2ldbm", "Unable to to
start database in [%s]\n",
li->li_directory);
if (task) {
- slapi_task_log_notice(task, "Failed to start the database in
"
- "%s",
+ slapi_task_log_notice(task, "Failed to start the database in
%s",
li->li_directory);
}
- goto out;
}
}
+
/* bring all backends and changelog back online */
plugin_call_plugins(pb, SLAPI_PLUGIN_BE_POST_OPEN_FN);
for (inst_obj = objset_first_obj(li->li_instance_set); inst_obj;
@@ -241,8 +241,7 @@ ldbm_back_archive2ldbm(Slapi_PBlock *pb)
"ldbm_back_archive2ldbm", "Unable to restart
'%s'\n",
inst->inst_name);
if (task) {
- slapi_task_log_notice(task, "Unable to restart
'%s'",
- inst->inst_name);
+ slapi_task_log_notice(task, "Unable to restart
'%s'", inst->inst_name);
}
} else {
slapi_mtn_be_enable(inst->inst_be);
@@ -250,6 +249,7 @@ ldbm_back_archive2ldbm(Slapi_PBlock *pb)
}
}
}
+
out:
if (run_from_cmdline && (0 == return_value)) {
dblayer_restore_file_update(li, directory);
diff --git a/ldap/servers/slapd/back-ldbm/dblayer.c
b/ldap/servers/slapd/back-ldbm/dblayer.c
index 1fee805..8c93119 100644
--- a/ldap/servers/slapd/back-ldbm/dblayer.c
+++ b/ldap/servers/slapd/back-ldbm/dblayer.c
@@ -6506,12 +6506,10 @@ dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task
*task, char *bena
/* We check on the source staging area, no point in going further if it
* isn't there */
if (stat(src_dir, &sbuf) < 0) {
- slapi_log_err(SLAPI_LOG_ERR, "dblayer_restore", "Backup directory
%s does not "
- "exist.\n",
+ slapi_log_err(SLAPI_LOG_ERR, "dblayer_restore", "Backup directory
%s does not exist.\n",
src_dir);
if (task) {
- slapi_task_log_notice(task, "Restore: backup directory %s does not
"
- "exist.\n",
+ slapi_task_log_notice(task, "Restore: backup directory %s does not
exist.",
src_dir);
}
return LDAP_UNWILLING_TO_PERFORM;
@@ -6520,8 +6518,7 @@ dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task
*task, char *bena
"a directory.\n",
src_dir);
if (task) {
- slapi_task_log_notice(task, "Restore: backup directory %s is not "
- "a directory.\n",
+ slapi_task_log_notice(task, "Restore: backup directory %s is not a
directory.",
src_dir);
}
return LDAP_UNWILLING_TO_PERFORM;
@@ -6565,12 +6562,13 @@ dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task
*task, char *bena
inst = ldbm_instance_find_by_name(li, (char *)direntry->name);
if (inst == NULL) {
slapi_log_err(SLAPI_LOG_ERR,
- "dblayer_restore", "Target server
has no %s configured\n",
+ "dblayer_restore", "Target server
has no backend (%s) configured\n",
direntry->name);
if (task) {
slapi_task_log_notice(task,
- "dblayer_restore - Target server
has no %s configured\n",
+ "dblayer_restore - Target server
has no backend (%s) configured",
direntry->name);
+ slapi_task_cancel(task, LDAP_UNWILLING_TO_PERFORM);
}
PR_CloseDir(dirhandle);
return_value = LDAP_UNWILLING_TO_PERFORM;
@@ -6581,7 +6579,7 @@ dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task
*task, char *bena
slapi_log_err(SLAPI_LOG_ERR, "dblayer_restore",
"Parent directory is not set, aborting
restore\n");
if (task) {
- slapi_task_log_notice(task, "dblayer_restore - Parent
directory is not set, aborting restore\n");
+ slapi_task_log_notice(task, "dblayer_restore - Parent
directory is not set, aborting restore");
}
PR_CloseDir(dirhandle);
return_value = LDAP_UNWILLING_TO_PERFORM;
@@ -6636,7 +6634,7 @@ dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task
*task, char *bena
"dblayer_restore", "Failed to open the directory
\"%s\"\n", real_src_dir);
if (task) {
slapi_task_log_notice(task,
- "Restore: failed to open the directory
\"%s\"\n", real_src_dir);
+ "Restore: failed to open the directory
\"%s\"", real_src_dir);
}
return_value = -1;
goto error_out;
@@ -6669,7 +6667,7 @@ dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task
*task, char *bena
changelogdir);
if (task) {
slapi_task_log_notice(task,
- "Restore: broken changelog dir
path %s\n",
+ "Restore: broken changelog dir
path %s",
changelogdir);
}
goto error_out;
@@ -6778,8 +6776,7 @@ dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task
*task, char *bena
char *dataversion = NULL;
if (dbversion_read(li, home_dir, &ldbmversion, &dataversion) != 0) {
- slapi_log_err(SLAPI_LOG_WARNING, "dblayer_restore", "Unable to
read dbversion "
- "file in
%s\n",
+ slapi_log_err(SLAPI_LOG_WARNING, "dblayer_restore", "Unable to
read dbversion file in %s\n",
home_dir);
} else {
adjust_idl_switch(ldbmversion, li);
diff --git a/src/lib389/lib389/cli_conf/backup.py b/src/lib389/lib389/cli_conf/backup.py
index 1e73fba..c53a39b 100644
--- a/src/lib389/lib389/cli_conf/backup.py
+++ b/src/lib389/lib389/cli_conf/backup.py
@@ -26,11 +26,12 @@ def backup_restore(inst, basedn, log, args):
task = inst.restore_online(archive=args.archive, db_type=args.db_type)
task.wait()
result = task.get_exit_code()
+ task_log = task.get_task_log()
if task.is_complete() and result == 0:
log.info("The backup restore task has finished successfully")
else:
- raise ValueError("The backup restore task has failed with the error code:
({})".format(result))
+ raise ValueError("The backup restore task has failed with the error code:
{}\n{}".format(result, task_log))
def create_parser(subparsers):
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.