MM-68316: recommend builtin DB readiness check for K8s deployments#8935
MM-68316: recommend builtin DB readiness check for K8s deployments#8935nickmisasi wants to merge 2 commits intomasterfrom
Conversation
Updates the Kubernetes deployment guides — particularly the air-gapped deployment guide — to recommend spec.database.readinessCheck.mode=builtin as the primary approach, removing the requirement to mirror postgres:13 into air-gapped registries. Notes that the legacy external/postgres:13 mode remains the default for backward compatibility but is slated for deprecation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThis PR updates Kubernetes deployment docs to document and recommend ChangesDatabase Readiness Check Configuration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
source/deployment-guide/server/kubernetes/deploy-k8s.rst (2)
141-143: ⚡ Quick winConsider upgrading this admonition to
important.The content describes a material difference between legacy and new deployment modes that affects image mirroring requirements in air-gapped environments and notes upcoming deprecation. This has a high impact on deployment planning and supportability. As per coding guidelines, use
importantadmonition for "prerequisites, constraints, or high-impact information that materially affects correctness, supportability, compliance, or success."📋 Suggested admonition upgrade
- .. note:: + .. important::🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/deployment-guide/server/kubernetes/deploy-k8s.rst` around lines 141 - 143, Replace the current note admonition that explains DB_CONNECTION_CHECK_URL and legacy vs builtin readiness behavior with an important admonition to highlight this high-impact information: update the admonition type around the paragraph referencing DB_CONNECTION_CHECK_URL, the operator's legacy "postgres:13" + "pg_isready" readiness init container, the default "external" mode of spec.database.readinessCheck, and the recommended spec.database.readinessCheck.mode: builtin (and its use of the in-image "mattermost db ping") so the text is presented as an important prerequisite/constraint and stands out for deployment planning and air-gapped image mirroring.
201-215: ⚡ Quick winConsider adding version-checking guidance for Novice Nate.
Line 213 states that builtin mode "requires a Mattermost release that ships the
mattermost db pingcommand" and links to a GitHub PR for availability. A novice administrator may not know how to verify whether their chosen Mattermost version includes this command. Consider adding a brief inline note such as "You can verify support by checking the release notes for your Mattermost version" or similar guidance to help readers confirm compatibility before proceeding.As per coding guidelines, define technical requirements clearly and explain prerequisites at the top of documentation sections to help novice users.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/deployment-guide/server/kubernetes/deploy-k8s.rst` around lines 201 - 215, The docs mention readinessCheck mode: builtin requires the mattermost db ping command but don't tell users how to verify support; add a short inline sentence after the parenthetical about availability that tells readers how to check their Mattermost version (e.g., “You can verify support by checking the release notes or changelog for your specific Mattermost version for the presence of the mattermost db ping command”) and reference the readinessCheck/mode: builtin and mattermost db ping symbols so novices know what to look for before switching modes.source/deployment-guide/reference-architecture/deployment-scenarios/air-gapped-deployment.rst (1)
79-99: ⚡ Quick winConsider changing this admonition to
tip.This note provides an optional best practice that helps air-gapped cluster operators succeed by avoiding the need to mirror
postgres:13. As per coding guidelines, usetipadmonition for "shortcuts, optional best practices, or advice that helps the reader succeed faster." The content clearly fits this definition with its explicit "We recommend this mode" language and practical benefit explanation.💡 Suggested admonition change
- .. note:: + .. tip::🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/deployment-guide/reference-architecture/deployment-scenarios/air-gapped-deployment.rst` around lines 79 - 99, The documentation uses a ".. note::" admonition titled "**Database readiness check (air-gapped recommendation)**" but it should be a tip; replace the ".. note::" directive with ".. tip::" for that block (the paragraph explaining builtin readinessCheck mode and the YAML example) so the content is styled as an optional best-practice shortcut; keep the existing title, body text, and code-block intact and ensure linking and formatting remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@source/deployment-guide/reference-architecture/deployment-scenarios/air-gapped-deployment.rst`:
- Around line 79-99: The documentation uses a ".. note::" admonition titled
"**Database readiness check (air-gapped recommendation)**" but it should be a
tip; replace the ".. note::" directive with ".. tip::" for that block (the
paragraph explaining builtin readinessCheck mode and the YAML example) so the
content is styled as an optional best-practice shortcut; keep the existing
title, body text, and code-block intact and ensure linking and formatting remain
unchanged.
In `@source/deployment-guide/server/kubernetes/deploy-k8s.rst`:
- Around line 141-143: Replace the current note admonition that explains
DB_CONNECTION_CHECK_URL and legacy vs builtin readiness behavior with an
important admonition to highlight this high-impact information: update the
admonition type around the paragraph referencing DB_CONNECTION_CHECK_URL, the
operator's legacy "postgres:13" + "pg_isready" readiness init container, the
default "external" mode of spec.database.readinessCheck, and the recommended
spec.database.readinessCheck.mode: builtin (and its use of the in-image
"mattermost db ping") so the text is presented as an important
prerequisite/constraint and stands out for deployment planning and air-gapped
image mirroring.
- Around line 201-215: The docs mention readinessCheck mode: builtin requires
the mattermost db ping command but don't tell users how to verify support; add a
short inline sentence after the parenthetical about availability that tells
readers how to check their Mattermost version (e.g., “You can verify support by
checking the release notes or changelog for your specific Mattermost version for
the presence of the mattermost db ping command”) and reference the
readinessCheck/mode: builtin and mattermost db ping symbols so novices know what
to look for before switching modes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dac83a35-bba9-464d-a85f-a862d2f73412
📒 Files selected for processing (2)
source/deployment-guide/reference-architecture/deployment-scenarios/air-gapped-deployment.rstsource/deployment-guide/server/kubernetes/deploy-k8s.rst
|
Newest code from mattermost has been published to preview environment for Git SHA e61dd48 |
There was a problem hiding this comment.
Pull request overview
Updates Mattermost Kubernetes deployment documentation to recommend the operator’s new spec.database.readinessCheck.mode: builtin database-readiness init container behavior (using mattermost db ping from the Mattermost image) to reduce dependency on mirroring postgres:13, especially for air-gapped environments.
Changes:
- Adds guidance and examples for configuring
spec.database.readinessCheck.mode: builtinin the Kubernetes deployment flow. - Adds an air-gapped-specific note recommending builtin readiness checks to avoid mirroring the
postgres:13image. - Documents legacy
externalreadiness-check mode as the default for backward compatibility and notes planned deprecation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
source/deployment-guide/server/kubernetes/deploy-k8s.rst |
Adds a deprecation note for the legacy DB_CONNECTION_CHECK_URL/external readiness flow and a recommended Step 5 configuration for readinessCheck.mode: builtin. |
source/deployment-guide/reference-architecture/deployment-scenarios/air-gapped-deployment.rst |
Adds an air-gapped recommendation note and example manifest snippet for readinessCheck.mode: builtin. |
| .. note:: | ||
|
|
||
| The ``DB_CONNECTION_CHECK_URL`` value is consumed by the operator's legacy ``postgres:13`` + ``pg_isready`` readiness init container (the default ``external`` mode of ``spec.database.readinessCheck``). New deployments are encouraged to set ``spec.database.readinessCheck.mode: builtin`` (see Step 5 below), in which case the readiness init container runs the in-image ``mattermost db ping`` command and the ``DB_CONNECTION_CHECK_URL`` field is no longer required. The legacy ``external`` mode remains the default for backward compatibility but is slated for deprecation in a future operator release. |
|
|
||
| Recent versions of the Mattermost Operator can run the database-readiness init container from the same Mattermost image as the main container by setting ``spec.database.readinessCheck.mode: builtin`` on the ``Mattermost`` custom resource. The init container then invokes the in-image ``mattermost db ping`` command instead of pulling ``postgres:13`` and running ``pg_isready``. | ||
|
|
||
| We recommend this mode for air-gapped clusters: it removes the requirement to mirror ``postgres:13`` into your private registry, since the only image needed for the readiness check is the Mattermost image you're already mirroring. Using ``builtin`` mode requires a Mattermost release that ships the ``mattermost db ping`` command (see the `Mattermost server release notes <https://github.com/mattermost/mattermost/pull/36406>`__ for availability). |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Newest code from mattermost has been published to preview environment for Git SHA e2610b8 |
Summary
Updates the Kubernetes deployment guides — particularly the air-gapped deployment guide — to recommend
spec.database.readinessCheck.mode: builtinas the primary approach for the operator's database-readiness init container. The new mode reuses the Mattermost image and runsmattermost db ping, removing the requirement to mirrorpostgres:13into air-gapped registries. The legacyexternalmode remains the default for backward compatibility and is noted as slated for deprecation.Files touched:
source/deployment-guide/reference-architecture/deployment-scenarios/air-gapped-deployment.rst— adds a recommendation note in the Kubernetes prerequisites tab.source/deployment-guide/server/kubernetes/deploy-k8s.rst— adds a recommendedreadinessCheckconfiguration step and a deprecation note next to the legacyDB_CONNECTION_CHECK_URLflow.Ticket Link
https://mattermost.atlassian.net/browse/MM-68316
Related PRs
mattermost db pingsubcommand mattermost#36406 — adds themattermost db pingCLI to the server binary.spec.database.readinessCheck.modefield.Notes
mattermost db pinglinks to the server PR. Update once concrete versions are decided.