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

fix healthcheck postgres user #1307

Merged
merged 1 commit into from Apr 20, 2024
Merged

Conversation

DrWarpMan
Copy link
Contributor

@DrWarpMan DrWarpMan commented Apr 18, 2024

Q A
License MIT
Doc issue/PR -

By default, root user is used to connect to the postgres database.
If the database does not have a "root" role, the following error is logged everytime the health check happens:
FATAL: role "root" does not exist

It does not seem to affect the 'healthy' status of the container (the command still returns zero exit code), but it spams the log:

database-1  | 2024-04-18 06:26:08.016 UTC [67] FATAL:  role "root" does not exist
database-1  | 2024-04-18 06:27:08.192 UTC [69] FATAL:  role "root" does not exist
database-1  | 2024-04-18 06:28:08.352 UTC [71] FATAL:  role "root" does not exist

By default, root user is used to connect to the database.
If the database does not have a "root" role, the following error is logged everytime the health check happens: 'FATAL:  role "root" does not exist', which spams the log.
@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) April 18, 2024 22:31
Copy link

Thanks for the PR 😍

How to test these changes in your application

  1. Define the SYMFONY_ENDPOINT environment variable:

    # On Unix-like (BSD, Linux and macOS)
    export SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1307/index.json
    # On Windows
    SET SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1307/index.json
  2. Install the package(s) related to this recipe:

    composer req 'symfony/flex:^1.16'
    composer req 'doctrine/doctrine-bundle:^2.12'
  3. Don't forget to unset the SYMFONY_ENDPOINT environment variable when done:

    # On Unix-like (BSD, Linux and macOS)
    unset SYMFONY_ENDPOINT
    # On Windows
    SET SYMFONY_ENDPOINT=

Diff between recipe versions

In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes.
I'm going keep this comment up to date with any updates of the attached patch.

doctrine/doctrine-bundle

1.6 vs 1.12
diff --git a/doctrine/doctrine-bundle/1.6/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml
index 2f611de..30d710d 100644
--- a/doctrine/doctrine-bundle/1.6/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml
@@ -10,9 +10,12 @@ doctrine:
         charset: utf8mb4
         default_table_options:
             collate: utf8mb4_unicode_ci
+
+        # backtrace queries in profiler (increases memory usage per request)
+        #profiling_collect_backtrace: '%kernel.debug%'
     orm:
         auto_generate_proxy_classes: true
-        naming_strategy: doctrine.orm.naming_strategy.underscore
+        naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
         auto_mapping: true
         mappings:
             App:
diff --git a/doctrine/doctrine-bundle/1.6/config/packages/prod/doctrine.yaml b/doctrine/doctrine-bundle/1.12/config/packages/prod/doctrine.yaml
index 0a7c53b..084f59a 100644
--- a/doctrine/doctrine-bundle/1.6/config/packages/prod/doctrine.yaml
+++ b/doctrine/doctrine-bundle/1.12/config/packages/prod/doctrine.yaml
@@ -2,26 +2,14 @@ doctrine:
     orm:
         auto_generate_proxy_classes: false
         metadata_cache_driver:
-            type: service
-            id: doctrine.system_cache_provider
+            type: pool
+            pool: doctrine.system_cache_pool
         query_cache_driver:
-            type: service
-            id: doctrine.system_cache_provider
+            type: pool
+            pool: doctrine.system_cache_pool
         result_cache_driver:
-            type: service
-            id: doctrine.result_cache_provider
-
-services:
-    doctrine.result_cache_provider:
-        class: Symfony\Component\Cache\DoctrineProvider
-        public: false
-        arguments:
-            - '@doctrine.result_cache_pool'
-    doctrine.system_cache_provider:
-        class: Symfony\Component\Cache\DoctrineProvider
-        public: false
-        arguments:
-            - '@doctrine.system_cache_pool'
+            type: pool
+            pool: doctrine.result_cache_pool
 
 framework:
     cache:
diff --git a/doctrine/doctrine-bundle/1.6/manifest.json b/doctrine/doctrine-bundle/1.12/manifest.json
index 254981d..b39e0ab 100644
--- a/doctrine/doctrine-bundle/1.6/manifest.json
+++ b/doctrine/doctrine-bundle/1.12/manifest.json
@@ -11,8 +11,8 @@
         "#2": "IMPORTANT: You MUST configure your server version, either here or in config/packages/doctrine.yaml",
         "#3": "",
         "#4": "DATABASE_URL=\"sqlite:///%kernel.project_dir%/var/data.db\"",
-        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8\"",
-        "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/app?serverVersion=16&charset=utf8"
+        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/db_name?serverVersion=8\"",
+        "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/db_name?serverVersion=16&charset=utf8"
     },
     "dockerfile": [
         "RUN install-php-extensions pdo_pgsql"
1.12 vs 2.0
diff --git a/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/2.0/config/packages/doctrine.yaml
index 30d710d..365fef1 100644
--- a/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.0/config/packages/doctrine.yaml
@@ -5,14 +5,7 @@ doctrine:
         # IMPORTANT: You MUST configure your server version,
         # either here or in the DATABASE_URL env var (see .env file)
         #server_version: '16'
-
-        # only needed for MySQL
-        charset: utf8mb4
-        default_table_options:
-            collate: utf8mb4_unicode_ci
-
-        # backtrace queries in profiler (increases memory usage per request)
-        #profiling_collect_backtrace: '%kernel.debug%'
+        use_savepoints: true
     orm:
         auto_generate_proxy_classes: true
         naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
diff --git a/doctrine/doctrine-bundle/1.12/manifest.json b/doctrine/doctrine-bundle/2.0/manifest.json
index b39e0ab..254981d 100644
--- a/doctrine/doctrine-bundle/1.12/manifest.json
+++ b/doctrine/doctrine-bundle/2.0/manifest.json
@@ -11,8 +11,8 @@
         "#2": "IMPORTANT: You MUST configure your server version, either here or in config/packages/doctrine.yaml",
         "#3": "",
         "#4": "DATABASE_URL=\"sqlite:///%kernel.project_dir%/var/data.db\"",
-        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/db_name?serverVersion=8\"",
-        "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/db_name?serverVersion=16&charset=utf8"
+        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8\"",
+        "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/app?serverVersion=16&charset=utf8"
     },
     "dockerfile": [
         "RUN install-php-extensions pdo_pgsql"
2.0 vs 2.3
diff --git a/doctrine/doctrine-bundle/2.0/config/packages/prod/doctrine.yaml b/doctrine/doctrine-bundle/2.3/config/packages/prod/doctrine.yaml
index 084f59a..17299e2 100644
--- a/doctrine/doctrine-bundle/2.0/config/packages/prod/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.3/config/packages/prod/doctrine.yaml
@@ -1,9 +1,6 @@
 doctrine:
     orm:
         auto_generate_proxy_classes: false
-        metadata_cache_driver:
-            type: pool
-            pool: doctrine.system_cache_pool
         query_cache_driver:
             type: pool
             pool: doctrine.system_cache_pool
diff --git a/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml b/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
new file mode 100644
index 0000000..2ace640
--- /dev/null
+++ b/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
@@ -0,0 +1,4 @@
+doctrine:
+    dbal:
+        # "TEST_TOKEN" is typically set by ParaTest
+        dbname: 'main_test%env(default::TEST_TOKEN)%'
diff --git a/doctrine/doctrine-bundle/2.0/manifest.json b/doctrine/doctrine-bundle/2.3/manifest.json
index 254981d..7203651 100644
--- a/doctrine/doctrine-bundle/2.0/manifest.json
+++ b/doctrine/doctrine-bundle/2.3/manifest.json
@@ -11,7 +11,7 @@
         "#2": "IMPORTANT: You MUST configure your server version, either here or in config/packages/doctrine.yaml",
         "#3": "",
         "#4": "DATABASE_URL=\"sqlite:///%kernel.project_dir%/var/data.db\"",
-        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8\"",
+        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8&charset=utf8mb4\"",
         "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/app?serverVersion=16&charset=utf8"
     },
     "dockerfile": [
2.3 vs 2.4
diff --git a/doctrine/doctrine-bundle/2.3/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/2.4/config/packages/doctrine.yaml
index 365fef1..e517e07 100644
--- a/doctrine/doctrine-bundle/2.3/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.4/config/packages/doctrine.yaml
@@ -13,7 +13,32 @@ doctrine:
         mappings:
             App:
                 is_bundle: false
-                type: annotation
                 dir: '%kernel.project_dir%/src/Entity'
                 prefix: 'App\Entity'
                 alias: App
+
+when@test:
+    doctrine:
+        dbal:
+            # "TEST_TOKEN" is typically set by ParaTest
+            dbname_suffix: '_test%env(default::TEST_TOKEN)%'
+
+when@prod:
+    doctrine:
+        orm:
+            auto_generate_proxy_classes: false
+            proxy_dir: '%kernel.build_dir%/doctrine/orm/Proxies'
+            query_cache_driver:
+                type: pool
+                pool: doctrine.system_cache_pool
+            result_cache_driver:
+                type: pool
+                pool: doctrine.result_cache_pool
+
+    framework:
+        cache:
+            pools:
+                doctrine.result_cache_pool:
+                    adapter: cache.app
+                doctrine.system_cache_pool:
+                    adapter: cache.system
diff --git a/doctrine/doctrine-bundle/2.3/config/packages/prod/doctrine.yaml b/doctrine/doctrine-bundle/2.3/config/packages/prod/doctrine.yaml
deleted file mode 100644
index 17299e2..0000000
--- a/doctrine/doctrine-bundle/2.3/config/packages/prod/doctrine.yaml
+++ /dev/null
@@ -1,17 +0,0 @@
-doctrine:
-    orm:
-        auto_generate_proxy_classes: false
-        query_cache_driver:
-            type: pool
-            pool: doctrine.system_cache_pool
-        result_cache_driver:
-            type: pool
-            pool: doctrine.result_cache_pool
-
-framework:
-    cache:
-        pools:
-            doctrine.result_cache_pool:
-                adapter: cache.app
-            doctrine.system_cache_pool:
-                adapter: cache.system
diff --git a/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml b/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
deleted file mode 100644
index 2ace640..0000000
--- a/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
+++ /dev/null
@@ -1,4 +0,0 @@
-doctrine:
-    dbal:
-        # "TEST_TOKEN" is typically set by ParaTest
-        dbname: 'main_test%env(default::TEST_TOKEN)%'
diff --git a/doctrine/doctrine-bundle/2.3/manifest.json b/doctrine/doctrine-bundle/2.4/manifest.json
index 7203651..c022d2f 100644
--- a/doctrine/doctrine-bundle/2.3/manifest.json
+++ b/doctrine/doctrine-bundle/2.4/manifest.json
@@ -41,5 +41,8 @@
                 "    - \"5432\""
             ]
         }
+    },
+    "conflict": {
+        "symfony/framework-bundle": "<5.3"
     }
 }
2.4 vs 2.8
diff --git a/doctrine/doctrine-bundle/2.4/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/2.8/config/packages/doctrine.yaml
index e517e07..c00894c 100644
--- a/doctrine/doctrine-bundle/2.4/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.8/config/packages/doctrine.yaml
@@ -8,6 +8,7 @@ doctrine:
         use_savepoints: true
     orm:
         auto_generate_proxy_classes: true
+        enable_lazy_ghost_objects: true
         naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
         auto_mapping: true
         mappings:
diff --git a/doctrine/doctrine-bundle/2.4/manifest.json b/doctrine/doctrine-bundle/2.8/manifest.json
index c022d2f..1d8996a 100644
--- a/doctrine/doctrine-bundle/2.4/manifest.json
+++ b/doctrine/doctrine-bundle/2.8/manifest.json
@@ -11,7 +11,8 @@
         "#2": "IMPORTANT: You MUST configure your server version, either here or in config/packages/doctrine.yaml",
         "#3": "",
         "#4": "DATABASE_URL=\"sqlite:///%kernel.project_dir%/var/data.db\"",
-        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8&charset=utf8mb4\"",
+        "#5": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=8.0.32&charset=utf8mb4\"",
+        "#6": "DATABASE_URL=\"mysql://app:!ChangeMe!@127.0.0.1:3306/app?serverVersion=10.11.2-MariaDB&charset=utf8mb4\"",
         "DATABASE_URL": "postgresql://app:!ChangeMe!@127.0.0.1:5432/app?serverVersion=16&charset=utf8"
     },
     "dockerfile": [
@@ -43,6 +44,8 @@
         }
     },
     "conflict": {
+        "doctrine/orm": "<2.14",
+        "symfony/dependency-injection": "<6.2",
         "symfony/framework-bundle": "<5.3"
     }
 }
2.8 vs 2.9
2.9 vs 2.10
diff --git a/doctrine/doctrine-bundle/2.9/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/2.10/config/packages/doctrine.yaml
index c00894c..d42c52d 100644
--- a/doctrine/doctrine-bundle/2.9/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.10/config/packages/doctrine.yaml
@@ -5,14 +5,19 @@ doctrine:
         # IMPORTANT: You MUST configure your server version,
         # either here or in the DATABASE_URL env var (see .env file)
         #server_version: '16'
+
+        profiling_collect_backtrace: '%kernel.debug%'
         use_savepoints: true
     orm:
         auto_generate_proxy_classes: true
         enable_lazy_ghost_objects: true
+        report_fields_where_declared: true
+        validate_xml_mapping: true
         naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
         auto_mapping: true
         mappings:
             App:
+                type: attribute
                 is_bundle: false
                 dir: '%kernel.project_dir%/src/Entity'
                 prefix: 'App\Entity'
2.10 vs 2.12
diff --git a/doctrine/doctrine-bundle/2.10/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/2.12/config/packages/doctrine.yaml
index d42c52d..75ec9e8 100644
--- a/doctrine/doctrine-bundle/2.10/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.12/config/packages/doctrine.yaml
@@ -22,6 +22,8 @@ doctrine:
                 dir: '%kernel.project_dir%/src/Entity'
                 prefix: 'App\Entity'
                 alias: App
+        controller_resolver:
+            auto_mapping: true
 
 when@test:
     doctrine:
diff --git a/doctrine/doctrine-bundle/2.10/manifest.json b/doctrine/doctrine-bundle/2.12/manifest.json
index 1d8996a..d7f4f04 100644
--- a/doctrine/doctrine-bundle/2.10/manifest.json
+++ b/doctrine/doctrine-bundle/2.12/manifest.json
@@ -28,6 +28,11 @@
                 "    # You should definitely change the password in production",
                 "    POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-!ChangeMe!}",
                 "    POSTGRES_USER: ${POSTGRES_USER:-app}",
+                "  healthcheck:",
+                "    test: [\"CMD\", \"pg_isready -U ${POSTGRES_USER:-app}\"]",
+                "    timeout: 5s",
+                "    retries: 5",
+                "    start_period: 60s",
                 "  volumes:",
                 "    - database_data:/var/lib/postgresql/data:rw",
                 "    # You may use a bind-mounted host directory instead, so that it is harder to accidentally remove the volume and lose all your data!",

@fabpot fabpot disabled auto-merge April 20, 2024 09:28
@fabpot fabpot merged commit 8cba886 into symfony:main Apr 20, 2024
1 of 2 checks passed
@gabrielpenide
Copy link
Contributor

I am having problems with this healthcheck change, as database containers state they are unhealthy. Turns out that making the test line look like

test: ["CMD", "pg_isready", "-U", "${POSTGRES_USER:-app}"]

partially solves the problem (containers are now healthy) but logs are populated with the following message:

FATAL:  database "<redacted - shows $POSTGRES_USER variable value>" does not exist

Further modification of the line solves the problem for me:

test: ["CMD", "pg_isready", "-d", "${POSTGRES_DB:-app}", "-U", "${POSTGRES_USER:-app}"]

Not sure, but it seems something related with using CMD vs. CMD-SHELL and/or incorrectly escaping variables. Can anyone share some thoughts about this?

@DrWarpMan
Copy link
Contributor Author

The reason for this seems to be in the logged error that you provided.

It appears that if you don't provide database name to the pg_isready command, it defaults to the name of the provided user.

I did not notice this, because I was using the default database & user name which is app.

@gabrielpenide Can you confirm that your database name and database user name are different in your application?

@gabrielpenide
Copy link
Contributor

gabrielpenide commented Apr 26, 2024

Yes. They are both different from the default value app and they are both different from each other.

@DrWarpMan
Copy link
Contributor Author

The change you made should solve the issue, would you like to make the pull request yourself?

test: ["CMD", "pg_isready -d ${POSTGRES_DB:-app} -U ${POSTGRES_USER:-app}"]

@gabrielpenide
Copy link
Contributor

I can make the pull request, of course. But first I want to notice that I wasn't able to make this work with your exact suggestion. I mean that I need to use the comma-separated style for the healthcheck to pass:

test: ["CMD", "pg_isready", "-d", "${POSTGRES_DB:-app}", "-U", "${POSTGRES_USER:-app}"]

I'm very far from a Docker expert. That's why I pointed out this on my original message:

Not sure, but it seems something related with using CMD vs. CMD-SHELL and/or incorrectly escaping variables.

I would appreciate if you or anyone else elaborates on this before I make the pull request, because it slightly changes your original choice.

@DrWarpMan
Copy link
Contributor Author

Yes, you are right. In this case your provided solution is better.
Apparently the two type of syntaxes are called "shell" and "exec" forms.

This answer explains it more: https://stackoverflow.com/a/47940538
It also references to the following Docker documentation, which provides even more information:
https://docs.docker.com/reference/dockerfile/#shell-and-exec-form

Does your database name contain any special characters, spaces or something?

@gabrielpenide
Copy link
Contributor

It doesn't. Just lowercase [a-z] letters, not even numbers or special characters.

@DrWarpMan
Copy link
Contributor Author

DrWarpMan commented Apr 26, 2024

From what I tested, it seems there are only two possible combinations, either you use:

test: ["CMD-SHELL", "pg_isready -d ${POSTGRES_DB:-app} -U ${POSTGRES_USER:-app} || exit 1"]

or

test: ["CMD", "pg_isready", "-d", "${POSTGRES_DB:-app}", "-U", "${POSTGRES_USER:-app}"]

(Apparently when using CMD, || exit 1 is appended automatically.)

Can you try both syntaxes and see if both of them work @gabrielpenide ?

@gabrielpenide
Copy link
Contributor

Both work for me. Personally, I would go with the CMD option, but I guess that's a matter of taste.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants