SEP 12 - Engines live configuration

[cbosdo]

Allow adding /removing engines without restart the minion

[waynew]

Thanks for the SEP! Sorry for the (apparent) lack of input - we have been focused on testing stability as well as getting release 2019.2.1 out. We've finally stabilized a bit, so we should be able to review and comment on this either later this week or early next week.

[waynew]

@cro do you have any thoughts about this? Someone suggested you might be familiar with this.

[thatch45]

I think this would be great!

[max-arnold]

+1 for this feature!

Recently I wrote a couple of beacons that take some time to run, and unfortunately, they block the minion event loop. An engine would be a better fit for this task; however, it is not so easy to configure and deploy/update.

My first concern is the automatic restart on each pillar refresh. I'm not sure when it happens, and there is a related bug saltstack/salt#54941. This could lead to unnecessary restarts, even if there are no actual changes in engine code or configuration.

Also, an engine could have some internal state that needs to be persisted/flushed, so maybe it makes sense to provide a pre-shutdown/restart callback (or a signal handler). On first glance, forced restarts could be disruptive for some of the existing engines.

And finally, given the recently implemented feature saltstack/salt#50059, it would be nice if each running engine provided an extended process name (when the setproctitle library is installed https://github.com/saltstack/salt/blob/develop/salt/utils/process.py#L55). This could help troubleshoot per-engine memory/cpu consumption issues.

[max-arnold]

And I just realized that engines could be run on a master, so it is also useful to restart them without bringing the master down. How could this be handled?

[waynew]

I think this would be great!

@cbosdo not much more to say than that.

We discussed this in our core meeting today, so the rest of the approvals should show up soon and we can get that merged in.

@garethgreenaway has some relevant information about this that he can provide, but he's pretty busy right now so it may be January before he can get to this.

[thatch45]

By relevant information, this means that he can outline what it will take to get this added in. Likely a runner for master engines and an execution module for minion engines. Just because we merge a SEP does not mean that we will be able to implement it, a SEP is an approval of a feature so someone can feel safe dedicating the time to create it :)

[cbosdo]

@thatch45 @waynew thanks for the infos. I am also rather busy with the virt module and states as well as other related things. Wouldn't be able to start on that before January either.

[waynew]

@cbosdo This SEP has been accepted 👍 When you (or anyone else) has time to start working on this, we look forward to seeing your implementation!

[cbosdo]

Thanks @waynew. I'm still fairly busy here, if someone wants to pick it feel free to do it. I'll try to come to it later, most probably around spring