Merging foreman-tasks into the core

Hello,

we briefly discussed this in a smaller group, but we’d like to revisit possibility of merging foreman-tasks into the core. It was discussed in the past and got support, but we ended up reverting the change, due to packaging issues. So this thread is created to get the feedback and find any concerns early, before the any effort is invested.

Summary of the reasons for the merge:

  • many plugins depend on it, katello, REX, ansible, salt, chef, scc_manager, wreckingball, foreman_template_tasks
  • it’s quite stable, without breaking changes, the main dependency (dynflow) is already a dependency of core
  • the list of runtime dependencies is not big - dynflow (already a dependency), foreman-tasks-core (depends on dynflow only), get_process_mem (requires ffi, already in), parse_cron (no deps), sinatra (this one can raise eyebrow, it’s used by dynflow console, we already have it pacakged in SCL for proxy)
  • we’d have mgmt console for active job tasks we already spawn on background, more cron based tasks could be converted and monitored/managed (audits expiration, config reports cleanup etc)

I think it shouldn’t be much work if we decide to merge it in as is. Meaning we’d keep the namespace for example. We should only modify the plugin registration from engine.rb to use native Foreman definitions.

Is there something else people would like to see changed before the merge? E.g. changes in API, like renaming recurring logics? Is there any concern with the merge in general?

Thanks for any feedback!

6 Likes

As a discovery maintainer, I would love to merge discovery as well long-term. But the plans are to base discovery on SSH, which actually requires tasks/ReX in core too. I think (bare-metal) provisioning with discovery should be vastly improved as one of the key features of Foreman going forward.

+1 from me.

1 Like

In the installer there’s also code to deploy a cronjob to clean up.

Should that be moved to core or can the cleanup be executed via a task? It has the benefit that deployment is easier due to fewer requirements, which also makes it easier run in HA setups. On the other hand, cleanin up a system from inside the system has its own challenges.

In packaging we also have a logrotate config that we deploy. That should also be moved into the Foreman package itself.

For sinatra we can probably make a separate foreman-tasks-console subpackage in our RPMs/debs to keep it optional if we want to. This requires a separate bundler group and of course the code to gracefully handle its absence.

However, for my understanding: what spawns the dynflow console process? AFAIK it’s listening on a separate TCP port. Does it have a systemd service? I can’t find it.

Also note that we currently have foreman-tasks-core which is also required by some smart_proxy parts. You can’t just merge that into foreman itself.

I don’t really like the idea of spawning tasks to remove tasks.

Is it worth it?

It just gets mounted under /foreman_tasks/dynflow route in the rails process and that’s it.

I’d say foreman-tasks-core part should stay a separate gem. foreman-tasks uses only a small subset of foreman-tasks-core and having it as a gem would mean we could load it into both foreman and into proxy, the same way we do it now. It also hardly ever changes nowadays so it would be a set it and forget it kind of thing.

:+1: This feels right given it’s place in the ecosystem and the simplification it provides to deployment and coding.

:+1:

I can see the hesitation there. Given tasks growth is a predictable and well known issue for users, having the tasks code self clean itself without a user having to deploy anything separate from the application itself has big advantages for maintenance. Is there another way we could do this from within the application? Or that tasks can safely clean themselves up?

I can see the hesitation there.

I’m not saying it couldn’t be done, we’d just have to make extra sure the task-cleaning-task doesn’t try to clean itself and so on.

Is there another way we could do this from within the application?

There’s the execution plan cleaner we use in dynflow on smart proxy to wipe old (older than a day) execution plans. This could be extended and used, but its operation is a bit hidden from the users.

Or that tasks can safely clean themselves up?

Something like ephemeral tasks which delete themselves when they succeed and stick around unless there’s an error?

I was assuming here, by task cleanup, this would only affect completed task runs and not destroy any running or scheduled tasks.

The more I think on that one the more I realize this would help the problem, but you can still end up with a growing table of tasks in the end and still need a cleaner.

IMHO the cleanup doesn’t have to be a blocker to merging, but it’s good to think about this. Also when we keep the containerization case in mind. Prior to merging this, it’s not something that you need to worry about. After, it probably is. For me it would be sufficient if the Foreman manual at least has a recommendation about best practices.