Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for extensions loading and creation. #399

Merged
merged 2 commits into from
Jul 31, 2024

Conversation

mscherer
Copy link
Contributor

Using POSTGRESQL_LIBRARIES and POSTGRESQL_EXTENSIONS allow
users to enable extensions (some bundled in the container) without
having to do anything but adding 2 variables.

@centos-ci
Copy link
Collaborator

Can one of the admins verify this patch?

2 similar comments
@centos-ci
Copy link
Collaborator

Can one of the admins verify this patch?

@centos-ci
Copy link
Collaborator

Can one of the admins verify this patch?

@phracek
Copy link
Member

phracek commented Jul 16, 2021

Why did you remove file 10_pgaudit.conf. shared_preload_libraries = 'pgaudit' is set only for test? It is not needed in the container for users at all?

@phracek
Copy link
Member

phracek commented Jul 16, 2021

[test]

@mscherer
Copy link
Contributor Author

I removed it because this is used in the test, and would interfer with them.

However, you are right, removing the file make the example incorrect, as the whole README is no longer correct.

A better solution would be to either remove the cp line and keep the example as is, or duplicate the test, so we can test the old way, and the new one.

I will send a updated PR once the tests are finished.

@jontrossbach
Copy link

jontrossbach commented Jul 16, 2021

I just got done deploying Postgres13 from these changes as a back end to my Discourse instance. They worked for me and allowed me to automate the processes I have been struggling to automate cleanly for a while now. We are going to either need to maintain these changes or have them upstreamed for a Discourse Template we plan to go live somewhere close to next week.

I built it from the instructions in the first README.md. The following might be useful if it was updated to reflect the steps to build on Fedora, e.g. from:

$ git clone --recursive https://github.com/sclorg/postgresql-container.git
$ cd postgresql
$ make build TARGET=rhel7 VERSIONS=13

to something like:

$ git clone --recursive https://github.com/sclorg/postgresql-container.git
$ cd postgresql
# dnf install podman-docker go-md2man distgen
$ make build TARGET=rhel7 VERSIONS=13

@mscherer mscherer force-pushed the add_extensions branch 2 times, most recently from 82b0b9b to 570df13 Compare July 20, 2021 12:45
@mscherer
Copy link
Contributor Author

Ok so I tried my best to test the old way and the new way.

And for the failure on testing-farm.io, I am unsure what to do, I rebased hoping this would somehow fix the problem.

@hhorak
Copy link
Member

hhorak commented Sep 15, 2021

[test]

@mscherer
Copy link
Contributor Author

Last push should fix the error:

12:17:33 test/run: line 862: DOCKER_EXTRA_ARGS: unbound variable

(which is the only error I see on https://ci.centos.org/job/SCLo-container-postgresql-rh-fedora/292/timestamps/?time=HH:mm:ss&timeZone=GMT+2&appendLog&locale=en_GB )

@mscherer
Copy link
Contributor Author

So I regenerated like the other PR.

@mscherer
Copy link
Contributor Author

I rebased again (conflicts).

@phracek
Copy link
Member

phracek commented Oct 3, 2023

@mscherer Please rebase it against master including common submodule.

Copy link
Member

@phracek phracek left a comment

Choose a reason for hiding this comment

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

I am not an postgresql expert. Except postgresql commands everything LGTM.

@fila43 @hhorak Does it make sense to you?

@phracek
Copy link
Member

phracek commented Oct 3, 2023

Rebased

[test]

@phracek
Copy link
Member

phracek commented Oct 3, 2023

Let's test after generated sources.

[test]

@phracek
Copy link
Member

phracek commented Oct 3, 2023

Strange behaviour.

There is an error in test/run script:

-----------------------------------------------
Running test run_pgaudit_test (starting at 2023-10-03 10:26:59+00:00) ... 
-----------------------------------------------
c644c8223c59eb64a932a2d1cd2e13e9f3bee4a1c7fd79ec69e1df6e37a788c0
Created container c644c8223c59eb64a932a2d1cd2e13e9f3bee4a1c7fd79ec69e1df6e37a788c0
test/run: line 340: $1: unbound variable
Tests finished with EXIT=1

Tests are passing:

==============================================
Tests were run for image quay.io/sclorg/postgresql-15-c9s:1
==============================================
Test cases results:

 [PASSED] for 'postgresql-container_tests' run_container_creation_tests (00:00:19)
 [PASSED] for 'postgresql-container_tests' run_general_tests (00:00:39)
 [PASSED] for 'postgresql-container_tests' run_change_password_test (00:00:07)
 [PASSED] for 'postgresql-container_tests' run_replication_test (00:00:06)
 [PASSED] for 'postgresql-container_tests' run_doc_test (00:00:00)
 [PASSED] for 'postgresql-container_tests' run_s2i_test (00:00:14)
 [PASSED] for 'postgresql-container_tests' run_test_cfg_hook (00:00:08)
 [PASSED] for 'postgresql-container_tests' run_s2i_bake_data_test (00:00:12)
 [PASSED] for 'postgresql-container_tests' run_s2i_enable_ssl_test (00:00:08)
 [PASSED] for 'postgresql-container_tests' run_upgrade_test (00:00:00)
 [PASSED] for 'postgresql-container_tests' run_migration_test (00:00:23)

phracek
phracek previously requested changes Oct 3, 2023
Copy link
Member

@phracek phracek left a comment

Choose a reason for hiding this comment

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

I hit some issues, that needs to be fixed.

test/run_test Outdated Show resolved Hide resolved
@phracek
Copy link
Member

phracek commented Oct 3, 2023

@mscherer Please address my comments, if they are relevant.

@zmiklank
Copy link
Contributor

Strange behaviour.

There is an error in test/run script:

-----------------------------------------------
Running test run_pgaudit_test (starting at 2023-10-03 10:26:59+00:00) ... 
-----------------------------------------------
c644c8223c59eb64a932a2d1cd2e13e9f3bee4a1c7fd79ec69e1df6e37a788c0
Created container c644c8223c59eb64a932a2d1cd2e13e9f3bee4a1c7fd79ec69e1df6e37a788c0
test/run: line 340: $1: unbound variable
Tests finished with EXIT=1

Tests are passing:

==============================================
Tests were run for image quay.io/sclorg/postgresql-15-c9s:1
==============================================
Test cases results:

 [PASSED] for 'postgresql-container_tests' run_container_creation_tests (00:00:19)
 [PASSED] for 'postgresql-container_tests' run_general_tests (00:00:39)
 [PASSED] for 'postgresql-container_tests' run_change_password_test (00:00:07)
 [PASSED] for 'postgresql-container_tests' run_replication_test (00:00:06)
 [PASSED] for 'postgresql-container_tests' run_doc_test (00:00:00)
 [PASSED] for 'postgresql-container_tests' run_s2i_test (00:00:14)
 [PASSED] for 'postgresql-container_tests' run_test_cfg_hook (00:00:08)
 [PASSED] for 'postgresql-container_tests' run_s2i_bake_data_test (00:00:12)
 [PASSED] for 'postgresql-container_tests' run_s2i_enable_ssl_test (00:00:08)
 [PASSED] for 'postgresql-container_tests' run_upgrade_test (00:00:00)
 [PASSED] for 'postgresql-container_tests' run_migration_test (00:00:23)

@phracek Hi, have you encountered this problem repeatedly? I will create a issue from this so we will not forget it.

mscherer added 2 commits July 30, 2024 10:19
Using POSTGRESQL_LIBRARIES and POSTGRESQL_EXTENSIONS allow
users to enable extensions (some bundled in the container) without
having to do anything but adding 2 variables.
@fila43
Copy link
Member

fila43 commented Jul 30, 2024

[test]

@fila43 fila43 dismissed phracek’s stale review July 31, 2024 08:21

not needed

@fila43
Copy link
Member

fila43 commented Jul 31, 2024

Tests passed; upgrade test is a known issue. Otherwise LGTM

@fila43 fila43 merged commit b455357 into sclorg:master Jul 31, 2024
12 of 17 checks passed
SlouchyButton added a commit to SlouchyButton/postgresql-container that referenced this pull request Oct 8, 2024
Initially command `test was missing`, but after it was added
test failed and crashed in a way that no more tests were run.
This behavior is known issue tracked here in issue sclorg#537

Problem is that `DOCKER_EXTRA_ARGS` variable is set already
which it shouldn't be, so the `test` command returns 0,
so no `cp` happens and we get the crash.

Variable is empty, but apparently do exist as
`test -v DOCKER_EXTRA_ARGS` is `TRUE`.

Introduced by PR sclorg#399
phracek pushed a commit that referenced this pull request Oct 9, 2024
Initially command `test was missing`, but after it was added
test failed and crashed in a way that no more tests were run.
This behavior is known issue tracked here in issue #537

Problem is that `DOCKER_EXTRA_ARGS` variable is set already
which it shouldn't be, so the `test` command returns 0,
so no `cp` happens and we get the crash.

Variable is empty, but apparently do exist as
`test -v DOCKER_EXTRA_ARGS` is `TRUE`.

Introduced by PR #399
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants