From f9bb6f7478493c35be4a2a63056f92a23d8052c6 Mon Sep 17 00:00:00 2001 From: Honza Horak Date: Thu, 4 Jan 2024 17:06:38 +0100 Subject: [PATCH 1/4] Revert "each container in run_change_password_test mounts its own volume" This reverts commit 62a0e88ae0e5f19c3dad1cb6fea3778dfa2dbf2f. Creating a new volume directory was actually NOOP, because the variable volume_options was not updated. What we need to get rid of SELinux messages that this fix tried to address is to properly shut down previous container. That will be done in the following commit. Background: podman's :Z modificator for volumes works the way that the latest container run with the same volume directory mounted with :Z modificator has access, the previous containers are kept running, but access to the shared directory is suddenly removed. That caused the first instance of PostgreSQL server to crash with SIGSEGV actually, while triggering some SELinux error message during that. --- test/run_test | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/run_test b/test/run_test index c560434c..9dc6929b 100755 --- a/test/run_test +++ b/test/run_test @@ -634,11 +634,6 @@ $volume_options echo " Changing passwords" - # create separate mounting directory for second container, as selinux does - # not allow two containers accesing one mounting directory if mounted with - # Z option - create_volume_dir || ret=1 - DOCKER_ARGS=" -e POSTGRESQL_DATABASE=${database} -e POSTGRESQL_USER=${user} From fc304f442433d7c5631c65ca1d5e96ba2e114f37 Mon Sep 17 00:00:00 2001 From: Honza Horak Date: Thu, 4 Jan 2024 17:12:00 +0100 Subject: [PATCH 2/4] Gracefully shut down the original container With keeping the container running, we see SELinux error messages and sudden crash of the PostgreSQL container. Let's properly shut down the container before using its volume privately mounted into a different container. Background: podman's :Z modificator for volumes works the way that the latest container run with the same volume directory mounted with :Z modificator has access, the previous containers are kept running, but access to the shared directory is suddenly removed. That caused the first instance of PostgreSQL server to crash with SIGSEGV actually, while triggering some SELinux error message during that. --- test/run_test | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/run_test b/test/run_test index 9dc6929b..a3269624 100755 --- a/test/run_test +++ b/test/run_test @@ -634,6 +634,13 @@ $volume_options echo " Changing passwords" + echo "Kill the previous container and create a new one" + local cidfile=$CID_FILE_DIR/"${name}" + docker kill $(cat $cidfile) + docker rm -f $(cat $cidfile) + # Don't forget to remove its .cid file + rm $cidfile + DOCKER_ARGS=" -e POSTGRESQL_DATABASE=${database} -e POSTGRESQL_USER=${user} From 652296efa64dccca96706aab41ad865145ae32e5 Mon Sep 17 00:00:00 2001 From: Honza Horak Date: Thu, 4 Jan 2024 15:02:16 +0100 Subject: [PATCH 3/4] Do not ignore SQL failures and check that we still work with previous data when changing password Fix also postgresql_cmd function that should not ignore SQL errors (thus using ON_ERROR_STOP option now) and was often called with an interactive input using <<< but podman run is run in non-interactive mode, resulting in the SQL commands to not be printed, so checking them later did not work and errors are not visible. Using -c option solves this. Function test_postgresql was called with a parameter that was not used anywhere. This function used CREATE EXTENSION to test proper PostgreSQL functionality, that only admin have permissions for. So, the parameter of test_postgresql function was changed to have a useful meaning. The meaning is that only if the parameter is "admin", the CREATE EXTENSION statement is run to not see "Not enough permissions" error when the contianer is run as a normal user. --- test/run_test | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/test/run_test b/test/run_test index a3269624..8c34cad5 100755 --- a/test/run_test +++ b/test/run_test @@ -107,7 +107,7 @@ function get_ip_from_cid() { } function postgresql_cmd() { - docker run --rm -e PGPASSWORD="$PASS" "$IMAGE_NAME" psql "postgresql://$PGUSER@$CONTAINER_IP:5432/${DB-db}" "$@" + docker run --rm -e PGPASSWORD="$PASS" "$IMAGE_NAME" psql -v ON_ERROR_STOP=1 "postgresql://$PGUSER@$CONTAINER_IP:5432/${DB-db}" "$@" } function test_connection() { @@ -121,9 +121,9 @@ function test_connection() { # Don't let the code come here if neither user nor admin is able to # connect. if [ -v PGUSER ] && [ -v PASS ]; then - CONTAINER_IP=$ip postgresql_cmd <<< "SELECT 1;" + CONTAINER_IP=$ip postgresql_cmd -At -c "SELECT 1;" else - PGUSER=postgres PASS=$ADMIN_PASS CONTAINER_IP=$ip DB=postgres postgresql_cmd <<< "SELECT 1;" + PGUSER=postgres PASS=$ADMIN_PASS CONTAINER_IP=$ip DB=postgres postgresql_cmd -At -c "SELECT 1;" fi status=$? if [ $status -eq 0 ]; then @@ -137,14 +137,19 @@ function test_connection() { function test_postgresql() { local ret=0 + local user=${1:-user} echo " Testing PostgreSQL" - postgresql_cmd <<< "CREATE EXTENSION 'uuid-ossp';" || ret=1 # to test contrib package - postgresql_cmd <<< "CREATE TABLE tbl (col1 VARCHAR(20), col2 VARCHAR(20));" || ret=2 - postgresql_cmd <<< "INSERT INTO tbl VALUES ('foo1', 'bar1');" || ret=3 - postgresql_cmd <<< "INSERT INTO tbl VALUES ('foo2', 'bar2');" || ret=4 - postgresql_cmd <<< "INSERT INTO tbl VALUES ('foo3', 'bar3');" || ret=5 - postgresql_cmd <<< "SELECT * FROM tbl;" || ret=6 - #postgresql_cmd <<< "DROP TABLE tbl;" + # test contrib only when having admin privileges + if [ "$user" == "admin" ] ; then + postgresql_cmd -At -c "CREATE EXTENSION \"uuid-ossp\";" || ret=1 # to test contrib package + fi + postgresql_cmd -At -c "CREATE TABLE tbl (col1 VARCHAR(20), col2 VARCHAR(20));" || ret=2 + postgresql_cmd -At -c "INSERT INTO tbl VALUES ('foo1', 'bar1');" || ret=3 + postgresql_cmd -At -c "INSERT INTO tbl VALUES ('foo2', 'bar2');" || ret=4 + postgresql_cmd -At -c "INSERT INTO tbl VALUES ('foo3', 'bar3');" || ret=5 + postgresql_cmd -At -c "SELECT * FROM tbl;" || ret=6 + # not droping table, other tests depend on having it created after this function is run + #postgresql_cmd -At -c "DROP TABLE tbl;" if [ $ret -eq 0 ]; then echo " Success!" fi @@ -195,7 +200,7 @@ function assert_login_access() { echo "testing login as $PGUSER:$PASS; should_success=$success" - if postgresql_cmd <<<'SELECT 1;' ; then + if postgresql_cmd -At -c 'SELECT 1;' ; then if $success ; then echo " $PGUSER($PASS) access granted as expected" return @@ -422,11 +427,11 @@ function run_tests() { run_configuration_tests $name || ret=8 if $user_login; then - test_postgresql $name || ret=9 + test_postgresql || ret=9 fi if $admin_login; then - DB=postgres PGUSER=postgres PASS=$ADMIN_PASS test_postgresql $name || ret=10 + DB=postgres PGUSER=postgres PASS=$ADMIN_PASS test_postgresql admin || ret=10 fi if [ $ret -eq 0 ]; then echo " Success!" @@ -631,6 +636,7 @@ $volume_options assert_login_access ${user} ${password} true || ret=4 assert_login_access 'postgres' ${admin_password} true || ret=5 + test_postgresql || ret 5 echo " Changing passwords" @@ -664,6 +670,9 @@ $volume_options assert_login_access 'postgres' "NEW_${admin_password}" true || ret=10 assert_login_access 'postgres' ${admin_password} false || ret=11 + # check that we still work with the original volume + postgresql_cmd -At -c "SELECT * FROM tbl;" | grep bar3 || ret=12 + if [ $ret -eq 0 ]; then echo " Success!" fi From 28bd46b4d0b9d3c87a6f06d9c4ee1e8bc8889edb Mon Sep 17 00:00:00 2001 From: Honza Horak Date: Fri, 5 Jan 2024 11:58:33 +0100 Subject: [PATCH 4/4] More shared functions and test log readability This combines two changes that both improve test script, so I did not bother to split it to two commits. One is using more functions from the test-lib shared library, and second is removing expected warnings from the test log, to make the log more readable and easier to spot the issues. --- test/run_test | 90 +++++++++++++++++++-------------------------------- 1 file changed, 33 insertions(+), 57 deletions(-) diff --git a/test/run_test b/test/run_test index 8c34cad5..9482d404 100755 --- a/test/run_test +++ b/test/run_test @@ -91,16 +91,6 @@ cleanup_volume_dir () rmdir "$1" } -function get_cid() { - local id="$1" ; shift || return 1 - echo $(cat "$CID_FILE_DIR/$id") -} - -function get_container_ip() { - local id="$1" ; shift - docker inspect --format='{{.NetworkSettings.IPAddress}}' $(get_cid "$id") -} - function get_ip_from_cid() { local cid="$1"; shift docker inspect --format='{{.NetworkSettings.IPAddress}}' $cid @@ -112,18 +102,19 @@ function postgresql_cmd() { function test_connection() { local name=$1 ; shift - ip=$(get_container_ip $name) + ip=$(ct_get_cip $name) echo " Testing PostgreSQL connection to $ip..." local max_attempts=20 local sleep_time=2 + echo -n " Trying to connect, waiting for 1: ..." for i in $(seq $max_attempts); do - echo " Trying to connect..." + echo -n "." # Don't let the code come here if neither user nor admin is able to # connect. if [ -v PGUSER ] && [ -v PASS ]; then - CONTAINER_IP=$ip postgresql_cmd -At -c "SELECT 1;" + CONTAINER_IP=$ip postgresql_cmd -At -c "SELECT 1;" 2>/dev/null else - PGUSER=postgres PASS=$ADMIN_PASS CONTAINER_IP=$ip DB=postgres postgresql_cmd -At -c "SELECT 1;" + PGUSER=postgres PASS=$ADMIN_PASS CONTAINER_IP=$ip DB=postgres postgresql_cmd -At -c "SELECT 1;" 2>/dev/null fi status=$? if [ $status -eq 0 ]; then @@ -132,6 +123,7 @@ function test_connection() { fi sleep $sleep_time done + echo " Failure to connect to server $ip :(" return 1 } @@ -200,7 +192,7 @@ function assert_login_access() { echo "testing login as $PGUSER:$PASS; should_success=$success" - if postgresql_cmd -At -c 'SELECT 1;' ; then + if postgresql_cmd -At -c 'SELECT 1;' 2>/dev/null ; then if $success ; then echo " $PGUSER($PASS) access granted as expected" return @@ -211,28 +203,31 @@ function assert_login_access() { return fi fi - echo " $PGUSER($PASS) login assertion failed" + echo " $PGUSER($PASS) login assertion failed (expected: $success)" return 1 } function assert_local_access() { local id="$1" ; shift - docker exec -i $(get_cid "$id") bash -c psql <<< "SELECT 1;" + docker exec -i $(ct_get_cid "$id") bash -c psql <<< "SELECT 1;" } # Make sure the invocation of docker run fails. function assert_container_creation_fails() { local ret=0 + local out= # Time the docker run command. It should fail. If it doesn't fail, # postgresql will keep running so we kill it with SIGKILL to make sure # timeout returns a non-zero value. - timeout -s 9 --preserve-status 60s docker run --rm "$@" $IMAGE_NAME + out=$(timeout -s 9 --preserve-status 60s docker run --rm "$@" $IMAGE_NAME 2>&1) ret=$? # Timeout will exit with a high number. if [ $ret -gt 30 ]; then + echo "Output of container that did not fail:" + echo "$out" return 1 fi } @@ -317,7 +312,7 @@ function test_config_option() { local setting=$1 ; shift local value=$1 ; shift - docker exec $(get_cid ${name}) grep -q "${setting} = ${value}" /var/lib/pgsql/openshift-custom-postgresql.conf + docker exec $(ct_get_cid ${name}) grep -q "${setting} = ${value}" /var/lib/pgsql/openshift-custom-postgresql.conf } @@ -326,7 +321,7 @@ function test_config_option() { # Wait until the PG container becomes ready wait_ready () { - while ! docker exec "$(get_cid "$1")" /usr/libexec/check-container ; do + while ! docker exec "$(ct_get_cid "$1")" /usr/libexec/check-container ; do sleep 1 done } @@ -338,7 +333,7 @@ assert_runtime_option () { local name=$1 option=$2 value=$3 wait_ready "$name" - set -- $(docker exec "$(get_cid "$name")" bash -c "psql -tA -c 'SHOW $option;'") + set -- $(docker exec "$(ct_get_cid "$name")" bash -c "psql -tA -c 'SHOW $option;'") test "$value" = "$1" } @@ -367,12 +362,12 @@ test_scl_usage() { echo "ERROR[/bin/bash -c "${run_cmd}"] Expected '${expected}', got '${out}'" return 1 fi - out=$(docker exec $(get_cid $name) /bin/bash -c "${run_cmd}" 2>&1) + out=$(docker exec $(ct_get_cid $name) /bin/bash -c "${run_cmd}" 2>&1) if ! echo "${out}" | grep -q "${expected}"; then echo "ERROR[exec /bin/bash -c "${run_cmd}"] Expected '${expected}', got '${out}'" return 1 fi - out=$(docker exec $(get_cid $name) /bin/sh -ic "${run_cmd}" 2>&1) + out=$(docker exec $(ct_get_cid $name) /bin/sh -ic "${run_cmd}" 2>&1) if ! echo "${out}" | grep -q "${expected}"; then echo "ERROR[exec /bin/sh -ic "${run_cmd}"] Expected '${expected}', got '${out}'" return 1 @@ -411,7 +406,7 @@ function run_tests() { envs="$envs -e POSTGRESQL_SHARED_BUFFERS=$POSTGRESQL_SHARED_BUFFERS" fi DOCKER_ARGS="${DOCKER_ARGS:-} $envs" create_container $name - CONTAINER_IP=$(get_container_ip $name) + CONTAINER_IP=$(ct_get_cip $name) test_connection $name || ret=1 echo " Testing scl usage" test_scl_usage $name 'psql --version' "$VERSION" || ret=2 @@ -464,7 +459,7 @@ function test_slave_visibility() { return 1 fi for i in $(seq $max_attempts); do - result="$(postgresql_cmd -c "select client_addr from pg_stat_replication;" | grep "$slave_ip" || true)" + result="$(postgresql_cmd -c "select client_addr from pg_stat_replication;" 2>/dev/null | grep "$slave_ip" || true)" if [[ -n "${result}" ]]; then echo "${slave_ip} successfully registered as SLAVE for ${master_ip}" break @@ -493,7 +488,7 @@ function test_value_replication() { slave_ip=$(get_ip_from_cid $slave) CONTAINER_IP=$slave_ip for i in $(seq $max_attempts); do - result="$(postgresql_cmd -At -c "select * from $table_name" || :)" + result="$(postgresql_cmd -At -c "select * from $table_name" 2>/dev/null || :)" if [[ "$result" == "$value" ]]; then echo "${slave_ip} successfully got value from MASTER ${master_ip}" break @@ -515,7 +510,7 @@ function setup_replication_cluster() { # Run the PostgreSQL slaves local i - master_ip=$(get_container_ip "master-$cid_suffix.cid") + master_ip=$(ct_get_cip "master-$cid_suffix.cid") local cluster_args="$cluster_args --add-host postgresql-master:$master_ip" local master_hostname="postgresql-master" for i in $(seq ${slave_num:-1}); do @@ -558,7 +553,7 @@ function run_master_restart_test() { rm $cidfile run_master $cid_suffix - CONTAINER_IP=$(get_container_ip master-$cid_suffix.cid) + CONTAINER_IP=$(ct_get_cip master-$cid_suffix.cid) # Update master_ip in slaves for slave in $slave_cids; do @@ -629,7 +624,7 @@ $volume_options PASS=${password} # need this to wait for the container to start up - CONTAINER_IP=$(get_container_ip ${name}) + CONTAINER_IP=$(ct_get_cip ${name}) test_connection ${name} || ret=3 echo " Testing login" @@ -659,7 +654,7 @@ $volume_options PASS="NEW_${password}" # need this to wait for the container to start up - CONTAINER_IP=$(get_container_ip "${name}_NEW") + CONTAINER_IP=$(ct_get_cip "${name}_NEW") test_connection "${name}_NEW" || ret=7 echo " Testing login with new passwords" @@ -733,27 +728,8 @@ run_migration_test () } run_doc_test() { - local tmpdir=$(mktemp -d) - local f - echo " Testing documentation in the container image" - # Extract the help files from the container - for f in help.1 ; do - docker run --rm ${IMAGE_NAME} /bin/bash -c "cat /${f}" >${tmpdir}/$(basename ${f}) - # Check whether the files include some important information - for term in 'POSTGRESQL\\?_ADMIN\\?_PASSWORD' Volume 5432 ; do - if ! cat ${tmpdir}/$(basename ${f}) | grep -E -q -e "${term}" ; then - echo "ERROR: File /${f} does not include '${term}'." - return 1 - fi - done - done - # Check whether the files use the correct format - if ! file ${tmpdir}/help.1 | grep -q roff ; then - echo "ERROR: /help.1 is not in troff or groff format" - return 1 - fi - echo " Success!" - echo + ct_doc_content_old 'POSTGRESQL\\?_ADMIN\\?_PASSWORD' Volume 5432 + return $? } test_the_app_image () { @@ -874,7 +850,7 @@ run_s2i_enable_ssl_test() IMAGE_NAME="$s2i_image_name" create_container "$container_name" wait_ready "$container_name" - CONTAINER_IP=$(get_container_ip $container_name) + CONTAINER_IP=$(ct_get_cip $container_name) DB=postgres assert_login_access postgres password true @@ -895,7 +871,7 @@ run_s2i_bake_data_test () wait_ready "$container_name" - test "hello world" == "$(docker exec "$(get_cid "$container_name")" \ + test "hello world" == "$(docker exec "$(ct_get_cid "$container_name")" \ bash -c "psql -tA -c 'SELECT * FROM test;'")" } @@ -932,14 +908,14 @@ run_pgaudit_test() # enable the pgaudit extension # Deliberately moving heredoc into the container, otherwise it does not work # in podman 1.6.x due to https://bugzilla.redhat.com/show_bug.cgi?id=1827324 - docker exec -i $(get_cid "$name") bash -c "psql <&1 | grep -q 'FATAL: role "nonexistent" does not exist' ; then + if docker logs $(ct_get_cid "$name") 2>&1 | grep -q 'FATAL: role "nonexistent" does not exist' ; then echo " PASS: the container log does include expected error message" else echo "ERROR: the container log does not include expected error message"