multi-install PostgresNode

I've been giving some thought to $subject. The initial impetus is the
promise I made to assist with testing of clients built with NSS against
servers built with openssl, and vice versa.

I've already generalized the process of saving binaries by the buildfarm
client. And we could proceed with a purely bespoke module for testing
the SSL components, as we already do for testing cross-version
pg_upgrade. But it struck me that it might be better to leverage our
existing investment in TAP tests. So I came up with the idea of creating
a child module of PostgresNode.pm, which would set the PATH and other
variables appropriately at the start of each method and restore them on
method exit. So then we could have things like:

    $openssl_node->start;
    my $connstr = $openssl_node->connstr;
    $nss_node->psql($connstr, ...);
      

To use this a TAP test would need to know two (or more) install paths
for the various nodes, presumably set in environment variables much as
we do now for things like TESTDIR. So for the above example, the TAP
test could set things up with:

    my
    $openssl_node=PostgresNodePath::get_new_node($ENV{OPENSSL_POSTGRES_INSTALL_PATH},'openssl');
    my
    $nss_node=PostgresNodePath::get_new_node($ENV{NSS_POSTGRES_INSTALL_PATH},'nss');

Other possible uses would be things like cross-version testing of
pg_dump (How do we know we haven't broken anything in dumping very old
versions?).

The proposed module would look something like this:

    package PostgresNodePath;

    use strict;
    use warnings;

    use parent PostgresNode;

    use Exporter qw(import);
    our @EXPORT = qw(get_new_node);

    sub get_new_node
    {
        my $installpath= shift;
        my $node = PostgresNode::get_new_node(@_);
        bless $node; # re-bless into current class
        $node->{_installpath} = $installpath;
        return $node;
    }

and then  for each class method in PostgresNode.pm we'd have an override
something like:

    sub foo
    {
        my $node=shift;
        my $inst = $node->{_installpath};
        local %ENV = %ENV;
        $ENV{PATH} = "$inst/bin:$ENV{PATH}";
        $ENV{LD_LIBRARY_PATH} = "$inst/lib:$ENV{LD_LIBRARY_PATH}";
        $node->SUPER::foo(@_);
    }

There might be more elegant ways of doing this, but that's what I came
up with.

My main question is: do we want something like this in the core code
(presumably in src/test/perl), or is it not of sufficiently general
interest? If it's wanted I'll submit a patch, probably for the March CF,
but January if I manage to get my running shoes on. If not, I'll put it
in the buildfarm code, but then any TAP tests that want it will likewise
need to live there.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On Thu, Dec 17, 2020 at 04:37:54PM -0500, Andrew Dunstan wrote: Passing down the installpath as argument and saving it within a
PostgresNode or child class looks like the correct way of doing things
to me.  This would require an extra routine to be able to get the
install path from a node as _installpath would remain internal to the
module file, right?  Shouldn't it be something that ought to be
directly part of PostgresNode actually, where we could enforce the lib
and bin paths to the output of pg_config if an _installpath is not
provided by the caller?  In short, I am not sure that we need an extra
module here.
As long as it does not become necessary to pass down _installpath to
all indidivual binary calls we have in PostgresNode or the extra
module, this gets a +1 from me.  So, if I am getting that right, the
key point is the use of local %ENV here to make sure that PATH and
LD_LIBRARY_PATH are only enforced when it comes to calls within
PostgresNode.pm, right?  That's an elegant solution.  This is
something I have wanted for a long time for pg_upgrade to be able to
get rid of its test.sh.

> My main question is: do we want something like this in the core code
> (presumably in src/test/perl), or is it not of sufficiently general
> interest? If it's wanted I'll submit a patch, probably for the March CF,
> but January if I manage to get my running shoes on. If not, I'll put it
> in the buildfarm code, but then any TAP tests that want it will likewise
> need to live there.

This facility gives us the possibility to clean up the test code of
pg_upgrade and move it to a TAP test, so I'd say that it is worth
having in the core code in the long-term.
--
Michael

On 12/17/20 7:55 PM, Michael Paquier wrote:
This turns out to be remarkably short, with the use of a little eval magic.

Give the attached, this test program works just fine:

    #!/bin/perl
    use PostgresNodePath;
    $ENV{PG_REGRESS} =
    '/home/andrew/pgl/vpath.12/src/test/regress/pg_regress';
    my $node = get_new_node('/home/andrew/pgl/inst.12.5711','blurfl');
    print $node->info;
    print $node->connstr(),"\n";
    $node->init();

cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On 12/19/20 11:19 AM, Andrew Dunstan wrote:
This time with a typo removed.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On 2020-12-20 18:09, Andrew Dunstan wrote:
What is proposed the destination of this file?  Is it meant to be part
of a patch?  Is it meant to be installed?  Is it meant for the buildfarm
code?

> On 17 Dec 2020, at 22:37, Andrew Dunstan <[hidden email]> wrote:

> I've been giving some thought to $subject. The initial impetus is the
> promise I made to assist with testing of clients built with NSS against
> servers built with openssl, and vice versa.

Thanks for tackling!

> My main question is: do we want something like this in the core code
> (presumably in src/test/perl), or is it not of sufficiently general
> interest?

To be able to implement pg_upgrade tests as TAP tests seems like enough of a
win to consider this for inclusion in core.

cheers ./daniel

On 1/11/21 9:34 AM, Peter Eisentraut wrote:

Core code, ideally. I will submit a patch.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com