Thread: custom postgres launcher for tests
This patch allow to use custom postgres launcher for tests (tap®ress)
by setting environment variable PGLAUNCHER.
Other known methods (like: https://wiki.postgresql.org/wiki/Valgrind) requires
to perform installation, build system modifications, executable replacement etc...
And proposed way is simpler and more flexible.
** Use-case: run checks under Valgrind
- prepare launcher
echo 'exec valgrind postgres "$@"' > /tmp/pgvalgrind
chmod +x /tmp/pgvalgrind
- execute regress tests under Valgrind
PGLAUNCHER=/tmp/pgvalgrind TESTS=gin make check-tests
- execute concrete tap-test under Valgrind
PGLAUNCHER=/tmp/pgvalgrind PROVE_TESTS=t/001_stream_rep.pl make \
check -C src/test/recovery
** Use-case: execute tests with different postgres versions
- prepare multi-launcher
cat <<EOF > /tmp/launcher
cp -f `pwd`/src/backend/postgres.v* \`pg_config --bindir\`
exec postgres.v\$V "\$@"
EOF
chmod +x /tmp/launcher
- make some versions of postgres binary
./configure "CFLAGS=..." && make
mv src/backend/postgres src/backend/postgres.v1
./configure "CFLAGS=..." && make
mv src/backend/postgres src/backend/postgres.v2
- run checks with different postgres binaries
PGLAUNCHER=/tmp/launcher V=1 make check -C contrib/bloom
PGLAUNCHER=/tmp/launcher V=2 make check -C contrib/bloom
by setting environment variable PGLAUNCHER.
Other known methods (like: https://wiki.postgresql.org/wiki/Valgrind) requires
to perform installation, build system modifications, executable replacement etc...
And proposed way is simpler and more flexible.
** Use-case: run checks under Valgrind
- prepare launcher
echo 'exec valgrind postgres "$@"' > /tmp/pgvalgrind
chmod +x /tmp/pgvalgrind
- execute regress tests under Valgrind
PGLAUNCHER=/tmp/pgvalgrind TESTS=gin make check-tests
- execute concrete tap-test under Valgrind
PGLAUNCHER=/tmp/pgvalgrind PROVE_TESTS=t/001_stream_rep.pl make \
check -C src/test/recovery
** Use-case: execute tests with different postgres versions
- prepare multi-launcher
cat <<EOF > /tmp/launcher
cp -f `pwd`/src/backend/postgres.v* \`pg_config --bindir\`
exec postgres.v\$V "\$@"
EOF
chmod +x /tmp/launcher
- make some versions of postgres binary
./configure "CFLAGS=..." && make
mv src/backend/postgres src/backend/postgres.v1
./configure "CFLAGS=..." && make
mv src/backend/postgres src/backend/postgres.v2
- run checks with different postgres binaries
PGLAUNCHER=/tmp/launcher V=1 make check -C contrib/bloom
PGLAUNCHER=/tmp/launcher V=2 make check -C contrib/bloom
Attachment
On Tue, 11 Feb 2020 at 19:33, Ivan Taranov <i.taranov@postgrespro.ru> wrote: > > This patch allow to use custom postgres launcher for tests (tap®ress) > by setting environment variable PGLAUNCHER. I thought I saw a related patch to this that proposed to add a pg_ctl argument. Was that you too? I can't find it at the moment. In any case I *very definitely* want the ability to wrap the 'postgres' executable we launch via some wrapper command. I currently do this with a horrid shellscript hack that uses next-on-path lookups and a wrapper 'postgres' executable. But because of how find_other_exec works this is rather far from optimal so I'm a fan of a cleaner, built-in alternative. I was initially going to object to using an env-var. But if someone controls your environment they can already LD_PRELOAD you or PATH-hack into doing whatever they want, so it's not a security concern. And you're right that getting a pg_ctl command line or the like into all the places where we'd want it deep inside TAP tests etc is just too much hassle otherwise. I haven't reviewed the code yet, but for the idea a strong +1000 or so. -- Craig Ringer http://www.2ndQuadrant.com/ 2ndQuadrant - PostgreSQL Solutions for the Enterprise
On Fri, Feb 21, 2020 at 4:49 AM Craig Ringer <craig@2ndquadrant.com> wrote: > I thought I saw a related patch to this that proposed to add a pg_ctl > argument. Was that you too? I can't find it at the moment. This very simple two-line patch for src/test/perl/PostgresNode.pm code, it set `pg_ctl -p <path>` argument, and one-line patch for src/test/regress/pg_regress.c it spawn postgres-launcher directly. This routine usable only for tap tests with used PostgresNode::get_new_node/start/restart, and for regress tests. Perhaps the name TEST_PGLAUNCHER is more correct for this env-var. >into doing whatever they want, so it's not a security concern >I currently do this with a horrid shellscript hack that uses next-on-path >lookups and a wrapper 'postgres' executable Its not security problem, because this kit only for developer, commonly - for iteratively build and run concrete tests. For more complexy replacing need patch for pg_ctl, or postgres wrapper, or replacing postgres bin and other ways... Thanks for the response!
On Fri, 21 Feb 2020 at 17:05, Ivan N. Taranov <i.taranov@postgrespro.ru> wrote:
>
> On Fri, Feb 21, 2020 at 4:49 AM Craig Ringer <craig@2ndquadrant.com> wrote:
>
> > I thought I saw a related patch to this that proposed to add a pg_ctl
> > argument. Was that you too? I can't find it at the moment.
I've had it on my TODO forever but I don't think it was me who posted
a patch. I honestly can't even remember. Too much going on at once.
> This routine usable only for tap tests with used
> PostgresNode::get_new_node/start/restart, and for regress tests.
>
> Perhaps the name TEST_PGLAUNCHER is more correct for this env-var.
>
> >into doing whatever they want, so it's not a security concern
>
> >I currently do this with a horrid shellscript hack that uses next-on-path
> >lookups and a wrapper 'postgres' executable
>
> Its not security problem, because this kit only for developer, commonly - for
> iteratively build and run concrete tests.
>
> For more complexy replacing need patch for pg_ctl, or postgres wrapper, or
> replacing postgres bin and other ways...
If we support a wrapper we should support it for all pg_ctl usage IMO.
Even if it's intended just for testing. Because the scope of "testing"
extends very far outside "in-core TAP and pg_regress tests". Testing
needs include extensions running their own tests under valgrind or
similar tools, tests simulating clustered environments using ansible
or other automation tools, and more.
So I'd rather stick with your original PGLAUNCHER proposal. I think
all the tools we care about already invoke postgres via pg_ctl, and
any that don't should probably be taught to.
(I wish pg_ctl had a --no-daemon, --foreground or --no-detach mode
though, to help with this.)
For the sake of others with similar needs I attach my current
wrapper/launcher script. To use it you have to:
mkdir pglauncher
cd pglauncher
cp $the_script postgres
chmod a+x postgres
ln -s postgres initdb
ln -s postgres pg_ctl
cd ..
Then ensure the bin directory for your target postgres is first on the
PATH and run with something like:
POSTGRES_SRC=/path/to/your/srcdir
PATH=${PWD}/pglauncher/:$PATH \
VG_LOG="$(mktemp -p . -d valgrind-log-XXXXXX)/%n-%p.log"
VG_ARGS="--verbose --leak-check=full
--show-leak-kinds=definite,possible --track-origins=yes
--read-var-info=yes --show-error-list=yes --num-callers=30
--malloc-fill=8f --free-fill=9f
--suppressions=$POSTGRES_SRC/src/tools/valgrind.supp
--gen-suppressions=all" \
pg_ctl blah blah
(BTW, we should install "valgrind.supp" when we install the optional
bits and pieces like the src/test/perl modules, pg_regress, and so on,
so it's available for extensions that run their own valgrind scans.)
--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise
Attachment
> If we support a wrapper we should support it for all pg_ctl usage IMO. As i understand it - you propose to patch pg_ctl.c & regress.c instead of PostgresNode.pm & regress.c? This is a deeper invasion to pg_ctl. There will be a conflict between the environment variable and the pg_ctl -p parameter, and possibly induced bugs. I suggest microscopic changes for iterative recompilation/test/debug without installation. > For the sake of others with similar needs I attach my current > wrapper/launcher script. To use it you have to: IMHO, the method what you proposed (wrapper/launcher) - is more suitable for complex testing. I agree that the my proposed way is incomplete.
> If we support a wrapper we should support it for all pg_ctl usage IMO. As i understand it - you propose to patch pg_ctl.c & regress.c instead of PostgresNode.pm & regress.c? This is a deeper invasion to pg_ctl. There will be a conflict between the environment variable and the pg_ctl -p parameter, and possibly induced bugs. I suggest microscopic changes for iterative recompilation/test/debug without installation. > For the sake of others with similar needs I attach my current > wrapper/launcher script. To use it you have to: IMHO, the method what you proposed (wrapper/launcher) - is more suitable for complex testing. I agree that the my proposed way is incomplete.