From 7e73ac0222634dc8ba7eb426c8c87b20f64cfa54 Mon Sep 17 00:00:00 2001 From: Andreas Rogge Date: Fri, 31 Dec 2021 12:29:37 +0100 Subject: [PATCH 1/3] python-dir: fix plugin-context confusion Previously when running multiple jobs concurrently, the python plugin would not always set the right context before running the python callback function. This patch makes the context thread_local and resets it before every callback. --- core/src/plugins/dird/python/module/bareosdir.h | 2 +- core/src/plugins/dird/python/python-dir.cc | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/core/src/plugins/dird/python/module/bareosdir.h b/core/src/plugins/dird/python/module/bareosdir.h index 50fb6773625..ff07a881db1 100644 --- a/core/src/plugins/dird/python/module/bareosdir.h +++ b/core/src/plugins/dird/python/module/bareosdir.h @@ -86,7 +86,7 @@ static bRC PyHandlePluginEvent(PluginContext* plugin_ctx, using namespace directordaemon; /* variables storing bareos pointers */ -PluginContext* plugin_context = NULL; +thread_local PluginContext* plugin_context = NULL; MOD_INIT(bareosdir) { diff --git a/core/src/plugins/dird/python/python-dir.cc b/core/src/plugins/dird/python/python-dir.cc index 6b526727561..46e923e42c6 100644 --- a/core/src/plugins/dird/python/python-dir.cc +++ b/core/src/plugins/dird/python/python-dir.cc @@ -136,6 +136,7 @@ static bRC getPluginValue(PluginContext* bareos_plugin_ctx, bRC retval = bRC_Error; if (!plugin_priv_ctx) { goto bail_out; } + Bareosdir_set_plugin_context(bareos_plugin_ctx); PyEval_AcquireThread(plugin_priv_ctx->interpreter); retval = Bareosdir_PyGetPluginValue(bareos_plugin_ctx, var, value); @@ -155,6 +156,7 @@ static bRC setPluginValue(PluginContext* bareos_plugin_ctx, bRC retval = bRC_Error; if (!plugin_priv_ctx) { return bRC_Error; } + Bareosdir_set_plugin_context(bareos_plugin_ctx); PyEval_AcquireThread(plugin_priv_ctx->interpreter); retval = Bareosdir_PySetPluginValue(bareos_plugin_ctx, var, value); @@ -303,6 +305,8 @@ static bRC handlePluginEvent(PluginContext* plugin_ctx, if (!plugin_priv_ctx) { goto bail_out; } + Bareosdir_set_plugin_context(plugin_ctx); + /* * First handle some events internally before calling python if it * want to do some special handling on the event triggered. From 7aa39674b1156809637bbaccf895f2c44cf7d4b4 Mon Sep 17 00:00:00 2001 From: Andreas Rogge Date: Fri, 31 Dec 2021 12:30:41 +0100 Subject: [PATCH 2/3] systemtests: py[23]plug-dir threading issue This patch lets the test run multiple jobs in parallel and checks that each plugin callback sees the correct plugin context. --- .../bareos-dir.d/client/bareos-fd.conf.in | 1 + .../bareos-dir.d/job/backup-bareos-fd.conf.in | 23 ++++++++++++++++--- .../bareos/bareos-dir.d/storage/File.conf.in | 1 + .../bareos-sd.d/device/FileStorage.conf | 10 +++++++- .../python-modules/BareosDirTest.py | 19 +++++++++++---- systemtests/tests/py2plug-dir/testrunner | 18 +++++++++++---- 6 files changed, 59 insertions(+), 13 deletions(-) diff --git a/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/client/bareos-fd.conf.in b/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/client/bareos-fd.conf.in index 59b61700966..086402a1c1b 100644 --- a/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/client/bareos-fd.conf.in +++ b/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/client/bareos-fd.conf.in @@ -4,4 +4,5 @@ Client { Address = @hostname@ Password = "@fd_password@" # password for FileDaemon FD PORT = @fd_port@ + Maximum Concurrent Jobs = 10 } diff --git a/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/job/backup-bareos-fd.conf.in b/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/job/backup-bareos-fd.conf.in index b77a57e34a4..406e5fb9b7d 100644 --- a/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/job/backup-bareos-fd.conf.in +++ b/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/job/backup-bareos-fd.conf.in @@ -1,6 +1,23 @@ Job { - Name = "backup-bareos-fd" - DIR Plugin Options ="@python_module_name@:module_path=@python_plugin_module_src_test_dir@:module_name=bareos-dir-test:output=@tmp@/test-plugin.log" + Name = "backup-bareos-fd1" + DIR Plugin Options ="@python_module_name@:module_path=@python_plugin_module_src_test_dir@:module_name=bareos-dir-test:output=@tmp@/test-plugin1.log" + JobDefs = "DefaultJob" +} + +Job { + Name = "backup-bareos-fd2" + DIR Plugin Options ="@python_module_name@:module_path=@python_plugin_module_src_test_dir@:module_name=bareos-dir-test:output=@tmp@/test-plugin2.log" + JobDefs = "DefaultJob" +} + +Job { + Name = "backup-bareos-fd3" + DIR Plugin Options ="@python_module_name@:module_path=@python_plugin_module_src_test_dir@:module_name=bareos-dir-test:output=@tmp@/test-plugin3.log" + JobDefs = "DefaultJob" +} + +Job { + Name = "backup-bareos-fd4" + DIR Plugin Options ="@python_module_name@:module_path=@python_plugin_module_src_test_dir@:module_name=bareos-dir-test:output=@tmp@/test-plugin4.log" JobDefs = "DefaultJob" - Client = "bareos-fd" } diff --git a/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/storage/File.conf.in b/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/storage/File.conf.in index a2622f719e2..ef4b3495440 100644 --- a/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/storage/File.conf.in +++ b/systemtests/tests/py2plug-dir/etc/bareos/bareos-dir.d/storage/File.conf.in @@ -5,4 +5,5 @@ Storage { Device = FileStorage Media Type = File SD Port = @sd_port@ + Maximum Concurrent Jobs = 10 } diff --git a/systemtests/tests/py2plug-dir/etc/bareos/bareos-sd.d/device/FileStorage.conf b/systemtests/tests/py2plug-dir/etc/bareos/bareos-sd.d/device/FileStorage.conf index 11a639bc688..87cac481d4a 100644 --- a/systemtests/tests/py2plug-dir/etc/bareos/bareos-sd.d/device/FileStorage.conf +++ b/systemtests/tests/py2plug-dir/etc/bareos/bareos-sd.d/device/FileStorage.conf @@ -1,5 +1,12 @@ -Device { +Autochanger { Name = FileStorage + Device = FileStorageDev + Changer Command = "" + Changer Device = /dev/null +} + +Device { + Name = FileStorageDev Media Type = File Archive Device = storage LabelMedia = yes; # lets Bareos label unlabeled media @@ -8,4 +15,5 @@ Device { RemovableMedia = no; AlwaysOpen = no; Description = "File device. A connecting Director must have the same Name and MediaType." + Count = 10 } diff --git a/systemtests/tests/py2plug-dir/python-modules/BareosDirTest.py b/systemtests/tests/py2plug-dir/python-modules/BareosDirTest.py index 9ff3a4e1e63..9d439f03d74 100644 --- a/systemtests/tests/py2plug-dir/python-modules/BareosDirTest.py +++ b/systemtests/tests/py2plug-dir/python-modules/BareosDirTest.py @@ -22,6 +22,7 @@ import bareosdir import BareosDirPluginBaseclass +from time import time from sys import version_info @@ -52,21 +53,31 @@ def parse_plugin_definition(self, plugindef): def handle_plugin_event(self, event): super(BareosDirTest, self).handle_plugin_event(event) + job_name = repr(bareosdir.GetValue(bareosdir.bDirVarJobName)) + job_id = repr(bareosdir.GetValue(bareosdir.bDirVarJobId)) + microtime = round(time() * 1000) + msg_f = "%s Job:" + job_name + " JobId: " + job_id + " Time: " + repr(microtime) + "\n" + if event == bareosdir.bDirEventJobStart: - self.toFile("bDirEventJobStart\n") + self.toFile(msg_f % "bDirEventJobStart") elif event == bareosdir.bDirEventJobEnd: - self.toFile("bDirEventJobEnd\n") + self.toFile(msg_f % "bDirEventJobEnd") elif event == bareosdir.bDirEventJobInit: - self.toFile("bDirEventJobInit\n") + self.toFile(msg_f % "bDirEventJobInit") elif event == bareosdir.bDirEventJobRun: - self.toFile("bDirEventJobRun\n") + self.toFile(msg_f % "bDirEventJobRun") return bareosdir.bRC_OK def toFile(self, text): + bareosdir.DebugMessage( + 100, + "Writing string '%s' to '%s'\n" + % (text, self.outputfile), + ) doc = open(self.outputfile, "a") doc.write(text) doc.close() diff --git a/systemtests/tests/py2plug-dir/testrunner b/systemtests/tests/py2plug-dir/testrunner index 77800673df9..3bd48571979 100755 --- a/systemtests/tests/py2plug-dir/testrunner +++ b/systemtests/tests/py2plug-dir/testrunner @@ -16,11 +16,9 @@ set -u TestName="$(basename "$(pwd)")" export TestName -JobName=backup-bareos-fd #shellcheck source=../environment.in . ./environment -JobName=backup-bareos-fd #shellcheck source=../scripts/functions . "${rscripts}"/functions "${rscripts}"/cleanup @@ -39,9 +37,12 @@ cat <$tmp/bconcmds @$out /dev/null messages @$out $tmp/log1.out -setdebug level=200 dir +setdebug level=200 trace=1 dir label volume=TestVolume001 storage=File pool=Full -run job=$JobName yes +run job=backup-bareos-fd1 yes +run job=backup-bareos-fd2 yes +run job=backup-bareos-fd3 yes +run job=backup-bareos-fd4 yes status director status client status storage=File @@ -55,9 +56,16 @@ check_for_zombie_jobs storage=File stop_bareos for i in bDirEventJobStart bDirEventJobInit bDirEventJobRun bDirEventJobEnd; do - if ! grep -q "$i" ${tmp}/test-plugin.log; then + if ! grep -q "$i" ${tmp}/test-plugin1.log; then set_error "Failed to find logged event $i" fi done +for i in 1 2 3 4; do + num="$(grep -c -F "Job:'backup-bareos-fd$i." "${tmp}/test-plugin$i.log")" + if [ $num -ne 4 ]; then + set_error "Mismatched job context on plugin event in backup-bareos-fd$i" + fi +done + end_test From c3557d0704b6128112fe91c41443b0c9a96d1900 Mon Sep 17 00:00:00 2001 From: Andreas Rogge Date: Tue, 8 Feb 2022 18:03:27 +0100 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b85ce6e40d..a63e0148ef3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and since Bareos version 20 this project adheres to [Semantic Versioning](https: - sql_get.cc: fix error logging in GetJobRecord() for jobname #1042 - webui: fix empty job timeline issue if date.timezone is not set in php.ini [PR #1051] - Fix for wrong update message when updating all volumes from all pools with no existing volumes [PR #1015] +- Fix context confusion in Director's Python plugins [PR #1047] ### Added - Python plugins: add default module_path to search path [PR #1038] @@ -58,6 +59,7 @@ and since Bareos version 20 this project adheres to [Semantic Versioning](https: [PR #1041]: https://github.com/bareos/bareos/pull/1041 [PR #1043]: https://github.com/bareos/bareos/pull/1043 [PR #1046]: https://github.com/bareos/bareos/pull/1046 +[PR #1047]: https://github.com/bareos/bareos/pull/1047 [PR #1048]: https://github.com/bareos/bareos/pull/1048 [PR #1050]: https://github.com/bareos/bareos/pull/1050 [PR #1051]: https://github.com/bareos/bareos/pull/1051