pgsql: pg_upgrade: Check version of target cluster binaries

Started by Peter Eisentrautover 5 years ago7 messagescomitters
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

pg_upgrade: Check version of target cluster binaries

This expands the binary validation in pg_upgrade with a version
check per binary to ensure that the target cluster installation
only contains binaries from the target version.

In order to reduce duplication, validate_exec is exported from
port.h and the local copy in pg_upgrade is removed.

Author: Daniel Gustafsson <daniel@yesql.se>
Discussion: /messages/by-id/9328.1552952117@sss.pgh.pa.us

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/f06b1c598254f8adb2b7f51d6a7685618a7fb121

Modified Files
--------------
src/bin/pg_upgrade/exec.c | 88 +++++++++++++++++++----------------------------
src/common/exec.c | 3 +-
src/include/port.h | 1 +
3 files changed, 38 insertions(+), 54 deletions(-)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: pgsql: pg_upgrade: Check version of target cluster binaries

Peter Eisentraut <peter@eisentraut.org> writes:

pg_upgrade: Check version of target cluster binaries

Crake says this patch broke its cross-version upgrade tests:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&amp;dt=2021-03-03%2009%3A02%3A31

where the important bit is

check for "/home/andrew/bf/root/upgrade.crake/REL9_2_STABLE/inst/bin/postgres" failed: incorrect version: found "postgres (PostgreSQL) 9.2.24", expected "postgres (PostgreSQL) 14devel"

It's not at all clear from here whether that's a bug in the patch
or in the buildfarm's test harness.

regards, tom lane

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#2)
Re: pgsql: pg_upgrade: Check version of target cluster binaries

On 3/3/21 3:28 PM, Tom Lane wrote:

Peter Eisentraut <peter@eisentraut.org> writes:

pg_upgrade: Check version of target cluster binaries

Crake says this patch broke its cross-version upgrade tests:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&amp;dt=2021-03-03%2009%3A02%3A31

where the important bit is

check for "/home/andrew/bf/root/upgrade.crake/REL9_2_STABLE/inst/bin/postgres" failed: incorrect version: found "postgres (PostgreSQL) 9.2.24", expected "postgres (PostgreSQL) 14devel"

It's not at all clear from here whether that's a bug in the patch
or in the buildfarm's test harness.

This has worked for years ... here's the latest code:
<https://github.com/PGBuildFarm/client-code/blob/master/PGBuild/Modules/TestUpgradeXversion.pm&gt;
The invocation of pg_upgrade is at line 560.

The log says:

check for
"/home/andrew/bf/root/upgrade.crake/REL9_2_STABLE/inst/bin/postgres"
failed: incorrect version: found "postgres (PostgreSQL) 9.2.24",
expected "postgres (PostgreSQL) 14devel"

But that makes no sense at all. Looks like we're confusing the source and the target.

cheers

andrew

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#3)
Re: pgsql: pg_upgrade: Check version of target cluster binaries

Andrew Dunstan <andrew@dunslane.net> writes:

On 3/3/21 3:28 PM, Tom Lane wrote:

Crake says this patch broke its cross-version upgrade tests:

The log says:
check for
"/home/andrew/bf/root/upgrade.crake/REL9_2_STABLE/inst/bin/postgres"
failed: incorrect version: found "postgres (PostgreSQL) 9.2.24",
expected "postgres (PostgreSQL) 14devel"
But that makes no sense at all. Looks like we're confusing the source and the target.

On looking closer, I think the patch is just several bricks shy of a
load. It's applying validate_exec (which insists on a match to its
own version number) to *both* the source and target binaries. It
must not check the source that way.

regards, tom lane

#5Daniel Gustafsson
daniel@yesql.se
In reply to: Tom Lane (#4)
Re: pgsql: pg_upgrade: Check version of target cluster binaries

On 3 Mar 2021, at 23:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 3/3/21 3:28 PM, Tom Lane wrote:

Crake says this patch broke its cross-version upgrade tests:

The log says:
check for
"/home/andrew/bf/root/upgrade.crake/REL9_2_STABLE/inst/bin/postgres"
failed: incorrect version: found "postgres (PostgreSQL) 9.2.24",
expected "postgres (PostgreSQL) 14devel"
But that makes no sense at all. Looks like we're confusing the source and the target.

On looking closer, I think the patch is just several bricks shy of a
load. It's applying validate_exec (which insists on a match to its
own version number) to *both* the source and target binaries. It
must not check the source that way.

It's much to late to focus here at the moment, I will take a look in the
morning unless beaten to it.

--
Daniel Gustafsson https://vmware.com/

#6Peter Eisentraut
peter_e@gmx.net
In reply to: Daniel Gustafsson (#5)
Re: pgsql: pg_upgrade: Check version of target cluster binaries

On 04.03.21 01:06, Daniel Gustafsson wrote:

On 3 Mar 2021, at 23:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 3/3/21 3:28 PM, Tom Lane wrote:

Crake says this patch broke its cross-version upgrade tests:

The log says:
check for
"/home/andrew/bf/root/upgrade.crake/REL9_2_STABLE/inst/bin/postgres"
failed: incorrect version: found "postgres (PostgreSQL) 9.2.24",
expected "postgres (PostgreSQL) 14devel"
But that makes no sense at all. Looks like we're confusing the source and the target.

On looking closer, I think the patch is just several bricks shy of a
load. It's applying validate_exec (which insists on a match to its
own version number) to *both* the source and target binaries. It
must not check the source that way.

It's much to late to focus here at the moment, I will take a look in the
morning unless beaten to it.

I think the attached would be a sensible fix.

Attachments:

0001-pg_upgrade-Fix-oversight-in-version-checking.patchtext/plain; charset=UTF-8; name=0001-pg_upgrade-Fix-oversight-in-version-checking.patch; x-mac-creator=0; x-mac-type=0Download+29-23
#7Daniel Gustafsson
daniel@yesql.se
In reply to: Peter Eisentraut (#6)
Re: pgsql: pg_upgrade: Check version of target cluster binaries

On 4 Mar 2021, at 08:24, Peter Eisentraut <peter@eisentraut.org> wrote:

I think the attached would be a sensible fix.

That's pretty much what I ended up with as well.

I was first looking at checking the version of the binary against the
major_version_str in ClusterInfo by passing down cluster instead, but that
turns into chicken-and-egg since we don't set the extracted version until we've
checked that pg_ctl exists and works.

Thanks!

--
Daniel Gustafsson https://vmware.com/