sclorg / postgresql-container

PostgreSQL container images based on Red Hat Software Collections and intended for OpenShift and general usage. Users can choose between Red Hat Enterprise Linux, Fedora, and CentOS based images.
http://softwarecollections.org
Apache License 2.0
164 stars 216 forks source link

Allow logging to /dev/stderr #366

Closed hhorak closed 1 year ago

hhorak commented 4 years ago

For easier debugging, it should be pretty easy to switch daemon to log into the /dev/stderr. This adds such a feature by setting POSTGRESQL_LOG_DESTINATION environment variable to /dev/stderr.

This should address #353.

phracek commented 4 years ago

[test]

pkubatrh commented 4 years ago

centos7 failure will be fixed by https://github.com/sclorg/postgresql-container/pull/368

phracek commented 4 years ago

@hhorak rhel8 test is failing by

+ echo 'psql -U nonexistent'
++ get_cid pg-test-logging
++ local id=pg-test-logging
++ shift
+++ cat /tmp/tmp.nwSOqgiZvspostgresql_test_cidfiles/pg-test-logging
++ echo b9893aa670ad049bae5a6db4fa5f6a2116c7fb9b38d5b79e75c4111f2e5f06b6
+ docker exec -i b9893aa670ad049bae5a6db4fa5f6a2116c7fb9b38d5b79e75c4111f2e5f06b6 bash
+ sleep 1
+ grep -q 'FATAL:  role "nonexistent" does not exist'
++ get_cid pg-test-logging
++ local id=pg-test-logging
++ shift
+++ cat /tmp/tmp.nwSOqgiZvspostgresql_test_cidfiles/pg-test-logging
++ echo b9893aa670ad049bae5a6db4fa5f6a2116c7fb9b38d5b79e75c4111f2e5f06b6
+ docker logs b9893aa670ad049bae5a6db4fa5f6a2116c7fb9b38d5b79e75c4111f2e5f06b6
ERROR: the container log does not include expected error message
+ echo 'ERROR: the container log does not include expected error message'

Unfortunately, I am not able to reproduce it locally :( Locally all tests are passing. I will try it once more.

phracek commented 4 years ago

[test]

phracek commented 4 years ago

[test]

pkubatrh commented 4 years ago

Needs a rebase now that #368 is merged

pkubatrh commented 4 years ago

Rebased [test-openshift]

phracek commented 4 years ago

[test]

fridex commented 3 years ago

A friendly ping - will this be eventually applied? It would definitely help us.

hhorak commented 3 years ago

A friendly ping - will this be eventually applied? It would definitely help us.

That's great feedback, thanks, we'll try to finish it.

phracek commented 2 years ago

Let's try to run all tests once more.

[test-all]

phracek commented 2 years ago

[test-all]

pkubatrh commented 1 year ago

Lets try to revisit and make sure this gets merged.

fila43 commented 1 year ago

[test]

hhorak commented 1 year ago

Rebased.

hhorak commented 1 year ago

[test]

hhorak commented 1 year ago

Test for this had some issues, so let's try again after improving it a bit. [test]

hhorak commented 1 year ago

[test]

phracek commented 1 year ago

RHEL8 tests are failing for this reason:

-----------------------------------------------
Running test run_logging_test (starting at 2023-03-01 06:04:51-05:00) ... 
-----------------------------------------------
640a6a2bef4835f14cf7403051c5a2d4e544647f643926977ac84fb9806f6889
Created container 640a6a2bef4835f14cf7403051c5a2d4e544647f643926977ac84fb9806f6889
Error: non zero exit code: 2: OCI runtime error
Error: non zero exit code: 2: OCI runtime error
Error: read unixpacket @->/var/run/libpod/socket/d0e9d591a8da0bcdbc8d9657fb9b3fdb958f81162accc0d33637c7ffe2de3973/attach: read: connection reset by peer
ERROR: the container log does not include expected error message
Test for image 'rhel8/postgresql-13:1' FAILED (exit code: 1)

[snipped]
 [PASSED] for 'postgresql-container_tests' run_pgaudit_test (00:00:05)
 [FAILED] for 'postgresql-container_tests' run_logging_test (00:00:05)

What about adding more logs for debugging issues?

fila43 commented 1 year ago

[test]

phracek commented 1 year ago

@hhorak Please have a look at RHEL8 tests. They are failing on:

 [FAILED] for 'postgresql-container_tests' run_logging_test (00:00:05). 

May be more logs for traces would be fine.

fila43 commented 1 year ago

@phracek I see [FAILED] for 'postgresql-container_tests' run_master_restart_test (00:01:14)

fila43 commented 1 year ago

The investigation in https://github.com/sclorg/postgresql-container/pull/503 leads me to the conclusion. Added functionality works as expected, and it's proven by passing all tests except RHEL8. Logging to stderr also should work in the case of RHEL8. Unfortunately, RHEL8 uses podman version 1.6.4. This version raises error Error: non zero exit code: 2: OCI runtime error for the docker exec command used in the test case.

zmiklank commented 1 year ago

The investigation in #503 leads me to the conclusion. Added functionality works as expected, and it's proven by passing all tests except RHEL8. Logging to stderr also should work in the case of RHEL8. Unfortunately, RHEL8 uses podman version 1.6.4. This version raises error Error: non zero exit code: 2: OCI runtime error for the docker exec command used in the test case.

Do you think it could be a bug in podman? Or some needed functionality is not implemented in this version of podman?

hhorak commented 1 year ago

@fila43 I see the docker exec exit code should not matter, as the line ends with || : -- can you explain why it might make the difference?

zmiklank commented 1 year ago

[test]

fila43 commented 1 year ago

[test]

fila43 commented 1 year ago

It seems that master_restart_test randomly fails. Since it isn't related to this PR, could we merge it @hhorak @zmiklank? I would prefer to fix the failing test in a separate PR later. Because it happens only occasionally and only within GitHub CI, it makes investigating the problem tricky.

zmiklank commented 1 year ago

It seems that master_restart_test randomly fails. Since it isn't related to this PR, could we merge it @hhorak @zmiklank? I would prefer to fix the failing test in a separate PR later. Because it happens only occasionally and only within GitHub CI, it makes investigating the problem tricky.

I agree to fix the test which is not connected to this PR separately. However I can not give lgtm to this PR now, as I did not review it yet.

fila43 commented 1 year ago

From my point of view, the PR is LGTM. There is only one unrelated problem with random failures. But I am ok to wait for your review. Thanks

zmiklank commented 1 year ago

From my point of view, the PR is LGTM. There is only one unrelated problem with random failures. But I am ok to wait for your review. Thanks

You do not really need to wait for my review :). I am ok with merging if you reviewed it.