Add PG CI to older PG releases

Started by Nazir Bilal Yavuzover 2 years ago7 messageshackers
Jump to latest
#1Nazir Bilal Yavuz
byavuz81@gmail.com

Hi,

PG CI is added starting from PG 15, adding PG CI to PG 14 and below
could be beneficial. So, firstly I tried adding it to the
REL_14_STABLE branch. If that makes sense, I will try to add PG CI to
other old PG releases.

'Add PG CI to PG 14' patch is attached. I merged both CI commits and
the least amount of commits to pass the CI on all OSes.

Addition to CI commits, other changes are:

1_ 76e38b37a5f179d4c9d2865ff31b79130407530b is added for debugging
Windows. Also a couple of SSL tests were failing without this because
the log file is empty. Example failures on 001_ssltests.pl:

# Failed test 'certificate authorization succeeds with DN mapping:
log matches'
# at c:/cirrus/src/test/perl/PostgresNode.pm line 2195.
# ''
# doesn't match '(?^:connection authenticated:
identity="CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG"
method=cert)'

# Failed test 'certificate authorization succeeds with CN mapping:
log matches'
# at c:/cirrus/src/test/perl/PostgresNode.pm line 2195.
# ''
# doesn't match '(?^:connection authenticated:
identity="CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG"
method=cert)'

# Failed test 'certificate authorization fails with client cert
belonging to another user: log matches'
# at c:/cirrus/src/test/perl/PostgresNode.pm line 2243.
# ''
# doesn't match '(?^:connection authenticated:
identity="CN=ssltestuser" method=cert)'
# Looks like you failed 3 tests of 110.

2_ 45f52709d86ceaaf282a440f6311c51fc526340b is added for fixing socket
directories on Windows.

3_ I removed '--with-zstd' since it was not supported yet.

Any kind of feedback would be appreciated.

Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v1-0001-ci-Add-PG-CI-to-PG-14.patchtext/x-diff; charset=US-ASCII; name=v1-0001-ci-Add-PG-CI-to-PG-14.patchDownload+841-4
#2Peter Eisentraut
peter_e@gmx.net
In reply to: Nazir Bilal Yavuz (#1)
Re: Add PG CI to older PG releases

On 10.08.23 16:43, Nazir Bilal Yavuz wrote:

1_ 76e38b37a5f179d4c9d2865ff31b79130407530b is added for debugging
Windows. Also a couple of SSL tests were failing without this because
the log file is empty. Example failures on 001_ssltests.pl:

I see only one attachment, so it's not clear what these commit hashes
refer to.

#3Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Peter Eisentraut (#2)
Re: Add PG CI to older PG releases

Hi,

On Thu, 10 Aug 2023 at 18:05, Peter Eisentraut <peter@eisentraut.org> wrote:

I see only one attachment, so it's not clear what these commit hashes
refer to.

I split the commit into 3 parts.

v2-0001-windows-Only-consider-us-to-be-running-as-service.patch is an
older commit (59751ae47fd43add30350a4258773537e98d4063). A couple of
tests were failing without this because the log file was empty and the
tests were comparing strings with the log files (like in the first
mail).

v2-0002-Make-PG_TEST_USE_UNIX_SOCKETS-work-for-tap-tests-.patch is
slightly modified version of 45f52709d86ceaaf282a440f6311c51fc526340b
to backpatch it to PG 14. Without this, Windows tests were failing
when they tried to create sockets with 'unix_socket_directories' path.

v2-0003-ci-Add-PG-CI-to-PG-14.patch adds files that are needed for CI
to work. I copied these files from the REL_15_STABLE branch. Then I
removed '--with-zstd' from .cirrus.yml since it was not supported yet.

Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v2-0002-Make-PG_TEST_USE_UNIX_SOCKETS-work-for-tap-tests-.patchtext/x-diff; charset=US-ASCII; name=v2-0002-Make-PG_TEST_USE_UNIX_SOCKETS-work-for-tap-tests-.patchDownload+13-2
v2-0003-ci-Add-PG-CI-to-PG-14.patchtext/x-diff; charset=US-ASCII; name=v2-0003-ci-Add-PG-CI-to-PG-14.patchDownload+788-1
v2-0001-windows-Only-consider-us-to-be-running-as-service.patchtext/x-diff; charset=US-ASCII; name=v2-0001-windows-Only-consider-us-to-be-running-as-service.patchDownload+48-4
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nazir Bilal Yavuz (#1)
Re: Add PG CI to older PG releases

Nazir Bilal Yavuz <byavuz81@gmail.com> writes:

PG CI is added starting from PG 15, adding PG CI to PG 14 and below
could be beneficial. So, firstly I tried adding it to the
REL_14_STABLE branch. If that makes sense, I will try to add PG CI to
other old PG releases.

I'm not actually sure this is worth spending time on. We don't
do new development in three-release-back branches, nor do we have
so many spare CI cycles that we can routinely run CI testing in
those branches [1]/messages/by-id/20230808021541.7lbzdefvma7qmn3w@awork3.anarazel.de.

regards, tom lane

[1]: /messages/by-id/20230808021541.7lbzdefvma7qmn3w@awork3.anarazel.de

#5Andres Freund
andres@anarazel.de
In reply to: Nazir Bilal Yavuz (#3)
Re: Add PG CI to older PG releases

Hi,

On 2023-08-10 19:55:15 +0300, Nazir Bilal Yavuz wrote:

v2-0001-windows-Only-consider-us-to-be-running-as-service.patch is an
older commit (59751ae47fd43add30350a4258773537e98d4063). A couple of
tests were failing without this because the log file was empty and the
tests were comparing strings with the log files (like in the first
mail).

Hm. I'm a bit worried about backpatching this one, it's not a small
behavioural change. But the prior behaviour is pretty awful and could very
justifiably be viewed as a bug.

At the very least this would need to be combined with

commit 950e64fa46b164df87b5eb7c6e15213ab9880f87
Author: Andres Freund <andres@anarazel.de>
Date: 2022-07-18 17:06:34 -0700

Use STDOUT/STDERR_FILENO in most of syslogger.

Greetings,

Andres Freund

#6Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#4)
Re: Add PG CI to older PG releases

Hi,

On 2023-08-10 18:09:03 -0400, Tom Lane wrote:

Nazir Bilal Yavuz <byavuz81@gmail.com> writes:

PG CI is added starting from PG 15, adding PG CI to PG 14 and below
could be beneficial. So, firstly I tried adding it to the
REL_14_STABLE branch. If that makes sense, I will try to add PG CI to
other old PG releases.

I'm not actually sure this is worth spending time on. We don't
do new development in three-release-back branches, nor do we have
so many spare CI cycles that we can routinely run CI testing in
those branches [1].

I find it much less stressful to backpatch if I can test the patches via CI
first. I don't think I'm alone in that - Alvaro for example has previously
done this in a more limited fashion (no windows):
/messages/by-id/20220928192226.4c6zeenujaoqq4bq@alvherre.pgsql

Greetings,

Andres Freund

#7Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Andres Freund (#5)
Re: Add PG CI to older PG releases

Hi,

On Fri, 11 Aug 2023 at 02:00, Andres Freund <andres@anarazel.de> wrote:

At the very least this would need to be combined with

commit 950e64fa46b164df87b5eb7c6e15213ab9880f87
Author: Andres Freund <andres@anarazel.de>
Date: 2022-07-18 17:06:34 -0700

Use STDOUT/STDERR_FILENO in most of syslogger.

950e64fa46b164df87b5eb7c6e15213ab9880f87 needs to be combined with
92f478657c5544eba560047c39eba8a030ddb83e. So, I combined
59751ae47fd43add30350a4258773537e98d4063,
950e64fa46b164df87b5eb7c6e15213ab9880f87 and
92f478657c5544eba560047c39eba8a030ddb83e in a single commit [1]and the rest are the same..

v2-0001-windows-Fix-windows-logging-problems.patch is a combination of
[1]: and the rest are the same.

Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v2-0002-Make-PG_TEST_USE_UNIX_SOCKETS-work-for-tap-tests-.patchtext/x-diff; charset=US-ASCII; name=v2-0002-Make-PG_TEST_USE_UNIX_SOCKETS-work-for-tap-tests-.patchDownload+13-2
v2-0001-windows-Fix-windows-logging-problems.patchtext/x-diff; charset=US-ASCII; name=v2-0001-windows-Fix-windows-logging-problems.patchDownload+65-13
v2-0003-ci-Add-PG-CI-to-PG-14.patchtext/x-diff; charset=US-ASCII; name=v2-0003-ci-Add-PG-CI-to-PG-14.patchDownload+788-1