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

Let systemd relationship handle dynflow restarts #1148

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Jan 23, 2024

When all services are stopped, a puppet run of this module (or the installer) will trigger services to start back up. Since I believe Puppet detects this as the start, this results in a restart of Foreman and Dynflow:

[28.25 seconds] /Service[foreman]:
[27.37 seconds] /Service[dynflow-sidekiq@orchestrator]:
[26.42 seconds] /Service[dynflow-sidekiq@worker-hosts-queue-1]:
[26.32 seconds] /Service[dynflow-sidekiq@worker-1]:

Via systemd relationships, dynflow is already part of Foreman (https://github.com/theforeman/foreman/blob/develop/extras/systemd/dynflow-sidekiq%40.service#L5) which means that anytime foreman.service receives a restart the dynflow* services will also get restarted. By allowing systemd to control this rather than Puppet, we can save ~1.5 minutes on my test system.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I'd think that if we make it require the Foreman service that the ordering means it'll already be running by the time it's evaluated, making it a noop

@ehelms
Copy link
Member Author

ehelms commented Jan 24, 2024 via email

@@ -47,7 +47,7 @@
content => foreman::to_symbolized_yaml($config),
}
~> service { $service:
ensure => running,
ensure => true,
Copy link
Member

Choose a reason for hiding this comment

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

What does ensure => true mean?

https://www.puppet.com/docs/puppet/8/types/service.html#service-attribute-ensure is not exactly verbose on that.

Copy link
Member

Choose a reason for hiding this comment

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

@ehelms
Copy link
Member Author

ehelms commented Jan 24, 2024

Yea.. weird I dunno why I thought I saw this work and then couldn't reproduce it. Digging through some history, I see this was brought up in the original systemd change:

I think the part that has me confused the most, and our use cases is PartOf declaration and it's caveat that it only works for stop and restart. Although from my testing restart doesn't always work. If foreman is stopped and you issue a restart it won't affect dynflow. I assume this is because it's actually the "stop" aspect that triggers the relationship.

Letting systemd perform actions in parallel though is more efficient than what currently appears to happen with Puppet.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I think the real cause isn't the ensure but that there's a refresh relationship.

On line 55 there's a Service <| tag == 'postgresql::server::service' |> ~> Service[$service]. This is because dynflow doesn't handle PostgreSQL reconnects, at least didn't in the past. That is one possible issue.

It may also be because of the config file on line 48, though we can't get rid of that.

The other one I'm uncertain about is how foreman::service propagates refreshes to foreman::dynflow::pool and that pool to individual workers.

I think debug logs tell you when a resource triggers a refresh.

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

Successfully merging this pull request may close these issues.

3 participants