From: Michal Rakowski <michal.rakowski@baculasystems.com>
Date: Wed, 17 Feb 2021 07:27:42 +0000 (+0100)
Subject: Fix #7266 About Failing Runscript after not aborting job
X-Git-Tag: Beta-15.0.0~896
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4a7df7ba930ddd28ea606a7464f93fda8acf1db5;p=thirdparty%2Fbacula.git

Fix #7266 About Failing Runscript after not aborting job
---

diff --git a/bacula/src/dird/admin.c b/bacula/src/dird/admin.c
index 5f4a86fc8..860cf81f0 100644
--- a/bacula/src/dird/admin.c
+++ b/bacula/src/dird/admin.c
@@ -77,6 +77,15 @@ void admin_cleanup(JCR *jcr, int TermCode)
 
    Dmsg0(100, "Enter admin_cleanup()\n");
 
+   /* Job needs to be marked as terminated before running the after runscript */
+   jcr->setJobStatus(TermCode);
+
+   run_scripts(jcr, jcr->job->RunScripts, "AfterJob");
+
+   /* Runscript could have changed JobStatus,
+    * now check if it should be changed in the report or not */
+   TermCode = compareJobStatus(TermCode, jcr->JobStatus);
+
    update_job_end(jcr, TermCode);
 
    if (!db_get_job_record(jcr, jcr->db, &jcr->jr)) {
@@ -88,7 +97,11 @@ void admin_cleanup(JCR *jcr, int TermCode)
    msg_type = M_INFO;                 /* by default INFO message */
    switch (jcr->JobStatus) {
    case JS_Terminated:
-      term_msg = _("Admin OK");
+      if (jcr->JobErrors) {
+         term_msg = _("Admin OK -- with warnings");
+      } else {
+         term_msg = _("Admin OK");
+      }
       break;
    case JS_FatalError:
    case JS_ErrorTerminated:
diff --git a/bacula/src/dird/backup.c b/bacula/src/dird/backup.c
index e716ad4c1..a97d26135 100644
--- a/bacula/src/dird/backup.c
+++ b/bacula/src/dird/backup.c
@@ -993,6 +993,15 @@ void backup_cleanup(JCR *jcr, int TermCode)
    Dmsg2(100, "Enter backup_cleanup %d %c\n", TermCode, TermCode);
    memset(&cr, 0, sizeof(cr));
 
+   /* Job needs to be marked as terminated before running the after runscript */
+   jcr->setJobStatus(TermCode);
+
+   run_scripts(jcr, jcr->job->RunScripts, "AfterJob");
+
+   /* Runscript could have changed JobStatus,
+    * now check if it should be changed in the report or not */
+   TermCode = compareJobStatus(TermCode, jcr->JobStatus);
+
 #ifdef xxxx
    /* The current implementation of the JS_Warning status is not
     * completed. SQL part looks to be ok, but the code is using
diff --git a/bacula/src/dird/job.c b/bacula/src/dird/job.c
index 274913d5f..d59a9a580 100644
--- a/bacula/src/dird/job.c
+++ b/bacula/src/dird/job.c
@@ -509,8 +509,6 @@ static void *job_thread(void *arg)
       break;
    }
 
-   run_scripts(jcr, jcr->job->RunScripts, "AfterJob");
-
    /* Send off any queued messages */
    if (jcr->msg_queue && jcr->msg_queue->size() > 0) {
       dequeue_messages(jcr);
diff --git a/bacula/src/dird/mac.c b/bacula/src/dird/mac.c
index 52c79bb01..76ac57a1d 100644
--- a/bacula/src/dird/mac.c
+++ b/bacula/src/dird/mac.c
@@ -703,7 +703,9 @@ void mac_cleanup(JCR *jcr, int TermCode, int writeTermCode)
    remove_dummy_jobmedia_records(jcr);
 
    Dmsg2(100, "Enter mac_cleanup %d %c\n", TermCode, TermCode);
-   update_job_end(jcr, TermCode);
+
+   /* Job needs to be marked as terminated before running the after runscript */
+   jcr->setJobStatus(TermCode);
 
    /*
     * Check if we actually did something.
@@ -848,6 +850,13 @@ void mac_cleanup(JCR *jcr, int TermCode, int writeTermCode)
          db_sql_query(wjcr->db, query.c_str(), NULL, NULL);
       }
    }
+   run_scripts(jcr, jcr->job->RunScripts, "AfterJob");
+
+   /* Runscript could have changed JobStatus,
+    * now check if it should be changed in the report or not */
+   TermCode = compareJobStatus(TermCode, jcr->JobStatus);
+
+   update_job_end(jcr, TermCode);
 
    switch (jcr->JobStatus) {
    case JS_Terminated:
diff --git a/bacula/src/dird/restore.c b/bacula/src/dird/restore.c
index 7b49410f8..89e09e350 100644
--- a/bacula/src/dird/restore.c
+++ b/bacula/src/dird/restore.c
@@ -658,6 +658,16 @@ void restore_cleanup(JCR *jcr, int TermCode)
    utime_t RunTime;
 
    Dmsg0(20, "In restore_cleanup\n");
+
+   /* Job needs to be marked as terminated before running the after runscript */
+   jcr->setJobStatus(TermCode);
+
+   run_scripts(jcr, jcr->job->RunScripts, "AfterJob");
+
+   /* Runscript could have changed JobStatus,
+    * now check if it should be changed in the report or not */
+   TermCode = compareJobStatus(TermCode, jcr->JobStatus);
+
    update_job_end(jcr, TermCode);
 
    memset(&cr, 0, sizeof(cr));
diff --git a/bacula/src/dird/vbackup.c b/bacula/src/dird/vbackup.c
index c9060f153..85a65977d 100644
--- a/bacula/src/dird/vbackup.c
+++ b/bacula/src/dird/vbackup.c
@@ -402,6 +402,15 @@ void vbackup_cleanup(JCR *jcr, int TermCode)
    Dmsg2(100, "Enter vbackup_cleanup %d %c\n", TermCode, TermCode);
    memset(&cr, 0, sizeof(cr));
 
+   /* Job needs to be marked as terminated before running the after runscript */
+   jcr->setJobStatus(TermCode);
+
+   run_scripts(jcr, jcr->job->RunScripts, "AfterJob");
+
+   /* Runscript could have changed JobStatus,
+    * now check if it should be changed in the report or not */
+   TermCode = compareJobStatus(TermCode, jcr->JobStatus);
+
    jcr->jr.JobLevel = L_FULL;   /* we want this to appear as a Full backup */
    jcr->JobFiles = jcr->SDJobFiles;
    jcr->JobBytes = jcr->SDJobBytes;
diff --git a/bacula/src/dird/verify.c b/bacula/src/dird/verify.c
index f03f1a432..cc26c46e8 100644
--- a/bacula/src/dird/verify.c
+++ b/bacula/src/dird/verify.c
@@ -456,6 +456,15 @@ void verify_cleanup(JCR *jcr, int TermCode)
       TermCode = JS_ErrorTerminated;
    }
 
+   /* Job needs to be marked as terminated before running the after runscript */
+   jcr->setJobStatus(TermCode);
+
+   run_scripts(jcr, jcr->job->RunScripts, "AfterJob");
+
+   /* Runscript could have changed JobStatus,
+    * now check if it should be changed in the report or not */
+   TermCode = compareJobStatus(TermCode, jcr->JobStatus);
+
    update_job_end(jcr, TermCode);
 
    if (job_canceled(jcr)) {
diff --git a/bacula/src/lib/jcr.c b/bacula/src/lib/jcr.c
index 7ab1fbffd..16be87520 100644
--- a/bacula/src/lib/jcr.c
+++ b/bacula/src/lib/jcr.c
@@ -923,22 +923,16 @@ void JCR::setJobStarted()
    job_started_time = time(NULL);
 }
 
-static pthread_mutex_t status_lock = PTHREAD_MUTEX_INITIALIZER;
-
-void JCR::setJobStatus(int newJobStatus)
+/*
+ * Compare two job statuses, return one with higher priority.
+ */
+int compareJobStatus(int oldJobStatus, int newJobStatus)
 {
    int priority, old_priority;
-   int oldJobStatus = JobStatus;
+   int ret_status = oldJobStatus;
 
-   P(status_lock);
    priority = get_status_priority(newJobStatus);
    old_priority = get_status_priority(oldJobStatus);
-
-   Dmsg2(800, "set_jcr_job_status(%ld, %c)\n", JobId, newJobStatus);
-
-   /* Update wait_time depending on newJobStatus and oldJobStatus */
-   update_wait_time(this, newJobStatus);
-
    /*
     * For a set of errors, ... keep the current status
     *   so it isn't lost. For all others, set it.
@@ -955,13 +949,31 @@ void JCR::setJobStatus(int newJobStatus)
        priority == 0 && old_priority == 0)) {
       Dmsg4(800, "Set new stat. old: %c,%d new: %c,%d\n",
             (oldJobStatus==0)?'0':oldJobStatus, old_priority, newJobStatus, priority);
-      JobStatus = newJobStatus;     /* replace with new status */
+      ret_status = newJobStatus;     /* replace with new status */
    }
 
-   if (oldJobStatus != JobStatus) {
+   if (oldJobStatus != newJobStatus) {
       Dmsg2(800, "leave setJobStatus old=%c new=%c\n", (oldJobStatus==0)?'0':oldJobStatus, newJobStatus);
 //    generate_plugin_event(this, bEventStatusChange, NULL);
    }
+
+   return ret_status;
+}
+
+static pthread_mutex_t status_lock = PTHREAD_MUTEX_INITIALIZER;
+
+void JCR::setJobStatus(int newJobStatus)
+{
+
+   P(status_lock);
+
+   Dmsg2(800, "set_jcr_job_status(%ld, %c)\n", JobId, newJobStatus);
+
+   /* Update wait_time depending on newJobStatus and oldJobStatus */
+   update_wait_time(this, newJobStatus);
+
+   JobStatus = compareJobStatus(JobStatus, newJobStatus);
+
    V(status_lock);
 }
 
diff --git a/bacula/src/lib/protos.h b/bacula/src/lib/protos.h
index d84306339..b3bd62c51 100644
--- a/bacula/src/lib/protos.h
+++ b/bacula/src/lib/protos.h
@@ -264,6 +264,7 @@ void     set_jcr_in_tsd(JCR *jcr);
 void     remove_jcr_from_tsd(JCR *jcr);
 uint32_t get_jobid_from_tsd();
 uint32_t get_jobid_from_tid(pthread_t tid);
+int compareJobStatus(int oldJobStatus, int newJobStatus);
 
 
 /* lex.c */
diff --git a/bacula/src/lib/runscript.c b/bacula/src/lib/runscript.c
index 967fa2e7d..c43b312a3 100644
--- a/bacula/src/lib/runscript.c
+++ b/bacula/src/lib/runscript.c
@@ -100,7 +100,7 @@ int run_scripts(JCR *jcr, alist *runscripts, const char *label)
 
    RUNSCRIPT *script;
    bool runit;
-
+   bool ret = true; /* Changed to false in case any of the scripts fails */
    int when;
 
    if (strstr(label, NT_("Before"))) {
@@ -166,10 +166,13 @@ int run_scripts(JCR *jcr, alist *runscripts, const char *label)
 
       /* we execute it */
       if (runit) {
-         script->run(jcr, label);
+         if (!script->run(jcr, label)) {
+            ret = false; /* return err in case any of the script commands failed */
+         }
       }
    }
-   return 1;
+
+   return ret;
 }
 
 bool RUNSCRIPT::is_local()