Skip to content
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.

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

Add Wants=postgresql.service to Pulpcore service files #359

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Sep 12, 2024

Wants is a weak requirement, so the service won't fail it can't be started but systemd will at least try it. That should work with help when postgresql is local.

This was reported in theforeman/foreman_maintain#824. I don't know if this is sufficient or if foreman_maintain also needs to be smarter.

An open question: if we know PostgreSQL is managed, should we make it a hard Requires= instead?

Wants is a weak requirement, so the service won't fail it can't be
started but systemd will at least try it. That should work with help
when postgresql is local.
@evgeni
Copy link
Member

evgeni commented Sep 12, 2024

I'd not add a hard requires

@ekohl ekohl added the Bug Something isn't working label Sep 12, 2024
@@ -1,7 +1,7 @@
[Unit]
Description=Pulp Worker
After=network-online.target
Wants=network-online.target
Wants=network-online.target postgresql.service
Copy link
Member Author

Choose a reason for hiding this comment

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

Reading this file I debated adding Redis as well, but wasn't quite sure about it.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense (also to foreman and dynflow)

Copy link

@sbernhard sbernhard left a comment

Choose a reason for hiding this comment

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

Make sense!

@ianballou
Copy link
Contributor

A user reported on Discourse that this wasn't sufficient to stop the coredumps when stopping Pulpcore: https://community.theforeman.org/t/pulpcore-coredumps-during-stopping-services/36512/6

@ehelms
Copy link
Member

ehelms commented Oct 11, 2024

We've identified this is still useful even if it does not address the particular problem discussed, which @evgeni has made headway with actually fixing through packaging.

@ehelms ehelms merged commit d75c952 into theforeman:master Oct 11, 2024
23 checks passed
@ekohl ekohl deleted the add-postgresql-wants branch October 11, 2024 16:25
@ekohl
Copy link
Member Author

ekohl commented Oct 11, 2024

@ehelms while I was out, has additional testing been done on the stronger requirement, which was also suggested in https://community.theforeman.org/t/pulpcore-coredumps-during-stopping-services/36512/6?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants