Prevent unintentional Postgres submodule update (#4692)

## Problem

Postgres submodule can be changed unintentionally, and these changes are
easy to miss during the review.

Adding a check that should prevent this from happening, the check fails
`build-neon` job with the following message:
```
Expected postgres-v14 rev to be at '1414141414141414141414141414141414141414', but it is at '1144aee1661c79eec65e784a8dad8bd450d9df79'
Expected postgres-v15 rev to be at '1515151515151515151515151515151515151515', but it is at '1984832c740a7fa0e468bb720f40c525b652835d'
Please update vendors/revisions.json if these changes are intentional.
```
This is an alternative approach to
https://github.com/neondatabase/neon/pull/4603

## Summary of changes
- Add `vendor/revisions.json` file with expected revisions
- Add built-time check (to `build-neon` job) that Postgres submodules
match revisions from `vendor/revisions.json`
- A couple of small improvements for logs from
https://github.com/neondatabase/neon/pull/4603
- Fixed GitHub autocomment for no tests was run case

---------

Co-authored-by: Joonas Koivunen <joonas@neon.tech>

.github/actions/allure-report-generate/action.yml

@@ -105,7 +105,7 @@ runs:
         # Get previously uploaded data for this run
         ZSTD_NBTHREADS=0
 
-        S3_FILEPATHS=$(aws s3api list-objects-v2 --bucket ${BUCKET} --prefix ${RAW_PREFIX}/ | jq --raw-output  '.Contents[].Key')
+        S3_FILEPATHS=$(aws s3api list-objects-v2 --bucket ${BUCKET} --prefix ${RAW_PREFIX}/ | jq --raw-output '.Contents[]?.Key')
         if [ -z "$S3_FILEPATHS" ]; then
           # There's no previously uploaded data for this $GITHUB_RUN_ID
           exit 0

.github/workflows/build_and_test.yml

@@ -174,6 +174,27 @@ jobs:
           submodules: true
           fetch-depth: 1
 
+      - name: Check Postgres submodules revision
+        shell: bash -euo pipefail {0}
+        run: |
+          # This is a temporary solution to ensure that the Postgres submodules revision is correct (i.e. the updated intentionally).
+          # Eventually it will be replaced by a regression test https://github.com/neondatabase/neon/pull/4603
+
+          FAILED=false
+          for postgres in postgres-v14 postgres-v15; do
+            expected=$(cat vendor/revisions.json | jq --raw-output '."'"${postgres}"'"')
+            actual=$(git rev-parse "HEAD:vendor/${postgres}")
+            if [ "${expected}" != "${actual}" ]; then
+              echo >&2 "Expected ${postgres} rev to be at '${expected}', but it is at '${actual}'"
+              FAILED=true
+            fi
+          done
+
+          if [ "${FAILED}" = "true" ]; then
+            echo >&2 "Please update vendors/revisions.json if these changes are intentional"
+            exit 1
+          fi
+
       - name: Set pg 14 revision for caching
         id: pg_v14_rev
         run: echo pg_rev=$(git rev-parse HEAD:vendor/postgres-v14) >> $GITHUB_OUTPUT

pageserver/src/tenant/timeline/walreceiver/connection_manager.rs

@@ -266,7 +266,7 @@ pub struct ConnectionManagerStatus {
 impl ConnectionManagerStatus {
     /// Generates a string, describing current connection status in a form, suitable for logging.
     pub fn to_human_readable_string(&self) -> String {
-        let mut resulting_string = "WalReceiver status".to_string();
+        let mut resulting_string = String::new();
         match &self.existing_connection {
             Some(connection) => {
                 if connection.has_processed_wal {

scripts/comment-test-report.js

@@ -205,29 +205,25 @@ module.exports = async ({ github, context, fetch, report }) => {
 
     const {reportUrl, reportJsonUrl} = report
 
-    if (!reportUrl || !reportJsonUrl) {
+    if (reportUrl && reportJsonUrl) {
+        try {
+            const parsed = await parseReportJson({ reportJsonUrl, fetch })
+            commentBody += await reportSummary({ ...parsed, reportUrl })
+        } catch (error) {
+            commentBody += `### [full report](${reportUrl})\n___\n`
+            commentBody += `#### Failed to create a summary for the test run: \n`
+            commentBody += "```\n"
+            commentBody += `${error.stack}\n`
+            commentBody += "```\n"
+            commentBody += "\nTo reproduce and debug the error locally run:\n"
+            commentBody += "```\n"
+            commentBody += `scripts/comment-test-report.js ${reportJsonUrl}`
+            commentBody += "\n```\n"
+        }
+    } else {
         commentBody += `#### No tests were run or test report is not available\n`
-        commentBody += autoupdateNotice
-        return
     }
 
-    try {
-        const parsed = await parseReportJson({ reportJsonUrl, fetch })
-        commentBody += await reportSummary({ ...parsed, reportUrl })
-    } catch (error) {
-        commentBody += `### [full report](${reportUrl})\n___\n`
-        commentBody += `#### Failed to create a summary for the test run: \n`
-        commentBody += "```\n"
-        commentBody += `${error.stack}\n`
-        commentBody += "```\n"
-        commentBody += "\nTo reproduce and debug the error locally run:\n"
-        commentBody += "```\n"
-        commentBody += `scripts/comment-test-report.js ${reportJsonUrl}`
-        commentBody += "\n```\n"
-    }
-
-    commentBody += autoupdateNotice
-
     let createCommentFn, listCommentsFn, updateCommentFn, issueNumberOrSha
     if (isPullRequest) {
         createCommentFn  = github.rest.issues.createComment

test_runner/regress/test_compatibility.py

@@ -73,9 +73,9 @@ def test_create_snapshot(
         ".*init_tenant_mgr: marking .* as locally complete, while it doesnt exist in remote index.*"
     )
 
-    pg_bin.run(["pgbench", "--initialize", "--scale=10", endpoint.connstr()])
-    pg_bin.run(["pgbench", "--time=60", "--progress=2", endpoint.connstr()])
-    pg_bin.run(
+    pg_bin.run_capture(["pgbench", "--initialize", "--scale=10", endpoint.connstr()])
+    pg_bin.run_capture(["pgbench", "--time=60", "--progress=2", endpoint.connstr()])
+    pg_bin.run_capture(
         ["pg_dumpall", f"--dbname={endpoint.connstr()}", f"--file={test_output_dir / 'dump.sql'}"]
     )
 
@@ -405,7 +405,9 @@ def check_neon_works(
     request.addfinalizer(lambda: cli_current.endpoint_stop("main"))
 
     connstr = f"host=127.0.0.1 port={pg_port} user=cloud_admin dbname=postgres"
-    pg_bin.run(["pg_dumpall", f"--dbname={connstr}", f"--file={test_output_dir / 'dump.sql'}"])
+    pg_bin.run_capture(
+        ["pg_dumpall", f"--dbname={connstr}", f"--file={test_output_dir / 'dump.sql'}"]
+    )
     initial_dump_differs = dump_differs(
         repo_dir.parent / "dump.sql",
         test_output_dir / "dump.sql",
@@ -425,7 +427,7 @@ def check_neon_works(
     shutil.rmtree(repo_dir / "local_fs_remote_storage")
     timeline_delete_wait_completed(pageserver_http, tenant_id, timeline_id)
     pageserver_http.timeline_create(pg_version, tenant_id, timeline_id)
-    pg_bin.run(
+    pg_bin.run_capture(
         ["pg_dumpall", f"--dbname={connstr}", f"--file={test_output_dir / 'dump-from-wal.sql'}"]
     )
     # The assert itself deferred to the end of the test
@@ -437,7 +439,7 @@ def check_neon_works(
     )
 
     # Check that we can interract with the data
-    pg_bin.run(["pgbench", "--time=10", "--progress=2", connstr])
+    pg_bin.run_capture(["pgbench", "--time=10", "--progress=2", connstr])
 
     assert not dump_from_wal_differs, "dump from WAL differs"
     assert not initial_dump_differs, "initial dump differs"

vendor/revisions.json

@@ -0,0 +1,4 @@
+{
+    "postgres-v15": "1984832c740a7fa0e468bb720f40c525b652835d",
+    "postgres-v14": "1144aee1661c79eec65e784a8dad8bd450d9df79"
+}