Skip to content

Add PostgreSQL role switching#1178

Merged
joergsteffens merged 2 commits intobareos:masterfrom
tuxmaster5000:pgsql_role
Sep 8, 2022
Merged

Add PostgreSQL role switching#1178
joergsteffens merged 2 commits intobareos:masterfrom
tuxmaster5000:pgsql_role

Conversation

@tuxmaster5000
Copy link
Contributor

@tuxmaster5000 tuxmaster5000 commented May 11, 2022

Add role switching for PostgreSQL.
See https://bugs.bareos.org/view.php?id=1456 for more.

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Separate commit for this PR in the CHANGELOG.md, PR number referenced is same
  • Commit descriptions are understandable and well formatted
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
  • bareos-check-sources --since-merge does not report any problems
  • git status should not report modifications in the source tree after building and testing
Tests
  • Decision taken that a test is required (if not, then remove this paragraph)
  • The choice of the type of test (unit test or systemtest) is reasonable
  • Testname matches exactly what is being tested
  • On a fail, output of the test leads quickly to the origin of the fault

@bruno-at-bareos bruno-at-bareos self-requested a review May 12, 2022 09:31
@bruno-at-bareos bruno-at-bareos self-assigned this May 12, 2022
@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented May 12, 2022

Thanks for the PR. Actually testing the build and systemtests in container (non rootless) make the test failed

12-May 12:42 py3plug-fd-postgres-fd JobId 1: ACL support is enabled
12-May 12:42 py3plug-fd-postgres-fd JobId 1: Fatal error: python3-fd-mod: Could not connect to database postgres, user root, host: /bareos/git/build/systemtests/tests/py3plug-fd-postgres/tmp/.s.PGSQL.5432: 'role'
12-May 12:42 py3plug-fd-postgres-fd JobId 1: Fatal error: filed/fd_plugins.cc:681 PluginSave: Command plugin "python3:module_path=/bareos/git/build/systemtests/tests/py3plug-fd-postgres/python-modules:module_name=bareos-fd-postgres:dbHost=/bareos/git/build/systemtests/tests/py3plug-fd-postgres/tmp:postgresDataDir=/bareos/git/build/systemtests/tests/py3plug-fd-postgres/database/data/:walArchive=/bareos/git/build/systemtests/tests/py3plug-fd-postgres/database/wal_archive/" requested, but job is already cancelled.

@tuxmaster5000
Copy link
Contributor Author

What is written in the log of the SQL server?

@bruno-at-bareos
Copy link
Contributor

What is written in the log of the SQL server?

I've increase the log level of postgresql in test

2022-05-16 11:43:17.435 UTC [unknown] [unknown] [8329]LOG:  00000: connection received: host=[local]
2022-05-16 11:43:17.435 UTC [unknown] [unknown] [8329]LOCATION:  BackendInitialize, postmaster.c:4413
2022-05-16 11:43:17.436 UTC postgres root [8329]LOG:  00000: connection authorized: user=root database=postgres
2022-05-16 11:43:17.436 UTC postgres root [8329]LOCATION:  PerformAuthentication, postinit.c:292
2022-05-16 11:43:17.553 UTC postgres root [8329]LOG:  00000: disconnection: session time: 0:00:00.117 user=root database=postgres host=[local]
2022-05-16 11:43:17.553 UTC postgres root [8329]LOCATION:  log_disconnections, postgres.c:4767

While during a valid communication

2022-05-16 11:43:15.027 UTC [unknown] [unknown] [8263]LOG:  00000: connection received: host=[local]
2022-05-16 11:43:15.027 UTC [unknown] [unknown] [8263]LOCATION:  BackendInitialize, postmaster.c:4413
2022-05-16 11:43:15.028 UTC backuptest root [8263]LOG:  00000: connection authorized: user=root database=backuptest application_name=psql
2022-05-16 11:43:15.028 UTC backuptest root [8263]LOCATION:  PerformAuthentication, postinit.c:292
2022-05-16 11:43:15.030 UTC backuptest root [8263]LOG:  00000: statement: SHOW server_version;
2022-05-16 11:43:15.030 UTC backuptest root [8263]LOCATION:  exec_simple_query, postgres.c:1044
2022-05-16 11:43:15.031 UTC backuptest root [8263]LOG:  00000: disconnection: session time: 0:00:00.003 user=root database=backuptest host=[local]
2022-05-16 11:43:15.031 UTC backuptest root [8263]LOCATION:  log_disconnections, postgres.c:4767

I'm attaching also the trace file of fd during that first backup
py3plug-fd-postgres-fd.trace.txt

diff --git i/systemtests/tests/py2plug-fd-postgres/database/setup_local_db.sh.in w/systemtests/tests/py2plug-fd-postgres/database/setup_local_db.sh.in
index 49f196e03..d95b0f398 100755
--- i/systemtests/tests/py2plug-fd-postgres/database/setup_local_db.sh.in
+++ w/systemtests/tests/py2plug-fd-postgres/database/setup_local_db.sh.in
@@ -55,6 +55,13 @@ local_db_prepare_files() {
     echo "archive_mode = on"
     echo "archive_command = 'cp %p ../wal_archive'"
     echo "max_wal_senders = 10"
+    echo "log_connections = on"
+    echo "log_disconnections = on"
+    echo "log_min_messages = info"
+    echo "log_min_error_statement = info"
+    echo "log_error_verbosity = verbose"
+    echo "log_statement = 'all'"
+    echo "log_temp_files = 0"
   } >> data/postgresql.conf
 }

diff --git i/systemtests/tests/py2plug-fd-postgres/testrunner w/systemtests/tests/py2plug-fd-postgres/testrunner
index ab0651501..ad64113cf 100755
--- i/systemtests/tests/py2plug-fd-postgres/testrunner
+++ w/systemtests/tests/py2plug-fd-postgres/testrunner
@@ -50,7 +50,7 @@ cat <<END_OF_DATA >$tmp/bconcmds
 @$out /dev/null
 messages
 @$out $tmp/log1.out
-setdebug level=100 storage=File
+setdebug level=200 trace=1 all
 label volume=TestVolume001 storage=File pool=Full
 run job=$JobName yes
 status director
@@ -63,6 +63,8 @@ END_OF_DATA

 run_bareos "$@"

+exit
+
 # Now add data to the database and run an incremental job
 echo "INSERT INTO t (text, created_on) values ('test for INCR backup', current_timestamp)" | ${PSQL} ${DBNAME}

@tuxmaster5000
Copy link
Contributor Author

Very strange, in this log's I don't see any errors.

@bruno-at-bareos
Copy link
Contributor

Actually this PR can't be build, nor can't be tested
system:py3plug-fd-postgres doesn't work.

@tuxmaster5000
Copy link
Contributor Author

Without the full log's(database server, jenkins,test etc.) I can't do anything because on my systems, it works.

@bruno-at-bareos
Copy link
Contributor

Without the full log's(database server, jenkins,test etc.) I can't do anything because on my systems, it works.

You have a make test working ? here not.

@tuxmaster5000
Copy link
Contributor Author

I test it in the live scenario, so I need the all log files to debug your test environment.

Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,

We would like to find the root cause why introducing your code broke the system:py3plug-fd-postgres plugin test present in master.
For this I've pushed also on this branch some increased debug level to the test.

See documentation https://docs.bareos.org/DeveloperGuide/BuildAndTestBareos/systemtests.html#systemtests how to run them.
Of course to be able to merge we need the test to be green like it was before.

To avoid running all the tests but just the interesting one, you can use the following
ctest -vvvv --output-on-failure -R fd-postgres

in your build/systemtests/tests/py3plug-fd-postgres/working subdir you will find the trace from bareos-fd
and postgresql log file is located in database/log/logfile

Maybe you will find why your proposed code is creating a failure in the plugin test.

@tuxmaster5000
Copy link
Contributor Author

I have try this, but no tests will start:
ctest -vvvv --output-on-failure -R fd-postgres
will only log this:
PRETEST: checking configured hostname (foo.bar) ... OK
Start 82: system:py2plug-fd-postgres
1/2 Test #82: system:py2plug-fd-postgres .......***Not Run (Disabled) 0.00 sec
Start 83: system:py3plug-fd-postgres
2/2 Test #83: system:py3plug-fd-postgres .......***Not Run (Disabled) 0.00 sec
No tests were found!!!

@bruno-at-bareos
Copy link
Contributor

Hi, you certainly miss the cmake message about needed modules : dateutils and pg8000
if they are present, the test will be set ON.

@tuxmaster5000
Copy link
Contributor Author

Very interesting both nodules are present.
In the console log of the cmake run I only see this:

disabling py2plug-fd-postgres, Python2 not supported for this plugin
-- Test: system:py2plug-fd-postgres => disabled ()
-- python module pyversion 3 pg8000 found
-- python module pyversion 3 dateutil found
-- Test: system:py3plug-fd-postgres => disabled ()

But no reason why py3plug-fd-postgres is disabled.

@bruno-at-bareos
Copy link
Contributor

But no reason why py3plug-fd-postgres is disabled.

Maybe you don't have the python3 devel stack, but just the python2 (sorry, I'm just guessing)
something similar to this list

 python3-appdirs
 python3-base
 python3-devel
 python3-mysqlclient
 python3-ordered-set
 python3-packaging
 python3-pip
 python3-pyparsing
 python3-rpm
 python3-setuptools
 python3-six
 python3-dateutils
 python3-pg8000

Name varying a lot depending on distribution used.

@tuxmaster5000
Copy link
Contributor Author

The python3 devel stuff is installed.
Ah I found it:
systemtests/tests/py2plug-fd-postgres/CMakeLists.txt
Only allow Python up to 3.9, but the test system comes with python 3.10.
So simple set AND (${Python3_VERSION_MINOR} LESS to 11 instant of 10
Now I can try to see if I see any errors on the code.

@bruno-at-bareos
Copy link
Contributor

The python3 devel stuff is installed. Ah I found it: systemtests/tests/py2plug-fd-postgres/CMakeLists.txt Only allow Python up to 3.9, but the test system comes with python 3.10. So simple set AND (${Python3_VERSION_MINOR} LESS to 11 instant of 10 Now I can try to see if I see any errors on the code.

Not sure I understand, it seems we check that major version is 3, but also maybe it will be a good idea to rebase from master anyway.

@tuxmaster5000
Copy link
Contributor Author

As far as I understood the test, it will only creates an test database for the test. But no director is set up.
The logs I only see that an connection to the director was failed. But I can't see, that an director was set up.
Only the test database was created.

@bruno-at-bareos
Copy link
Contributor

The bareos dir, fd, sd is setup during those part of the code
. "${rscripts}"/functions
"${rscripts}"/cleanup
"${rscripts}"/setup

start_test
then
run_bareos "$@"

with your code the test is failing with a word "role" if you checkout master the test is working

@tuxmaster5000
Copy link
Contributor Author

Using the master from today, will result in the same problem.
It looks like the test will not start the director.
The tests starts, hangs for 5 seconds and then it fails.
The only log entry that I can see is in the OS log itself:

Jul 15 10:44:46 bconsole-py3plug-fd-postgres[3108256]: lib/bsock_tcp.cc:128 Unable to connect to Director daemon on <myLocalSystem>:30431. ERR=Connection refused
Jul 15 10:44:46 bareos_fd-py3plug-fd-postgres[3108221]: Shutting down BAREOS service: py3plug-fd-postgres-fd ...
Jul 15 10:44:46 bareos_sd-py3plug-fd-postgres[3108220]: Shutting down BAREOS service: bareos-sd ...
Jul 15 10:44:47 bareos_dir-py3plug-fd-postgres[3108219]: Shutting down BAREOS service: bareos-dir ...

The log dir for the test are empty. Only an log for the database was created:
cat systemtests/tests/py3plug-fd-postgres/database/data/log/postgresql-Fri.log

2022-07-15 10:44:30.690 CEST [3108089] LOG:  database system was shut down at 2022-07-15 10:44:30 CEST
2022-07-15 10:44:30.693 CEST [3108087] LOG:  database system is ready to accept connections

@tuxmaster5000
Copy link
Contributor Author

After an weekend debug session, it looks like as an general database problem.
The director can't start, because the socket path to the pgsql db is not set in the catalog config file.
But when I set it manual, then the option "DB Socket" is complete ignored. The director will always only try "/var/run/postgresql/.s.PGSQL.5432" But correct will be "systemtests/tests/py3plug-fd-postgres/tmp/.s.PGSQL.5432"

@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Jul 18, 2022

Hello, I think you didn't catch the way the tests is run, the regression test use a global server in your environment, bareos_regress or whatever user defined in cmake call is used to connect and create a regress test database used by each bareos test.
Here we have an additional database cluster setup, which will be saved by the plugin, then destroyed and then restored. This cluster will not be used by bareos to store anything in.
Sorry if this wasn't clear enough in our documentation here
https://docs.bareos.org/bareos-21/DeveloperGuide/BuildAndTestBareos/systemtests.html

@tuxmaster5000
Copy link
Contributor Author

Now it is very strange. Using an external db, then the director starts, but the test itself will also use this external db.
But as I understand the test config, it will try to setup an second database. But this will not happens. Or misunderstand I the test?

@tuxmaster5000
Copy link
Contributor Author

Now I can repeat the problem with the test. But as far as work in the live system. I have rewritten it, so that it will work in both scenarios. (test and live system)

@bruno-at-bareos
Copy link
Contributor

Nice addition, I wonder if we can't trick a bit more the test, to not only test without "role" needed but also when it is needed,
like adding some parameter to the local tested PG ?
So code changes occur later, we will not silently loose or broke this functionality, What your thoughts on this ?

@tuxmaster5000
Copy link
Contributor Author

For complete testing, 3 tests are needed. But I have not knowledge of writing cmake tests.
Here the "list" of needed tests

  1. user with backup rights, without setting an role -> must pass
  2. user without backup rights, without setting an role -> must fail
  3. user without backup rights, with role switching -> must pass
    For all tests an clean test data base is needed.

@bruno-at-bareos
Copy link
Contributor

Hi @tuxmaster5000 I completely agree with the 3 cases which would be the nice things to have.
The tests for this are not gtest but systemtest basically all written in bash ;-)

See the testrunner script and also the setup_local_database.sh.

@tuxmaster5000
Copy link
Contributor Author

I seen an testrunner file, but how will it split into multiple test? I have never used the ctest framework before.

@bruno-at-bareos
Copy link
Contributor

Hello @tuxmaster5000 (sorry for the delay, overloaded by other tasks).
I was thinking about hacking the database/setup_local_db.sh.in to create the needed role, then also modify etc/bareos/ configuration to add a test with the plugin and role
I'm not sure if we want to drive the failing test in testrunner on first attempt then upgrade the right on the to be backuped pg cluster, and the retry should work. I guess this will depend of role management.

@tuxmaster5000
Copy link
Contributor Author

As far as I understand the bareos test world I have created the 3 tests. But I don't see the way do build the "no role" test as an "experted fail" test like it can be done under python.

@bruno-at-bareos
Copy link
Contributor

Hello thanks for the work already done, I'm doing some research how to handle this the best way, and will come back next week with some code proposal. Sorry for the time its taken.

@tuxmaster5000
Copy link
Contributor Author

No problem, in the meantime I use my one patched version internal. Thanks for your work.

Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After last fixes, approved as all tests pass.

tuxmaster5000 and others added 2 commits September 8, 2022 09:31
- move py2plug-fd-postgres to py3plug-fd-postgres
- add tests (roles) in py3plug-fd-postgres
- add postgresql configuration to increase log verbosity
- increase debug level of fd
- cleanup extra configuration
- use expect_grep for testing result

Signed-off-by: Bruno Friedmann <bruno.friedmann@bareos.com>
Signed-off-by: Bruno Friedmann <bruno.friedmann@bareos.com>
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final approval after renamed py2 to py3

@joergsteffens joergsteffens merged commit c8aefc3 into bareos:master Sep 8, 2022
@tuxmaster5000 tuxmaster5000 deleted the pgsql_role branch September 17, 2022 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants