Skip to content

fix(server): set HOME in podSecuritycontext for migration job#606

Open
mathijscarlu wants to merge 1 commit intoPrefectHQ:mainfrom
mathijscarlu:main
Open

fix(server): set HOME in podSecuritycontext for migration job#606
mathijscarlu wants to merge 1 commit intoPrefectHQ:mainfrom
mathijscarlu:main

Conversation

@mathijscarlu
Copy link

@mathijscarlu mathijscarlu commented Mar 6, 2026

Summary

Closes #605

This change adds the following to the separate migrations job:

  • HOME env variable
  • podSecurityContext, as specified in the backgroundServices section

Requirements

  • Contributing guide has been read
  • Title follows the conventional commits format
  • Body includes Closes <issue>, if available
  • Added/modified configuration includes a descriptive comment for documentation generation
  • Unit tests are added/updated
  • Deprecation/removal checks are added in templates/NOTES.txt
  • Relevant labels are added
  • Draft status is used until ready for review

@mathijscarlu mathijscarlu requested a review from a team as a code owner March 6, 2026 15:36
@mathijscarlu mathijscarlu requested a deployment to Acceptance Tests (External) March 6, 2026 15:36 — with GitHub Actions Waiting
@mathijscarlu mathijscarlu requested a deployment to Acceptance Tests (External) March 6, 2026 15:36 — with GitHub Actions Waiting
@mathijscarlu mathijscarlu requested a deployment to Acceptance Tests (External) March 6, 2026 15:36 — with GitHub Actions Waiting
@mathijscarlu mathijscarlu requested a deployment to Acceptance Tests (External) March 6, 2026 15:36 — with GitHub Actions Waiting
@mathijscarlu mathijscarlu requested a deployment to Acceptance Tests (External) March 6, 2026 15:36 — with GitHub Actions Waiting
@mathijscarlu mathijscarlu requested a deployment to Acceptance Tests (External) March 6, 2026 15:36 — with GitHub Actions Waiting
@mathijscarlu mathijscarlu requested a deployment to Acceptance Tests (External) March 6, 2026 15:36 — with GitHub Actions Waiting
@mathijscarlu mathijscarlu requested a deployment to Acceptance Tests (External) March 6, 2026 15:36 — with GitHub Actions Waiting
@mathijscarlu mathijscarlu marked this pull request as draft March 6, 2026 15:36
@mathijscarlu mathijscarlu marked this pull request as ready for review March 9, 2026 07:19
@mitchnielsen mitchnielsen changed the title fix(server): set HOME en podSecuritycontext for migration job fix(server): set HOME in podSecuritycontext for migration job Mar 9, 2026
@mitchnielsen mitchnielsen self-requested a review March 9, 2026 21:06
@mitchnielsen mitchnielsen added the fix A fix for a bug in an existing feature label Mar 9, 2026
@mitchnielsen
Copy link
Contributor

Thanks for the contribution @mathijscarlu!

The HOME env var addition looks good. I believe the podSecurityContext implementation has a couple of issues that need fixing:

  1. {{- include . | nindent 4 }} won't work because include expects a template name string, not a dict. The pattern used elsewhere in this chart is:

    {{- with .Values.backgroundServices.podSecurityContext }}
    securityContext: {{- . | toYaml | nindent 8 }}
    {{- end }}
  2. The second test case (Should set correct pod security context) is indented one level too deep, making it part of the previous test's asserts list rather than a separate test. It also needs a set: block to provide the values and to enable backgroundServices.runAsSeparateDeployment. You can see the test failure here and you should be able to run it locally to confirm it works before pushing up the commit.

Would you also consider adding migrations.podSecurityContext as its own values key (similar to how the migration job already has its own securityContext, affinity, tolerations, etc.) rather than pulling from backgroundServices? This would be more consistent with the existing pattern where the migration job has independent configuration.

Copy link
Contributor

@mitchnielsen mitchnielsen left a comment

Choose a reason for hiding this comment

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

Left a comment requesting small changes 👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix A fix for a bug in an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Running migrations in job does not work with ssl enabled

2 participants