Skip to content

change hook weight to -1 (job has to run after config maps placed) #776

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.

Sign up for GitHub

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

Open
masbarnes wants to merge 2 commits into
base: main
Choose a base branch
from
Open

change hook weight to -1 (job has to run after config maps placed) #776

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6a75ba
change hook weight to -1 (job has to run after config maps are alread…
masbarnes Mar 10, 2026
73cda88
Merge branch 'main' into hook-weight-bug-fix
masbarnes Mar 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/model-engine/templates/database_init_job.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ metadata:
{{- include "modelEngine.labels" . | nindent 4 }}
annotations:
"helm.sh/hook": pre-install,pre-upgrade
"helm.sh/hook-weight": "-2"
"helm.sh/hook-weight": "-1"
Copy link

@greptile-apps greptile-apps bot Mar 13, 2026

Choose a reason for hiding this comment

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

Init and migration jobs now run concurrently

This change correctly ensures the config maps (weight -2) are created before the database_init_job runs. However, before this PR the ordering was:

  • -2: config maps and database_init_job (concurrent, which was itself the bug being fixed)
  • -1: database_migration_job

After this PR, both database_init_job and database_migration_job share weight -1, meaning Helm will submit and run them concurrently. If db.runDbInitScript and db.runDbMigrationScript are ever both enabled in the same release (e.g., a first-install that also needs migrations), the migration job could start before the init job finishes — which could cause the migration to fail against an uninitialized database.

If these two flags are guaranteed to be mutually exclusive, this is fine. If there is any scenario where both could be true simultaneously, consider bumping database_migration_job to weight 0 to preserve a sequential init → migrate order.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: charts/model-engine/templates/database_init_job.yaml
Line: 10

Comment:
**Init and migration jobs now run concurrently**

This change correctly ensures the config maps (weight `-2`) are created before the `database_init_job` runs. However, before this PR the ordering was:

- `-2`: config maps **and** `database_init_job` (concurrent, which was itself the bug being fixed)
- `-1`: `database_migration_job`

After this PR, both `database_init_job` and `database_migration_job` share weight `-1`, meaning Helm will submit and run them **concurrently**. If `db.runDbInitScript` and `db.runDbMigrationScript` are ever both enabled in the same release (e.g., a first-install that also needs migrations), the migration job could start before the init job finishes — which could cause the migration to fail against an uninitialized database.

If these two flags are guaranteed to be mutually exclusive, this is fine. If there is any scenario where both could be `true` simultaneously, consider bumping `database_migration_job` to weight `0` to preserve a sequential init → migrate order.

How can I resolve this? If you propose a fix, please make it concise.

"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
spec:
backoffLimit: 0
Expand Down