Skip to content

docs(container): warn about K8s init-container tag mismatch + RollingUpdate hazard on shared data volumes (refs #9940)#10014

Closed
asheshv wants to merge 1 commit into
pgadmin-org:masterfrom
asheshv:docs/k8s-init-container-tag-pinning
Closed

docs(container): warn about K8s init-container tag mismatch + RollingUpdate hazard on shared data volumes (refs #9940)#10014
asheshv wants to merge 1 commit into
pgadmin-org:masterfrom
asheshv:docs/k8s-init-container-tag-pinning

Conversation

@asheshv

@asheshv asheshv commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Refs #9940.

Why

Issue #9940 reported (sqlite3.OperationalError) no such column: sharedserver.db_res from a Kubernetes pgAdmin deployment where the main container and an init container ran on different image tags but shared /var/lib/pgadmin. Root cause: PR #9835 (Apr 2026, ships in 9.15) added a migration that drops db_res from sharedserver. When the init container ran pgadmin4:latest (had the drop migration) and the main container ran 9.13 (didn't), the init container advanced the schema past 9.13's ORM and the main container's SELECT sharedserver.db_res … queries started failing.

Discussion on the issue exposed a second, related hazard the operator hadn't asked about: even with both image tags pinned correctly, a K8s rolling upgrade across pgAdmin versions is unsafe when old and new pods share a data volume. During the RollingUpdate overlap window, the new pod migrates the schema while the old pod is still serving traffic — destructive migrations (column drops, table renames, etc.) immediately break the still-alive old pod's queries.

Both hazards are real, both are operator-side configuration concerns, and neither is documented anywhere.

What this PR does

Adds a section to docs/en_US/container_deployment.rst (after the existing Traefik section, at the end of the file) titled "Sharing the data volume across containers (Kubernetes init containers)", with two subsections:

  1. Init-container tag pinningthe rule: the init container's tag must not be newer than the main container's. Example: Helm values showing both pinned to the same version, with an explicit "Do NOT use :latest" note. Why: pgAdmin migrations are forward-only with destructive operations.

  2. Use strategy: Recreate, not RollingUpdatethe rule: deployment strategy must be Recreate. Why: the rolling-update overlap window between old and new pods sharing a data volume is unsafe with single-step destructive migrations (no expand-contract pattern); the only safe path is to kill the old pod before the new one migrates. Trade-off: a few seconds of downtime per upgrade, in exchange for not serving traffic from a pod whose ORM is misaligned with the database schema underneath it.

What this PR doesn't do

This is a docs-only change — no code, no migration changes, no behavior change. The reporter on #9940 has a concrete workaround (pin the init container tag and switch to Recreate); this PR ensures future operators don't hit either trap without warning.

A longer-term, code-side improvement would be a startup-time alembic_version mismatch check: at pgAdmin main-container boot, refuse to start with a clear error if the DB has been migrated by a newer pgAdmin than the running code. That's its own design discussion and out of scope here.

A still-longer-term option — adopting the expand-contract migration pattern so destructive operations span multiple releases and rolling upgrades become safe — would be a substantial design change to pgAdmin's migration policy, also out of scope.

Stats

  • 1 file, +75 lines
  • RST parses cleanly (no new warnings vs master; the existing Sphinx-only role warnings on this file are pre-existing)

Test plan

  • docutils.core.publish_doctree parses without new warnings/errors
  • Recommended for maintainers: run make docs (Sphinx HTML build) and confirm both subsections render correctly in docs/en_US/_build/html/container_deployment.html.

@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Adds a Kubernetes section describing safe sharing of /var/lib/pgadmin via init containers, requiring init/sidecar image tags not be newer than the main pgAdmin image and recommending strategy: Recreate. Includes YAML examples for extraInitContainers and type: Recreate.

Changes

Kubernetes Data Volume Sharing Documentation

Layer / File(s) Summary
Init container guidance and version-pinning
docs/en_US/container_deployment.rst
Documents sharing /var/lib/pgadmin using init containers, warns that init/sidecar image tags must not be newer than the main pgAdmin image (forward-only migrations), and provides an extraInitContainers YAML example with matching tags.
Deployment upgrade strategy: Recreate
docs/en_US/container_deployment.rst
Adds a subsection requiring strategy: Recreate instead of RollingUpdate, explains pod-overlap failure mode during rolling upgrades, and shows a YAML snippet with type: Recreate.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main documentation change: warning about Kubernetes init-container tag mismatches and RollingUpdate hazards on shared data volumes, directly addressing the PR's core objective.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

…umes

Refs pgadmin-org#9940.

Issue pgadmin-org#9940 reported a SQLite "no such column: sharedserver.db_res"
error on a Kubernetes deployment where the main pgAdmin container
was pinned to 9.13 but an init container (running
`setup.py load-servers` to import a servers.json) used the
`pgadmin4:latest` image. The init container's migrations
advanced the schema past 9.13's ORM expectations — specifically
the `sharedserver_feature_parity_` migration (introduced in 9.15
via PR pgadmin-org#9835) drops the `db_res` column from `sharedserver`.

This is a real hazard of pgAdmin's forward-only-with-destructive-
operations migration model when combined with the K8s init-container
pattern, but it wasn't documented anywhere. Operators hitting this
have no guidance on how to avoid it.

Add a short section to docs/en_US/container_deployment.rst:

- The single rule: init container's image tag must NOT be newer
  than the main container's, when they share a data volume.
- A concrete Helm-values example showing the pinned tags side by
  side, with an explicit "Do NOT use :latest" note.
- A reminder that during upgrades, every reference to the image
  tag must move together.

Pure docs-only change — no code touched. Reviewable on its own.
@asheshv asheshv force-pushed the docs/k8s-init-container-tag-pinning branch from b02627c to a3f998f Compare June 7, 2026 07:55
@asheshv asheshv changed the title docs(container): warn about K8s init-container tag mismatch on shared data volumes (refs #9940) docs(container): warn about K8s init-container tag mismatch + RollingUpdate hazard on shared data volumes (refs #9940) Jun 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@docs/en_US/container_deployment.rst`:
- Around line 593-600: Update the examples so Helm-values keys are not mixed
with raw Deployment fields: either split the shown snippets into two labeled
blocks (one demonstrating Helm values using keys like image.registry,
image.repository, image.tag and strategy.type in values.yaml, and a separate raw
Deployment manifest showing image: docker.io/dpage/pgadmin4:9.14,
spec.strategy.type, and extraInitContainers), or replace the current examples
with Helm-values-native keys only (remove Deployment-style fields such as image:
docker.io/dpage/pgadmin4:9.14 and spec.strategy.type from the values example and
instead show image.registry/repository/tag and strategy.type). Ensure the same
change is applied to the other occurrence noted (the snippet around
spec.strategy.type).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2f971ea4-af50-42de-b36b-fef4b8d523c1

📥 Commits

Reviewing files that changed from the base of the PR and between b02627c and a3f998f.

📒 Files selected for processing (1)
  • docs/en_US/container_deployment.rst

Comment thread docs/en_US/container_deployment.rst
dpage pushed a commit that referenced this pull request Jun 9, 2026
@dpage

dpage commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Merged to master as 25415b8 (squashed, preserving @asheshv as author). I addressed @coderabbitai's point — the first example now shows a chart-agnostic raw Deployment manifest fragment (spec.template.spec.initContainers/containers[].image) instead of mixing a extraInitContainers Helm-values key with a Deployment image field, plus a note to set the equivalent values if deploying via a Helm chart. Verified the Sphinx HTML build is warning-free for this file. The squash rewrites the SHA so GitHub won't auto-close this — closing manually. Thanks @asheshv!

@dpage dpage closed this Jun 9, 2026
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

@dpage Thanks for the update, and for taking the time to refine the example into a chart-agnostic Deployment manifest — that makes the guidance cleaner and more broadly applicable. Great collaboration with @asheshv on getting this documented. Congratulations on the merge! 🎉

(∧ _ ∧)
(。・ω・。) 🐰

✅ Action performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.

asheshv added a commit that referenced this pull request Jun 16, 2026
Add 14 release-note entries that were merged on master but not yet
captured in the 9.16 notes: 1 new feature (#2431), 6 housekeeping
(#9817, #9866, #9917, #9959, #10014, #10023) and 7 bug fixes (#9701,
#9782, #9933, #9952, #9985, #10013, #10030). Entries inserted in
numeric order within each section.

Also add an "Additional changes (no associated issue) -> Dependencies"
section mirroring the 9.15 format, listing net direct dep bumps
between REL-9_15 and HEAD across Python (requirements.txt,
tools/requirements.txt, web/regression/requirements.txt), web/
package.json, and runtime/package.json. Transitive yarn resolutions
and the setuptools pin (covered by bug fix #9829) are excluded.
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.

2 participants