Skip to content

Restart PostgreSQL when quadlet env vars change#479

Merged
evgeni merged 1 commit intotheforeman:masterfrom
shubhamsg199:fix_reset_tuning
Apr 29, 2026
Merged

Restart PostgreSQL when quadlet env vars change#479
evgeni merged 1 commit intotheforeman:masterfrom
shubhamsg199:fix_reset_tuning

Conversation

@shubhamsg199
Copy link
Copy Markdown
Contributor

@shubhamsg199 shubhamsg199 commented Apr 28, 2026

Why are you introducing these changes? (Problem description, related links)

The Deploy PostgreSQL container task updates the quadlet file with tuning-specific env vars (POSTGRESQL_MAX_CONNECTIONS, POSTGRESQL_SHARED_BUFFERS, POSTGRESQL_EFFECTIVE_CACHE_SIZE) but does not notify the Restart postgresql handler. As a result, switching tuning profiles (e.g. medium to default) updates the quadlet file and triggers daemon_reload, but PostgreSQL keeps running with stale env vars from the previous profile.

What are the changes introduced in this pull request?

  • Adding notify: Restart postgresql to the container task ensures the handler fires whenever the quadlet content changes, restarting the service with the updated env vars.

How to test this pull request

Steps to reproduce:

  • Run foremanctl deploy --tuning medium
  • Check the postgresql tuning values
  • Run foremanctl deploy --reset-tuning
  • Check the postgresql tuning values again
  • The tuning values will be for the medium profile

Checklist

  • Tests added/updated (if applicable)
  • Documentation updated (if applicable)

@ehelms
Copy link
Copy Markdown
Member

ehelms commented Apr 28, 2026

This reads like we would benefit from a test that switches tuning profiles and checks that the values for said tuning profile are updated within the running container(s) that it applies to.

@evgeni
Copy link
Copy Markdown
Member

evgeni commented Apr 29, 2026

This reads like we would benefit from a test that switches tuning profiles and checks that the values for said tuning profile are updated within the running container(s) that it applies to.

That's what SatelliteQE/robottelo#21357 is about where this was found, yeah.

I am not sure we want to test every permutation of parameters in our primary CI, as that will be expensive (time wise)

Copy link
Copy Markdown
Member

@evgeni evgeni left a comment

Choose a reason for hiding this comment

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

Tested with foremanctl-2.0.0-1.20260428184728543158.pr479.12.ge60ca68.el9.noarch, PostgreSQL is correctly restarted and the new max_connections value is used.

@evgeni evgeni enabled auto-merge (rebase) April 29, 2026 06:26
@evgeni evgeni merged commit 0541cc9 into theforeman:master Apr 29, 2026
18 of 20 checks passed
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.

3 participants