Skip to content

🐛(helm) show the database error while jobs wait for it to be ready - #2578

Open
RISK-alt wants to merge 1 commit into
suitenumerique:mainfrom
RISK-alt:fix/helm-migrate-job-error-output
Open

🐛(helm) show the database error while jobs wait for it to be ready#2578
RISK-alt wants to merge 1 commit into
suitenumerique:mainfrom
RISK-alt:fix/helm-migrate-job-error-output

Conversation

@RISK-alt

Copy link
Copy Markdown
Contributor

Purpose

Fixes #1966

The migrate and createsuperuser jobs wait for the database like this:

while ! python manage.py check --database default > /dev/null 2>&1
do
  echo "Database not ready"
  sleep 2
done

Two problems, both of which the issue points at:

  • > /dev/null 2>&1 throws away everything the check has to say
  • manage.py check --database default fails for far more than an unreachable database — a missing required setting, a bad credential, a broken INSTALLED_APPS entry — and every one of those prints "Database not ready"

So the job loops forever, at two seconds an iteration, and its logs contain nothing but a message that is usually wrong.

Proposal

  • capture the check output instead of discarding it, and print it on each failed attempt
  • number the attempts, so a log where the error changes between two attempts is readable
  • drop the "Database not ready" wording, which asserted a cause the check never established
  • leave the retry behaviour untouched — same 2s interval, same unbounded loop
attempt=0
until output=$(python manage.py check --database default 2>&1)
do
  attempt=$((attempt + 1))
  echo "Database check failed (attempt $attempt), retrying in 2s:"
  echo "$output"
  sleep 2
done

until VAR=$(cmd) takes the exit status of the command substitution, so the loop condition is unchanged; it only gains a copy of the output. Nothing here is bashism: until, $(( )) and command substitution are all POSIX, and the backend image is python:3.14.6-alpine, whose /bin/sh is busybox ash.

The issue notes the snippet is duplicated. It is, in six places, all updated:

  • src/helm/impress/values.yamlbackend.migrate and backend.createsuperuser
  • src/helm/env.d/dev/values.impress.yaml.gotmpl — same two jobs
  • src/helm/env.d/feature/values.impress.yaml.gotmpl — same two jobs

I did not try to factor the snippet into a single template helper: the four job definitions live in values files that are meant to be overridden by operators, and hiding the command behind a helper would take that away.

Testing

helm is not installed on my machine, so the chart rendering rests on the helmfile-lint job.

What I did check locally:

  • values.yaml parses, and the command list round-trips to the three expected /bin/sh -c <script> elements
  • the loop itself, replayed under sh against a stub check that fails twice with a two-line stderr then succeeds — both failures print with their attempt number and the full stderr, and the loop exits on success
Database check failed (attempt 1), retrying in 2s:
django.db.utils.OperationalError: could not connect to server
extra stderr line
Database check failed (attempt 2), retrying in 2s:
django.db.utils.OperationalError: could not connect to server
extra stderr line
Database is ready

One note on src/helm/impress/README.md: the two backend.*.command rows are generated from the values, so the table had to be re-rendered, which re-pads every row of that section — hence the large but purely cosmetic diff there. I could not run generate-readme.sh (it needs Docker) nor the generator directly: version 3.0.1 stops on a pre-existing metadata mismatch in this chart, unrelated to this PR.

ERROR: Missing metadata for key: yProvider.converter.service
ERROR: Metadata provided for non existing key: yProvider.converter.service.type
ERROR: Metadata provided for non existing key: yProvider.converter.service.port
ERROR: Metadata provided for non existing key: yProvider.converter.service.targetPort
ERROR: Metadata provided for non existing key: yProvider.converter.service.annotations

So I reproduced the generator's output for those two cells by hand: same compact JSON serialization of the command list, same column padding as the rest of the table. Happy to fix that metadata in a separate PR if you want the generator runnable again.

External contributions

General requirements

  • I have read and followed the contributing guidelines
  • I have read and agreed to the Code of Conduct
  • I have added corresponding tests for new features or bug fixes (if applicable) — N/A, chart values only

CI requirements

  • I made sure that all existing tests are passing — waiting on the CI, helmfile lint is the relevant job and I cannot run it locally
  • I have signed off my commits with git commit --signoff (DCO compliance)
  • I have signed my commits with my SSH or GPG key (git commit -S)
  • My commit messages follow the required format: <gitmoji>(type) title description
  • I have added a changelog entry under ## [Unreleased] section (if noticeable change)

AI requirements

  • I used AI assistance to produce part or all of this contribution
  • I have read, reviewed, understood and can explain the code I am submitting
  • I can jump in a call or a chat to explain my work to a maintainer

The migrate and createsuperuser jobs polled `manage.py check` with its
output sent to /dev/null, then printed "Database not ready" whatever
the reason. Any failure the check reports, a missing setting or a bad
credential for instance, looked like a database that had not started
yet, and the job looped forever without a single clue in its logs.

The check output is now captured and printed on each failed attempt,
along with the attempt number, so the reason the job is still waiting
is readable with kubectl logs. The retry behaviour is unchanged.

Applied to both jobs in the chart values and in the dev and feature
helmfile environments. The chart README table is regenerated
accordingly.

Signed-off-by: risk-alt <aldu6974@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 55977f37-1c94-4f32-be02-77030970597a

📥 Commits

Reviewing files that changed from the base of the PR and between c9e32e3 and d8afb33.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • src/helm/env.d/dev/values.impress.yaml.gotmpl
  • src/helm/env.d/feature/values.impress.yaml.gotmpl
  • src/helm/impress/README.md
  • src/helm/impress/values.yaml

Walkthrough

Helm migration and superuser initialization jobs now capture database check output, count retry attempts, and print diagnostic details while waiting for database readiness. The behavior is applied to production, development, and feature configurations. The backend Helm documentation and changelog now describe the updated retry reporting.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: antolc

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary change: showing database errors while Helm jobs wait for database readiness.
Description check ✅ Passed The description explains the database polling issue, proposed fix, affected files, preserved retry behavior, and validation performed.
Linked Issues check ✅ Passed The changes address issue #1966 by exposing actual check errors and updating all duplicated Helm polling instances.
Out of Scope Changes check ✅ Passed All changes support the linked issue, including duplicated chart values, generated README documentation, and the changelog entry.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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.

@AntoLC
AntoLC self-requested a review August 10, 2026 12:49
@AntoLC
AntoLC removed their request for review August 10, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛Helm error migrate createsuperuser

3 participants